diff options
author | Lukas Rytz <lukas.rytz@gmail.com> | 2014-08-28 15:47:07 +0200 |
---|---|---|
committer | Lukas Rytz <lukas.rytz@gmail.com> | 2014-09-10 00:05:11 +0200 |
commit | 630bd29f9e92ad26b8ce05835e2edc115633072c (patch) | |
tree | 1719a5bae79c084a545f33fd6b403c71243a4849 | |
parent | 35c53af7e3bbe19d50845e698c02a49d0a022409 (diff) | |
download | scala-630bd29f9e92ad26b8ce05835e2edc115633072c.tar.gz scala-630bd29f9e92ad26b8ce05835e2edc115633072c.tar.bz2 scala-630bd29f9e92ad26b8ce05835e2edc115633072c.zip |
Clarify why we emit ATHROW after expressions of type Nothing
Tests for emitting expressions of type Nothing.
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala | 46 | ||||
-rw-r--r-- | test/files/run/nothingTypeDce.flags | 1 | ||||
-rw-r--r-- | test/files/run/nothingTypeDce.scala | 61 | ||||
-rw-r--r-- | test/files/run/nothingTypeNoFramesNoDce.check | 1 | ||||
-rw-r--r-- | test/files/run/nothingTypeNoFramesNoDce.flags | 1 | ||||
-rw-r--r-- | test/files/run/nothingTypeNoFramesNoDce.scala | 61 | ||||
-rw-r--r-- | test/files/run/nothingTypeNoOpt.flags | 1 | ||||
-rw-r--r-- | test/files/run/nothingTypeNoOpt.scala | 61 | ||||
-rw-r--r-- | test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala | 11 | ||||
-rw-r--r-- | test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala | 45 |
10 files changed, 282 insertions, 7 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index d71d71362e..68b6f1af37 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala @@ -814,7 +814,51 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder { case _ => bc.emitT2T(from, to) } } else if (from.isNothingType) { - emit(asm.Opcodes.ATHROW) // ICode enters here into enterIgnoreMode, we'll rely instead on DCE at ClassNode level. + /* There are two possibilities for from.isNothingType: emitting a "throw e" expressions and + * loading a (phantom) value of type Nothing. + * + * The Nothing type in Scala's type system does not exist in the JVM. In bytecode, Nothing + * is mapped to scala.runtime.Nothing$. To the JVM, a call to Predef.??? looks like it would + * return an object of type Nothing$. We need to do something with that phantom object on + * the stack. "Phantom" because it never exists: such methods always throw, but the JVM does + * not know that. + * + * Note: The two verifiers (old: type inference, new: type checking) have differnet + * requirements. Very briefly: + * + * Old (http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.10.2.1): at + * each program point, no matter what branches were taken to get there + * - Stack is same size and has same typed values + * - Local and stack values need to have consistent types + * - In practice, the old verifier seems to ignore unreachable code and accept any + * instrucitons after an ATHROW. For example, there can be another ATHROW (without + * loading another throwable first). + * + * New (http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.10.1) + * - Requires consistent stack map frames. GenBCode generates stack frames if -target:jvm-1.6 + * or higher. + * - In practice: the ASM library computes stack map frames for us (ClassWriter). Emitting + * correct frames after an ATHROW is probably complex, so ASM uses the following strategy: + * - Every time when generating an ATHROW, a new basic block is started. + * - During classfile writing, such basic blocks are found to be dead: no branches go there + * - Eliminating dead code would probably require complex shifts in the output byte buffer + * - But there's an easy solution: replace all code in the dead block with with + * `nop; nop; ... nop; athrow`, making sure the bytecode size stays the same + * - The corresponding stack frame can be easily generated: on entering a dead the block, + * the frame requires a single Throwable on the stack. + * - Since there are no branches to the dead block, the frame requirements are never violated. + * + * To summarize the above: it does matter what we emit after an ATHROW. + * + * NOW: if we end up here because we emitted a load of a (phantom) value of type Nothing$, + * there was no ATHROW emitted. So, we have to make the verifier happy and do something + * with that value. Since Nothing$ extends Throwable, the easiest is to just emit an ATHROW. + * + * If we ended up here because we generated a "throw e" expression, we know the last + * emitted instruction was an ATHROW. As explained above, it is OK to emit a second ATHROW, + * the verifiers will be happy. + */ + emit(asm.Opcodes.ATHROW) } else if (from.isNullType) { bc drop from emit(asm.Opcodes.ACONST_NULL) diff --git a/test/files/run/nothingTypeDce.flags b/test/files/run/nothingTypeDce.flags new file mode 100644 index 0000000000..d85321ca0e --- /dev/null +++ b/test/files/run/nothingTypeDce.flags @@ -0,0 +1 @@ +-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code diff --git a/test/files/run/nothingTypeDce.scala b/test/files/run/nothingTypeDce.scala new file mode 100644 index 0000000000..4b9d934eac --- /dev/null +++ b/test/files/run/nothingTypeDce.scala @@ -0,0 +1,61 @@ +// See comment in BCodeBodyBuilder + +// -target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code +// target enables stack map frames generation + +class C { + // can't just emit a call to ???, that returns value of type Nothing$ (not Int). + def f1: Int = ??? + + def f2: Int = throw new Error("") + + def f3(x: Boolean) = { + var y = 0 + // cannot assign an object of type Nothing$ to Int + if (x) y = ??? + else y = 1 + y + } + + def f4(x: Boolean) = { + var y = 0 + // tests that whatever is emitted after the throw is valid (what? depends on opts, presence of stack map frames) + if (x) y = throw new Error("") + else y = 1 + y + } + + def f5(x: Boolean) = { + // stack heights need to be the smae. ??? looks to the jvm like returning a value of + // type Nothing$, need to drop or throw it. + println( + if (x) { ???; 10 } + else 20 + ) + } + + def f6(x: Boolean) = { + println( + if (x) { throw new Error(""); 10 } + else 20 + ) + } + + def f7(x: Boolean) = { + println( + if (x) throw new Error("") + else 20 + ) + } + + def f8(x: Boolean) = { + println( + if (x) throw new Error("") + else 20 + ) + } +} + +object Test extends App { + new C() +} diff --git a/test/files/run/nothingTypeNoFramesNoDce.check b/test/files/run/nothingTypeNoFramesNoDce.check new file mode 100644 index 0000000000..b1d08b45ff --- /dev/null +++ b/test/files/run/nothingTypeNoFramesNoDce.check @@ -0,0 +1 @@ +warning: -target:jvm-1.5 is deprecated: use target for Java 1.6 or above. diff --git a/test/files/run/nothingTypeNoFramesNoDce.flags b/test/files/run/nothingTypeNoFramesNoDce.flags new file mode 100644 index 0000000000..a035c86179 --- /dev/null +++ b/test/files/run/nothingTypeNoFramesNoDce.flags @@ -0,0 +1 @@ +-target:jvm-1.5 -Ybackend:GenBCode -Yopt:l:none -deprecation diff --git a/test/files/run/nothingTypeNoFramesNoDce.scala b/test/files/run/nothingTypeNoFramesNoDce.scala new file mode 100644 index 0000000000..3d1298303a --- /dev/null +++ b/test/files/run/nothingTypeNoFramesNoDce.scala @@ -0,0 +1,61 @@ +// See comment in BCodeBodyBuilder + +// -target:jvm-1.5 -Ybackend:GenBCode -Yopt:l:none +// target disables stack map frame generation. in this mode, the ClssWriter just emits dead code as is. + +class C { + // can't just emit a call to ???, that returns value of type Nothing$ (not Int). + def f1: Int = ??? + + def f2: Int = throw new Error("") + + def f3(x: Boolean) = { + var y = 0 + // cannot assign an object of type Nothing$ to Int + if (x) y = ??? + else y = 1 + y + } + + def f4(x: Boolean) = { + var y = 0 + // tests that whatever is emitted after the throw is valid (what? depends on opts, presence of stack map frames) + if (x) y = throw new Error("") + else y = 1 + y + } + + def f5(x: Boolean) = { + // stack heights need to be the smae. ??? looks to the jvm like returning a value of + // type Nothing$, need to drop or throw it. + println( + if (x) { ???; 10 } + else 20 + ) + } + + def f6(x: Boolean) = { + println( + if (x) { throw new Error(""); 10 } + else 20 + ) + } + + def f7(x: Boolean) = { + println( + if (x) throw new Error("") + else 20 + ) + } + + def f8(x: Boolean) = { + println( + if (x) throw new Error("") + else 20 + ) + } +} + +object Test extends App { + new C() +} diff --git a/test/files/run/nothingTypeNoOpt.flags b/test/files/run/nothingTypeNoOpt.flags new file mode 100644 index 0000000000..b3b518051b --- /dev/null +++ b/test/files/run/nothingTypeNoOpt.flags @@ -0,0 +1 @@ +-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none diff --git a/test/files/run/nothingTypeNoOpt.scala b/test/files/run/nothingTypeNoOpt.scala new file mode 100644 index 0000000000..5c5a20fa3b --- /dev/null +++ b/test/files/run/nothingTypeNoOpt.scala @@ -0,0 +1,61 @@ +// See comment in BCodeBodyBuilder + +// -target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none +// target enables stack map frame generation + +class C { + // can't just emit a call to ???, that returns value of type Nothing$ (not Int). + def f1: Int = ??? + + def f2: Int = throw new Error("") + + def f3(x: Boolean) = { + var y = 0 + // cannot assign an object of type Nothing$ to Int + if (x) y = ??? + else y = 1 + y + } + + def f4(x: Boolean) = { + var y = 0 + // tests that whatever is emitted after the throw is valid (what? depends on opts, presence of stack map frames) + if (x) y = throw new Error("") + else y = 1 + y + } + + def f5(x: Boolean) = { + // stack heights need to be the smae. ??? looks to the jvm like returning a value of + // type Nothing$, need to drop or throw it. + println( + if (x) { ???; 10 } + else 20 + ) + } + + def f6(x: Boolean) = { + println( + if (x) { throw new Error(""); 10 } + else 20 + ) + } + + def f7(x: Boolean) = { + println( + if (x) throw new Error("") + else 20 + ) + } + + def f8(x: Boolean) = { + println( + if (x) throw new Error("") + else 20 + ) + } +} + +object Test extends App { + new C() +} diff --git a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala index 8518d5c832..fb677fc84b 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala @@ -42,7 +42,7 @@ object CodeGenTools { compiler } - def compile(compiler: Global = newCompiler())(code: String): List[(String, Array[Byte])] = { + def compile(compiler: Global)(code: String): List[(String, Array[Byte])] = { compiler.reporter.reset() resetOutput(compiler) val run = new compiler.Run() @@ -51,11 +51,16 @@ object CodeGenTools { (for (f <- outDir.iterator if !f.isDirectory) yield (f.name, f.toByteArray)).toList } - def compileClasses(compiler: Global = newCompiler())(code: String): List[ClassNode] = { + def compileClasses(compiler: Global)(code: String): List[ClassNode] = { compile(compiler)(code).map(p => AsmUtils.readClass(p._2)).sortBy(_.name) } - def compileMethods(compiler: Global = newCompiler())(code: String): List[MethodNode] = { + def compileMethods(compiler: Global)(code: String): List[MethodNode] = { compileClasses(compiler)(s"class C { $code }").head.methods.asScala.toList.filterNot(_.name == "<init>") } + + def singleMethodInstructions(compiler: Global)(code: String): List[Instruction] = { + val List(m) = compileMethods(compiler)(code) + instructionsFromMethod(m) + } } diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala index 4e62cc64e4..ae08d10b79 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala @@ -18,6 +18,14 @@ import ASMConverters._ class UnreachableCodeTest { import UnreachableCodeTest._ + // jvm-1.6 enables emitting stack map frames, which impacts the code generation wrt dead basic blocks, + // see comment in BCodeBodyBuilder + val dceCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code") + val noOptCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none") + + // jvm-1.5 disables computing stack map frames, and it emits dead code as-is. + val noOptNoFramesCompiler = newCompiler(extraArgs = "-target:jvm-1.5 -Ybackend:GenBCode -Yopt:l:none") + @Test def basicElimination(): Unit = { assertEliminateDead( @@ -97,6 +105,36 @@ class UnreachableCodeTest { } @Test + def basicEliminationCompiler(): Unit = { + val code = "def f: Int = { return 1; 2 }" + val withDce = singleMethodInstructions(dceCompiler)(code) + assertSameCode(withDce.dropNonOp, List(Op(ICONST_1), Op(IRETURN))) + + val noDce = singleMethodInstructions(noOptCompiler)(code) + + // The emitted code is ICONST_1, IRETURN, ICONST_2, IRETURN. The latter two are dead. + // + // GenBCode puts the last IRETURN into a new basic block: it emits a label before the second + // IRETURN. This is an implementation detail, it may change; it affects the outcome of this test. + // + // During classfile writing with COMPUTE_FAMES (-target:jvm-1.6 or larger), the ClassfileWriter + // puts the ICONST_2 into a new basic block, because the preceding operation (IRETURN) ends + // the current block. We get something like + // + // L1: ICONST_1; IRETURN + // L2: ICONST_2 << dead + // L3: IRETURN << dead + // + // Finally, instructions in the dead basic blocks are replaced by ATHROW, as explained in + // a comment in BCodeBodyBuilder. + assertSameCode(noDce.dropNonOp, List(Op(ICONST_1), Op(IRETURN), Op(ATHROW), Op(ATHROW))) + + // when NOT computing stack map frames, ASM's ClassWriter does not replace dead code by NOP/ATHROW + val noDceNoFrames = singleMethodInstructions(noOptNoFramesCompiler)(code) + assertSameCode(noDceNoFrames.dropNonOp, List(Op(ICONST_1), Op(IRETURN), Op(ICONST_2), Op(IRETURN))) + } + + @Test def metaTest(): Unit = { assertEliminateDead() // no instructions @@ -153,10 +191,11 @@ object UnreachableCodeTest { val cls = wrapInClass(genMethod()(code.map(_._1): _*)) LocalOpt.removeUnreachableCode(cls) val nonEliminated = instructionsFromMethod(cls.methods.get(0)) - val expectedLive = code.filter(_._2).map(_._1).toList - - assertTrue(s"\nExpected: $expectedLive\nActual : $nonEliminated", nonEliminated === expectedLive) + assertSameCode(nonEliminated, expectedLive) } + def assertSameCode(actual: List[Instruction], expected: List[Instruction]): Unit = { + assertTrue(s"\nExpected: $expected\nActual : $actual", actual === expected) + } } |