RFR: 8229118: [TESTBUG] serviceability/sa/ClhsdbFindPC fails on AArch64

[Chris Plummer]

Hi Nick,

Adding to Andrew comments, maybe the solution is to have the test extend 
LingeredApp so it can produce a more reliable stack trace other than the 
default one you get with LingeredApp. If that's too much trouble, I 
don't mind the solution you came up with, but seems writing a 
LingeredApp subclass that is specific for this test would be cleaner.

thanks,

Chris

On 8/12/19 3:24 AM, Nick Gasson wrote:
> Thanks Andrew. Can someone from the serviceability team check this is 
> OK to push?
>
> Nick
>
>
> On 08/08/2019 18:16, Andrew Dinn wrote:
>> Hi Nick,
>>
>> On 08/08/2019 10:32, Nick Gasson wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8229118
>>> Webrev: http://cr.openjdk.java.net/~ngasson/8229118/webrev.0/
>>>
>>> This test starts a sub-process with -Xcomp and then uses the SA to 
>>> get a
>>> stack trace of it. It expects to see this line:
>>>
>>>    In code in NMethod for jdk/test/lib/apps/LingeredApp.main
>>>
>>> But actually on AArch64 the stack trace looks like this:
>>>
>>>   - java.lang.Thread.sleep(long) @bci=0, pc=0x0000ffff74603d08, 
>>> Method*=0x0000ffff031baf98 (Compiled frame; information may be 
>>> imprecise)
>>>   - jdk.test.lib.apps.LingeredApp.main(java.lang.String[]) @bci=53, 
>>> line=502, pc=0x0000ffff6c9276e0, Method*=0x0000ffff03611d48 
>>> (Interpreted frame)
>>>
>>> The main method is interpreted even though we're running with
>>> -Xcomp. That's because it is deoptimized almost immediately, because
>>> main calls some methods on java.nio.file.Paths, but that class hasn't
>>> been loaded when main is compiled.
>>>
>>> X86 can patch in the address of the method on-the-fly, but AArch64 
>>> can't
>>> do this because of restrictions on which instructions can be legally
>>> rewritten.
>>>
>>> This patch lifts the code that uses the java.nio classes out of
>>> LingeredApp::main into a separate static method. LingeredApp.main now
>>> only uses classes that are loaded very early in boot, before main is
>>> compiled. The stack trace now looks like:
>>>
>>> "main" #1 prio=5 tid=0x0000ffffb4022800 nid=0xd610 waiting on 
>>> condition [0x0000ffffbb755000]
>>>     java.lang.Thread.State: TIMED_WAITING (sleeping)
>>>     JavaThread state: _thread_blocked
>>>   - java.lang.Thread.sleep(long) @bci=0, pc=0x0000fffface414c8, 
>>> Method*=0x0000ffff3dac8a28 (Compiled frame; information may be 
>>> imprecise)
>>>   - jdk.test.lib.apps.LingeredApp.pollLockFile(java.lang.String) 
>>> @bci=30, line=499, pc=0x0000ffffa50818e0, Method*=0x0000ffff3c122cf0 
>>> (Interpreted frame)
>>>   - jdk.test.lib.apps.LingeredApp.main(java.lang.String[]) @bci=25, 
>>> line=529, pc=0x0000ffffa59afd58, Method*=0x0000ffff3c122de0 
>>> (Compiled frame)
>>>
>>> I.e. pollLockFile was deoptimized to an interpreted frame but
>>> LingeredApp.main is still a compiled frame which is what 
>>> ClhsdbFindPC is
>>> looking for.
>>>
>>> This solution does seem a bit hacky, so if it's not acceptable an
>>> alternative is to just skip the -Xcomp part of the test on AArch64.
>>>
>>> Ran a full jtreg test on AArch64/x86 to check for regressions.
>> Yuck! That's a nice hack to avoid the indeterminate effect of -Xcomp.
>> However, my gut feeling is still that relying on -Xcomp in tests is just
>> a /really/ bad idea and I'd prefer to omit it but . . .
>>
>> I'm not 100% clear what the point of this test is but it looks like it
>> is meant to exercise the stack backtrace code when there is a compiled
>> method on the stack. If so then I guess your hack fits the bill while
>> removing the -Xcomp flag from the command line would not fulfil the
>> test's remit. If that is the point of the test then I agree,
>> reluctantly, that your hack is the right solution. On those grounds I'm
>> happy to accept the patch. However, I'd prefer someone else (Andrew
>> Haley?) also to review this before it gets pushed.
>>
>> regards,
>>
>>
>> Andrew Dinn
>> -----------
>> Senior Principal Software Engineer
>> Red Hat UK Ltd
>> Registered in England and Wales under Company Registration No. 03798903
>> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>>