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

[B. Blaser]

On Mon, 25 Mar 2019 at 19:33, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>
> I've updated the patch with a version that strives to handle the boolean
> switch expressions. Passes all tests except
> tools/javac/switchexpr/CRT.java, which depends on the exact structure of
> the generated code. Among other things, I'd like to see if we could
> generate the current bytecode when there is no try-finally, which should
> help with the CRT.java test. More testing still needed, though.
>
> Jan

It seems we had the same problem with tests like
'switchexpr/ExpressionSwitchEmbedding.java'. You solved it with
'code.newRegSegment()' but you still allocate one temporary variable
per 'break'.

The following patch based on yours create only one temporary variable
per switch expression and preserves roughly the same byte-code without
'try-finally' making tests like 'switchexpr/CRT.java' succeed.

So, maybe you'll have an even better solution but 'langtools:tier1'
along with the test you provided are OK.

What do you think?

Thanks,
Bernard

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
@@ -165,6 +165,7 @@
     Chain switchExpressionTrueChain;
     Chain switchExpressionFalseChain;
     List<LocalItem> stackBeforeSwitchExpression;
+    LocalItem switchResult;

     /** Generate code to load an integer constant.
      *  @param n     The integer to be loaded.
@@ -1178,9 +1179,11 @@

     private void doHandleSwitchExpression(JCSwitchExpression tree) {
         List<LocalItem> prevStackBeforeSwitchExpression =
stackBeforeSwitchExpression;
+        LocalItem prevSwitchResult = switchResult;
         int limit = code.nextreg;
         try {
             stackBeforeSwitchExpression = List.nil();
+            switchResult = null;
             if (hasTry(tree)) {
                 //if the switch expression contains try-catch, the
catch handlers need to have
                 //an empty stack. So stash whole stack to local
variables, and restore it before
@@ -1199,6 +1202,7 @@
                     stackBeforeSwitchExpression =
stackBeforeSwitchExpression.prepend(item);
                     item.store();
                 }
+                switchResult = makeTemp(tree.type);
             }
             int prevLetExprStart =
code.setLetExprStackPos(code.state.stacksize);
             try {
@@ -1208,6 +1212,7 @@
             }
         } finally {
             stackBeforeSwitchExpression = prevStackBeforeSwitchExpression;
+            switchResult = prevSwitchResult;
             code.endScopes(limit);
         }
     }
@@ -1707,16 +1712,22 @@
         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 (inCondSwitchExpression) {
                 CondItem value = genCond(tree.value, CRT_FLOW_TARGET);
                 Chain falseJumps = value.jumpFalse();
-                targetEnv = unwindBreak(tree);
+                Env<GenContext> localEnv = unwindBreak(tree);
                 code.resolve(value.trueJumps);
+                for (LocalItem li : stackBeforeSwitchExpression) {
+                   li.load();
+                }
                 Chain trueJumps = code.branch(goto_);
+                endFinalizerGaps(env, localEnv);
+                code.resolve(falseJumps);
+                targetEnv = unwindBreak(tree);
+                for (LocalItem li : stackBeforeSwitchExpression) {
+                    li.load();
+                }
+                falseJumps = code.branch(goto_);
                 if (switchExpressionTrueChain == null) {
                     switchExpressionTrueChain = trueJumps;
                 } else {
@@ -1731,9 +1742,17 @@
                 }
             } else {
                 genExpr(tree.value, pt).load();
-                code.state.forceStackTop(tree.target.type);
+                if (switchResult != null) switchResult.store();
                 targetEnv = unwindBreak(tree);
-                targetEnv.info.addExit(code.branch(goto_));
+                if (code.isAlive()) {
+                    for (LocalItem li : stackBeforeSwitchExpression) {
+                        li.load();
+                    }
+                    if (switchResult != null) switchResult.load();
+                    code.state.forceStackTop(tree.target.type);
+                    targetEnv.info.addExit(code.branch(goto_));
+                    code.markDead();
+                }
             }
         } else {
             targetEnv = unwindBreak(tree);