8024039: javac, previous solution for JDK-8022186 was incorrect

Reviewed-by: jjg

src/share/classes/com/sun/tools/javac/comp/Lower.java

@@ -49,7 +49,6 @@ import static com.sun.tools.javac.code.Kinds.*;
 import static com.sun.tools.javac.code.TypeTag.*;
 import static com.sun.tools.javac.jvm.ByteCodes.*;
 import static com.sun.tools.javac.tree.JCTree.Tag.*;
-import javax.lang.model.type.TypeKind;
 
 /** This pass translates away some syntactic sugar: inner classes,
  *  class literals, assertions, foreach loops, etc.
@@ -3830,15 +3829,26 @@ public class Lower extends TreeTranslator {
 
     @Override
     public void visitTry(JCTry tree) {
-        /* special case of try without catchers and with finally emtpy.
-         * Don't give it a try, translate only the body.
-         */
         if (tree.resources.isEmpty()) {
+            /* special case of try without catchers and with finally emtpy.
+             * Don't give it a try, translate only the body.
+             */
             if (tree.catchers.isEmpty() &&
                 tree.finalizer.getStatements().isEmpty()) {
                 result = translate(tree.body);
             } else {
-                super.visitTry(tree);
+                /* also if the body is empty we only need to generate the finalizer
+                 * provided that it's not empty.
+                 */
+                if (tree.body.getStatements().isEmpty()) {
+                    if (tree.finalizer.getStatements().isEmpty()) {
+                        result = translate(tree.body);
+                    } else {
+                        result = translate(tree.finalizer);
+                    }
+                } else {
+                    super.visitTry(tree);
+                }
             }
         } else {
             result = makeTwrTry(tree);

src/share/classes/com/sun/tools/javac/jvm/Gen.java

@@ -1478,82 +1478,74 @@ public class Gen extends JCTree.Visitor {
             code.statBegin(TreeInfo.endPos(body));
             genFinalizer(env);
             code.statBegin(TreeInfo.endPos(env.tree));
-            Chain exitChain;
-            if (startpc != endpc) {
-                exitChain = code.branch(goto_);
-            } else {
-                exitChain = code.branch(dontgoto);
-            }
+            Chain exitChain = code.branch(goto_);
             endFinalizerGap(env);
-            if (startpc != endpc) {
-                for (List<JCCatch> l = catchers; l.nonEmpty(); l = l.tail) {
-                    // start off with exception on stack
-                    code.entryPoint(stateTry, l.head.param.sym.type);
-                    genCatch(l.head, env, startpc, endpc, gaps);
-                    genFinalizer(env);
-                    if (hasFinalizer || l.tail.nonEmpty()) {
-                        code.statBegin(TreeInfo.endPos(env.tree));
-                        exitChain = Code.mergeChains(exitChain,
-                                                     code.branch(goto_));
-                    }
-                    endFinalizerGap(env);
+            if (startpc != endpc) for (List<JCCatch> l = catchers; l.nonEmpty(); l = l.tail) {
+                // start off with exception on stack
+                code.entryPoint(stateTry, l.head.param.sym.type);
+                genCatch(l.head, env, startpc, endpc, gaps);
+                genFinalizer(env);
+                if (hasFinalizer || l.tail.nonEmpty()) {
+                    code.statBegin(TreeInfo.endPos(env.tree));
+                    exitChain = Code.mergeChains(exitChain,
+                                                 code.branch(goto_));
                 }
+                endFinalizerGap(env);
+            }
+            if (hasFinalizer) {
+                // Create a new register segement to avoid allocating
+                // the same variables in finalizers and other statements.
+                code.newRegSegment();
+
+                // Add a catch-all clause.
+
+                // start off with exception on stack
+                int catchallpc = code.entryPoint(stateTry, syms.throwableType);
+
+                // Register all exception ranges for catch all clause.
+                // The range of the catch all clause is from the beginning
+                // of the try or synchronized block until the present
+                // code pointer excluding all gaps in the current
+                // environment's GenContext.
+                int startseg = startpc;
+                while (env.info.gaps.nonEmpty()) {
+                    int endseg = env.info.gaps.next().intValue();
+                    registerCatch(body.pos(), startseg, endseg,
+                                  catchallpc, 0);
+                    startseg = env.info.gaps.next().intValue();
+                }
+                code.statBegin(TreeInfo.finalizerPos(env.tree));
+                code.markStatBegin();
 
-                if (hasFinalizer) {
-                    // Create a new register segement to avoid allocating
-                    // the same variables in finalizers and other statements.
-                    code.newRegSegment();
-
-                    // Add a catch-all clause.
-
-                    // start off with exception on stack
-                    int catchallpc = code.entryPoint(stateTry, syms.throwableType);
-
-                    // Register all exception ranges for catch all clause.
-                    // The range of the catch all clause is from the beginning
-                    // of the try or synchronized block until the present
-                    // code pointer excluding all gaps in the current
-                    // environment's GenContext.
-                    int startseg = startpc;
-                    while (env.info.gaps.nonEmpty()) {
-                        int endseg = env.info.gaps.next().intValue();
-                        registerCatch(body.pos(), startseg, endseg,
-                                      catchallpc, 0);
-                        startseg = env.info.gaps.next().intValue();
-                    }
+                Item excVar = makeTemp(syms.throwableType);
+                excVar.store();
+                genFinalizer(env);
+                excVar.load();
+                registerCatch(body.pos(), startseg,
+                              env.info.gaps.next().intValue(),
+                              catchallpc, 0);
+                code.emitop0(athrow);
+                code.markDead();
+
+                // If there are jsr's to this finalizer, ...
+                if (env.info.cont != null) {
+                    // Resolve all jsr's.
+                    code.resolve(env.info.cont);
+
+                    // Mark statement line number
                     code.statBegin(TreeInfo.finalizerPos(env.tree));
                     code.markStatBegin();
 
-                    Item excVar = makeTemp(syms.throwableType);
-                    excVar.store();
-                    genFinalizer(env);
-                    excVar.load();
-                    registerCatch(body.pos(), startseg,
-                                  env.info.gaps.next().intValue(),
-                                  catchallpc, 0);
-                    code.emitop0(athrow);
-                    code.markDead();
-
-                    // If there are jsr's to this finalizer, ...
-                    if (env.info.cont != null) {
-                        // Resolve all jsr's.
-                        code.resolve(env.info.cont);
+                    // Save return address.
+                    LocalItem retVar = makeTemp(syms.throwableType);
+                    retVar.store();
 
-                        // Mark statement line number
-                        code.statBegin(TreeInfo.finalizerPos(env.tree));
-                        code.markStatBegin();
+                    // Generate finalizer code.
+                    env.info.finalize.genLast();
 
-                        // Save return address.
-                        LocalItem retVar = makeTemp(syms.throwableType);
-                        retVar.store();
-
-                        // Generate finalizer code.
-                        env.info.finalize.genLast();
-
-                        // Return.
-                        code.emitop1w(ret, retVar.reg);
-                        code.markDead();
-                    }
+                    // Return.
+                    code.emitop1w(ret, retVar.reg);
+                    code.markDead();
                 }
             }
             // Resolve all breaks.

test/tools/javac/T8024039/NoDeadCodeGenerationOnTrySmtTest.java

+/*
+ * Copyright (c) 2013, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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 8024039
+ * @summary javac, previous solution for JDK-8022186 was incorrect
+ * @library /tools/javac/lib
+ * @build ToolBox
+ * @run main NoDeadCodeGenerationOnTrySmtTest
+ */
+
+import java.io.File;
+import java.nio.file.Paths;
+
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.Code_attribute;
+import com.sun.tools.classfile.Code_attribute.Exception_data;
+import com.sun.tools.classfile.Method;
+import com.sun.tools.javac.util.Assert;
+
+public class NoDeadCodeGenerationOnTrySmtTest {
+
+    static final String testSource =
+        "public class Test {\n" +
+        "    void m1(int arg) {\n" +
+        "        synchronized (new Integer(arg)) {\n" +
+        "            {\n" +
+        "                label0:\n" +
+        "                do {\n" +
+        "                    break label0;\n" +
+        "                } while (arg != 0);\n" +
+        "            }\n" +
+        "        }\n" +
+        "    }\n" +
+
+        "    void m2(int arg) {\n" +
+        "        synchronized (new Integer(arg)) {\n" +
+        "            {\n" +
+        "                label0:\n" +
+        "                {\n" +
+        "                    break label0;\n" +
+        "                }\n" +
+        "            }\n" +
+        "        }\n" +
+        "    }\n" +
+        "}";
+
+    static final int[][] expectedExceptionTable = {
+    //  {from,         to,         target,      type},
+        {11,           13,         16,          0},
+        {16,           19,         16,          0}
+    };
+
+    static final String[] methodsToLookFor = {"m1", "m2"};
+
+    public static void main(String[] args) throws Exception {
+        new NoDeadCodeGenerationOnTrySmtTest().run();
+    }
+
+    void run() throws Exception {
+        compileTestClass();
+        checkClassFile(new File(Paths.get(System.getProperty("user.dir"),
+                "Test.class").toUri()), methodsToLookFor);
+    }
+
+    void compileTestClass() throws Exception {
+        ToolBox.JavaToolArgs javacSuccessArgs =
+                new ToolBox.JavaToolArgs().setSources(testSource);
+        ToolBox.javac(javacSuccessArgs);
+    }
+
+    void checkClassFile(final File cfile, String[] methodsToFind) throws Exception {
+        ClassFile classFile = ClassFile.read(cfile);
+        int numberOfmethodsFound = 0;
+        for (String methodToFind : methodsToFind) {
+            for (Method method : classFile.methods) {
+                if (method.getName(classFile.constant_pool).equals(methodToFind)) {
+                    numberOfmethodsFound++;
+                    Code_attribute code = (Code_attribute) method.attributes.get("Code");
+                    Assert.check(code.exception_table_langth == expectedExceptionTable.length,
+                            "The ExceptionTable found has a length different to the expected one");
+                    int i = 0;
+                    for (Exception_data entry: code.exception_table) {
+                        Assert.check(entry.start_pc == expectedExceptionTable[i][0] &&
+                                entry.end_pc == expectedExceptionTable[i][1] &&
+                                entry.handler_pc == expectedExceptionTable[i][2] &&
+                                entry.catch_type == expectedExceptionTable[i][3],
+                                "Exception table entry at pos " + i + " differ from expected.");
+                        i++;
+                    }
+                }
+            }
+        }
+        Assert.check(numberOfmethodsFound == 2, "Some seek methods were not found");
+    }
+
+    void error(String msg) {
+        throw new AssertionError(msg);
+    }
+
+}