Skip to content

Commit 055a373

Browse files
committed
SI-9376 don't crash when inlining a closure body that throws.
If the closure body method has return type Nothing$, add an `ATHROW` instruction after the callsite. This is required for computing stack map frames, as explained in a comment in BCodeBodyBuilder.adapt. Similar for closure bodies with return type Null$.
1 parent 6ae2dd8 commit 055a373

File tree

4 files changed

+99
-2
lines changed

4 files changed

+99
-2
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
843843
*
844844
* New (http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.10.1)
845845
* - Requires consistent stack map frames. GenBCode always generates stack frames.
846-
* or higher.
847846
* - In practice: the ASM library computes stack map frames for us (ClassWriter). Emitting
848847
* correct frames after an ATHROW is probably complex, so ASM uses the following strategy:
849848
* - Every time when generating an ATHROW, a new basic block is started.

src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import scala.collection.mutable
1212
import scala.reflect.internal.util.Collections._
1313
import scala.tools.asm.commons.CodeSizeEvaluator
1414
import scala.tools.asm.tree.analysis._
15-
import scala.tools.asm.{MethodWriter, ClassWriter, Label, Opcodes}
15+
import scala.tools.asm.{MethodWriter, ClassWriter, Label, Opcodes, Type}
1616
import scala.tools.asm.tree._
1717
import GenBCode._
1818
import scala.collection.convert.decorateAsScala._
@@ -330,6 +330,26 @@ object BytecodeUtils {
330330
)).toList
331331
}
332332

333+
/**
334+
* This method is used by optimizer components to eliminate phantom values of instruction
335+
* that load a value of type `Nothing$` or `Null$`. Such values on the stack don't interact well
336+
* with stack map frames.
337+
*
338+
* For example, `opt.getOrElse(throw e)` is re-written to an invocation of the lambda body, a
339+
* method with return type `Nothing$`. Similarly for `opt.getOrElse(null)` and `Null$`.
340+
*
341+
* During bytecode generation this is handled by BCodeBodyBuilder.adapt. See the comment in that
342+
* method which explains the issue with such phantom values.
343+
*/
344+
def fixLoadedNothingOrNullValue(loadedType: Type, loadInstr: AbstractInsnNode, methodNode: MethodNode, bTypes: BTypes): Unit = {
345+
if (loadedType == bTypes.coreBTypes.RT_NOTHING.toASMType) {
346+
methodNode.instructions.insert(loadInstr, new InsnNode(Opcodes.ATHROW))
347+
} else if (loadedType == bTypes.coreBTypes.RT_NULL.toASMType) {
348+
methodNode.instructions.insert(loadInstr, new InsnNode(Opcodes.ACONST_NULL))
349+
methodNode.instructions.insert(loadInstr, new InsnNode(Opcodes.POP))
350+
}
351+
}
352+
333353
/**
334354
* A wrapper to make ASM's Analyzer a bit easier to use.
335355
*/

src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,10 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) {
283283
val isInterface = bodyOpcode == INVOKEINTERFACE
284284
val bodyInvocation = new MethodInsnNode(bodyOpcode, lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc, isInterface)
285285
methodNode.instructions.insertBefore(invocation, bodyInvocation)
286+
287+
val returnType = Type.getReturnType(lambdaBodyHandle.getDesc)
288+
fixLoadedNothingOrNullValue(returnType, bodyInvocation, methodNode, btypes) // see comment of that method
289+
286290
methodNode.instructions.remove(invocation)
287291

288292
// update the call graph
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package scala.tools.nsc
2+
package backend.jvm
3+
package opt
4+
5+
import org.junit.runner.RunWith
6+
import org.junit.runners.JUnit4
7+
import org.junit.Test
8+
import scala.collection.generic.Clearable
9+
import scala.collection.mutable.ListBuffer
10+
import scala.reflect.internal.util.BatchSourceFile
11+
import scala.tools.asm.Opcodes._
12+
import org.junit.Assert._
13+
14+
import scala.tools.asm.tree._
15+
import scala.tools.asm.tree.analysis._
16+
import scala.tools.nsc.backend.jvm.opt.BytecodeUtils.AsmAnalyzer
17+
import scala.tools.nsc.io._
18+
import scala.tools.nsc.reporters.StoreReporter
19+
import scala.tools.testing.AssertUtil._
20+
21+
import CodeGenTools._
22+
import scala.tools.partest.ASMConverters
23+
import ASMConverters._
24+
import AsmUtils._
25+
26+
import BackendReporting._
27+
28+
import scala.collection.convert.decorateAsScala._
29+
import scala.tools.testing.ClearAfterClass
30+
31+
object ClosureOptimizerTest extends ClearAfterClass.Clearable {
32+
var compiler = newCompiler(extraArgs = "-Yopt:l:classpath -Yopt-warnings")
33+
def clear(): Unit = { compiler = null }
34+
}
35+
36+
@RunWith(classOf[JUnit4])
37+
class ClosureOptimizerTest extends ClearAfterClass {
38+
ClearAfterClass.stateToClear = ClosureOptimizerTest
39+
40+
val compiler = ClosureOptimizerTest.compiler
41+
42+
@Test
43+
def nothingTypedClosureBody(): Unit = {
44+
val code =
45+
"""abstract class C {
46+
| def isEmpty: Boolean
47+
| @inline final def getOrElse[T >: C](f: => T) = if (isEmpty) f else this
48+
| def t = getOrElse(throw new Error(""))
49+
|}
50+
""".stripMargin
51+
52+
val List(c) = compileClasses(compiler)(code)
53+
val t = c.methods.asScala.toList.find(_.name == "t").get
54+
val List(bodyCall) = findInstr(t, "INVOKESTATIC C.C$$$anonfun$1 ()Lscala/runtime/Nothing$")
55+
assert(bodyCall.getNext.getOpcode == ATHROW)
56+
}
57+
58+
@Test
59+
def nullTypedClosureBody(): Unit = {
60+
val code =
61+
"""abstract class C {
62+
| def isEmpty: Boolean
63+
| @inline final def getOrElse[T >: C](f: => T) = if (isEmpty) f else this
64+
| def t = getOrElse(null)
65+
|}
66+
""".stripMargin
67+
68+
val List(c) = compileClasses(compiler)(code)
69+
val t = c.methods.asScala.toList.find(_.name == "t").get
70+
val List(bodyCall) = findInstr(t, "INVOKESTATIC C.C$$$anonfun$1 ()Lscala/runtime/Null$")
71+
assert(bodyCall.getNext.getOpcode == POP)
72+
assert(bodyCall.getNext.getNext.getOpcode == ACONST_NULL)
73+
}
74+
}

0 commit comments

Comments
 (0)