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$.

4 files changed, 99 insertions, 2 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
index c3f71969f6..063fb81d46 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -843,7 +843,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
          *
          * New (http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.10.1)
          *   - Requires consistent stack map frames. GenBCode always generates stack frames.
-         *     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.
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala
index 0ec550981a..cd36fd8bba 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala
@@ -12,7 +12,7 @@ import scala.collection.mutable
 import scala.reflect.internal.util.Collections._
 import scala.tools.asm.commons.CodeSizeEvaluator
 import scala.tools.asm.tree.analysis._
-import scala.tools.asm.{MethodWriter, ClassWriter, Label, Opcodes}
+import scala.tools.asm.{MethodWriter, ClassWriter, Label, Opcodes, Type}
 import scala.tools.asm.tree._
 import GenBCode._
 import scala.collection.convert.decorateAsScala._
@@ -331,6 +331,26 @@ object BytecodeUtils {
   }
 
   /**
+   * This method is used by optimizer components to eliminate phantom values of instruction
+   * that load a value of type `Nothing$` or `Null$`. Such values on the stack don't interact well
+   * with stack map frames.
+   *
+   * For example, `opt.getOrElse(throw e)` is re-written to an invocation of the lambda body, a
+   * method with return type `Nothing$`. Similarly for `opt.getOrElse(null)` and `Null$`.
+   *
+   * During bytecode generation this is handled by BCodeBodyBuilder.adapt. See the comment in that
+   * method which explains the issue with such phantom values.
+   */
+  def fixLoadedNothingOrNullValue(loadedType: Type, loadInstr: AbstractInsnNode, methodNode: MethodNode, bTypes: BTypes): Unit = {
+    if (loadedType == bTypes.coreBTypes.RT_NOTHING.toASMType) {
+      methodNode.instructions.insert(loadInstr, new InsnNode(Opcodes.ATHROW))
+    } else if (loadedType == bTypes.coreBTypes.RT_NULL.toASMType) {
+      methodNode.instructions.insert(loadInstr, new InsnNode(Opcodes.ACONST_NULL))
+      methodNode.instructions.insert(loadInstr, new InsnNode(Opcodes.POP))
+    }
+  }
+
+  /**
    * A wrapper to make ASM's Analyzer a bit easier to use.
    */
   class AsmAnalyzer[V <: Value](methodNode: MethodNode, classInternalName: InternalName, interpreter: Interpreter[V] = new BasicInterpreter) {
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala
index 1648a53ed8..743a454678 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala
@@ -283,6 +283,10 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) {
           val isInterface = bodyOpcode == INVOKEINTERFACE
           val bodyInvocation = new MethodInsnNode(bodyOpcode, lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc, isInterface)
           methodNode.instructions.insertBefore(invocation, bodyInvocation)
+
+          val returnType = Type.getReturnType(lambdaBodyHandle.getDesc)
+          fixLoadedNothingOrNullValue(returnType, bodyInvocation, methodNode, btypes) // see comment of that method
+
           methodNode.instructions.remove(invocation)
 
           // update the call graph
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala
new file mode 100644
index 0000000000..69eed1f75d
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala
@@ -0,0 +1,74 @@
+package scala.tools.nsc
+package backend.jvm
+package opt
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.collection.generic.Clearable
+import scala.collection.mutable.ListBuffer
+import scala.reflect.internal.util.BatchSourceFile
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+
+import scala.tools.asm.tree._
+import scala.tools.asm.tree.analysis._
+import scala.tools.nsc.backend.jvm.opt.BytecodeUtils.AsmAnalyzer
+import scala.tools.nsc.io._
+import scala.tools.nsc.reporters.StoreReporter
+import scala.tools.testing.AssertUtil._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+import AsmUtils._
+
+import BackendReporting._
+
+import scala.collection.convert.decorateAsScala._
+import scala.tools.testing.ClearAfterClass
+
+object ClosureOptimizerTest extends ClearAfterClass.Clearable {
+  var compiler = newCompiler(extraArgs = "-Yopt:l:classpath -Yopt-warnings")
+  def clear(): Unit = { compiler = null }
+}
+
+@RunWith(classOf[JUnit4])
+class ClosureOptimizerTest extends ClearAfterClass {
+  ClearAfterClass.stateToClear = ClosureOptimizerTest
+
+  val compiler = ClosureOptimizerTest.compiler
+
+  @Test
+  def nothingTypedClosureBody(): Unit = {
+    val code =
+      """abstract class C {
+        |  def isEmpty: Boolean
+        |  @inline final def getOrElse[T >: C](f: => T) = if (isEmpty) f else this
+        |  def t = getOrElse(throw new Error(""))
+        |}
+      """.stripMargin
+
+    val List(c) = compileClasses(compiler)(code)
+    val t = c.methods.asScala.toList.find(_.name == "t").get
+    val List(bodyCall) = findInstr(t, "INVOKESTATIC C.C$$$anonfun$1 ()Lscala/runtime/Nothing$")
+    assert(bodyCall.getNext.getOpcode == ATHROW)
+  }
+
+  @Test
+  def nullTypedClosureBody(): Unit = {
+    val code =
+      """abstract class C {
+        |  def isEmpty: Boolean
+        |  @inline final def getOrElse[T >: C](f: => T) = if (isEmpty) f else this
+        |  def t = getOrElse(null)
+        |}
+      """.stripMargin
+
+    val List(c) = compileClasses(compiler)(code)
+    val t = c.methods.asScala.toList.find(_.name == "t").get
+    val List(bodyCall) = findInstr(t, "INVOKESTATIC C.C$$$anonfun$1 ()Lscala/runtime/Null$")
+    assert(bodyCall.getNext.getOpcode == POP)
+    assert(bodyCall.getNext.getNext.getOpcode == ACONST_NULL)
+  }
+}