> On Mar 16, 2020, at 11:43 AM, "[email protected]" > <[email protected]> wrote: > > >> On 3/16/20 11:26, Chris Plummer wrote: >> I had to make another change. TestMutuallyExclusivePlatformPredicates.java >> failed when I ran tier 3. I had fixed it a long while back due to >> Platform.shouldSAAttach() being removed, but there were more changes to >> Platform.java after that that I didn't account for. isRoot() was added and >> canPtraceAttrachLinux() was made public. So this is what the diff looks like >> now: >> >> --- >> a/test/hotspot/jtreg/testlibrary_tests/TestMutuallyExclusivePlatformPredicates.java >> +++ >> b/test/hotspot/jtreg/testlibrary_tests/TestMutuallyExclusivePlatformPredicates.java >> @@ -51,9 +51,9 @@ >> VM_TYPE("isClient", "isServer", "isMinimal", "isZero", >> "isEmbedded"), >> MODE("isInt", "isMixed", "isComp"), >> IGNORED("isEmulatedClient", "isDebugBuild", "isFastDebugBuild", >> - "isSlowDebugBuild", "hasSA", "shouldSAAttach", >> "isTieredSupported", >> + "isSlowDebugBuild", "hasSA", "canPtraceAttachLinux", >> "isTieredSupported", >> "areCustomLoadersSupportedForCDS", >> "isDefaultCDSArchiveSupported", >> - "isSignedOSX"); >> + "isSignedOSX", "isRoot"); >> >> However, I'm thinking maybe I should just move canPtraceAttachLinux() to >> SATestUtils.java since that's the only user, and it is an SA specific API. >> What do you think? > > The approach to localize canPtraceAttachLinux() in SATestUtils.java sounds > right to me if it is an SA specific API. > +1 — Igor
> Thanks, > Serguei > >> >> Chris >> >>> On 3/15/20 10:22 PM, [email protected] wrote: >>> Hi Chris, >>> >>> Looks good. >>> Thank you for update! >>> >>> Thanks, >>> Serguei >>> >>> >>>> On 3/15/20 17:47, Chris Plummer wrote: >>>> I changed them all to "SA Attach" and grepped to make sure there are no >>>> other occurrences of "SA attach". >>>> >>>> thanks, >>>> >>>> Chris >>>> >>>>> On 3/15/20 4:49 PM, Igor Ignatyev wrote: >>>>> Hi Chris, >>>>> >>>>> looks good, thanks! >>>>> >>>>> one minor nit, in SATestUtils::skipIfCannotAttach, you have two exception >>>>> messages which start with 'SA attach not expected to work.', and one w/ >>>>> 'SA Attach not expected to work.' (w/ Attach instead of attach), it'd be >>>>> nicer to have them uniform. >>>>> >>>>> Cheers, >>>>> -- Igor >>>>> >>>>>> On Mar 15, 2020, at 4:35 PM, Chris Plummer <[email protected]> >>>>>> wrote: >>>>>> >>>>>> Hi Igor, >>>>>> >>>>>> Thanks for the review. Here's and updated webrev with all of the >>>>>> suggestions from you and Serguei: >>>>>> >>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.01/index.html >>>>>> >>>>>> Also some comments inline below. >>>>>> >>>>>>> On 3/13/20 9:26 AM, Igor Ignatyev wrote: >>>>>>> HI Chris, >>>>>>> >>>>>>> overall looks good to me, a few comments though: >>>>>>> 1. since you removed vm.hasSAandCanAttach from VMProps, you also need >>>>>>> to remove it from all TEST.ROOT files which mention it >>>>>>> (test/jdk/TEST.ROOT and test/hotspot/jtreg/TEST.ROOT) so people won't >>>>>>> be confused by undefined property and jtreg will be able to properly >>>>>>> report invalid usages of it if any. >>>>>> Ok, but it's unclear to me what requires.properties is even for, and >>>>>> what is the impact of extra or missing properties. What kind of test >>>>>> would catch these errors? >>>>> jtreg uses 'requires.properties' as a list of extra variables for >>>>> @require expressions, if a test uses a name which isn't in >>>>> 'requires.properties' (or known to jtreg), jtreg won't execute such test >>>>> and will set its status to Error w/ 'invalid name ...' message. >>>>> >>>>>>> >>>>>>> 2. in SATestUtils::canAddPrivileges, could you please add some >>>>>>> meaningful message to the RuntimeException at L#102? >>>>>>> >>>>>> Ok. >>>>>> >>>>>> throw new RuntimeException("sudo process interrupted", e); >>>>>> >>>>>>> 3. SATestUtils::checkAttachOk method name is somewhat misleading (hence >>>>>>> you had to put comment every time you used it), I'd recommend you to >>>>>>> rename to smth like skipIfCannotAttach(). >>>>>> Ok, but I still left the comment in place. >>>>>>> >>>>>>> 4. in SATestUtils::checkAttachOk's javadoc, it would be better to use >>>>>>> @throws tag like: >>>>>>>> + /** >>>>>>>> + * Checks if SA Attach is expected to work. >>>>>>>> +. * @throws SkippedException ifSA Attach is not expected to work. >>>>>>>> + */ >>>>>>> >>>>>>> >>>>>> Ok. >>>>>>> 5. it also might make sense to catch IOException within >>>>>>> SATestUtils::checkAttachOk and throw it as Error or RuntimeException. >>>>>>> >>>>>> Ok. >>>>>>> I've briefly looked at all the changed tests and they look good. >>>>>> >>>>>> Thanks! >>>>>> >>>>>> Chris >>>>>>> >>>>>>> Thanks, >>>>>>> -- Igor >>>>>>> >>>>>>> >>>>>>>> On Mar 12, 2020, at 11:06 PM, Chris Plummer <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Hi Serguei, >>>>>>>> >>>>>>>> Thanks for the review! >>>>>>>> >>>>>>>> Can I get one more reviewer please? >>>>>>>> >>>>>>>> thanks, >>>>>>>> >>>>>>>> Chris >>>>>>>> >>>>>>>> On 3/12/20 12:06 AM, [email protected] wrote: >>>>>>>>> Hi Chris, >>>>>>>>> >>>>>>>>> >>>>>>>>> On 3/12/20 00:03, Chris Plummer wrote: >>>>>>>>>> Hi Serguei, >>>>>>>>>> >>>>>>>>>> That check used to be in Platform.shouldSAAttach(), which >>>>>>>>>> essentially was moved to SATestUtils.checkAttachOk() and reworked >>>>>>>>>> some. It was necessary in Platform.shouldSAAttach() since that was >>>>>>>>>> used to evaluation vm.hasSAandCanAttach (which is now gone). When I >>>>>>>>>> moved everything to SATestUtils.checkAttachOk(), I recall thinking >>>>>>>>>> it wasn't really necessary since all tests that call it should have >>>>>>>>>> @require vm.hasSA, but left it in anyway just to be extra safe. I'm >>>>>>>>>> still inclined to just leave it in, but would not be opposed to >>>>>>>>>> removing it. >>>>>>>>> >>>>>>>>> I agree, it is more safe to keep it, at list for now. >>>>>>>>> >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Serguei >>>>>>>>> >>>>>>>>>> thanks, >>>>>>>>>> >>>>>>>>>> Chris >>>>>>>>>> >>>>>>>>>> On 3/11/20 11:20 PM, [email protected] wrote: >>>>>>>>>>> Hi Chris, >>>>>>>>>>> >>>>>>>>>>> I've made another pass today. >>>>>>>>>>> It looks good to me. >>>>>>>>>>> >>>>>>>>>>> I have just one minor questions. >>>>>>>>>>> >>>>>>>>>>> There is some overlap between the requires vm.hasSA check and >>>>>>>>>>> checkAttachOk: >>>>>>>>>>> + public static void checkAttachOk() throws IOException { >>>>>>>>>>> + if (!Platform.hasSA()) { >>>>>>>>>>> + throw new SkippedException("SA not supported."); >>>>>>>>>>> + } >>>>>>>>>>> In the former case, the test is not run but in the latter the >>>>>>>>>>> SkippedException is thrown. >>>>>>>>>>> As I see, all tests with the checkAttachOk call use requires >>>>>>>>>>> vm.hasSA as well. >>>>>>>>>>> It can be that the first check "if (!Platform.hasSA())" in the >>>>>>>>>>> checkAttachOk is redundant. >>>>>>>>>>> It is okay and more safe in general but generates little confusion. >>>>>>>>>>> I'm okay if you don't do anything with this but wanted to know your >>>>>>>>>>> view. >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> Serguei >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 3/10/20 18:57, Chris Plummer wrote: >>>>>>>>>>>> On 3/10/20 6:07 PM, [email protected] wrote: >>>>>>>>>>>>> Hi Chris, >>>>>>>>>>>>> >>>>>>>>>>>>> Overall, this looks as a right direction to me while it is not >>>>>>>>>>>>> easy to verify all the details. >>>>>>>>>>>> Yes, there are a lot of tests with quite a few different types of >>>>>>>>>>>> changes. I did a lot of testing and verified that when the tests >>>>>>>>>>>> pass, they pass for the right reasons (really ran the test, >>>>>>>>>>>> skipped due to lack of privileges, or skipped due to running >>>>>>>>>>>> signed on OSX 10.14 or later). I also verified locally running as >>>>>>>>>>>> root, running with a cached sudo, and running without sudo. >>>>>>>>>>>>> I'll make another pass tomorrow. >>>>>>>>>>>> Thanks! >>>>>>>>>>>>> >>>>>>>>>>>>> A couple of quick nits so far: >>>>>>>>>>>>> >>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/test/hotspot/jtreg/serviceability/sa/TestCpoolForInvokeDynamic.java.udiff.html >>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/test/hotspot/jtreg/serviceability/sa/TestDefaultMethods.java.udiff.html >>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/test/hotspot/jtreg/serviceability/sa/TestHeapDumpForInvokeDynamic.java.udiff.html >>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/test/hotspot/jtreg/serviceability/sa/TestInstanceKlassSizeForInterface.java.udiff.html >>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/test/hotspot/jtreg/serviceability/sa/TestJhsdbJstackMixed.java.udiff.html >>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/test/hotspot/jtreg/serviceability/sa/TestRevPtrsForInvokeDynamic.java.udiff.html >>>>>>>>>>>>> import jdk.test.lib.Utils; >>>>>>>>>>>>> -import jdk.test.lib.Asserts; >>>>>>>>>>>>> +import jdk.test.lib.SA.SATestUtils; >>>>>>>>>>>>> Need to swap these exports. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> Ok >>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238268/webrev.00/test/lib/jdk/test/lib/SA/SATestUtils.java.frames.html >>>>>>>>>>>>> 48 if (SATestUtils.needsPrivileges()) { >>>>>>>>>>>>> 49 cmdStringList = >>>>>>>>>>>>> SATestUtils.addPrivileges(cmdStringList); >>>>>>>>>>>>> The method calls are local, so the class name can be omitted in >>>>>>>>>>>>> the method names: >>>>>>>>>>>>> SATestUtils.needsPrivileges and SATestUtils.addPrivileges. >>>>>>>>>>>> Ok >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> 94 try { >>>>>>>>>>>>> 95 if (echoProcess.waitFor(60, TimeUnit.SECONDS) == >>>>>>>>>>>>> false) { >>>>>>>>>>>>> 96 // Due to using the "-n" option, sudo should >>>>>>>>>>>>> complete almost immediately. 60 seconds >>>>>>>>>>>>> 97 // is more than generous. If it didn't >>>>>>>>>>>>> complete in that time, something went very wrong. >>>>>>>>>>>>> 98 echoProcess.destroyForcibly(); >>>>>>>>>>>>> 99 throw new RuntimeException("Timed out waiting >>>>>>>>>>>>> for sudo to execute."); >>>>>>>>>>>>> 100 } >>>>>>>>>>>>> 101 } catch (InterruptedException e) { >>>>>>>>>>>>> 102 throw new RuntimeException(e); >>>>>>>>>>>>> 103 } >>>>>>>>>>>>> The lines 101/103 are misaligned. >>>>>>>>>>>> Ok. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks, >>>>>>>>>>>>> Serguei >>>>>>>>>>>>> >>>>>>>>>>>> Thanks, >>>>>>>>>>>> >>>>>>>>>>>> Chris >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On 3/9/20 19:29, Chris Plummer wrote: >>>>>>>>>>>>>> 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 >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
