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.

4 files changed, 173 insertions, 8 deletions

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
 
@@ -51,6 +54,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]].
    */
   def classBTypeFromParsedClassfile(internalName: InternalName): 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(_))
+  }
+}