[9] RFR (S): 8168926: C2: Bytecode escape analyzer crashes due to stack overflow

[Vladimir Kozlov]

Thank you, Zoltan

Very nice investigation and evaluation. I agree that EA Analyzer should 
use only information in ciMethod otherwise it will be inconsistent.

Changes look good to me.

Thanks
Vladimir

On 1/10/17 5:25 AM, Zoltán Majó wrote:
> Hi,
>
>
> please review the fix for 8168926.
>
> https://bugs.openjdk.java.net/browse/JDK-8168926
> http://cr.openjdk.java.net/~zmajo/8168926/webrev.00/
>
> This is a bug in C2's escape analyzer (EA) I've been chasing for more
> than a year now.
>
> The bug reproduces very rarely (<10 appearances since Sep '15) and in
> different forms/with different tests (see JDK-8135159 for a set of
> different manifestations of the same bug).
>
> I tried to reproduce the crash on at least five different occasions and
> with different tests, but did not succeed, unfortunately. So my findings
> (and the fix) rely only on source-code/test inspection and post-mortem
> analysis of crashes I've seen.
>
> The bug is caused by the EA having an inconsistent view of the number of
> parameters taken by a call site 'c'. If call site 'c' in a method 'm' is
> dynamic (i.e., 'c' is targeted by an invokehandle or invokedynamic
> instruction), the number of parameters taken by 'c' is different before
> and after 'c' is resolved. That is, after 'c' is resolved, 'c' takes one
> more argument than the number of arguments pushed onto the stack by 'm'
> (as 'c' is dynamic, it needs an extra appendix argument after resolution).
>
> In its current state, EA can have two views of 'c' for the analysis of
> 'c'. I.e., EA can use both a "before-resolution" and an
> "after-resolution" view of 'c'. As a result, EA can pop fewer elements
> from the stack than there were pushed onto the stack, which results in a
> stack overflow.
>
> Here is a detailed scenario to illustrate the problem. Let's assume the
> following sequence of operations to take place while EA is analyzing
> method 'm'.
>
>
> Step (1): EA obtains the method targeted by call site 'c' in 'm'. The
> result is saved into ciMethod 'target':
>
> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/026ff073b5ad/src/share/vm/ci/bcEscapeAnalyzer.cpp#l895
>
>
> Let's assume that 'c' is not yet resolved at this point of time, i.e.,
> the number of arguments N of 'target' does not include the appendix
> argument (i.e., N is equal to the number of items pushed onto the stack
> by the the bytecodes of method 'm').
>
>
> Step (2): A thread different than the compiler thread performing EA of
> 'm' reaches call site 'c' and executes it. As a result, 'c' is resolved
> (and bootstrapped) and it now points to a method taking N+1 parameters
> (one more parameter than before, because the parameters also include the
> appendix argument).
>
>
> Step (3): EA checks if call site 'c' has an appendix argument.
>
> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/026ff073b5ad/src/share/vm/ci/bcEscapeAnalyzer.cpp#l899
>
>
> As there is an appendix argument, an extra (unknown) argument is pushed
> onto the stack. I.e., there are N+1 elements on the stack at this point
> of time.
>
>
> Step (4): EA continues with analyzing the call site
>
> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/026ff073b5ad/src/share/vm/ci/bcEscapeAnalyzer.cpp#l903
>
>
> After being done with the analysis, EA removes 'arg_size' number of
> arguments from the stack. For example, here:
>
> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/026ff073b5ad/src/share/vm/ci/bcEscapeAnalyzer.cpp#l294
>
>
> The number 'arg_size' of arguments is, however, only N. The reason is
> that 'arg_size' is obtained from ciMethod 'target' constructed back at
> Step (1), i.e., from the unresolved call site, and does not include the
> appendix argument.
>
>
> Summary: If the sequence of operations is executed as outlined by Step
> (1)-(4), the stack can overflow after call site is analyzed, because
> some arguments pushed onto it are not popped EA is done with analyzing
> call site'c'. For the problem to appear, the resolution of call site 'c'
> has to happen concurrently with EA and exactly after Step (1) and before
> Step (3). That explains why the problem reproduces so rarely. For more
> information on the investigation please see [1].
>
> The fix I propose determines if a call site 'c' needs an appendix
> argument solely by looking at the ciMethod 'target' and the current
> bytecode instruction. By that, EA has only one (consistent) view of call
> site 'c' (which is either resolved or not).
>
> I tested the fix with
> - RBT (all hotspot tests both with -Xmixed and -Xcomp);
> - JPRT;
> - locally executed all jdk/test/java/lang/invoke tests (both with
> -Xmixed and -Xcomp).
>
> No (new) failures appeared.
>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>
> [1]
> https://bugs.openjdk.java.net/browse/JDK-8168926?focusedCommentId=14035888&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14035888
>