RFR(L) 8238268: Many SA tests are not running on OSX because they do not attempt to use sudo when available

[Chris Plummer]

Hi,

Please help review the following:

https://bugs.openjdk.java.net/browse/JDK-8238268
http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/

I'll try to give enough background first to make it easier to understand 
the changes. On OSX you must run SA tests that attach to a live process 
as root or using sudo. For example:

   sudo make run-test TEST=serviceability/sa/ClhsdbJstackXcompStress.java

Whether running as root or under sudo, the check to allow the test to 
run is done with:

     private static boolean canAttachOSX() {
           return userName.equals("root");
     }

Any test using "@requires vm.hasSAandCanAttach" must pass this check via 
Platform.shouldSAAttach(), which for OSX returns:

              return canAttachOSX() && !isSignedOSX();

So if running as root the "@requires vm.hasSAandCanAttach" passes, 
otherwise it does not. However, using a root login to run tests is not a 
very desirable, nor is issuing a "sudo make run-test" (any created file 
ends up with root ownership). Because of this support was previously 
added for just running the attaching process using sudo, not the entire 
test. This was only done for the 20 or so tests that use ClhsdbLauncher. 
These tests use "@requires vm.hasSA", and then while running the test 
will do a "sudo" check if canAttachOSX() returns false:

         if (!Platform.shouldSAAttach()) {
             if (Platform.isOSX()) {
                 if (Platform.isSignedOSX()) {
                     throw new SkippedException("SA attach not expected 
to work. JDK is signed.");
                 } else if (SATestUtils.canAddPrivileges()) {
                     needPrivileges = true;
                 }
             }
             if (!needPrivileges)  {
                // Skip the test if we don't have enough permissions to 
attach
                // and cannot add privileges.
                throw new SkippedException(
                    "SA attach not expected to work. Insufficient 
privileges.");
            }
         }

So basically it does a runtime check of vm.hasSAandCanAttach, and if it 
fails then checks if running with sudo will work. This allows for either 
a passwordless sudo to be used when running clhsdb, or for the user to 
be prompted for the sudo password (note I've remove support for the 
latter with my changes).

That brings us to the CR that is being fixed. ClhsdbLauncher tests 
support sudo and will therefore run with our CI testing on OSX, but the 
25 or so tests that use "@requires vm.hasSAandCanAttach" do not, and 
therefore are never run with our CI OSX testing. The changes in this 
webrev fix that.

There are two possible approaches to the fix. One is having the check 
for sudo be done as part of the vm.hasSAandCanAttach evaluation. The 
other approach is to do the check in the test at runtime similar to how 
ClhsdbLauncher currently does. This would mean just using "@requires 
vm.hasSA" for all the tests instead of "@requires vm.hasSAandCanAttach". 
I chose the later because there is an advantage to throwing 
SkippedException rather than just silently skipping the test using 
@requires. The advantage is that mdash tells you how many tests were 
skipped, and when you hover over the reason you can see the 
SkippedException message, which will differentiate between reasons like 
the JDK was signed or there are insufficient privileges. If all the 
checking was done by the vm.hasSAandCanAttach evaluation, you would not 
know why the test wasn't run.

The "support" related changes made are all in the following 3 files. The 
rest of the changes are in the tests:

test/jtreg-ext/requires/VMProps.java
test/lib/jdk/test/lib/Platform.java
test/lib/jdk/test/lib/SA/SATestUtils.java

You'll noticed that one change I made to the sudo support in 
SATestUtils.canAddPrivileges() is to make sudo non-interactive, which 
means no password prompt. So that means either the user does not require 
a password, or the credentials have been cached. Otherwise the sudo 
check will fail. On most platforms if you execute a sudo command, the 
credentials are cached for 5 minutes. So if your user is not setup for 
passwordless sudo, then a sudo command can be issued before running the 
tests, and will likely remain cached until the test is run. The reason 
for using passwordless is because prompting in the middle of running 
tests can be confusing (you usually walk way once launching the tests 
and miss the prompt anyway), and avoids unnecessary delays in automated 
testing due to waiting for the password prompt to timeout (it used to 
wait 1 minute).

There are essentially 3 types of tests that SA Attach to a process, each 
needing a slightly different fix:

1. Tests that directly launch a jdk.hotspot.agent class, such as 
TestClassDump.java. They need to call SATestUtils.checkAttachOk() to 
verify that attaching will be possible, and then 
SATestUtils.addPrivilegesIfNeeded(pb) to get the sudo command added if 
needed.They also need to switch from using hasSAandCanAttach to using hasSA.

2. Tests that launch command line tools such has jhsdb. They need to 
call SATestUtils.checkAttachOk() to verify that attaching will be 
possible, and then SATestUtils.createProcessBuilder() to create a 
process that will be launched using sudo if necessary.They also need to 
switch from using hasSAandCanAttach to using hasSA.

3. Tests that use ClhsdbLauncher. They already use hasSA instead of 
hasSAandCanAttach, and rely on ClhsdbLauncher to do check at runtime if 
attaching will work, so for the most part all the these tests are 
unchanged. ClhsdbLauncher was modified to take advantage of the new 
SATestUtils.createProcessBuilder() and SATestUtils.checkAttachOk() APIs.

Some tests required special handling:

test/hotspot/jtreg/compiler/ciReplay/TestSAClient.java
test/hotspot/jtreg/compiler/ciReplay/TestSAServer.java

- These two tests SA Attach to a core file, not to a process, so only 
need hasSA,
   not hasSAandCanAttach. No other changes were needed.

test/hotspot/jtreg/serviceability/sa/ClhsdbFindPC.java

- The output should never be null. If the test was skipped due to lack 
of privileges, you
   would never get to this section of the test.

test/hotspot/jtreg/serviceability/sa/TestClhsdbJstackLock.java
test/hotspot/jtreg/serviceability/sa/TestIntConstant.java
test/hotspot/jtreg/serviceability/sa/TestPrintMdo.java
test/hotspot/jtreg/serviceability/sa/TestType.java
test/hotspot/jtreg/serviceability/sa/TestUniverse.java

- These are ClhsdbLauncher tests, so they should have been using hasSA 
instead of
   hasSAandCanAttachin the first place. No other changes were needed.

test/hotspot/jtreg/serviceability/sa/TestCpoolForInvokeDynamic.java
test/hotspot/jtreg/serviceability/sa/TestDefaultMethods.java
test/hotspot/jtreg/serviceability/sa/TestG1HeapRegion.java

- These tests used to "@require mac" but seem run fine on OSX, so I 
removed this requirement.

test/jdk/sun/tools/jhsdb/BasicLauncherTest.java

- This test had a runtime check to not run on OSX due to not having core 
file stack
   walking support. However, this tests always attaches to a process, 
not a core file,
   and seems to run just fine on OSX.

test/jdk/sun/tools/jstack/DeadlockDetectionTest.java

- I changed the test to throw a SkippedException if it gets the 
unexpected error code
   rather than just println.

And a few other miscellaneous changes not already covered:

test/lib/jdk/test/lib/Platform.java
- Made canPtraceAttachLinux() public so it can be called from SATestUtils.
- vm.hasSAandCanAttach is now gone.

thanks,

Chris