Accessibility checks for methods with an InvokeDynamic instruction

Implements the necessary tests to check if a method with an
InvokeDynamic instruction can be inlined into a destination class.

Only InvokeDynamic instructions with LambdaMetaFactory as bootstrap
methods can be inlined. The accessibility checks cannot be implemented
generically, because it depends on what the bootstrap method is doing.
In particular, the bootstrap method receives a Lookup object as
argument which can be used to access private methods of the class
where the InvokeDynamic method is located.

A comment in the inliner explains the details.

Author: lrytz

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

@@ -1,7 +1,7 @@
 package scala.tools.nsc
 package backend.jvm
 
-import scala.tools.asm.tree.{AbstractInsnNode, MethodNode}
+import scala.tools.asm.tree.{InvokeDynamicInsnNode, AbstractInsnNode, MethodNode}
 import scala.tools.nsc.backend.jvm.BTypes.InternalName
 import scala.reflect.internal.util.Position
 import scala.tools.nsc.settings.ScalaSettings
@@ -246,6 +246,11 @@ object BackendReporting {
   case class ResultingMethodTooLarge(calleeDeclarationClass: InternalName, name: String, descriptor: String,
                                      callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning
 
+  case object UnknownInvokeDynamicInstruction extends OptimizerWarning {
+    override def toString = "The callee contains an InvokeDynamic instruction with an unknown bootstrap method (not a LambdaMetaFactory)."
+    def emitWarning(settings: ScalaSettings): Boolean = settings.YoptWarningEmitAtInlineFailed
+  }
+
   /**
    * Used in `rewriteClosureApplyInvocations` when a closure apply callsite cannot be rewritten
    * to the closure body method.

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

@@ -9,6 +9,7 @@ package opt
 
 import scala.annotation.tailrec
 import scala.tools.asm
+import asm.Handle
 import asm.Opcodes._
 import asm.tree._
 import scala.collection.convert.decorateAsScala._
@@ -687,9 +688,67 @@ class Inliner[BT <: BTypes](val btypes: BT) {
           }
         }
 
-      case ivd: InvokeDynamicInsnNode =>
-        // TODO @lry check necessary conditions to inline an indy, instead of giving up
-        Right(false)
+      case indy: InvokeDynamicInsnNode =>
+        // an indy instr points to a "call site specifier" (CSP) [1]
+        //  - a reference to a bootstrap method [2]
+        //    - bootstrap method name
+        //    - references to constant arguments, which can be:
+        //      - constant (string, long, int, float, double)
+        //      - class
+        //      - method type (without name)
+        //      - method handle
+        //  - a method name+type
+        //
+        // execution [3]
+        //  - resolve the CSP, yielding the boostrap method handle, the static args and the name+type
+        //    - resolution entails accessibility checking [4]
+        //  - execute the `invoke` method of the boostrap method handle (which is signature polymorphic, check its javadoc)
+        //    - the descriptor for the call is made up from the actual arguments on the stack:
+        //      - the first parameters are "MethodHandles.Lookup, String, MethodType", then the types of the constant arguments,
+        //      - the return type is CallSite
+        //    - the values for the call are
+        //      - the bootstrap method handle of the CSP is the receiver
+        //      - the Lookup object for the class in which the callsite occurs (obtained as through calling MethodHandles.lookup())
+        //      - the method name of the CSP
+        //      - the method type of the CSP
+        //      - the constants of the CSP (primitives are not boxed)
+        //  - the resulting `CallSite` object
+        //    - has as `type` the method type of the CSP
+        //    - is popped from the operand stack
+        //  - the `invokeExact` method (signature polymorphic!) of the `target` method handle of the CallSite is invoked
+        //    - the method descriptor is that of the CSP
+        //    - the receiver is the target of the CallSite
+        //    - the other argument values are those that were on the operand stack at the indy instruction (indyLambda: the captured values)
+        //
+        // [1] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4.10
+        // [2] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.23
+        // [3] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.invokedynamic
+        // [4] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.4.3
+
+        // We cannot generically check if an `invokedynamic` instruction can be safely inlined into
+        // a different class, that depends on the bootstrap method. The Lookup object passed to the
+        // bootstrap method is a capability to access private members of the callsite class. We can
+        // only move the invokedynamic to a new class if we know that the bootstrap method doesn't
+        // use this capability for otherwise non-accessible members.
+        // In the case of indyLambda, it depends on the visibility of the implMethod handle. If
+        // the implMethod is public, lambdaMetaFactory doesn't use the Lookup object's extended
+        // capability, and we can safely inline the instruction into a different class.
+
+        if (destinationClass == calleeDeclarationClass) {
+          Right(true) // within the same class, any indy instruction can be inlined
+        } else if (closureOptimizer.isClosureInstantiation(indy)) {
+          val implMethod = indy.bsmArgs(1).asInstanceOf[Handle] // safe, checked in isClosureInstantiation
+          val methodRefClass = classBTypeFromParsedClassfile(implMethod.getOwner)
+          for {
+            (methodNode, methodDeclClassNode) <- byteCodeRepository.methodNode(methodRefClass.internalName, implMethod.getName, implMethod.getDesc): Either[OptimizerWarning, (MethodNode, InternalName)]
+            methodDeclClass                   =  classBTypeFromParsedClassfile(methodDeclClassNode)
+            res                               <- memberIsAccessible(methodNode.access, methodDeclClass, methodRefClass, destinationClass)
+          } yield {
+            res
+          }
+        } else {
+          Left(UnknownInvokeDynamicInstruction)
+        }
 
       case ci: LdcInsnNode => ci.cst match {
         case t: asm.Type => classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(t.getInternalName), destinationClass)

src/partest-extras/scala/tools/partest/ASMConverters.scala

@@ -58,21 +58,24 @@ object ASMConverters {
 
   case class Method(instructions: List[Instruction], handlers: List[ExceptionHandler], localVars: List[LocalVariable])
 
-  case class Field       (opcode: Int, owner: String, name: String, desc: String)               extends Instruction
-  case class Incr        (opcode: Int, `var`: Int, incr: Int)                                   extends Instruction
-  case class Op          (opcode: Int)                                                          extends Instruction
-  case class IntOp       (opcode: Int, operand: Int)                                            extends Instruction
-  case class Jump        (opcode: Int, label: Label)                                            extends Instruction
-  case class Ldc         (opcode: Int, cst: Any)                                                extends Instruction
-  case class LookupSwitch(opcode: Int, dflt: Label, keys: List[Int], labels: List[Label])       extends Instruction
-  case class TableSwitch (opcode: Int, min: Int, max: Int, dflt: Label, labels: List[Label])    extends Instruction
-  case class Invoke      (opcode: Int, owner: String, name: String, desc: String, itf: Boolean) extends Instruction
-  case class NewArray    (opcode: Int, desc: String, dims: Int)                                 extends Instruction
-  case class TypeOp      (opcode: Int, desc: String)                                            extends Instruction
-  case class VarOp       (opcode: Int, `var`: Int)                                              extends Instruction
-  case class Label       (offset: Int)                                                          extends Instruction { def opcode: Int = -1 }
-  case class FrameEntry  (`type`: Int, local: List[Any], stack: List[Any])                      extends Instruction { def opcode: Int = -1 }
-  case class LineNumber  (line: Int, start: Label)                                              extends Instruction { def opcode: Int = -1 }
+  case class Field        (opcode: Int, owner: String, name: String, desc: String)                            extends Instruction
+  case class Incr         (opcode: Int, `var`: Int, incr: Int)                                                extends Instruction
+  case class Op           (opcode: Int)                                                                       extends Instruction
+  case class IntOp        (opcode: Int, operand: Int)                                                         extends Instruction
+  case class Jump         (opcode: Int, label: Label)                                                         extends Instruction
+  case class Ldc          (opcode: Int, cst: Any)                                                             extends Instruction
+  case class LookupSwitch (opcode: Int, dflt: Label, keys: List[Int], labels: List[Label])                    extends Instruction
+  case class TableSwitch  (opcode: Int, min: Int, max: Int, dflt: Label, labels: List[Label])                 extends Instruction
+  case class Invoke       (opcode: Int, owner: String, name: String, desc: String, itf: Boolean)              extends Instruction
+  case class InvokeDynamic(opcode: Int, name: String, desc: String, bsm: MethodHandle, bsmArgs: List[AnyRef]) extends Instruction
+  case class NewArray     (opcode: Int, desc: String, dims: Int)                                              extends Instruction
+  case class TypeOp       (opcode: Int, desc: String)                                                         extends Instruction
+  case class VarOp        (opcode: Int, `var`: Int)                                                           extends Instruction
+  case class Label        (offset: Int)                                                                       extends Instruction { def opcode: Int = -1 }
+  case class FrameEntry   (`type`: Int, local: List[Any], stack: List[Any])                                   extends Instruction { def opcode: Int = -1 }
+  case class LineNumber   (line: Int, start: Label)                                                           extends Instruction { def opcode: Int = -1 }
+
+  case class MethodHandle(tag: Int, owner: String, name: String, desc: String)
 
   case class ExceptionHandler(start: Label, end: Label, handler: Label, desc: Option[String])
   case class LocalVariable(name: String, desc: String, signature: Option[String], start: Label, end: Label, index: Int)
@@ -111,6 +114,7 @@ object ASMConverters {
       case i: t.LookupSwitchInsnNode   => LookupSwitch (op(i), applyLabel(i.dflt), lst(i.keys) map (x => x: Int), lst(i.labels) map applyLabel)
       case i: t.TableSwitchInsnNode    => TableSwitch  (op(i), i.min, i.max, applyLabel(i.dflt), lst(i.labels) map applyLabel)
       case i: t.MethodInsnNode         => Invoke       (op(i), i.owner, i.name, i.desc, i.itf)
+      case i: t.InvokeDynamicInsnNode  => InvokeDynamic(op(i), i.name, i.desc, convertMethodHandle(i.bsm), convertBsmArgs(i.bsmArgs))
       case i: t.MultiANewArrayInsnNode => NewArray     (op(i), i.desc, i.dims)
       case i: t.TypeInsnNode           => TypeOp       (op(i), i.desc)
       case i: t.VarInsnNode            => VarOp        (op(i), i.`var`)
@@ -119,6 +123,13 @@ object ASMConverters {
       case i: t.LineNumberNode         => LineNumber   (i.line, applyLabel(i.start))
     }
 
+    private def convertBsmArgs(a: Array[Object]): List[Object] = a.map({
+      case h: asm.Handle => convertMethodHandle(h)
+      case _ => a // can be: Class, method Type, primitive constant
+    })(collection.breakOut)
+
+    private def convertMethodHandle(h: asm.Handle): MethodHandle = MethodHandle(h.getTag, h.getOwner, h.getName, h.getDesc)
+
     private def convertHandlers(method: t.MethodNode): List[ExceptionHandler] = {
       method.tryCatchBlocks.asScala.map(h => ExceptionHandler(applyLabel(h.start), applyLabel(h.end), applyLabel(h.handler), Option(h.`type`)))(collection.breakOut)
     }
@@ -197,21 +208,28 @@ object ASMConverters {
     case x => x.asInstanceOf[Object]
   }
 
+  def unconvertMethodHandle(h: MethodHandle): asm.Handle = new asm.Handle(h.tag, h.owner, h.name, h.desc)
+  def unconvertBsmArgs(a: List[Object]): Array[Object] = a.map({
+    case h: MethodHandle => unconvertMethodHandle(h)
+    case o => o
+  })(collection.breakOut)
+
   private def visitMethod(method: t.MethodNode, instruction: Instruction, asmLabel: Map[Label, asm.Label]): Unit = instruction match {
-    case Field(op, owner, name, desc)            => method.visitFieldInsn(op, owner, name, desc)
-    case Incr(op, vr, incr)                      => method.visitIincInsn(vr, incr)
-    case Op(op)                                  => method.visitInsn(op)
-    case IntOp(op, operand)                      => method.visitIntInsn(op, operand)
-    case Jump(op, label)                         => method.visitJumpInsn(op, asmLabel(label))
-    case Ldc(op, cst)                            => method.visitLdcInsn(cst)
-    case LookupSwitch(op, dflt, keys, labels)    => method.visitLookupSwitchInsn(asmLabel(dflt), keys.toArray, (labels map asmLabel).toArray)
-    case TableSwitch(op, min, max, dflt, labels) => method.visitTableSwitchInsn(min, max, asmLabel(dflt), (labels map asmLabel).toArray: _*)
-    case Invoke(op, owner, name, desc, itf)      => method.visitMethodInsn(op, owner, name, desc, itf)
-    case NewArray(op, desc, dims)                => method.visitMultiANewArrayInsn(desc, dims)
-    case TypeOp(op, desc)                        => method.visitTypeInsn(op, desc)
-    case VarOp(op, vr)                           => method.visitVarInsn(op, vr)
-    case l: Label                                => method.visitLabel(asmLabel(l))
-    case FrameEntry(tp, local, stack)            => method.visitFrame(tp, local.length, frameTypesToAsm(local, asmLabel).toArray, stack.length, frameTypesToAsm(stack, asmLabel).toArray)
-    case LineNumber(line, start)                 => method.visitLineNumber(line, asmLabel(start))
+    case Field(op, owner, name, desc)                => method.visitFieldInsn(op, owner, name, desc)
+    case Incr(op, vr, incr)                          => method.visitIincInsn(vr, incr)
+    case Op(op)                                      => method.visitInsn(op)
+    case IntOp(op, operand)                          => method.visitIntInsn(op, operand)
+    case Jump(op, label)                             => method.visitJumpInsn(op, asmLabel(label))
+    case Ldc(op, cst)                                => method.visitLdcInsn(cst)
+    case LookupSwitch(op, dflt, keys, labels)        => method.visitLookupSwitchInsn(asmLabel(dflt), keys.toArray, (labels map asmLabel).toArray)
+    case TableSwitch(op, min, max, dflt, labels)     => method.visitTableSwitchInsn(min, max, asmLabel(dflt), (labels map asmLabel).toArray: _*)
+    case Invoke(op, owner, name, desc, itf)          => method.visitMethodInsn(op, owner, name, desc, itf)
+    case InvokeDynamic(op, name, desc, bsm, bsmArgs) => method.visitInvokeDynamicInsn(name, desc, unconvertMethodHandle(bsm), unconvertBsmArgs(bsmArgs))
+    case NewArray(op, desc, dims)                    => method.visitMultiANewArrayInsn(desc, dims)
+    case TypeOp(op, desc)                            => method.visitTypeInsn(op, desc)
+    case VarOp(op, vr)                               => method.visitVarInsn(op, vr)
+    case l: Label                                    => method.visitLabel(asmLabel(l))
+    case FrameEntry(tp, local, stack)                => method.visitFrame(tp, local.length, frameTypesToAsm(local, asmLabel).toArray, stack.length, frameTypesToAsm(stack, asmLabel).toArray)
+    case LineNumber(line, start)                     => method.visitLineNumber(line, asmLabel(start))
   }
 }

test/files/presentation/t7678/Runner.scala

@@ -1,6 +1,3 @@
-/*
- * filter: inliner warnings; re-run with
- */
 import scala.tools.nsc.interactive.tests._
 import scala.reflect.internal.util._
 

test/files/run/t8029.scala

@@ -1,6 +1,3 @@
-/*
- * filter: inliner warning; re-run with
- */
 import scala.tools.partest._
 import scala.tools.nsc._
 

test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala

@@ -991,4 +991,41 @@ class InlinerTest extends ClearAfterClass {
     assert(2 == t.collect({case Ldc(_, "hai!") => }).size)     // twice the body of f
     assert(1 == t.collect({case Jump(IFNONNULL, _) => }).size) // one single null check
   }
+
+  @Test
+  def inlineIndyLambda(): Unit = {
+    val code =
+      """object M {
+        |  @inline def m(s: String) = {
+        |    val f = (x: String) => x.trim
+        |    f(s)
+        |  }
+        |}
+        |class C {
+        |  @inline final def m(s: String) = {
+        |    val f = (x: String) => x.trim
+        |    f(s)
+        |  }
+        |  def t1 = m("foo")
+        |  def t2 = M.m("bar")
+        |}
+      """.stripMargin
+
+    val List(c, _, _) = compile(code)
+
+    val t1 = getSingleMethod(c, "t1")
+    assert(t1.instructions exists {
+      case _: InvokeDynamic => true
+      case _ => false
+    })
+    // the indy call is inlined into t, and the closure elimination rewrites the closure invocation to the body method
+    assertInvoke(t1, "C", "C$$$anonfun$2")
+
+    val t2 = getSingleMethod(c, "t2")
+    assert(t2.instructions exists {
+      case _: InvokeDynamic => true
+      case _ => false
+    })
+    assertInvoke(t2, "M$", "M$$$anonfun$1")
+  }
 }