From ff67161f946c515a3b0a719ce80531fa14a06a8f Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Sat, 17 Jan 2015 15:13:01 +0100 Subject: Find instructions that would cause an IllegalAccessError when inlined Some instructions would cause an IllegalAccessError if they are inlined into a different class. Based on Miguel's implementation in 6efc0528c6. --- src/asm/scala/tools/asm/tree/MethodInsnNode.java | 1 + .../scala/tools/nsc/backend/jvm/BTypes.scala | 43 +++++++- .../tools/nsc/backend/jvm/BTypesFromSymbols.scala | 4 +- .../nsc/backend/jvm/opt/ByteCodeRepository.scala | 16 +-- .../scala/tools/nsc/backend/jvm/opt/Inliner.scala | 118 +++++++++++++++++++++ 5 files changed, 174 insertions(+), 8 deletions(-) create mode 100644 src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala (limited to 'src') diff --git a/src/asm/scala/tools/asm/tree/MethodInsnNode.java b/src/asm/scala/tools/asm/tree/MethodInsnNode.java index 1ec46d473d..30c7854646 100644 --- a/src/asm/scala/tools/asm/tree/MethodInsnNode.java +++ b/src/asm/scala/tools/asm/tree/MethodInsnNode.java @@ -45,6 +45,7 @@ public class MethodInsnNode extends AbstractInsnNode { /** * The internal name of the method's owner class (see * {@link scala.tools.asm.Type#getInternalName() getInternalName}). + * For methods of arrays, e.g., clone(), the array type descriptor. */ public String owner; diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index ec26ef9b48..ff30631c10 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -6,10 +6,11 @@ package scala.tools.nsc package backend.jvm +import scala.annotation.switch import scala.tools.asm import asm.Opcodes import scala.tools.asm.tree.{InnerClassNode, ClassNode} -import opt.ByteCodeRepository +import opt.{ByteCodeRepository, Inliner} import scala.collection.convert.decorateAsScala._ /** @@ -34,6 +35,8 @@ abstract class BTypes { */ val byteCodeRepository: ByteCodeRepository + val inliner: Inliner[this.type] + // Allows to define per-run caches here and in the CallGraph component, which don't have a global def recordPerRunCache[T <: collection.generic.Clearable](cache: T): T @@ -50,6 +53,31 @@ abstract class BTypes { */ val classBTypeFromInternalName: collection.concurrent.Map[InternalName, ClassBType] = recordPerRunCache(collection.concurrent.TrieMap.empty[InternalName, ClassBType]) + /** + * Obtain the BType for a type descriptor or internal name. For class descriptors, the ClassBType + * is constructed by parsing the corresponding classfile. + * + * Some JVM operations use either a full descriptor or only an internal name. Example: + * ANEWARRAY java/lang/String // a new array of strings (internal name for the String class) + * ANEWARRAY [Ljava/lang/String; // a new array of array of string (full descriptor for the String class) + * + * This method supports both descriptors and internal names. + */ + def bTypeForDescriptorOrInternalNameFromClassfile(desc: String): BType = (desc(0): @switch) match { + case 'V' => UNIT + case 'Z' => BOOL + case 'C' => CHAR + case 'B' => BYTE + case 'S' => SHORT + case 'I' => INT + case 'F' => FLOAT + case 'J' => LONG + case 'D' => DOUBLE + case '[' => ArrayBType(bTypeForDescriptorOrInternalNameFromClassfile(desc.substring(1))) + case 'L' if desc.last == ';' => classBTypeFromParsedClassfile(desc.substring(1, desc.length - 1)) + case _ => classBTypeFromParsedClassfile(desc) + } + /** * Parse the classfile for `internalName` and construct the [[ClassBType]]. */ @@ -725,6 +753,19 @@ abstract class BTypes { case Some(sc) => sc :: sc.superClassesTransitive } + /** + * The prefix of the internal name until the last '/', or the empty string. + */ + def packageInternalName: String = { + val name = internalName + name.lastIndexOf('/') match { + case -1 => "" + case i => name.substring(0, i) + } + } + + def isPublic = (info.flags & asm.Opcodes.ACC_PUBLIC) != 0 + def isNestedClass = info.nestedInfo.isDefined def enclosingNestedClassesChain: List[ClassBType] = diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala index 2af665d31c..117b377622 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala @@ -7,9 +7,9 @@ package scala.tools.nsc package backend.jvm import scala.tools.asm -import opt.ByteCodeRepository import scala.tools.asm.tree.ClassNode import scala.tools.nsc.backend.jvm.opt.ByteCodeRepository.Source +import scala.tools.nsc.backend.jvm.opt.{Inliner, ByteCodeRepository} import BTypes.InternalName /** @@ -38,6 +38,8 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { val byteCodeRepository = new ByteCodeRepository(global.classPath, recordPerRunCache(collection.concurrent.TrieMap.empty[InternalName, (ClassNode, Source)])) + val inliner: Inliner[this.type] = new Inliner(this) + final def initializeCoreBTypes(): Unit = { coreBTypes.setBTypes(new CoreBTypes[this.type](this)) } diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala index 7b424d2107..9e56f25888 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala @@ -59,12 +59,16 @@ class ByteCodeRepository(val classPath: ClassFileLookup[AbstractFile], val class * * @return The [[MethodNode]] of the requested method and the [[InternalName]] of its declaring class. */ - def methodNode(classInternalName: InternalName, name: String, descriptor: String): Option[(MethodNode, InternalName)] = { - val c = classNode(classInternalName) - c.methods.asScala.find(m => m.name == name && m.desc == descriptor).map((_, classInternalName)) orElse { - val parents = Option(c.superName) ++ c.interfaces.asScala - // `view` to stop at the first result - parents.view.flatMap(methodNode(_, name, descriptor)).headOption + def methodNode(ownerInternalNameOrArrayDescriptor: String, name: String, descriptor: String): Option[(MethodNode, InternalName)] = { + // In a MethodInsnNode, the `owner` field may be an array descriptor, for exmple when invoking `clone`. + if (ownerInternalNameOrArrayDescriptor.charAt(0) == '[') None + else { + val c = classNode(ownerInternalNameOrArrayDescriptor) + c.methods.asScala.find(m => m.name == name && m.desc == descriptor).map((_, ownerInternalNameOrArrayDescriptor)) orElse { + val parents = Option(c.superName) ++ c.interfaces.asScala + // `view` to stop at the first result + parents.view.flatMap(methodNode(_, name, descriptor)).headOption + } } } diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala new file mode 100644 index 0000000000..6e5e03f730 --- /dev/null +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -0,0 +1,118 @@ +/* NSC -- new Scala compiler + * Copyright 2005-2014 LAMP/EPFL + * @author Martin Odersky + */ + +package scala.tools.nsc +package backend.jvm +package opt + +import scala.tools.asm +import asm.Opcodes +import asm.tree._ +import scala.collection.convert.decorateAsScala._ +import OptimizerReporting._ + +class Inliner[BT <: BTypes](val btypes: BT) { + import btypes._ + import btypes.byteCodeRepository + + def findIllegalAccess(instructions: InsnList, destinationClass: ClassBType): Option[AbstractInsnNode] = { + + /** + * Check if a type is accessible to some class, as defined in JVMS 5.4.4. + * (A1) C is public + * (A2) C and D are members of the same run-time package + */ + def classIsAccessible(accessed: BType, from: ClassBType = destinationClass): Boolean = (accessed: @unchecked) match { + // TODO: A2 requires "same run-time package", which seems to be package + classloader (JMVS 5.3.). is the below ok? + case c: ClassBType => c.isPublic || c.packageInternalName == from.packageInternalName + case a: ArrayBType => classIsAccessible(a.elementType, from) + case _: PrimitiveBType => true + } + + /** + * Check if a member reference is accessible from the [[destinationClass]], as defined in the + * JVMS 5.4.4. Note that the class name in a field / method reference is not necessarily the + * class in which the member is declared: + * + * class A { def f = 0 }; class B extends A { f } + * + * The INVOKEVIRTUAL instruction uses a method reference "B.f ()I". Therefore this method has + * two parameters: + * + * @param memberDeclClass The class in which the member is declared (A) + * @param memberRefClass The class used in the member reference (B) + * + * JVMS 5.4.4 summary: A field or method R is accessible to a class D (destinationClass) iff + * (B1) R is public + * (B2) R is protected, declared in C (memberDeclClass) and D is a subclass of C. + * If R is not static, R must contain a symbolic reference to a class T (memberRefClass), + * such that T is either a subclass of D, a superclass of D, or D itself. + * (B3) R is either protected or has default access and declared by a class in the same + * run-time package as D. + * (B4) R is private and is declared in D. + */ + def memberIsAccessible(memberFlags: Int, memberDeclClass: ClassBType, memberRefClass: ClassBType): Boolean = { + // TODO: B3 requires "same run-time package", which seems to be package + classloader (JMVS 5.3.). is the below ok? + def samePackageAsDestination = memberDeclClass.packageInternalName == destinationClass.packageInternalName + + val key = (Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED | Opcodes.ACC_PRIVATE) & memberFlags + key match { + case Opcodes.ACC_PUBLIC => // B1 + true + + case Opcodes.ACC_PROTECTED => // B2 + val condB2 = destinationClass.isSubtypeOf(memberDeclClass) && { + val isStatic = (Opcodes.ACC_STATIC & memberFlags) != 0 + isStatic || memberRefClass.isSubtypeOf(destinationClass) || destinationClass.isSubtypeOf(memberRefClass) + } + condB2 || samePackageAsDestination // B3 (protected) + + case 0 => // B3 (default access) + samePackageAsDestination + + case Opcodes.ACC_PRIVATE => // B4 + memberDeclClass == destinationClass + } + } + + def isLegal(instruction: AbstractInsnNode): Boolean = instruction match { + case ti: TypeInsnNode => + // NEW, ANEWARRAY, CHECKCAST or INSTANCEOF. For these instructions, the reference + // "must be a symbolic reference to a class, array, or interface type" (JVMS 6), so + // it can be an internal name, or a full array descriptor. + classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(ti.desc)) + + case ma: MultiANewArrayInsnNode => + // "a symbolic reference to a class, array, or interface type" + classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(ma.desc)) + + case fi: FieldInsnNode => + val fieldRefClass = classBTypeFromParsedClassfile(fi.owner) + val (fieldNode, fieldDeclClass) = byteCodeRepository.fieldNode(fieldRefClass.internalName, fi.name, fi.desc).get + memberIsAccessible(fieldNode.access, classBTypeFromParsedClassfile(fieldDeclClass), fieldRefClass) + + case mi: MethodInsnNode => + if (mi.owner.charAt(0) == '[') true // array methods are accessible + else { + val methodRefClass = classBTypeFromParsedClassfile(mi.owner) + val (methodNode, methodDeclClass) = byteCodeRepository.methodNode(methodRefClass.internalName, mi.name, mi.desc).get + memberIsAccessible(methodNode.access, classBTypeFromParsedClassfile(methodDeclClass), methodRefClass) + } + + case ivd: InvokeDynamicInsnNode => + // TODO @lry check necessary conditions to inline an indy, instead of giving up + false + + case ci: LdcInsnNode => ci.cst match { + case t: asm.Type => classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(t.getInternalName)) + case _ => true + } + + case _ => true + } + + instructions.iterator.asScala.find(!isLegal(_)) + } +} -- cgit v1.2.3