Some of the tests were skipped on OSX due to stackwalking issues on OSX. (Some were pstack related, and some for jstack on corefiles).

Thanks,
Jini.

On 6/25/2018 10:31 PM, Chris Plummer wrote:
Hi Goetz,

The changes look good. Just one question though. In places where you have:

   42  * @requires vm.hasSAandCanAttach & os.family != "mac"

Do you know why these tests don't run on osx? I just wonder if it related to attach not working unless root, and that is now covered by hasSAandCanAttach(). If that is the case, the "mac" check can be removed.

thanks,

Chris

On 6/22/18 1:20 AM, Lindenmaier, Goetz wrote:
Hi,

Ok, so I now added
   vm.hasSA and
   vm.hasSAandCanAttach
I use the first in the JMapCore tests, and in the tests that don't check shouldSAAttach()
right at the beginning.
Wherever the test is immediately ended after checking shouldSAAttach(), I
removed shouldSAAttach(), and added vm.hasSAandCanAttach.  The others
depend on  ClhsdbLauncher. Just checking vm.hasSA is on the safe side
if runOnCore() is changed.
http://cr.openjdk.java.net/~goetz/wr18/8205419-requiresHasSA/02/

This also makes the implementation of VMProps and VMPlatform more clear
as requested by Chris.

Best regards,
  Goetz.


-----Original Message-----
From: Jini George <jini.geo...@oracle.com>
Sent: Thursday, June 21, 2018 12:18 PM
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; 'Chris Plummer'
<chris.plum...@oracle.com>; serviceability-dev (serviceability-
d...@openjdk.java.net) <serviceability-dev@openjdk.java.net>
Subject: Re: RFR(S/M): 8205419: [testbug] TestJmapCore failing without SA:
introduce @requires vm.hasSA

Hi Goetz,

Tests like TestJmapCore which involve only corefile debugging by SA (and
not attaching to a live process) would only need to check for the
presence of SA, and would not need to check for ptrace related attach
permissions like what is done for Linux and OSX -- since super user
privileges are not required to debug corefiles. So a single @requires
catering to both these might not be desirable.

Having said that, I realized that runOnCore() of ClhsdbLauncher
incorrectly checks for the ptrace related attach permissions. (Will file
an issue to correct this).

An @requires instead of a shouldSAAttach() might help in avoiding fake
passes like those on OSX when not run as "root".
(https://bugs.openjdk.java.net/browse/JDK-8199700)

Thanks,
Jini.




On 6/21/2018 1:52 AM, Lindenmaier, Goetz wrote:
Hi Chris,

Thanks for looking at this change.

Can't VMProps.vmHasSA() just return Platform.shouldSAAttach()? It
seems
you are unnecessarily replicating Platform.shouldSAAttach() logic.
I think there are two things to distinguish:
   - platforms that don't implement SA
   - systems/setups where SA does not work.
If both can be recognized when VMProps is evaluated, shouldSAAttach()
is the one that is superfluous in the long run?!  If not, shouldSAAttach
should only return those where it is necessary to check in the test.
But this is probably too much brains in this case 😊

If you are adding "@requires vm.hasSAandCanAttach" to a test, shouldn't
you remove the test's Platform.shouldSAAttach() check?
Well I asked in my mail whether I should do that. So I take this as a yes.
But it depends on whether it must be evaluated in the test.

Is there a reason not to take the more direct and established approach
of just adding the Platform.shouldSAAttach() check to TestJmapCore? It
would be a lot less disruptive.  I know the vm.hasSAandCanAttach
approach has advantages in not even attempting to compile and run the
test, but I'm not so sure those advantages outweigh the simplicity of
just adding a Platform.shouldSAAttach() check to one test. I can go
either way here. Just wondering if there are additional reasons I might
not be seeing.
I like the @requires much more. It tells right where the test is described that it does not run with SA. The other is quite hidden in the test, e.g., in
helper class ClhsdbLauncher.java.
Also, it's more easy to remember (The implementor of
TestJmapCore forgot it...).  And, last but not least, it seems best practice
nowadays. There are lots of excludes for mac, they are also done by
@requires
and not as a check using Platform at runtime.

Sorry, I don't have an answer to your VMProps evaluation question. You
might need to ask a wider audience than just svc.
Whom should I ask? Hotspot-dev?

Best regards,
    Goetz.


thanks,

Chris

On 6/20/18 6:49 AM, Lindenmaier, Goetz wrote:
Hi,

TestJmapCore is failing on aix because there jhsdb is not implemented. So far, such tests check at runtime Platform.shouldSAAttach() which is
missing
in TestJmapCore.

Instead, I implement @requires vm.hasSAandCanAttach.
This makes jtreg skip the tests without compiling and starting them.

I added the new property to all tests that check shouldSAAttach in jdk
and
hostpot test
directories.
This is the implementation of the new property, the rest is just
repetitive
checking it:
http://cr.openjdk.java.net/~goetz/wr18/8205419-
requiresHasSA/01/test/jtreg-ext/requires/VMProps.java.udiff.html
webrev:
http://cr.openjdk.java.net/~goetz/wr18/8205419-requiresHasSA/01/

Is it correct to assume the VMProps is evaluated on the same machine
and
with the same
user as used for executing the test?  canPtraceAttachLinux directly
accesses the system.
(Should I remove the checks for shouldSAAttach() from the changed
tests?)
Best regards,
     Goetz.



Reply via email to