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

[B. Blaser]

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 {