From 4e982451decdc3821febfe975e1b8e406a3741e8 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 20 Jan 2015 23:20:26 +0100 Subject: Don't crash the inliner in mixed compilation In mixed compilation, the bytecode of Java classes is not availalbe: the Scala compiler does not produce any, and there are no classfiles yet. When inlining a (Scala defined) method that contains an invocation to a Java method, we need the Java method's bytecode in order to check whether that invocation can be transplanted to the new location without causing an IllegalAccessError. If the bytecode cannot be found, inlining won't be allowed. --- .../scala/tools/nsc/backend/jvm/BTypes.scala | 60 ++++++++++++---------- .../scala/tools/nsc/backend/jvm/opt/Inliner.scala | 41 +++++++++++---- 2 files changed, 64 insertions(+), 37 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index f07c7b7764..e617c86b23 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -87,32 +87,29 @@ abstract class BTypes { * * 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))) + def bTypeForDescriptorOrInternalNameFromClassfile(desc: String): Option[BType] = (desc(0): @switch) match { + case 'V' => Some(UNIT) + case 'Z' => Some(BOOL) + case 'C' => Some(CHAR) + case 'B' => Some(BYTE) + case 'S' => Some(SHORT) + case 'I' => Some(INT) + case 'F' => Some(FLOAT) + case 'J' => Some(LONG) + case 'D' => Some(DOUBLE) + case '[' => bTypeForDescriptorOrInternalNameFromClassfile(desc.substring(1)) map ArrayBType case 'L' if desc.last == ';' => classBTypeFromParsedClassfile(desc.substring(1, desc.length - 1)) case _ => classBTypeFromParsedClassfile(desc) } /** - * Parse the classfile for `internalName` and construct the [[ClassBType]]. + * Parse the classfile for `internalName` and construct the [[ClassBType]]. Returns `None` if the + * classfile cannot be found in the `byteCodeRepository`. */ - def classBTypeFromParsedClassfile(internalName: InternalName): ClassBType = { - val classNode = byteCodeRepository.classNode(internalName) getOrElse { - // There's no way out, we need the ClassBType. I (lry) only know one case byteCodeRepository.classNode - // returns None: for Java classes in mixed compilation. In this case we should not end up here, - // because there exists a symbol for that Java class, the ClassBType should be built from the symbol. - assertionError(s"Could not find bytecode for class $internalName") + def classBTypeFromParsedClassfile(internalName: InternalName): Option[ClassBType] = { + classBTypeFromInternalName.get(internalName) orElse { + byteCodeRepository.classNode(internalName) map classBTypeFromClassNode } - classBTypeFromClassNode(classNode) } /** @@ -120,20 +117,31 @@ abstract class BTypes { */ def classBTypeFromClassNode(classNode: ClassNode): ClassBType = { classBTypeFromInternalName.getOrElse(classNode.name, { - setClassInfo(classNode, ClassBType(classNode.name)) + setClassInfoFromParsedClassfile(classNode, ClassBType(classNode.name)) }) } - private def setClassInfo(classNode: ClassNode, classBType: ClassBType): ClassBType = { + private def setClassInfoFromParsedClassfile(classNode: ClassNode, classBType: ClassBType): ClassBType = { + def ensureClassBTypeFromParsedClassfile(internalName: InternalName): ClassBType = { + classBTypeFromParsedClassfile(internalName) getOrElse { + // When building a ClassBType from a parsed classfile, we need the ClassBTypes for all + // referenced types. + // TODO: make this more robust with respect to incomplete classpaths. + // Maybe not those parts of the ClassBType that require the missing class are not actually + // queried during the backend, so every part of a ClassBType that requires parsing a + // (potentially missing) classfile should be computed lazily. + assertionError(s"Could not find bytecode for class $internalName") + } + } val superClass = classNode.superName match { case null => assert(classNode.name == ObjectReference.internalName, s"class with missing super type: ${classNode.name}") None case superName => - Some(classBTypeFromParsedClassfile(superName)) + Some(ensureClassBTypeFromParsedClassfile(superName)) } - val interfaces: List[ClassBType] = classNode.interfaces.asScala.map(classBTypeFromParsedClassfile)(collection.breakOut) + val interfaces: List[ClassBType] = classNode.interfaces.asScala.map(ensureClassBTypeFromParsedClassfile)(collection.breakOut) val flags = classNode.access @@ -159,7 +167,7 @@ abstract class BTypes { } val nestedClasses: List[ClassBType] = classNode.innerClasses.asScala.collect({ - case i if nestedInCurrentClass(i) => classBTypeFromParsedClassfile(i.name) + case i if nestedInCurrentClass(i) => ensureClassBTypeFromParsedClassfile(i.name) })(collection.breakOut) // if classNode is a nested class, it has an innerClass attribute for itself. in this @@ -169,11 +177,11 @@ abstract class BTypes { val enclosingClass = if (innerEntry.outerName != null) { // if classNode is a member class, the outerName is non-null - classBTypeFromParsedClassfile(innerEntry.outerName) + ensureClassBTypeFromParsedClassfile(innerEntry.outerName) } else { // for anonymous or local classes, the outerName is null, but the enclosing class is // stored in the EnclosingMethod attribute (which ASM encodes in classNode.outerClass). - classBTypeFromParsedClassfile(classNode.outerClass) + ensureClassBTypeFromParsedClassfile(classNode.outerClass) } val staticFlag = (innerEntry.access & Opcodes.ACC_STATIC) != 0 NestedInfo(enclosingClass, Option(innerEntry.outerName), Option(innerEntry.innerName), staticFlag) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala index b74c7ba86c..2ca8e8b8c4 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -353,6 +353,11 @@ class Inliner[BT <: BTypes](val btypes: BT) { } } + /** + * Returns the first instruction in the `instructions` list that would cause a + * [[java.lang.IllegalAccessError]] when inlined into the `destinationClass`. Returns `None` if + * all instructions can be legally transplanted. + */ def findIllegalAccess(instructions: InsnList, destinationClass: ClassBType): Option[AbstractInsnNode] = { /** @@ -413,36 +418,50 @@ class Inliner[BT <: BTypes](val btypes: BT) { } } + /** + * Check if `instruction` can be transplanted to `destinationClass`. + * + * If the instruction references a class, method or field that cannot be found in the + * byteCodeRepository, it is considered as not legal. This is known to happen in mixed + * compilation: for Java classes there is no classfile that could be parsed, nor does the + * compiler generate any bytecode. + */ 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)) + bTypeForDescriptorOrInternalNameFromClassfile(ti.desc).exists(classIsAccessible(_)) case ma: MultiANewArrayInsnNode => // "a symbolic reference to a class, array, or interface type" - classIsAccessible(bTypeForDescriptorOrInternalNameFromClassfile(ma.desc)) + bTypeForDescriptorOrInternalNameFromClassfile(ma.desc).exists(classIsAccessible(_)) 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) + (for { + fieldRefClass <- classBTypeFromParsedClassfile(fi.owner) + (fieldNode, fieldDeclClassNode) <- byteCodeRepository.fieldNode(fieldRefClass.internalName, fi.name, fi.desc) + fieldDeclClass <- classBTypeFromParsedClassfile(fieldDeclClassNode) + } yield { + memberIsAccessible(fieldNode.access, fieldDeclClass, fieldRefClass) + }) getOrElse false 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) - } + else (for { + methodRefClass <- classBTypeFromParsedClassfile(mi.owner) + (methodNode, methodDeclClassNode) <- byteCodeRepository.methodNode(methodRefClass.internalName, mi.name, mi.desc) + methodDeclClass <- classBTypeFromParsedClassfile(methodDeclClassNode) + } yield { + memberIsAccessible(methodNode.access, methodDeclClass, methodRefClass) + }) getOrElse false 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 t: asm.Type => bTypeForDescriptorOrInternalNameFromClassfile(t.getInternalName).exists(classIsAccessible(_)) case _ => true } -- cgit v1.2.3