Skip to content

Commit e9c742e

Browse files
committed
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.
1 parent 4e9be26 commit e9c742e

File tree

6 files changed

+153
-40
lines changed

6 files changed

+153
-40
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package scala.tools.nsc
22
package backend.jvm
33

4-
import scala.tools.asm.tree.{AbstractInsnNode, MethodNode}
4+
import scala.tools.asm.tree.{InvokeDynamicInsnNode, AbstractInsnNode, MethodNode}
55
import scala.tools.nsc.backend.jvm.BTypes.InternalName
66
import scala.reflect.internal.util.Position
77
import scala.tools.nsc.settings.ScalaSettings
@@ -246,6 +246,11 @@ object BackendReporting {
246246
case class ResultingMethodTooLarge(calleeDeclarationClass: InternalName, name: String, descriptor: String,
247247
callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning
248248

249+
case object UnknownInvokeDynamicInstruction extends OptimizerWarning {
250+
override def toString = "The callee contains an InvokeDynamic instruction with an unknown bootstrap method (not a LambdaMetaFactory)."
251+
def emitWarning(settings: ScalaSettings): Boolean = settings.YoptWarningEmitAtInlineFailed
252+
}
253+
249254
/**
250255
* Used in `rewriteClosureApplyInvocations` when a closure apply callsite cannot be rewritten
251256
* to the closure body method.

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

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package opt
99

1010
import scala.annotation.tailrec
1111
import scala.tools.asm
12+
import asm.Handle
1213
import asm.Opcodes._
1314
import asm.tree._
1415
import scala.collection.convert.decorateAsScala._
@@ -687,9 +688,67 @@ class Inliner[BT <: BTypes](val btypes: BT) {
687688
}
688689
}
689690

690-
case ivd: InvokeDynamicInsnNode =>
691-
// TODO @lry check necessary conditions to inline an indy, instead of giving up
692-
Right(false)
691+
case indy: InvokeDynamicInsnNode =>
692+
// an indy instr points to a "call site specifier" (CSP) [1]
693+
// - a reference to a bootstrap method [2]
694+
// - bootstrap method name
695+
// - references to constant arguments, which can be:
696+
// - constant (string, long, int, float, double)
697+
// - class
698+
// - method type (without name)
699+
// - method handle
700+
// - a method name+type
701+
//
702+
// execution [3]
703+
// - resolve the CSP, yielding the boostrap method handle, the static args and the name+type
704+
// - resolution entails accessibility checking [4]
705+
// - execute the `invoke` method of the boostrap method handle (which is signature polymorphic, check its javadoc)
706+
// - the descriptor for the call is made up from the actual arguments on the stack:
707+
// - the first parameters are "MethodHandles.Lookup, String, MethodType", then the types of the constant arguments,
708+
// - the return type is CallSite
709+
// - the values for the call are
710+
// - the bootstrap method handle of the CSP is the receiver
711+
// - the Lookup object for the class in which the callsite occurs (obtained as through calling MethodHandles.lookup())
712+
// - the method name of the CSP
713+
// - the method type of the CSP
714+
// - the constants of the CSP (primitives are not boxed)
715+
// - the resulting `CallSite` object
716+
// - has as `type` the method type of the CSP
717+
// - is popped from the operand stack
718+
// - the `invokeExact` method (signature polymorphic!) of the `target` method handle of the CallSite is invoked
719+
// - the method descriptor is that of the CSP
720+
// - the receiver is the target of the CallSite
721+
// - the other argument values are those that were on the operand stack at the indy instruction (indyLambda: the captured values)
722+
//
723+
// [1] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4.10
724+
// [2] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.23
725+
// [3] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.invokedynamic
726+
// [4] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.4.3
727+
728+
// We cannot generically check if an `invokedynamic` instruction can be safely inlined into
729+
// a different class, that depends on the bootstrap method. The Lookup object passed to the
730+
// bootstrap method is a capability to access private members of the callsite class. We can
731+
// only move the invokedynamic to a new class if we know that the bootstrap method doesn't
732+
// use this capability for otherwise non-accessible members.
733+
// In the case of indyLambda, it depends on the visibility of the implMethod handle. If
734+
// the implMethod is public, lambdaMetaFactory doesn't use the Lookup object's extended
735+
// capability, and we can safely inline the instruction into a different class.
736+
737+
if (destinationClass == calleeDeclarationClass) {
738+
Right(true) // within the same class, any indy instruction can be inlined
739+
} else if (closureOptimizer.isClosureInstantiation(indy)) {
740+
val implMethod = indy.bsmArgs(1).asInstanceOf[Handle] // safe, checked in isClosureInstantiation
741+
val methodRefClass = classBTypeFromParsedClassfile(implMethod.getOwner)
742+
for {
743+
(methodNode, methodDeclClassNode) <- byteCodeRepository.methodNode(methodRefClass.internalName, implMethod.getName, implMethod.getDesc): Either[OptimizerWarning, (MethodNode, InternalName)]
744+
methodDeclClass = classBTypeFromParsedClassfile(methodDeclClassNode)
745+
res <- memberIsAccessible(methodNode.access, methodDeclClass, methodRefClass, destinationClass)
746+
} yield {
747+
res
748+
}
749+
} else {
750+
Left(UnknownInvokeDynamicInstruction)
751+
}
693752

694753
case ci: LdcInsnNode => ci.cst match {
695754
case t: asm.Type => classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(t.getInternalName), destinationClass)

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

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,24 @@ object ASMConverters {
5858

5959
case class Method(instructions: List[Instruction], handlers: List[ExceptionHandler], localVars: List[LocalVariable])
6060

61-
case class Field (opcode: Int, owner: String, name: String, desc: String) extends Instruction
62-
case class Incr (opcode: Int, `var`: Int, incr: Int) extends Instruction
63-
case class Op (opcode: Int) extends Instruction
64-
case class IntOp (opcode: Int, operand: Int) extends Instruction
65-
case class Jump (opcode: Int, label: Label) extends Instruction
66-
case class Ldc (opcode: Int, cst: Any) extends Instruction
67-
case class LookupSwitch(opcode: Int, dflt: Label, keys: List[Int], labels: List[Label]) extends Instruction
68-
case class TableSwitch (opcode: Int, min: Int, max: Int, dflt: Label, labels: List[Label]) extends Instruction
69-
case class Invoke (opcode: Int, owner: String, name: String, desc: String, itf: Boolean) extends Instruction
70-
case class NewArray (opcode: Int, desc: String, dims: Int) extends Instruction
71-
case class TypeOp (opcode: Int, desc: String) extends Instruction
72-
case class VarOp (opcode: Int, `var`: Int) extends Instruction
73-
case class Label (offset: Int) extends Instruction { def opcode: Int = -1 }
74-
case class FrameEntry (`type`: Int, local: List[Any], stack: List[Any]) extends Instruction { def opcode: Int = -1 }
75-
case class LineNumber (line: Int, start: Label) extends Instruction { def opcode: Int = -1 }
61+
case class Field (opcode: Int, owner: String, name: String, desc: String) extends Instruction
62+
case class Incr (opcode: Int, `var`: Int, incr: Int) extends Instruction
63+
case class Op (opcode: Int) extends Instruction
64+
case class IntOp (opcode: Int, operand: Int) extends Instruction
65+
case class Jump (opcode: Int, label: Label) extends Instruction
66+
case class Ldc (opcode: Int, cst: Any) extends Instruction
67+
case class LookupSwitch (opcode: Int, dflt: Label, keys: List[Int], labels: List[Label]) extends Instruction
68+
case class TableSwitch (opcode: Int, min: Int, max: Int, dflt: Label, labels: List[Label]) extends Instruction
69+
case class Invoke (opcode: Int, owner: String, name: String, desc: String, itf: Boolean) extends Instruction
70+
case class InvokeDynamic(opcode: Int, name: String, desc: String, bsm: MethodHandle, bsmArgs: List[AnyRef]) extends Instruction
71+
case class NewArray (opcode: Int, desc: String, dims: Int) extends Instruction
72+
case class TypeOp (opcode: Int, desc: String) extends Instruction
73+
case class VarOp (opcode: Int, `var`: Int) extends Instruction
74+
case class Label (offset: Int) extends Instruction { def opcode: Int = -1 }
75+
case class FrameEntry (`type`: Int, local: List[Any], stack: List[Any]) extends Instruction { def opcode: Int = -1 }
76+
case class LineNumber (line: Int, start: Label) extends Instruction { def opcode: Int = -1 }
77+
78+
case class MethodHandle(tag: Int, owner: String, name: String, desc: String)
7679

7780
case class ExceptionHandler(start: Label, end: Label, handler: Label, desc: Option[String])
7881
case class LocalVariable(name: String, desc: String, signature: Option[String], start: Label, end: Label, index: Int)
@@ -111,6 +114,7 @@ object ASMConverters {
111114
case i: t.LookupSwitchInsnNode => LookupSwitch (op(i), applyLabel(i.dflt), lst(i.keys) map (x => x: Int), lst(i.labels) map applyLabel)
112115
case i: t.TableSwitchInsnNode => TableSwitch (op(i), i.min, i.max, applyLabel(i.dflt), lst(i.labels) map applyLabel)
113116
case i: t.MethodInsnNode => Invoke (op(i), i.owner, i.name, i.desc, i.itf)
117+
case i: t.InvokeDynamicInsnNode => InvokeDynamic(op(i), i.name, i.desc, convertMethodHandle(i.bsm), convertBsmArgs(i.bsmArgs))
114118
case i: t.MultiANewArrayInsnNode => NewArray (op(i), i.desc, i.dims)
115119
case i: t.TypeInsnNode => TypeOp (op(i), i.desc)
116120
case i: t.VarInsnNode => VarOp (op(i), i.`var`)
@@ -119,6 +123,13 @@ object ASMConverters {
119123
case i: t.LineNumberNode => LineNumber (i.line, applyLabel(i.start))
120124
}
121125

126+
private def convertBsmArgs(a: Array[Object]): List[Object] = a.map({
127+
case h: asm.Handle => convertMethodHandle(h)
128+
case _ => a // can be: Class, method Type, primitive constant
129+
})(collection.breakOut)
130+
131+
private def convertMethodHandle(h: asm.Handle): MethodHandle = MethodHandle(h.getTag, h.getOwner, h.getName, h.getDesc)
132+
122133
private def convertHandlers(method: t.MethodNode): List[ExceptionHandler] = {
123134
method.tryCatchBlocks.asScala.map(h => ExceptionHandler(applyLabel(h.start), applyLabel(h.end), applyLabel(h.handler), Option(h.`type`)))(collection.breakOut)
124135
}
@@ -197,21 +208,28 @@ object ASMConverters {
197208
case x => x.asInstanceOf[Object]
198209
}
199210

211+
def unconvertMethodHandle(h: MethodHandle): asm.Handle = new asm.Handle(h.tag, h.owner, h.name, h.desc)
212+
def unconvertBsmArgs(a: List[Object]): Array[Object] = a.map({
213+
case h: MethodHandle => unconvertMethodHandle(h)
214+
case o => o
215+
})(collection.breakOut)
216+
200217
private def visitMethod(method: t.MethodNode, instruction: Instruction, asmLabel: Map[Label, asm.Label]): Unit = instruction match {
201-
case Field(op, owner, name, desc) => method.visitFieldInsn(op, owner, name, desc)
202-
case Incr(op, vr, incr) => method.visitIincInsn(vr, incr)
203-
case Op(op) => method.visitInsn(op)
204-
case IntOp(op, operand) => method.visitIntInsn(op, operand)
205-
case Jump(op, label) => method.visitJumpInsn(op, asmLabel(label))
206-
case Ldc(op, cst) => method.visitLdcInsn(cst)
207-
case LookupSwitch(op, dflt, keys, labels) => method.visitLookupSwitchInsn(asmLabel(dflt), keys.toArray, (labels map asmLabel).toArray)
208-
case TableSwitch(op, min, max, dflt, labels) => method.visitTableSwitchInsn(min, max, asmLabel(dflt), (labels map asmLabel).toArray: _*)
209-
case Invoke(op, owner, name, desc, itf) => method.visitMethodInsn(op, owner, name, desc, itf)
210-
case NewArray(op, desc, dims) => method.visitMultiANewArrayInsn(desc, dims)
211-
case TypeOp(op, desc) => method.visitTypeInsn(op, desc)
212-
case VarOp(op, vr) => method.visitVarInsn(op, vr)
213-
case l: Label => method.visitLabel(asmLabel(l))
214-
case FrameEntry(tp, local, stack) => method.visitFrame(tp, local.length, frameTypesToAsm(local, asmLabel).toArray, stack.length, frameTypesToAsm(stack, asmLabel).toArray)
215-
case LineNumber(line, start) => method.visitLineNumber(line, asmLabel(start))
218+
case Field(op, owner, name, desc) => method.visitFieldInsn(op, owner, name, desc)
219+
case Incr(op, vr, incr) => method.visitIincInsn(vr, incr)
220+
case Op(op) => method.visitInsn(op)
221+
case IntOp(op, operand) => method.visitIntInsn(op, operand)
222+
case Jump(op, label) => method.visitJumpInsn(op, asmLabel(label))
223+
case Ldc(op, cst) => method.visitLdcInsn(cst)
224+
case LookupSwitch(op, dflt, keys, labels) => method.visitLookupSwitchInsn(asmLabel(dflt), keys.toArray, (labels map asmLabel).toArray)
225+
case TableSwitch(op, min, max, dflt, labels) => method.visitTableSwitchInsn(min, max, asmLabel(dflt), (labels map asmLabel).toArray: _*)
226+
case Invoke(op, owner, name, desc, itf) => method.visitMethodInsn(op, owner, name, desc, itf)
227+
case InvokeDynamic(op, name, desc, bsm, bsmArgs) => method.visitInvokeDynamicInsn(name, desc, unconvertMethodHandle(bsm), unconvertBsmArgs(bsmArgs))
228+
case NewArray(op, desc, dims) => method.visitMultiANewArrayInsn(desc, dims)
229+
case TypeOp(op, desc) => method.visitTypeInsn(op, desc)
230+
case VarOp(op, vr) => method.visitVarInsn(op, vr)
231+
case l: Label => method.visitLabel(asmLabel(l))
232+
case FrameEntry(tp, local, stack) => method.visitFrame(tp, local.length, frameTypesToAsm(local, asmLabel).toArray, stack.length, frameTypesToAsm(stack, asmLabel).toArray)
233+
case LineNumber(line, start) => method.visitLineNumber(line, asmLabel(start))
216234
}
217235
}

test/files/presentation/t7678/Runner.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
/*
2-
* filter: inliner warnings; re-run with
3-
*/
41
import scala.tools.nsc.interactive.tests._
52
import scala.reflect.internal.util._
63

test/files/run/t8029.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
/*
2-
* filter: inliner warning; re-run with
3-
*/
41
import scala.tools.partest._
52
import scala.tools.nsc._
63

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -991,4 +991,41 @@ class InlinerTest extends ClearAfterClass {
991991
assert(2 == t.collect({case Ldc(_, "hai!") => }).size) // twice the body of f
992992
assert(1 == t.collect({case Jump(IFNONNULL, _) => }).size) // one single null check
993993
}
994+
995+
@Test
996+
def inlineIndyLambda(): Unit = {
997+
val code =
998+
"""object M {
999+
| @inline def m(s: String) = {
1000+
| val f = (x: String) => x.trim
1001+
| f(s)
1002+
| }
1003+
|}
1004+
|class C {
1005+
| @inline final def m(s: String) = {
1006+
| val f = (x: String) => x.trim
1007+
| f(s)
1008+
| }
1009+
| def t1 = m("foo")
1010+
| def t2 = M.m("bar")
1011+
|}
1012+
""".stripMargin
1013+
1014+
val List(c, _, _) = compile(code)
1015+
1016+
val t1 = getSingleMethod(c, "t1")
1017+
assert(t1.instructions exists {
1018+
case _: InvokeDynamic => true
1019+
case _ => false
1020+
})
1021+
// the indy call is inlined into t, and the closure elimination rewrites the closure invocation to the body method
1022+
assertInvoke(t1, "C", "C$$$anonfun$2")
1023+
1024+
val t2 = getSingleMethod(c, "t2")
1025+
assert(t2.instructions exists {
1026+
case _: InvokeDynamic => true
1027+
case _ => false
1028+
})
1029+
assertInvoke(t2, "M$", "M$$$anonfun$1")
1030+
}
9941031
}

0 commit comments

Comments
 (0)