summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLukas Rytz <lukas.rytz@gmail.com>2016-04-11 13:34:17 +0200
committerLukas Rytz <lukas.rytz@gmail.com>2016-04-20 08:53:46 +0200
commit765eb29a769488d56f9ff2e4dde105961dbd55db (patch)
treee991412e6db200b45d30b60292cbe87d8a37c44f
parentd176f102bb6d0a6467e510583e61f363ff76d75e (diff)
downloadscala-765eb29a769488d56f9ff2e4dde105961dbd55db.tar.gz
scala-765eb29a769488d56f9ff2e4dde105961dbd55db.tar.bz2
scala-765eb29a769488d56f9ff2e4dde105961dbd55db.zip
Clean up code gen for method invocations
The code was patched many times in the history and became a bit scattered. When emitting a virtual call, the receiver in the bytecode cannot just be the method's owner (the class in which it is declared), because that class may not be accessible at the callsite. Instead we use the type of the receiver. This was basically done to fix - aladdin bug 455 (9954eaf) - SI-1430 (0bea2ab) - basically the same bug, slightly different - SI-4283 (8707c9e) - the same for field reads In this patch we extend the fix to field writes, and clean up the code. This patch basically reverts 6eb55d4b, the fix for SI-4560, which was rather a workaround than a fix. The underlying problem was that in some cases, in a method invocation `foo.bar()`, the method `bar` was not actually a member of `foo.tpe`, causing a NoSuchMethodErrors. The issue was related to trait implementation classes. The idea of the fix was to check, at code-gen time, `foo.tpe.member("bar")`, and if that returns `NoSymbol`, use `barSym.owner`. With the new trait encoding the underlying problem seems to be fixed - all tests still pass (run/t4560.scala and run/t4560b.scala).
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala357
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala28
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala2
-rw-r--r--test/files/run/t2106.check2
-rw-r--r--test/junit/scala/issues/BytecodeTest.scala134
-rw-r--r--test/junit/scala/issues/RunTest.scala7
-rw-r--r--test/junit/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzerTest.scala12
7 files changed, 324 insertions, 218 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
index 6d3d458324..a429cfa0f2 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -34,14 +34,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
* Functionality to build the body of ASM MethodNode, except for `synchronized` and `try` expressions.
*/
abstract class PlainBodyBuilder(cunit: CompilationUnit) extends PlainSkelBuilder(cunit) {
- /* If the selector type has a member with the right name,
- * it is the host class; otherwise the symbol's owner.
- */
- def findHostClass(selector: Type, sym: Symbol) = selector member sym.name match {
- case NoSymbol => debuglog(s"Rejecting $selector as host class for $sym") ; sym.owner
- case _ => selector.typeSymbol
- }
-
/* ---------------- helper utils for generating methods and code ---------------- */
def emit(opc: Int) { mnode.visitInsn(opc) }
@@ -69,12 +61,14 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
def genStat(tree: Tree) {
lineNumber(tree)
tree match {
- case Assign(lhs @ Select(_, _), rhs) =>
+ case Assign(lhs @ Select(qual, _), rhs) =>
val isStatic = lhs.symbol.isStaticMember
if (!isStatic) { genLoadQualifier(lhs) }
genLoad(rhs, symInfoTK(lhs.symbol))
lineNumber(tree)
- fieldStore(lhs.symbol)
+ // receiverClass is used in the bytecode to access the field. using sym.owner may lead to IllegalAccessError, SI-4283
+ val receiverClass = qual.tpe.typeSymbol
+ fieldStore(lhs.symbol, receiverClass)
case Assign(lhs, rhs) =>
val s = lhs.symbol
@@ -169,21 +163,13 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
genLoad(args.head, INT)
generatedType = k.asArrayBType.componentType
bc.aload(elementType)
- }
- else if (scalaPrimitives.isArraySet(code)) {
- args match {
- case a1 :: a2 :: Nil =>
- genLoad(a1, INT)
- genLoad(a2)
- // the following line should really be here, but because of bugs in erasure
- // we pretend we generate whatever type is expected from us.
- //generatedType = UNIT
- bc.astore(elementType)
- case _ =>
- abort(s"Too many arguments for array set operation: $tree")
- }
- }
- else {
+ } else if (scalaPrimitives.isArraySet(code)) {
+ val List(a1, a2) = args
+ genLoad(a1, INT)
+ genLoad(a2)
+ generatedType = UNIT
+ bc.astore(elementType)
+ } else {
generatedType = INT
emit(asm.Opcodes.ARRAYLENGTH)
}
@@ -338,26 +324,22 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
assert(tree.symbol.isModule, s"Selection of non-module from empty package: $tree sym: ${tree.symbol} at: ${tree.pos}")
genLoadModule(tree)
- case Select(qualifier, selector) =>
+ case Select(qualifier, _) =>
val sym = tree.symbol
generatedType = symInfoTK(sym)
- val hostClass = findHostClass(qualifier.tpe, sym)
- debuglog(s"Host class of $sym with qual $qualifier (${qualifier.tpe}) is $hostClass")
val qualSafeToElide = treeInfo isQualifierSafeToElide qualifier
-
def genLoadQualUnlessElidable() { if (!qualSafeToElide) { genLoadQualifier(tree) } }
-
+ // receiverClass is used in the bytecode to access the field. using sym.owner may lead to IllegalAccessError, SI-4283
+ def receiverClass = qualifier.tpe.typeSymbol
if (sym.isModule) {
genLoadQualUnlessElidable()
genLoadModule(tree)
- }
- else if (sym.isStaticMember) {
+ } else if (sym.isStaticMember) {
genLoadQualUnlessElidable()
- fieldLoad(sym, hostClass)
- }
- else {
+ fieldLoad(sym, receiverClass)
+ } else {
genLoadQualifier(tree)
- fieldLoad(sym, hostClass)
+ fieldLoad(sym, receiverClass)
}
case Ident(name) =>
@@ -410,24 +392,18 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
/*
* must-single-thread
*/
- def fieldLoad( field: Symbol, hostClass: Symbol = null) {
- fieldOp(field, isLoad = true, hostClass)
- }
+ def fieldLoad(field: Symbol, hostClass: Symbol): Unit = fieldOp(field, isLoad = true, hostClass)
+
/*
* must-single-thread
*/
- def fieldStore(field: Symbol, hostClass: Symbol = null) {
- fieldOp(field, isLoad = false, hostClass)
- }
+ def fieldStore(field: Symbol, hostClass: Symbol): Unit = fieldOp(field, isLoad = false, hostClass)
/*
* must-single-thread
*/
- private def fieldOp(field: Symbol, isLoad: Boolean, hostClass: Symbol) {
- // LOAD_FIELD.hostClass , CALL_METHOD.hostClass , and #4283
- val owner =
- if (hostClass == null) internalName(field.owner)
- else internalName(hostClass)
+ private def fieldOp(field: Symbol, isLoad: Boolean, hostClass: Symbol): Unit = {
+ val owner = internalName(if (hostClass == null) field.owner else hostClass)
val fieldJName = field.javaSimpleName.toString
val fieldDescr = symInfoTK(field).descriptor
val isStatic = field.isStaticMember
@@ -435,7 +411,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
if (isLoad) { if (isStatic) asm.Opcodes.GETSTATIC else asm.Opcodes.GETFIELD }
else { if (isStatic) asm.Opcodes.PUTSTATIC else asm.Opcodes.PUTFIELD }
mnode.visitFieldInsn(opc, owner, fieldJName, fieldDescr)
-
}
// ---------------- emitting constant values ----------------
@@ -532,21 +507,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
var generatedType = expectedType
lineNumber(app)
- def genSuperApply(hostClass: Symbol, fun: Symbol, args: List[Tree]) = {
- // 'super' call: Note: since constructors are supposed to
- // return an instance of what they construct, we have to take
- // special care. On JVM they are 'void', and Scala forbids (syntactically)
- // to call super constructors explicitly and/or use their 'returned' value.
- // therefore, we can ignore this fact, and generate code that leaves nothing
- // on the stack (contrary to what the type in the AST says).
-
- val invokeStyle = InvokeStyle.Super
- mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
- genLoadArguments(args, paramTKs(app))
- genCallMethod(fun, invokeStyle, app.pos, hostClass)
- generatedType = methodBTypeFromSymbol(fun).returnType
- }
-
app match {
case Apply(TypeApply(fun, targs), _) =>
@@ -594,19 +554,33 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
generatedType = genTypeApply()
- case Apply(fun @ Select(Super(qual, mix), _), args) =>
- val hostClass = qual.symbol.parentSymbols.filter(_.name == mix) match {
- case Nil =>
- // We get here for trees created by SuperSelect which use tpnme.EMPTY as the super qualifier
- // Subsequent code uses the owner of fun.symbol to target the call.
- null
- case parent :: Nil=>
- parent
- case parents =>
- devWarning("ambiguous parent class qualifier: " + qual.symbol.parentSymbols)
- null
+ case Apply(fun @ Select(Super(_, _), _), args) =>
+ def initModule() {
+ // we initialize the MODULE$ field immediately after the super ctor
+ if (!isModuleInitialized &&
+ jMethodName == INSTANCE_CONSTRUCTOR_NAME &&
+ fun.symbol.javaSimpleName.toString == INSTANCE_CONSTRUCTOR_NAME &&
+ isStaticModuleClass(claszSymbol)) {
+ isModuleInitialized = true
+ mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
+ mnode.visitFieldInsn(
+ asm.Opcodes.PUTSTATIC,
+ thisBType.internalName,
+ strMODULE_INSTANCE_FIELD,
+ thisBType.descriptor
+ )
+ }
}
- genSuperApply(hostClass, fun.symbol, args)
+ // 'super' call: Note: since constructors are supposed to
+ // return an instance of what they construct, we have to take
+ // special care. On JVM they are 'void', and Scala forbids (syntactically)
+ // to call super constructors explicitly and/or use their 'returned' value.
+ // therefore, we can ignore this fact, and generate code that leaves nothing
+ // on the stack (contrary to what the type in the AST says).
+ mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
+ genLoadArguments(args, paramTKs(app))
+ generatedType = genCallMethod(fun.symbol, InvokeStyle.Super, app.pos)
+ initModule()
// 'new' constructor call: Note: since constructors are
// thought to return an instance of what they construct,
@@ -675,80 +649,73 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
case app @ Apply(fun, args) =>
val sym = fun.symbol
- if (sym.isLabel) { // jump to a label
+ if (sym.isLabel) { // jump to a label
genLoadLabelArguments(args, labelDef(sym), app.pos)
bc goTo programPoint(sym)
} else if (isPrimitive(sym)) { // primitive method call
generatedType = genPrimitiveOp(app, expectedType)
- } else { // normal method call
-
- def genNormalMethodCall() {
-
- val invokeStyle =
- if (sym.isStaticMember) InvokeStyle.Static
- else if (sym.isPrivate || sym.isClassConstructor) InvokeStyle.Special
- else InvokeStyle.Virtual
-
- if (invokeStyle.hasInstance) {
- genLoadQualifier(fun)
+ } else { // normal method call
+ val invokeStyle =
+ if (sym.isStaticMember) InvokeStyle.Static
+ else if (sym.isPrivate || sym.isClassConstructor) InvokeStyle.Special
+ else InvokeStyle.Virtual
+
+ if (invokeStyle.hasInstance) genLoadQualifier(fun)
+ genLoadArguments(args, paramTKs(app))
+
+ val Select(qual, _) = fun // fun is a Select, also checked in genLoadQualifier
+ if (sym == definitions.Array_clone) {
+ // Special-case Array.clone, introduced in 36ef60e. The goal is to generate this call
+ // as "[I.clone" instead of "java/lang/Object.clone". This is consistent with javac.
+ // Arrays have a public method `clone` (jls 10.7).
+ //
+ // The JVMS is not explicit about this, but that receiver type can be an array type
+ // descriptor (instead of a class internal name):
+ // invokevirtual #2; //Method "[I".clone:()Ljava/lang/Object
+ //
+ // Note that using `Object.clone()` would work as well, but only because the JVM
+ // relaxes protected access specifically if the receiver is an array:
+ // http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/87ee5ee27509/src/share/vm/interpreter/linkResolver.cpp#l439
+ // Example: `class C { override def clone(): Object = "hi" }`
+ // Emitting `def f(c: C) = c.clone()` as `Object.clone()` gives a VerifyError.
+ val target: String = tpeTK(qual).asRefBType.classOrArrayType
+ val methodBType = methodBTypeFromSymbol(sym)
+ bc.invokevirtual(target, sym.javaSimpleName.toString, methodBType.descriptor, app.pos)
+ generatedType = methodBType.returnType
+ } else {
+ val receiverClass = if (!invokeStyle.isVirtual) null else {
+ // receiverClass is used in the bytecode to as the method receiver. using sym.owner
+ // may lead to IllegalAccessErrors, see 9954eaf / aladdin bug 455.
+ val qualSym = qual.tpe.typeSymbol
+ if (qualSym == ArrayClass) {
+ // For invocations like `Array(1).hashCode` or `.wait()`, use Object as receiver
+ // in the bytecode. Using the array descriptor (like we do for clone above) seems
+ // to work as well, but it seems safer not to change this. Javac also uses Object.
+ // Note that array apply/update/length are handled by isPrimitive (above).
+ assert(sym.owner == ObjectClass, s"unexpected array call: ${show(app)}")
+ ObjectClass
+ } else qualSym
}
- genLoadArguments(args, paramTKs(app))
-
- // In "a couple cases", squirrel away a extra information (hostClass, targetTypeKind). TODO Document what "in a couple cases" refers to.
- var hostClass: Symbol = null
- var targetTypeKind: BType = null
- fun match {
- case Select(qual, _) =>
- val qualSym = findHostClass(qual.tpe, sym)
- if (qualSym == ArrayClass) {
- targetTypeKind = tpeTK(qual)
- log(s"Stored target type kind for ${sym.fullName} as $targetTypeKind")
- }
- else {
- hostClass = qualSym
- if (qual.tpe.typeSymbol != qualSym) {
- log(s"Precisified host class for $sym from ${qual.tpe.typeSymbol.fullName} to ${qualSym.fullName}")
- }
- }
-
- case _ =>
- }
- if ((targetTypeKind != null) && (sym == definitions.Array_clone) && invokeStyle.isVirtual) {
- // An invokevirtual points to a CONSTANT_Methodref_info which in turn points to a
- // CONSTANT_Class_info of the receiver type.
- // The JVMS is not explicit about this, but that receiver type may be an array type
- // descriptor (instead of a class internal name):
- // invokevirtual #2; //Method "[I".clone:()Ljava/lang/Object
- val target: String = targetTypeKind.asRefBType.classOrArrayType
- bc.invokevirtual(target, "clone", "()Ljava/lang/Object;", app.pos)
- }
- else {
- genCallMethod(sym, invokeStyle, app.pos, hostClass)
- // Check if the Apply tree has an InlineAnnotatedAttachment, added by the typer
- // for callsites marked `f(): @inline/noinline`. For nullary calls, the attachment
- // is on the Select node (not on the Apply node added by UnCurry).
- def checkInlineAnnotated(t: Tree): Unit = {
- if (t.hasAttachment[InlineAnnotatedAttachment]) lastInsn match {
- case m: MethodInsnNode =>
- if (app.hasAttachment[NoInlineCallsiteAttachment.type]) noInlineAnnotatedCallsites += m
- else inlineAnnotatedCallsites += m
- case _ =>
- } else t match {
- case Apply(fun, _) => checkInlineAnnotated(fun)
- case _ =>
- }
+ generatedType = genCallMethod(sym, invokeStyle, app.pos, receiverClass)
+
+ // Check if the Apply tree has an InlineAnnotatedAttachment, added by the typer
+ // for callsites marked `f(): @inline/noinline`. For nullary calls, the attachment
+ // is on the Select node (not on the Apply node added by UnCurry).
+ def recordInlineAnnotated(t: Tree): Unit = {
+ if (t.hasAttachment[InlineAnnotatedAttachment]) lastInsn match {
+ case m: MethodInsnNode =>
+ if (app.hasAttachment[NoInlineCallsiteAttachment.type]) noInlineAnnotatedCallsites += m
+ else inlineAnnotatedCallsites += m
+ case _ =>
+ } else t match {
+ case Apply(fun, _) => recordInlineAnnotated(fun)
+ case _ =>
}
- checkInlineAnnotated(app)
}
-
- } // end of genNormalMethodCall()
-
- genNormalMethodCall()
-
- generatedType = methodBTypeFromSymbol(sym).returnType
+ recordInlineAnnotated(app)
+ }
}
-
}
generatedType
@@ -1026,11 +993,11 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
def genStringConcat(tree: Tree): BType = {
lineNumber(tree)
liftStringConcat(tree) match {
-
// Optimization for expressions of the form "" + x. We can avoid the StringBuilder.
case List(Literal(Constant("")), arg) =>
genLoad(arg, ObjectRef)
genCallMethod(String_valueOf, InvokeStyle.Static, arg.pos)
+
case concatenations =>
bc.genStartConcat(tree.pos)
for (elem <- concatenations) {
@@ -1047,74 +1014,75 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
bc.genConcat(elemType, loadedElem.pos)
}
bc.genEndConcat(tree.pos)
-
}
-
StringRef
}
- def genCallMethod(method: Symbol, style: InvokeStyle, pos: Position, hostClass0: Symbol = null) {
-
- val siteSymbol = claszSymbol
- val hostSymbol = if (hostClass0 == null) method.owner else hostClass0
+ /**
+ * Generate a method invocation. If `specificReceiver != null`, it is used as receiver in the
+ * invocation instruction, otherwise `method.owner`. A specific receiver class is needed to
+ * prevent an IllegalAccessError, (aladdin bug 455).
+ */
+ def genCallMethod(method: Symbol, style: InvokeStyle, pos: Position, specificReceiver: Symbol = null): BType = {
val methodOwner = method.owner
- // info calls so that types are up to date; erasure may add lateINTERFACE to traits
- hostSymbol.info ; methodOwner.info
-
- def needsInterfaceCall(sym: Symbol) = (
- sym.isTraitOrInterface
- || sym.isJavaDefined && sym.isNonBottomSubClass(definitions.ClassfileAnnotationClass)
- )
+ // the class used in the invocation's method descriptor in the classfile
+ val receiverClass = {
+ if (specificReceiver != null)
+ assert(style.isVirtual || specificReceiver == methodOwner, s"specificReceiver can only be specified for virtual calls. $method - $specificReceiver")
+
+ val useSpecificReceiver = specificReceiver != null && !specificReceiver.isBottomClass
+ val receiver = if (useSpecificReceiver) specificReceiver else methodOwner
+
+ // workaround for a JVM bug: https://bugs.openjdk.java.net/browse/JDK-8154587
+ // when an interface method overrides a member of Object (note that all interfaces implicitly
+ // have superclass Object), the receiver needs to be the interface declaring the override (and
+ // not a sub-interface that inherits it). example:
+ // trait T { override def clone(): Object = "" }
+ // trait U extends T
+ // class C extends U
+ // class D { def f(u: U) = u.clone() }
+ // The invocation `u.clone()` needs `T` as a receiver:
+ // - using Object is illegal, as Object.clone is protected
+ // - using U results in a `NoSuchMethodError: U.clone. This is the JVM bug.
+ // Note that a mixin forwarder is generated, so the correct method is executed in the end:
+ // class C { override def clone(): Object = super[T].clone() }
+ val isTraitMethodOverridingObjectMember = {
+ receiver != methodOwner && // fast path - the boolean is used to pick either of these two, if they are the same it does not matter
+ style.isVirtual &&
+ receiver.isTraitOrInterface &&
+ ObjectTpe.decl(method.name).exists && // fast path - compute overrideChain on the next line only if necessary
+ method.overrideChain.last.owner == ObjectClass
+ }
+ if (isTraitMethodOverridingObjectMember) methodOwner else receiver
+ }
- val isTraitCallToObjectMethod =
- hostSymbol != methodOwner && methodOwner.isTraitOrInterface && ObjectTpe.decl(method.name) != NoSymbol && method.overrideChain.last.owner == ObjectClass
+ receiverClass.info // ensure types the type is up to date; erasure may add lateINTERFACE to traits
+ val receiverName = internalName(receiverClass)
- // whether to reference the type of the receiver or
- // the type of the method owner
- val useMethodOwner = ((
- !style.isVirtual
- || hostSymbol.isBottomClass
- || methodOwner == definitions.ObjectClass
- ) && !(style.isSuper && hostSymbol != null)) || isTraitCallToObjectMethod
- val receiver = if (useMethodOwner) methodOwner else hostSymbol
- val jowner = internalName(receiver)
+ // super calls are only allowed to direct parents
+ if (style.isSuper && receiverClass.isTraitOrInterface && !cnode.interfaces.contains(receiverName))
+ cnode.interfaces.add(receiverName)
- if (style.isSuper && (isTraitCallToObjectMethod || receiver.isTraitOrInterface) && !cnode.interfaces.contains(jowner))
- cnode.interfaces.add(jowner)
+ def needsInterfaceCall(sym: Symbol) = {
+ sym.isTraitOrInterface ||
+ sym.isJavaDefined && sym.isNonBottomSubClass(definitions.ClassfileAnnotationClass)
+ }
val jname = method.javaSimpleName.toString
val bmType = methodBTypeFromSymbol(method)
val mdescr = bmType.descriptor
- def initModule() {
- // we initialize the MODULE$ field immediately after the super ctor
- if (!isModuleInitialized &&
- jMethodName == INSTANCE_CONSTRUCTOR_NAME &&
- jname == INSTANCE_CONSTRUCTOR_NAME &&
- isStaticModuleClass(siteSymbol)) {
- isModuleInitialized = true
- mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
- mnode.visitFieldInsn(
- asm.Opcodes.PUTSTATIC,
- thisName,
- strMODULE_INSTANCE_FIELD,
- "L" + thisName + ";"
- )
- }
- }
-
- if (style.isStatic) { bc.invokestatic (jowner, jname, mdescr, pos) }
- else if (style.isSpecial) { bc.invokespecial (jowner, jname, mdescr, pos) }
- else if (style.isVirtual) {
- if (needsInterfaceCall(receiver)) { bc.invokeinterface(jowner, jname, mdescr, pos) }
- else { bc.invokevirtual (jowner, jname, mdescr, pos) }
- }
- else {
- assert(style.isSuper, s"An unknown InvokeStyle: $style")
- bc.invokespecial(jowner, jname, mdescr, pos)
- initModule()
+ import InvokeStyle._
+ style match {
+ case Static => bc.invokestatic (receiverName, jname, mdescr, pos)
+ case Special => bc.invokespecial (receiverName, jname, mdescr, pos)
+ case Virtual =>
+ if (needsInterfaceCall(receiverClass)) bc.invokeinterface(receiverName, jname, mdescr, pos)
+ else bc.invokevirtual (receiverName, jname, mdescr, pos)
+ case Super => bc.invokespecial (receiverName, jname, mdescr, pos)
}
+ bmType.returnType
} // end of genCallMethod()
/* Generate the scala ## method. */
@@ -1122,7 +1090,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
genLoadModule(ScalaRunTimeModule) // TODO why load ScalaRunTimeModule if ## has InvokeStyle of Static(false) ?
genLoad(tree, ObjectRef)
genCallMethod(hashMethodSym, InvokeStyle.Static, applyPos)
- INT
}
/*
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
index 4c41cfc380..47884da560 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
@@ -59,7 +59,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
// current class
var cnode: asm.tree.ClassNode = null
- var thisName: String = null // the internal name of the class being emitted
+ var thisBType: ClassBType = null
var claszSymbol: Symbol = null
var isCZParcelable = false
@@ -91,9 +91,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
isCZParcelable = isAndroidParcelableClass(claszSymbol)
isCZStaticModule = isStaticModuleClass(claszSymbol)
isCZRemote = isRemote(claszSymbol)
- thisName = internalName(claszSymbol)
-
- val classBType = classBTypeFromSymbol(claszSymbol)
+ thisBType = classBTypeFromSymbol(claszSymbol)
cnode = new asm.tree.ClassNode()
@@ -123,7 +121,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
if (shouldAddLambdaDeserialize)
backendUtils.addLambdaDeserialize(cnode)
- cnode.visitAttribute(classBType.inlineInfoAttribute.get)
+ cnode.visitAttribute(thisBType.inlineInfoAttribute.get)
if (AsmUtils.traceClassEnabled && cnode.name.contains(AsmUtils.traceClassPattern))
AsmUtils.traceClass(cnode)
@@ -144,7 +142,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
val thisSignature = getGenericSignature(claszSymbol, claszSymbol.owner)
cnode.visit(classfileVersion, flags,
- thisName, thisSignature,
+ thisBType.internalName, thisSignature,
superClass, interfaceNames.toArray)
if (emitSource) {
@@ -157,7 +155,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
case _ => ()
}
- val ssa = getAnnotPickle(thisName, claszSymbol)
+ val ssa = getAnnotPickle(thisBType.internalName, claszSymbol)
cnode.visitAttribute(if (ssa.isDefined) pickleMarkerLocal else pickleMarkerForeign)
emitAnnotations(cnode, claszSymbol.annotations ++ ssa)
@@ -178,7 +176,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
}
if (isCandidateForForwarders) {
log(s"Adding static forwarders from '$claszSymbol' to implementations in '$lmoc'")
- addForwarders(isRemote(claszSymbol), cnode, thisName, lmoc.moduleClass)
+ addForwarders(isRemote(claszSymbol), cnode, thisBType.internalName, lmoc.moduleClass)
}
}
}
@@ -196,7 +194,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
val fv =
cnode.visitField(GenBCode.PublicStaticFinal, // TODO confirm whether we really don't want ACC_SYNTHETIC nor ACC_DEPRECATED
strMODULE_INSTANCE_FIELD,
- "L" + thisName + ";",
+ thisBType.descriptor,
null, // no java-generic-signature
null // no initial value
)
@@ -220,11 +218,11 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
/* "legacy static initialization" */
if (isCZStaticModule) {
- clinit.visitTypeInsn(asm.Opcodes.NEW, thisName)
+ clinit.visitTypeInsn(asm.Opcodes.NEW, thisBType.internalName)
clinit.visitMethodInsn(asm.Opcodes.INVOKESPECIAL,
- thisName, INSTANCE_CONSTRUCTOR_NAME, "()V", false)
+ thisBType.internalName, INSTANCE_CONSTRUCTOR_NAME, "()V", false)
}
- if (isCZParcelable) { legacyAddCreatorCode(clinit, cnode, thisName) }
+ if (isCZParcelable) { legacyAddCreatorCode(clinit, cnode, thisBType.internalName) }
clinit.visitInsn(asm.Opcodes.RETURN)
clinit.visitMaxs(0, 0) // just to follow protocol, dummy arguments
@@ -604,7 +602,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
if (!hasStaticBitSet) {
mnode.visitLocalVariable(
"this",
- "L" + thisName + ";",
+ thisBType.descriptor,
null,
veryFirstProgramPoint,
onePastLastProgramPoint,
@@ -686,8 +684,8 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
val jname = callee.javaSimpleName.toString
val jtype = methodBTypeFromSymbol(callee).descriptor
insnParcA = new asm.tree.MethodInsnNode(asm.Opcodes.INVOKESTATIC, jowner, jname, jtype, false)
- // PUTSTATIC `thisName`.CREATOR;
- insnParcB = new asm.tree.FieldInsnNode(asm.Opcodes.PUTSTATIC, thisName, "CREATOR", andrFieldDescr)
+ // PUTSTATIC `thisBType.internalName`.CREATOR;
+ insnParcB = new asm.tree.FieldInsnNode(asm.Opcodes.PUTSTATIC, thisBType.internalName, "CREATOR", andrFieldDescr)
}
// insert a few instructions for initialization before each return instruction
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala
index b0ec37db97..65bd62a413 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala
@@ -187,7 +187,7 @@ abstract class GenBCode extends BCodeSyncAndTry {
// -------------- "plain" class --------------
val pcb = new PlainClassBuilder(cunit)
pcb.genPlainClass(cd)
- val outF = if (needsOutFolder) getOutFolder(claszSymbol, pcb.thisName, cunit) else null
+ val outF = if (needsOutFolder) getOutFolder(claszSymbol, pcb.thisBType.internalName, cunit) else null
val plainC = pcb.cnode
// -------------- bean info class, if needed --------------
diff --git a/test/files/run/t2106.check b/test/files/run/t2106.check
index b19165824b..c8ebe575f0 100644
--- a/test/files/run/t2106.check
+++ b/test/files/run/t2106.check
@@ -1,5 +1,5 @@
t2106.scala:7: warning: A::foo()Ljava/lang/Object; is annotated @inline but could not be inlined:
-The callee A::foo()Ljava/lang/Object; contains the instruction INVOKEVIRTUAL java/lang/Object.clone ()Ljava/lang/Object;
+The callee A::foo()Ljava/lang/Object; contains the instruction INVOKEVIRTUAL A.clone ()Ljava/lang/Object;
that would cause an IllegalAccessError when inlined into class Test$.
def main(args: Array[String]): Unit = x.foo
^
diff --git a/test/junit/scala/issues/BytecodeTest.scala b/test/junit/scala/issues/BytecodeTest.scala
index 08b984a622..cf5c7f9ec3 100644
--- a/test/junit/scala/issues/BytecodeTest.scala
+++ b/test/junit/scala/issues/BytecodeTest.scala
@@ -10,6 +10,7 @@ import scala.tools.nsc.backend.jvm.CodeGenTools._
import org.junit.Assert._
import scala.collection.JavaConverters._
+import scala.tools.asm.Opcodes
import scala.tools.asm.tree.ClassNode
import scala.tools.partest.ASMConverters._
import scala.tools.testing.ClearAfterClass
@@ -341,4 +342,137 @@ class BytecodeTest extends ClearAfterClass {
checkForwarder('K12, "T2")
}
+
+ @Test
+ def invocationReceivers(): Unit = {
+ val List(c1, c2, t, u) = compileClasses(compiler)(invocationReceiversTestCode.definitions("Object"))
+ // mixin forwarder in C1
+ assertSameCode(getSingleMethod(c1, "clone"), List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "T", "clone", "()Ljava/lang/Object;", false), Op(ARETURN)))
+ assertInvoke(getSingleMethod(c1, "f1"), "T", "clone")
+ assertInvoke(getSingleMethod(c1, "f2"), "T", "clone")
+ assertInvoke(getSingleMethod(c1, "f3"), "C1", "clone")
+ assertInvoke(getSingleMethod(c2, "f1"), "T", "clone")
+ assertInvoke(getSingleMethod(c2, "f2"), "T", "clone")
+ assertInvoke(getSingleMethod(c2, "f3"), "C1", "clone")
+
+ val List(c1b, c2b, tb, ub) = compileClasses(compiler)(invocationReceiversTestCode.definitions("String"))
+ def ms(c: ClassNode, n: String) = c.methods.asScala.toList.filter(_.name == n)
+ assert(ms(tb, "clone").length == 1)
+ assert(ms(ub, "clone").isEmpty)
+ val List(c1Clone) = ms(c1b, "clone")
+ assertEquals(c1Clone.desc, "()Ljava/lang/Object;")
+ assert((c1Clone.access | Opcodes.ACC_BRIDGE) != 0)
+ assertSameCode(convertMethod(c1Clone), List(VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "C1", "clone", "()Ljava/lang/String;", false), Op(ARETURN)))
+
+ def iv(m: Method) = getSingleMethod(c1b, "f1").instructions.collect({case i: Invoke => i})
+ assertSameCode(iv(getSingleMethod(c1b, "f1")), List(Invoke(INVOKEINTERFACE, "T", "clone", "()Ljava/lang/String;", true)))
+ assertSameCode(iv(getSingleMethod(c1b, "f2")), List(Invoke(INVOKEINTERFACE, "T", "clone", "()Ljava/lang/String;", true)))
+ // invokeinterface T.clone in C1 is OK here because it is not an override of Object.clone (different siganture)
+ assertSameCode(iv(getSingleMethod(c1b, "f3")), List(Invoke(INVOKEINTERFACE, "T", "clone", "()Ljava/lang/String;", true)))
+ }
+
+ @Test
+ def invocationReceiversProtected(): Unit = {
+ // http://lrytz.github.io/scala-aladdin-bugtracker/displayItem.do%3Fid=455.html / 9954eaf
+ // also https://issues.scala-lang.org/browse/SI-1430 / 0bea2ab (same but with interfaces)
+ val aC =
+ """package a;
+ |/*package private*/ abstract class A {
+ | public int f() { return 1; }
+ | public int t;
+ |}
+ """.stripMargin
+ val bC =
+ """package a;
+ |public class B extends A { }
+ """.stripMargin
+ val iC =
+ """package a;
+ |/*package private*/ interface I { int f(); }
+ """.stripMargin
+ val jC =
+ """package a;
+ |public interface J extends I { }
+ """.stripMargin
+ val cC =
+ """package b
+ |class C {
+ | def f1(b: a.B) = b.f
+ | def f2(b: a.B) = { b.t = b.t + 1 }
+ | def f3(j: a.J) = j.f
+ |}
+ """.stripMargin
+ val List(c) = compileClasses(compiler)(cC, javaCode = List((aC, "A.java"), (bC, "B.java"), (iC, "I.java"), (jC, "J.java")))
+ assertInvoke(getSingleMethod(c, "f1"), "a/B", "f") // receiver needs to be B (A is not accessible in class C, package b)
+ println(getSingleMethod(c, "f2").instructions.stringLines)
+ assertInvoke(getSingleMethod(c, "f3"), "a/J", "f") // receiver needs to be J
+ }
+
+ @Test
+ def specialInvocationReceivers(): Unit = {
+ val code =
+ """class C {
+ | def f1(a: Array[String]) = a.clone()
+ | def f2(a: Array[Int]) = a.hashCode()
+ | def f3(n: Nothing) = n.hashCode()
+ | def f4(n: Null) = n.toString()
+ |
+ |}
+ """.stripMargin
+ val List(c) = compileClasses(compiler)(code)
+ assertInvoke(getSingleMethod(c, "f1"), "[Ljava/lang/String;", "clone") // array descriptor as receiver
+ assertInvoke(getSingleMethod(c, "f2"), "java/lang/Object", "hashCode") // object receiver
+ assertInvoke(getSingleMethod(c, "f3"), "java/lang/Object", "hashCode")
+ assertInvoke(getSingleMethod(c, "f4"), "java/lang/Object", "toString")
+ }
+}
+
+object invocationReceiversTestCode {
+ // if cloneType is more specific than Object (e.g., String), a bridge method is generated.
+ def definitions(cloneType: String) =
+ s"""trait T { override def clone(): $cloneType = "hi" }
+ |trait U extends T
+ |class C1 extends U with Cloneable {
+ | // The comments below are true when $cloneType is Object.
+ | // C1 gets a forwarder for clone that invokes T.clone. this is needed because JVM method
+ | // resolution always prefers class members, so it would resolve to Object.clone, even if
+ | // C1 is a subtype of the interface T which has an overriding default method for clone.
+ |
+ | // invokeinterface T.clone
+ | def f1 = (this: T).clone()
+ |
+ | // cannot invokeinterface U.clone (NoSuchMethodError). Object.clone would work here, but
+ | // not in the example in C2 (illegal access to protected). T.clone works in all cases and
+ | // resolves correctly.
+ | def f2 = (this: U).clone()
+ |
+ | // invokevirtual C1.clone()
+ | def f3 = (this: C1).clone()
+ |}
+ |
+ |class C2 {
+ | def f1(t: T) = t.clone() // invokeinterface T.clone
+ | def f2(t: U) = t.clone() // invokeinterface T.clone -- Object.clone would be illegal (protected, explained in C1)
+ | def f3(t: C1) = t.clone() // invokevirtual C1.clone -- Object.clone would be illegal
+ |}
+ """.stripMargin
+
+ val runCode =
+ """
+ |val r = new StringBuffer()
+ |val c1 = new C1
+ |r.append(c1.f1)
+ |r.append(c1.f2)
+ |r.append(c1.f3)
+ |val t = new T { }
+ |val u = new U { }
+ |val c2 = new C2
+ |r.append(c2.f1(t))
+ |r.append(c2.f1(u))
+ |r.append(c2.f1(c1))
+ |r.append(c2.f2(u))
+ |r.append(c2.f2(c1))
+ |r.append(c2.f3(c1))
+ |r.toString
+ """.stripMargin
}
diff --git a/test/junit/scala/issues/RunTest.scala b/test/junit/scala/issues/RunTest.scala
index 2bc8008222..781f2ef343 100644
--- a/test/junit/scala/issues/RunTest.scala
+++ b/test/junit/scala/issues/RunTest.scala
@@ -140,4 +140,11 @@ class RunTest extends ClearAfterClass {
val i = Integer.TYPE
assertEquals(run[List[Class[_]]](code), List(i, i))
}
+
+ @Test
+ def invocationReceivers(): Unit = {
+ import invocationReceiversTestCode._
+ assertEquals(run[String](definitions("Object") + runCode), "hi" * 9)
+ assertEquals(run[String](definitions("String") + runCode), "hi" * 9) // bridge method for clone generated
+ }
}
diff --git a/test/junit/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzerTest.scala
index 78dbab82f4..ab05c15e85 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzerTest.scala
@@ -68,12 +68,12 @@ class NullnessAnalyzerTest extends ClearAfterClass {
// So in the frame for `ALOAD 0`, the stack is still empty.
val res =
- """ L0: 0: NotNull
- | LINENUMBER 1 L0: 0: NotNull
- | ALOAD 0: 0: NotNull
- |INVOKEVIRTUAL java/lang/Object.toString ()Ljava/lang/String;: 0: NotNull, 1: NotNull
- | ARETURN: 0: NotNull, 1: Unknown1
- | L0: null""".stripMargin
+ """ L0: 0: NotNull
+ | LINENUMBER 1 L0: 0: NotNull
+ | ALOAD 0: 0: NotNull
+ |INVOKEVIRTUAL C.toString ()Ljava/lang/String;: 0: NotNull, 1: NotNull
+ | ARETURN: 0: NotNull, 1: Unknown1
+ | L0: null""".stripMargin
// println(showAllNullnessFrames(newNullnessAnalyzer(m), m))
assertEquals(showAllNullnessFrames(newNullnessAnalyzer(m), m), res)
}