RFR(S/M): 8205419: [testbug] TestJmapCore failing without SA: introduce @requires vm.hasSA

[Lindenmaier, Goetz]

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.george at oracle.com>
> Sent: Thursday, June 21, 2018 12:18 PM
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'Chris Plummer'
> <chris.plummer at oracle.com>; serviceability-dev (serviceability-
> dev at openjdk.java.net) <serviceability-dev at 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.
> >>
> >>
> >