diff options
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/icode/GenICode.scala | 75 | ||||
-rw-r--r-- | test/files/jvm/t7181/Foo_1.scala | 26 | ||||
-rw-r--r-- | test/files/jvm/t7181/Test.scala | 24 | ||||
-rw-r--r-- | test/files/run/inline-ex-handlers.check | 150 | ||||
-rw-r--r-- | test/files/run/t7181.check | 23 | ||||
-rw-r--r-- | test/files/run/t7181.scala | 78 |
6 files changed, 279 insertions, 97 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala index 122972039b..5438fd8590 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala @@ -1921,15 +1921,46 @@ abstract class GenICode extends SubComponent { * }), (AnotherExceptionClass, * ctx => {... * } ))` + * + * The resulting structure will look something like + * + * outer: + * // this 'useless' jump will be removed later, + * // for now it separates the try body's blocks from previous + * // code since the try body needs its own exception handlers + * JUMP body + * + * body: + * [ try body ] + * JUMP normalExit + * + * catch[i]: + * [ handler[i] body ] + * JUMP normalExit + * + * catchAll: + * STORE exception + * [ finally body ] + * THROW exception + * + * normalExit: + * [ finally body ] + * + * each catch[i] will cover body. catchAll will cover both body and each catch[i] + * Additional finally copies are created on the emission of every RETURN in the try body and exception handlers. + * + * This could result in unreachable code which has to be cleaned up later, e.g. if the try and all the exception + * handlers always end in RETURN then there will be no "normal" flow out of the try/catch/finally. + * Later reachability analysis will remove unreacahble code. */ def Try(body: Context => Context, handlers: List[(Symbol, TypeKind, Context => Context)], finalizer: Tree, tree: Tree) = { - val outerCtx = this.dup // context for generating exception handlers, covered by finalizer + val outerCtx = this.dup // context for generating exception handlers, covered by the catch-all finalizer val finalizerCtx = this.dup // context for generating finalizer handler - val afterCtx = outerCtx.newBlock() + val normalExitCtx = outerCtx.newBlock() // context where flow will go on a "normal" (non-return, non-throw) exit from a try or catch handler var tmp: Local = null val kind = toTypeKind(tree.tpe) val guardResult = kind != UNIT && mayCleanStack(finalizer) @@ -1956,30 +1987,31 @@ abstract class GenICode extends SubComponent { } else ctx + // Generate the catch-all exception handler that deals with uncaught exceptions coming + // from the try or exception handlers. It catches the exception, runs the finally code, then rethrows + // the exception if (finalizer != EmptyTree) { val exh = outerCtx.newExceptionHandler(NoSymbol, finalizer.pos) // finalizer covers exception handlers this.addActiveHandler(exh) // .. and body aswell - val ctx = finalizerCtx.enterExceptionHandler(exh) - val exception = ctx.makeLocal(finalizer.pos, ThrowableClass.tpe, "exc") - loadException(ctx, exh, finalizer.pos) - ctx.bb.emit(STORE_LOCAL(exception)) - val ctx1 = genLoad(finalizer, ctx, UNIT) - ctx1.bb.emit(LOAD_LOCAL(exception)) - ctx1.bb.emit(THROW(ThrowableClass)) - ctx1.bb.enterIgnoreMode() - ctx1.bb.close() + val exhStartCtx = finalizerCtx.enterExceptionHandler(exh) + val exception = exhStartCtx.makeLocal(finalizer.pos, ThrowableClass.tpe, "exc") + loadException(exhStartCtx, exh, finalizer.pos) + exhStartCtx.bb.emit(STORE_LOCAL(exception)) + val exhEndCtx = genLoad(finalizer, exhStartCtx, UNIT) + exhEndCtx.bb.emit(LOAD_LOCAL(exception)) + exhEndCtx.bb.closeWith(THROW(ThrowableClass)) + exhEndCtx.bb.enterIgnoreMode() finalizerCtx.endHandler() } + // Generate each exception handler for ((sym, kind, handler) <- handlers) { val exh = this.newExceptionHandler(sym, tree.pos) - var ctx1 = outerCtx.enterExceptionHandler(exh) - ctx1.addFinalizer(finalizer, finalizerCtx) - loadException(ctx1, exh, tree.pos) - ctx1 = handler(ctx1) - // emit finalizer - val ctx2 = emitFinalizer(ctx1) - ctx2.bb.closeWith(JUMP(afterCtx.bb)) + val exhStartCtx = outerCtx.enterExceptionHandler(exh) + exhStartCtx.addFinalizer(finalizer, finalizerCtx) + loadException(exhStartCtx, exh, tree.pos) + val exhEndCtx = handler(exhStartCtx) + exhEndCtx.bb.closeWith(JUMP(normalExitCtx.bb)) outerCtx.endHandler() } @@ -1987,14 +2019,13 @@ abstract class GenICode extends SubComponent { if (finalizer != EmptyTree) bodyCtx.addFinalizer(finalizer, finalizerCtx) - var finalCtx = body(bodyCtx) - finalCtx = emitFinalizer(finalCtx) + val bodyEndCtx = body(bodyCtx) outerCtx.bb.closeWith(JUMP(bodyCtx.bb)) - finalCtx.bb.closeWith(JUMP(afterCtx.bb)) + bodyEndCtx.bb.closeWith(JUMP(normalExitCtx.bb)) - afterCtx + emitFinalizer(normalExitCtx) } } } diff --git a/test/files/jvm/t7181/Foo_1.scala b/test/files/jvm/t7181/Foo_1.scala new file mode 100644 index 0000000000..f9dfdd4442 --- /dev/null +++ b/test/files/jvm/t7181/Foo_1.scala @@ -0,0 +1,26 @@ +class Exception1 extends RuntimeException +class Exception2 extends RuntimeException + +class Foo_1 { + def foo(baz: Baz) { + try { + baz.bar + } catch { + case _: Exception1 => println("exception 1") + case _: Exception2 => println("exception 2") + } finally { + // this should be the only copy of the magic constant 3 + // making it easy to detect copies of this finally block + println(s"finally ${3}") + } + println(s"normal flow") + } +} + +trait Baz { + // does it throw? who knows? This way + // I can ensure that no optimization that honors + // separate compilation could ever + // change the exception handling structure + def bar: Unit +} diff --git a/test/files/jvm/t7181/Test.scala b/test/files/jvm/t7181/Test.scala new file mode 100644 index 0000000000..35dba436c1 --- /dev/null +++ b/test/files/jvm/t7181/Test.scala @@ -0,0 +1,24 @@ +import scala.tools.partest.BytecodeTest +import scala.tools.asm +import asm.tree.InsnList +import scala.collection.JavaConverters._ + +object Test extends BytecodeTest { + def show: Unit = { + val classNode = loadClassNode("Foo_1") + val methodNode = getMethod(classNode, "foo") + // there should be 2 copies of the finally block, each with the magic constant 3 + // one for the "normal" exit + // one for the uncaught exception exit + // prior to this PR there would have been 4 since each exception handler would also get a copy + val expected = 2 + val got = countMagicThrees(methodNode.instructions) + assert(got == expected, s"expected $expected but got $got magic threes") + } + + def countMagicThrees(insnList: InsnList): Int = { + def isMagicThree(node: asm.tree.AbstractInsnNode): Boolean = + (node.getOpcode == asm.Opcodes.ICONST_3) + insnList.iterator.asScala.count(isMagicThree) + } +} diff --git a/test/files/run/inline-ex-handlers.check b/test/files/run/inline-ex-handlers.check index f2f0b60687..0a234e2659 100644 --- a/test/files/run/inline-ex-handlers.check +++ b/test/files/run/inline-ex-handlers.check @@ -107,27 +107,27 @@ --- > catch (Throwable) in ArrayBuffer(4, 6, 7, 8, 9, 10, 11, 12) starting at: 3 619c642 -< blocks: [1,2,3,4,5,6,7,9,10] +< blocks: [1,3,4,5,6,8,9] --- -> blocks: [1,2,3,4,5,6,7,9,10,11,12] +> blocks: [1,3,4,5,6,8,9,10,11] 643c666,667 < 78 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) -> ? JUMP 11 +> ? JUMP 10 644a669,673 -> 11: +> 10: > 81 LOAD_LOCAL(value e) > ? STORE_LOCAL(variable exc1) -> ? JUMP 12 +> ? JUMP 11 > -672c701,702 +669c698,699 < 81 THROW(Exception) --- > ? STORE_LOCAL(variable exc1) -> ? JUMP 12 -688a719,731 -> 12: +> ? JUMP 11 +685a716,728 +> 11: > 83 LOAD_MODULE object Predef > 83 CONSTANT("finally") > 83 CALL_METHOD scala.Predef.println (dynamic) @@ -140,88 +140,88 @@ > 84 LOAD_LOCAL(variable exc1) > 84 THROW(Throwable) > -694c737 -< catch (<none>) in ArrayBuffer(4, 6, 7, 9) starting at: 3 +691c734 +< catch (<none>) in ArrayBuffer(4, 5, 6, 8) starting at: 3 --- -> catch (<none>) in ArrayBuffer(4, 6, 7, 9, 11) starting at: 3 -718c761 +> catch (<none>) in ArrayBuffer(4, 5, 6, 8, 10) starting at: 3 +715c758 < locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value message, value x, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value x, value ex6, value x4, value x5, value x -720c763 -< blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25] +717c760 +< blocks: [1,3,4,5,6,9,13,14,15,18,20,21,23,24] --- -> blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25,26,27,28] -744c787,794 +> blocks: [1,3,4,5,6,9,13,14,15,18,20,21,23,24,25,26,27] +741c784,791 < 172 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 26 +> ? JUMP 25 > -> 26: +> 25: > 170 LOAD_LOCAL(value ex6) > 170 STORE_LOCAL(value x4) > 170 SCOPE_ENTER value x4 -> 170 JUMP 15 -787,790d836 +> 170 JUMP 14 +781,784d830 < 175 LOAD_LOCAL(value x5) < 175 CALL_METHOD MyException.message (dynamic) < 175 STORE_LOCAL(value message) < 175 SCOPE_ENTER value message -792c838,839 +786c832,833 < 176 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 176 CALL_METHOD MyException.message (dynamic) -796c843,844 +790c837,838 < 177 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 177 CALL_METHOD MyException.message (dynamic) -798c846,847 +792c840,841 < 177 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 27 -802c851,852 +> ? JUMP 26 +796c845,846 < 170 THROW(Throwable) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 27 -811a862,867 -> 27: +> ? JUMP 26 +805a856,861 +> 26: > 169 LOAD_LOCAL(value ex6) > 169 STORE_LOCAL(value x4) > 169 SCOPE_ENTER value x4 > 169 JUMP 5 > -822,825d877 +816,819d871 < 180 LOAD_LOCAL(value x5) < 180 CALL_METHOD MyException.message (dynamic) < 180 STORE_LOCAL(value message) < 180 SCOPE_ENTER value message -827c879,880 +821c873,874 < 181 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 181 CALL_METHOD MyException.message (dynamic) -831c884,885 +825c878,879 < 182 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 182 CALL_METHOD MyException.message (dynamic) -833c887,888 +827c881,882 < 182 THROW(MyException) --- > ? STORE_LOCAL(variable exc2) -> ? JUMP 28 -837c892,893 +> ? JUMP 27 +831c886,887 < 169 THROW(Throwable) --- > ? STORE_LOCAL(variable exc2) -> ? JUMP 28 -853a910,922 -> 28: +> ? JUMP 27 +847a904,916 +> 27: > 184 LOAD_MODULE object Predef > 184 CONSTANT("finally") > 184 CALL_METHOD scala.Predef.println (dynamic) @@ -234,23 +234,23 @@ > 185 LOAD_LOCAL(variable exc2) > 185 THROW(Throwable) > -859c928 -< catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24) starting at: 4 +853c922 +< catch (Throwable) in ArrayBuffer(13, 14, 15, 18, 20, 21, 23) starting at: 4 --- -> catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24, 26) starting at: 4 -862c931 -< catch (<none>) in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24) starting at: 3 +> catch (Throwable) in ArrayBuffer(13, 14, 15, 18, 20, 21, 23, 25) starting at: 4 +856c925 +< catch (<none>) in ArrayBuffer(4, 5, 6, 9, 13, 14, 15, 18, 20, 21, 23) starting at: 3 --- -> catch (<none>) in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24, 26, 27) starting at: 3 -886c955 +> catch (<none>) in ArrayBuffer(4, 5, 6, 9, 13, 14, 15, 18, 20, 21, 23, 25, 26) starting at: 3 +880c949 < locals: value args, variable result, value e, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value e, value ex6, value x4, value x5, value x -888c957 +882c951 < blocks: [1,2,3,6,7,8,11,13,14,16] --- > blocks: [1,2,3,6,7,8,11,13,14,16,17] -912c981,988 +906c975,982 < 124 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -261,29 +261,29 @@ > 122 STORE_LOCAL(value x4) > 122 SCOPE_ENTER value x4 > 122 JUMP 7 -937,940d1012 +931,934d1006 < 127 LOAD_LOCAL(value x5) < 127 CALL_METHOD MyException.message (dynamic) < 127 STORE_LOCAL(value message) < 127 SCOPE_ENTER value message -942c1014,1015 +936c1008,1009 < 127 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 127 CALL_METHOD MyException.message (dynamic) -971c1044 +965c1038 < catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16) starting at: 3 --- > catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16, 17) starting at: 3 -995c1068 +989c1062 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x, value e --- > locals: value args, variable result, value ex6, value x4, value x5, value x, value e -997c1070 +991c1064 < blocks: [1,2,3,4,5,8,12,13,14,16] --- > blocks: [1,2,3,5,8,12,13,14,16,17] -1021c1094,1103 +1015c1088,1097 < 148 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -296,25 +296,25 @@ > 154 LOAD_LOCAL(value x4) > 154 IS_INSTANCE REF(class MyException) > 154 CZJUMP (BOOL)NE ? 5 : 8 -1042,1044d1123 +1036,1038d1117 < 145 JUMP 4 < < 4: -1054,1057d1132 +1048,1051d1126 < 154 LOAD_LOCAL(value x5) < 154 CALL_METHOD MyException.message (dynamic) < 154 STORE_LOCAL(value message) < 154 SCOPE_ENTER value message -1059c1134,1135 +1053c1128,1129 < 154 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 154 CALL_METHOD MyException.message (dynamic) -1276c1352 +1270c1346 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1300c1376,1383 +1294c1370,1377 < 38 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) @@ -325,20 +325,20 @@ > 42 CONSTANT("IllegalArgumentException") > 42 CALL_METHOD scala.Predef.println (dynamic) > 42 JUMP 2 -1347c1430 +1341c1424 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, value x4, value x5, value x -1349c1432 +1343c1426 < blocks: [1,2,3,4,5,8,10,11,13,14,16] --- > blocks: [1,2,3,5,8,10,11,13,14,16,17] -1373c1456,1457 +1367c1450,1451 < 203 THROW(MyException) --- > ? STORE_LOCAL(value ex6) > ? JUMP 17 -1393c1477,1486 +1387c1471,1480 < 209 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -351,41 +351,41 @@ > 212 LOAD_LOCAL(value x4) > 212 IS_INSTANCE REF(class MyException) > 212 CZJUMP (BOOL)NE ? 5 : 8 -1406,1408d1498 +1400,1402d1492 < 200 JUMP 4 < < 4: -1418,1421d1507 +1412,1415d1501 < 212 LOAD_LOCAL(value x5) < 212 CALL_METHOD MyException.message (dynamic) < 212 STORE_LOCAL(value message) < 212 SCOPE_ENTER value message -1423c1509,1510 +1417c1503,1504 < 213 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 213 CALL_METHOD MyException.message (dynamic) -1467c1554 +1461c1548 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1491c1578,1579 +1485c1572,1573 < 58 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) > ? JUMP 8 -1492a1581,1586 +1486a1575,1580 > 8: > 62 LOAD_MODULE object Predef > 62 CONSTANT("RuntimeException") > 62 CALL_METHOD scala.Predef.println (dynamic) > 62 JUMP 2 > -1540c1634 +1534c1628 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1560c1654,1659 +1554c1648,1653 < 229 THROW(MyException) --- > ? JUMP 5 @@ -394,19 +394,19 @@ > ? LOAD_LOCAL(variable monitor1) > 228 MONITOR_EXIT > 228 THROW(Throwable) -1566c1665 +1560c1659 < ? THROW(Throwable) --- > 228 THROW(Throwable) -1594c1693 +1588c1687 < locals: value args, variable result, variable monitor2, variable monitorResult1 --- > locals: value exception$1, value args, variable result, variable monitor2, variable monitorResult1 -1596c1695 +1590c1689 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1619c1718,1726 +1613c1712,1720 < 245 THROW(MyException) --- > ? STORE_LOCAL(value exception$1) @@ -418,7 +418,7 @@ > ? LOAD_LOCAL(variable monitor2) > 244 MONITOR_EXIT > 244 THROW(Throwable) -1625c1732 +1619c1726 < ? THROW(Throwable) --- > 244 THROW(Throwable) diff --git a/test/files/run/t7181.check b/test/files/run/t7181.check new file mode 100644 index 0000000000..e4b8e30dfe --- /dev/null +++ b/test/files/run/t7181.check @@ -0,0 +1,23 @@ +normal exit MainNormalExit +finally MainNormalExit +normal flow MainNormalExit + +return MainReturn +finally MainReturn + +uncaught exception MainUncaughtException +finally MainUncaughtException + +caught exception ExceptionNormalExit +normal exit ExceptionNormalExit +finally ExceptionNormalExit +normal flow ExceptionNormalExit + +caught exception ExceptionReturn +return ExceptionReturn +finally ExceptionReturn + +caught exception ExceptionUncaughtException +uncaught exception ExceptionUncaughtException +finally ExceptionUncaughtException + diff --git a/test/files/run/t7181.scala b/test/files/run/t7181.scala new file mode 100644 index 0000000000..a055e43481 --- /dev/null +++ b/test/files/run/t7181.scala @@ -0,0 +1,78 @@ +sealed abstract class Action +// exit the try body normally +case object MainNormalExit extends Action +// exit the try body with a 'return' +case object MainReturn extends Action +// exit the try body with an uncaught exception +case object MainUncaughtException extends Action +// exit the try body with a caught exception and exit the exception handler normally +case object ExceptionNormalExit extends Action +// exit the try body with a caught exception and exit the exception handler with a 'return' +case object ExceptionReturn extends Action +// exit the try body with a caught exception and exit the exception handler with an uncaught exception +case object ExceptionUncaughtException extends Action + +case class UncaughtException(action: Action) extends RuntimeException +case class CaughtException(action: Action) extends RuntimeException + +object Test extends App { + def test(action: Action, expectException: Boolean = false) { + var gotException = false + val result = try + driver(action) + catch { + case UncaughtException(a) => + gotException = true + a + } + if (gotException) assert(expectException, "Got unexpected exception") + else assert(!expectException, "Did not get expected exception") + + assert(result == action, s"Expected $action but got $result") + println() + } + + def driver(action: Action): Action = { + val result = try { + action match { + case MainNormalExit => + println(s"normal exit $action") + action + case MainReturn => + println(s"return $action") + return action + case MainUncaughtException => + println(s"uncaught exception $action") + throw UncaughtException(action) + case _ => + println(s"caught exception $action") + throw CaughtException(action) + } + } catch { + case CaughtException(action) => action match { + case ExceptionNormalExit => + println(s"normal exit $action") + action + case ExceptionReturn => + println(s"return $action") + return action + case ExceptionUncaughtException => + println(s"uncaught exception $action") + throw UncaughtException(action) + case _ => + sys.error(s"unexpected $action in exception handler") + } + } finally { + println(s"finally $action") + } + println(s"normal flow $action") + result + } + + test(MainNormalExit) + test(MainReturn) + test(MainUncaughtException, true) + test(ExceptionNormalExit) + test(ExceptionReturn) + test(ExceptionUncaughtException, true) +} |