Java 12: javac crash with switch expression containing try-catch-finally

[Jan Lahoda]

Hi Bernard,

My current thinking is that we will need to restore the stack right 
before the final goto for the given break (which can be skipped if 
code.isAlive() == false).

I've put my current patch here (incl. test) (I wasn't running other 
tests on this patch yet, though):
http://cr.openjdk.java.net/~jlahoda/8220018/8220018

This does not cover the switch expressions over booleans yet, that is 
something I am looking at right now.

Jan

On 23.3.2019 18:24, B. Blaser wrote:
> Hi Jan,
>
> On Thu, 21 Mar 2019 at 18:24, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>>
>> Hi Bernard,
>>
>> I was peeking at this, and expected to start to work on this.
>>
>> I do agree we will need to strip the extra values from the stack, but we
>> may also need to set the letExprStackPos at some point.
>>
>> Jan
>
> Right, this refers to Tagir's second example:
>
>      public static void test2() {
>          System.out.println(switch (0) {
>              default -> {
>                  try(Stream<?> s = Stream.empty()) {
>                      break 1;
>                  }
>              }
>          });
>      }
>
> The following patch fixes both cases, does this look more reasonable?
>
> Thanks,
> Bernard
>
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
> @@ -1223,6 +1223,18 @@
>           return !alive || state.stacksize == letExprStackPos;
>       }
>
> +    private boolean valueBreak = false;
> +
> +    public boolean setValueBreak(boolean set) {
> +        boolean b = valueBreak;
> +        valueBreak = set;
> +        return b;
> +    }
> +
> +    public boolean isValueBreak() {
> +        return valueBreak;
> +    }
> +
>   /**************************************************************************
>    * Stack map generation
>    *************************************************************************/
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
> @@ -1201,10 +1201,12 @@
>                   }
>               }
>               int prevLetExprStart =
> code.setLetExprStackPos(code.state.stacksize);
> +            boolean prevValueBreak = code.setValueBreak(false);
>               try {
>                   handleSwitch(tree, tree.selector, tree.cases);
>               } finally {
>                   code.setLetExprStackPos(prevLetExprStart);
> +                code.setValueBreak(prevValueBreak);
>               }
>           } finally {
>               stackBeforeSwitchExpression = prevStackBeforeSwitchExpression;
> @@ -1707,9 +1709,14 @@
>           Assert.check(code.isStatementStart());
>           final Env<GenContext> targetEnv;
>           if (tree.isValueBreak()) {
> -            //restore stack as it was before the switch expression:
> -            for (LocalItem li : stackBeforeSwitchExpression) {
> -                li.load();
> +            if (code.isValueBreak()) {
> +                code.emitop0(pop);
> +            }
> +            else {
> +                //restore stack as it was before the switch expression:
> +                for (LocalItem li : stackBeforeSwitchExpression) {
> +                    li.load();
> +                }
>               }
>               if (inCondSwitchExpression) {
>                   CondItem value = genCond(tree.value, CRT_FLOW_TARGET);
> @@ -1732,7 +1739,17 @@
>               } else {
>                   genExpr(tree.value, pt).load();
>                   code.state.forceStackTop(tree.target.type);
> -                targetEnv = unwindBreak(tree);
> +
> +                int prevLetStackPos =
> code.setLetExprStackPos(code.state.stacksize);
> +                boolean prevValueBreak = code.setValueBreak(true);
> +                try {
> +                    targetEnv = unwindBreak(tree);
> +                }
> +                finally {
> +                    code.setLetExprStackPos(prevLetStackPos);
> +                    code.setValueBreak(prevValueBreak);
> +                }
> +
>                   targetEnv.info.addExit(code.branch(goto_));
>               }
>           } else {
>