From 9d901b6598701d48091486d95c7c4e1fd286a126 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Wed, 23 Sep 2015 15:53:05 +0200 Subject: SD-33 Consider methods annotated @CallerSensitive not safe to inline Fixes https://github.com/scala/scala-dev/issues/33 Methods annotated `sun.reflect.CallerSensitive` should not be inlined, their implementation may depend on the call stack. --- .../tools/nsc/backend/jvm/opt/BytecodeUtils.scala | 2 + .../tools/nsc/backend/jvm/opt/CallGraph.scala | 3 +- .../tools/nsc/backend/jvm/opt/CallGraphTest.scala | 68 ++++++++++++++-------- 3 files changed, 47 insertions(+), 26 deletions(-) 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 ea186f9a1b..6bbcfae170 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala @@ -98,6 +98,8 @@ object BytecodeUtils { def isNativeMethod(methodNode: MethodNode): Boolean = (methodNode.access & Opcodes.ACC_NATIVE) != 0 + def hasCallerSensitiveAnnotation(methodNode: MethodNode) = methodNode.visibleAnnotations != null && methodNode.visibleAnnotations.asScala.exists(_.desc == "Lsun/reflect/CallerSensitive;") + def isFinalClass(classNode: ClassNode): Boolean = (classNode.access & Opcodes.ACC_FINAL) != 0 def isFinalMethod(methodNode: MethodNode): Boolean = (methodNode.access & (Opcodes.ACC_FINAL | Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC)) != 0 diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala index b9788c3f56..ea9fe31a7f 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala @@ -311,7 +311,8 @@ class CallGraph[BT <: BTypes](val btypes: BT) { isStaticallyResolved && // (1) !isAbstract && !BytecodeUtils.isConstructor(calleeMethodNode) && - !BytecodeUtils.isNativeMethod(calleeMethodNode), + !BytecodeUtils.isNativeMethod(calleeMethodNode) && + !BytecodeUtils.hasCallerSensitiveAnnotation(calleeMethodNode), safeToRewrite = canInlineFromSource && isRewritableTraitCall, // (2) annotatedInline = methodInlineInfo.annotatedInline, annotatedNoInline = methodInlineInfo.annotatedNoInline, diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala index 995e008912..efd88f10c3 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala @@ -47,13 +47,34 @@ class CallGraphTest extends ClearAfterClass { def compile(code: String, allowMessage: StoreReporter#Info => Boolean = _ => false): List[ClassNode] = { CallGraphTest.notPerRun.foreach(_.clear()) - compileClasses(compiler)(code, allowMessage = allowMessage) + compileClasses(compiler)(code, allowMessage = allowMessage).map(c => byteCodeRepository.classNode(c.name).get) } + def getMethods(c: ClassNode, p: String => Boolean) = c.methods.iterator.asScala.filter(m => p(m.name)).toList.sortBy(_.name) + def getMethod(c: ClassNode, name: String) = getMethods(c, _ == name).head + def callsInMethod(methodNode: MethodNode): List[MethodInsnNode] = methodNode.instructions.iterator.asScala.collect({ case call: MethodInsnNode => call }).toList + def checkCallsite(call: MethodInsnNode, callsiteMethod: MethodNode, target: MethodNode, calleeDeclClass: ClassBType, + safeToInline: Boolean, atInline: Boolean, atNoInline: Boolean, argInfos: IntMap[ArgInfo] = IntMap.empty) = { + val callsite = callGraph.callsites(callsiteMethod)(call) + try { + assert(callsite.callsiteInstruction == call) + assert(callsite.callsiteMethod == callsiteMethod) + val callee = callsite.callee.get + assert(callee.callee == target) + assert(callee.calleeDeclarationClass == calleeDeclClass) + assert(callee.safeToInline == safeToInline) + assert(callee.annotatedInline == atInline) + assert(callee.annotatedNoInline == atNoInline) + assert(callsite.argInfos == argInfos) + } catch { + case e: Throwable => println(callsite); throw e + } + } + @Test def callGraphStructure(): Unit = { val code = @@ -97,36 +118,17 @@ class CallGraphTest extends ClearAfterClass { msgCount += 1 ok exists (m.msg contains _) } - val List(cCls, cMod, dCls, testCls) = compile(code, checkMsg).map(c => byteCodeRepository.classNode(c.name).get) + val List(cCls, cMod, dCls, testCls) = compile(code, checkMsg) assert(msgCount == 6, msgCount) - val List(cf1, cf2, cf3, cf4, cf5, cf6, cf7) = cCls.methods.iterator.asScala.filter(_.name.startsWith("f")).toList.sortBy(_.name) - val List(df1, df3) = dCls.methods.iterator.asScala.filter(_.name.startsWith("f")).toList.sortBy(_.name) - val g1 = cMod.methods.iterator.asScala.find(_.name == "g1").get - val List(t1, t2) = testCls.methods.iterator.asScala.filter(_.name.startsWith("t")).toList.sortBy(_.name) + val List(cf1, cf2, cf3, cf4, cf5, cf6, cf7) = getMethods(cCls, _.startsWith("f")) + val List(df1, df3) = getMethods(dCls, _.startsWith("f")) + val g1 = getMethod(cMod, "g1") + val List(t1, t2) = getMethods(testCls, _.startsWith("t")) val List(cf1Call, cf2Call, cf3Call, cf4Call, cf5Call, cf6Call, cf7Call, cg1Call) = callsInMethod(t1) val List(df1Call, df2Call, df3Call, df4Call, df5Call, df6Call, df7Call, dg1Call) = callsInMethod(t2) - def checkCallsite(call: MethodInsnNode, callsiteMethod: MethodNode, target: MethodNode, calleeDeclClass: ClassBType, - safeToInline: Boolean, atInline: Boolean, atNoInline: Boolean) = { - val callsite = callGraph.callsites(callsiteMethod)(call) - try { - assert(callsite.callsiteInstruction == call) - assert(callsite.callsiteMethod == callsiteMethod) - val callee = callsite.callee.get - assert(callee.callee == target) - assert(callee.calleeDeclarationClass == calleeDeclClass) - assert(callee.safeToInline == safeToInline) - assert(callee.annotatedInline == atInline) - assert(callee.annotatedNoInline == atNoInline) - - assert(callsite.argInfos == IntMap.empty) // no higher-order methods - } catch { - case e: Throwable => println(callsite); throw e - } - } - val cClassBType = classBTypeFromClassNode(cCls) val cMClassBType = classBTypeFromClassNode(cMod) val dClassBType = classBTypeFromClassNode(dCls) @@ -150,6 +152,22 @@ class CallGraphTest extends ClearAfterClass { checkCallsite(dg1Call, t2, g1, cMClassBType, true, false, false) } + @Test + def callerSensitiveNotSafeToInline(): Unit = { + val code = + """class C { + | def m = java.lang.Class.forName("C") + |} + """.stripMargin + val List(c) = compile(code) + val m = getMethod(c, "m") + val List(fn) = callsInMethod(m) + val forNameMeth = byteCodeRepository.methodNode("java/lang/Class", "forName", "(Ljava/lang/String;)Ljava/lang/Class;").get._1 + val classTp = classBTypeFromInternalName("java/lang/Class") + val r = callGraph.callsites(m)(fn) + checkCallsite(fn, m, forNameMeth, classTp, safeToInline = false, atInline = false, atNoInline = false) + } + /** * NOTE: if this test fails for you when running within the IDE, it's probably because you're * using 2.12.0-M2 for compilining within the IDE, which doesn't add SAM information to the -- cgit v1.2.3