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

Reply via email to