changeset 55840:ef577fa0dd10

8220018: javac crash when compiling try-catch-finally inside switch expression Summary: Correcting handling of finally that yields from a switch expression. Reviewed-by: mcimadamore, jlahoda
author jlahoda
date Wed, 12 Jun 2019 13:49:43 +0200
parents 0530705ca300
children c63b9b87c28a
files src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java test/langtools/tools/javac/switchexpr/TryCatchFinally.java
diffstat 2 files changed, 349 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java	Wed Jun 12 13:21:25 2019 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java	Wed Jun 12 13:49:43 2019 +0200
@@ -166,6 +166,7 @@
     Chain switchExpressionTrueChain;
     Chain switchExpressionFalseChain;
     List<LocalItem> stackBeforeSwitchExpression;
+    LocalItem switchResult;
 
     /** Generate code to load an integer constant.
      *  @param n     The integer to be loaded.
@@ -1190,9 +1191,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
@@ -1211,6 +1214,7 @@
                     stackBeforeSwitchExpression = stackBeforeSwitchExpression.prepend(item);
                     item.store();
                 }
+                switchResult = makeTemp(tree.type);
             }
             int prevLetExprStart = code.setLetExprStackPos(code.state.stacksize);
             try {
@@ -1220,6 +1224,7 @@
             }
         } finally {
             stackBeforeSwitchExpression = prevStackBeforeSwitchExpression;
+            switchResult = prevSwitchResult;
             code.endScopes(limit);
         }
     }
@@ -1725,16 +1730,22 @@
     public void visitYield(JCYield tree) {
         Assert.check(code.isStatementStart());
         final Env<GenContext> targetEnv;
-        //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();
+
+            code.resolve(value.trueJumps);
+            Env<GenContext> localEnv = unwindBreak(tree.target);
+            reloadStackBeforeSwitchExpr();
+            Chain trueJumps = code.branch(goto_);
+
+            endFinalizerGaps(env, localEnv);
+
+            code.resolve(falseJumps);
             targetEnv = unwindBreak(tree.target);
-            code.resolve(value.trueJumps);
-            Chain trueJumps = code.branch(goto_);
+            reloadStackBeforeSwitchExpr();
+            falseJumps = code.branch(goto_);
+
             if (switchExpressionTrueChain == null) {
                 switchExpressionTrueChain = trueJumps;
             } else {
@@ -1749,13 +1760,26 @@
             }
         } else {
             genExpr(tree.value, pt).load();
-            code.state.forceStackTop(tree.target.type);
+            if (switchResult != null)
+                switchResult.store();
+
             targetEnv = unwindBreak(tree.target);
-            targetEnv.info.addExit(code.branch(goto_));
+
+            if (code.isAlive()) {
+                reloadStackBeforeSwitchExpr();
+                if (switchResult != null)
+                    switchResult.load();
+
+                code.state.forceStackTop(tree.target.type);
+                targetEnv.info.addExit(code.branch(goto_));
+                code.markDead();
+            }
         }
         endFinalizerGaps(env, targetEnv);
     }
     //where:
+        /** As side-effect, might mark code as dead disabling any further emission.
+         */
         private Env<GenContext> unwindBreak(JCTree target) {
             int tmpPos = code.pendingStatPos;
             Env<GenContext> targetEnv = unwind(target, env);
@@ -1763,6 +1787,11 @@
             return targetEnv;
         }
 
+        private void reloadStackBeforeSwitchExpr() {
+            for (LocalItem li : stackBeforeSwitchExpression)
+                li.load();
+        }
+
     public void visitContinue(JCContinue tree) {
         int tmpPos = code.pendingStatPos;
         Env<GenContext> targetEnv = unwind(tree.target, env);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/switchexpr/TryCatchFinally.java	Wed Jun 12 13:49:43 2019 +0200
@@ -0,0 +1,312 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8220018
+ * @summary Verify that try-catch-finally inside a switch expression works properly.
+ * @compile --enable-preview -source ${jdk.version} TryCatchFinally.java
+ * @run main/othervm --enable-preview TryCatchFinally
+ */
+public class TryCatchFinally {//TODO: yield <double>
+    public static void main(String[] args) {
+        for (int p1 = 0; p1 < 2; p1++) {
+            for (int p2 = 0; p2 < 2; p2++) {
+                for (int p3 = 0; p3 < 2; p3++) {
+                    for (int p4 = 0; p4 < 2; p4++) {
+                        for (int p5 = 0; p5 < 2; p5++) {
+                            for (int p6 = 0; p6 < 3; p6++) {
+                                int actual = runSwitchesOrdinary(p1, p2, p3, p4, p5, p6);
+                                int expected = computeExpectedOrdinary(p1, p2, p3, p4, p5, p6);
+                                if (actual != expected) {
+                                    throw new IllegalStateException("actual=" + actual + "; " +
+                                                                    "expected=" + expected + ", parameters: " + p1 + ", " + p2 + ", " + p3 + ", " + p4 + ", " + p5 + ", " + p6 + ", ");
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+        {
+            boolean correct = false;
+            int v;
+            if (switch (0) {
+                case 0:
+                    try {
+                        if (true) {
+                            throw new MarkerException();
+                        }
+                        yield false;
+                    } catch (MarkerException ex) {
+                        yield false;
+                    } finally {
+                        v = 0;
+                        yield true;
+                    }
+                default: yield false;
+            } && v == 0) {
+                correct = true;
+            }
+            if (!correct) {
+                throw new IllegalStateException();
+            }
+        }
+        {
+            boolean correct = false;
+            if (switch (0) {
+                case 0:
+                    try {
+                        if (true) {
+                            throw new MarkerException();
+                        }
+                        yield new TryCatchFinally().fls();
+                    } catch (MarkerException ex) {
+                        yield new TryCatchFinally().fls();
+                    } finally {
+                        yield true;
+                    }
+                default: yield new TryCatchFinally().fls();
+            }) {
+                correct = true;
+            }
+            if (!correct) {
+                throw new IllegalStateException();
+            }
+        }
+        {
+            E e = E.A;
+            boolean correct = false;
+            int v;
+            if (switch (0) {
+                case 0:
+                    try {
+                        if (true) {
+                            throw new MarkerException();
+                        }
+                        yield false;
+                    } catch (MarkerException ex) {
+                        v = 0;
+                        yield true;
+                    } finally {
+                        try {
+                            if (true)
+                                throw new MarkerException();
+                        } catch (MarkerException ex) {
+                            e = e.next();
+                        }
+                    }
+                default: yield false;
+            } && v == 0) {
+                correct = true;
+            }
+            if (!correct) {
+                throw new IllegalStateException();
+            }
+        }
+        {
+            E e = E.A;
+            boolean correct = false;
+            int v;
+            if (switch (0) {
+                case 0:
+                    try {
+                        if (true) {
+                            throw new MarkerException();
+                        }
+                        yield false;
+                    } catch (MarkerException ex) {
+                        yield false;
+                    } finally {
+                        try {
+                            if (true)
+                                throw new MarkerException();
+                        } catch (MarkerException ex) {
+                            e = e.next();
+                        } finally {
+                            v = 0;
+                            yield true;
+                        }
+                    }
+                default: yield false;
+            } && v == 0) {
+                correct = true;
+            }
+            if (!correct) {
+                throw new IllegalStateException();
+            }
+        }
+        {
+            boolean correct = false;
+            if (!switch (0) {
+                default -> {
+                    try {
+                        yield switch(0) { default -> true; };
+                    }
+                    finally {
+                        yield false;
+                    }
+                }
+            }) {
+                correct = true;
+            }
+            if (!correct) {
+                throw new IllegalStateException();
+            }
+        }
+    }
+
+    private static int runSwitchesOrdinary(int p1, int p2, int p3, int p4, int p5, int p6) {
+        return 1 + switch (p1) {
+            case 0:
+                try {
+                    if (p2 == 0) {
+                        new TryCatchFinally().throwException();
+                    }
+                    try {
+                        yield 10 + switch (p3) {
+                            case 0 -> {
+                                try {
+                                    if (p4  == 0) {
+                                        new TryCatchFinally().throwException();
+                                    }
+                                    yield 100;
+                                } catch (Throwable ex) {
+                                    yield 200;
+                                } finally {
+                                    if (p6 == 0) {
+                                        yield 300;
+                                    } else if (p6 == 1) {
+                                        throw new MarkerException();
+                                    }
+                                }
+                            }
+                            default -> 400;
+                        };
+                    } catch (MarkerException me) {
+                        yield 510;
+                    }
+                } catch(Throwable ex) {
+                    try {
+                        yield 20 + switch (p3) {
+                            case 0 -> {
+                                try {
+                                    if (p4  == 0) {
+                                        new TryCatchFinally().throwException();
+                                    }
+                                    yield 100;
+                                } catch (Throwable ex2) {
+                                    yield 200;
+                                } finally {
+                                    if (p6 == 0) {
+                                        yield 300;
+                                    } else if (p6 == 1) {
+                                        throw new MarkerException();
+                                    }
+                                }
+                            }
+                            default -> 400;
+                        };
+                    } catch (MarkerException me) {
+                        yield 520;
+                    }
+                } finally {
+                    if (p5 == 0) {
+                        try {
+                            yield 30 + switch (p3) {
+                                case 0 -> {
+                                    try {
+                                        if (p4  == 0) {
+                                            new TryCatchFinally().throwException();
+                                        }
+                                        yield 100;
+                                    } catch (Throwable ex) {
+                                        yield 200;
+                                    } finally {
+                                        if (p6 == 0) {
+                                            yield 300;
+                                        } else if (p6 == 1) {
+                                            throw new MarkerException();
+                                        }
+                                    }
+                                }
+                                default -> 400;
+                            };
+                        } catch (MarkerException me) {
+                            yield 530;
+                        }
+                    }
+                }
+            default: yield 40;
+        };
+    }
+
+    private static int computeExpectedOrdinary(int p1, int p2, int p3, int p4, int p5, int p6) {
+        int expected = 0;
+
+        if (p1 == 0) {
+            if (p5 == 0) {
+                expected = 30;
+            } else if (p2 == 0) {
+                expected = 20;
+            } else {
+                expected = 10;
+            }
+            if (p3 == 0) {
+                if (p6 == 0) {
+                    expected += 300;
+                } else if (p6 == 1) {
+                    expected += 500;
+                } else if (p4 == 0) {
+                    expected += 200;
+                } else {
+                    expected += 100;
+                }
+            } else {
+                expected += 400;
+            }
+        } else {
+            expected = 40;
+        }
+
+        expected += 1;
+
+        return expected;
+    }
+
+    private boolean fls() {
+        return false;
+    }
+    private void throwException() {
+        throw new RuntimeException();
+    }
+
+    static class MarkerException extends Throwable {}
+
+    enum E {
+        A, B, C;
+        public E next() {
+            return values()[(ordinal() + 1) % values().length];
+        }
+    }
+}