wrong assumption about invokespecial

[lrytz]

I went again through the invokespecial spec. One thing i didn't realize before is that the invoked method may be defined in a different class than the one used as receiver in the invocation instruction.

The compiler assumes that invokespecial A.m will always invoke method m defined in class A, which is not the case: it may invoke an overriding member in a subclass of A.

Related / affected issues:

• inline new mixin methods #86: the optimizer considers invokespecial as statically resolved, this may not hold under separate compilation
• new trait encoding: skip unnecessary mixin methods #98
• https://issues.scala-lang.org/browse/SI-4989

Java example:

public class A {
  public int m() { return 1; }
}
public class B extends A {
  // public int m() { return 2; }
}
public class C extends B {
  public int m() { return super.m(); }

  public static void main(String[] args) {
    System.out.println((new C()).m());
  }
}

Method C.m has the invocation INVOKESPECIAL B.m ()I, even though there's no method m in B (it's commented out). According to the spec, the method to be invoked is selected by searching for a matching member (name and signature), starting at the superclass of C, continuing with further superclasses, and finally interfaces (default methods). So it will invoke A.m and print 1

If we un-comment the definition of B.m and only re-compile B.java, the INVOKESPECIAL will now invoke B.m and print 2.

Here's an example where we get things wrong:

class A {
  def m = 1
}

class B extends A {
  override def m = 2
}

trait T extends A

class C extends B with T {
  override def m = super[T].m
}

object Test {
  def main(args: Array[String]): Unit = {
    println((new C).m)
  }
}

According to the spec this should print 1 (right?)

C.super[T].x [...] is called a static super reference. In this case, the reference is to the type or method of x in the parent trait of C whose simple name is T.

In M4 we get

public class C extends B  implements T  {
  public m()I
    ALOAD 0
    INVOKESPECIAL T.m ()I
    IRETURN
}
public abstract interface T {
  // no method m
}

Which is incorrect, running the example gives java.lang.NoSuchMethodError: T.m()I.

In current 2.12.x we get this - the change is probably due to my recent PR scala/scala#5096:

public class C extends B  implements T  {
  public m()I
    ALOAD 0
    INVOKESPECIAL A.m ()I
    IRETURN
}

Running the example gives 2. What happens is that method selection (see the invokespecial spec) starts at the superclass of C, which is B, and looks for a member matching m()I. Selection does NOT start at A.

We make the same mistake in 2.11: the body of C.m contains INVOKESPECIAL A.m ()I, so running the example also gives 2.

[lrytz]

I added a lot of labels to this bug to make it look more important 🤓

[sjrd]

From the linked spec:

If all of the following are true, let C be the direct superclass of the current class:

• The resolved method is not an instance initialization method (§2.9).
• If the symbolic reference names a class (not an interface), then that class is a superclass of the current class.
• The ACC_SUPER flag is set for the class file (§4.1).

Otherwise, let C be the class or interface named by the symbolic reference.

Note the last sentence. Apparently we could achieve what we initially want simply by not setting ACC_SUPER on Scala-generated class files. I have no idea what else is impacted by this, though.

[sjrd]

Also, FTR, the corresponding Scala.js IR node (ApplyStatically) behaves as was previously assumed, i.e., the lookup does start at A.

[sjrd]

Uh re ACC_SUPER, my comment is wrong:

In Java SE 8 and above, the Java Virtual Machine considers the ACC_SUPER flag to be set in every class file, regardless of the actual value of the flag in the class file and the version of the class file.

(from https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.1)

[lrytz]

yeah, ACC_SUPER is really a historical artifact, we should not use it, see http://stackoverflow.com/questions/8949933/what-is-the-purpose-of-the-acc-super-access-flag-on-java-class-files

[retronym]

Eek. If only we could do in regular bytecode as https://docs.oracle.com/javase/7/docs/api/java/lang/invoke/MethodHandles.Lookup.html unreflectSpecial is able to do reflectively.

[retronym]

Static methods (paired with non static forwarders) are looking more and more like a way out of our invokespecial blues.

[lrytz]

Shockingly, even INVOKESTATIC B.m ()I does not necessarily mean that B.m is executed. Java example:

public class A {
  public static int m() { return 1; }
}
public class B extends A {
  public static int m() { return 2; }
}
public class C extends B {
  public static void main(String[] args) {
    System.out.println(B.m());
  }
}

This produces INVOKESTATIC B.m ()I, running C prints 2. Now delete the definition of m in B and re-compile only B.java. Method resolution does not fail, but instead returns A.m, so running C now prints 1.

[retronym]

It sure is hard to disclaim one's inheritance.

In addition to the separate compilation example, it is legal in Java source to write B.m in the absence of the method in B if it exists in A. If A is inaccessible, this can be the only way to call it.

[lrytz]

Ah, right! Anyway this issue is less problematic because we can statically resolve the method (in the scala compiler / optimizer), assuming the correct classes are on the classpath, which is a core assumption of the optimizer anyway.

[milessabin]

I thought you might be interested in the shenanigans I had to go through to do a bit of monkey-patching of scala.reflect.internal.Types in a compiler plugin, because I think it's related.

I wanted to "override" the nested object TypeVar in Types, and attempted to do that via this Java shim. That compiles, but fails at runtime in TypeVarSubs constructor when it attempts to make the superclass constructor call.

That happens because javac expects the super constructor to have the signature (Lscala/reflect/internal/Types;)V and generates the corresponding invokespecial. However scalac has compiled the constructor with the signature (Lscala/reflect/internal/SymbolTable;)V ... the result is a NoSuchMethodError at runtime.

The workaround was monstrous but effective.

[retronym]

I am prototyping a backend change to use invokestatic rather than invokespecial for calls to statically resolved trait methods: https://github.com/scala/scala/compare/2.12.x...retronym:topic/trait-statics?expand=1

[sjrd]

@retronym Under that scheme, what will happen to super calls to Java-defined default methods?

[retronym]

@sjrd Good question. Here's a test case to consider:

trait T extends java.lang.Iterable[String] {

  override def spliterator(): java.util.Spliterator[String] = {
    println("T.spliterator")
    println("super.spliterator = " + super.spliterator)
    null
  }
  def foo = {
    println("T.foo")
    println("super[Iterable].spliterator = " + super[Iterable].spliterator)
    ()
  }
  def iterator(): java.util.Iterator[String] = java.util.Collections.emptyList().iterator()
}
class C extends T
object Test {
  def main(args: Array[String]): Unit = {
    val t = new C
    t.foo
    t.spliterator

  }
}

Clearly we can't rewrite super calls to Java defaults as invokestatic-s. I've updated my WIP branch to handle this, but now we get an IncompatibleClassChangeError in

java.lang.IncompatibleClassChangeError: Interface method reference: java.lang.Iterable.spliterator()Ljava/util/Spliterator;, is in an indirect superinterface of C
    at C.T$$super$spliterator(test3.scala:14)

So we either need to:

• keep the ugly "lateinterfaces" plumbing in place, to add Iterable as a direct parent of C in order to emit the super accessor, or
• outlaw super.spliterator, saying "cannot use unqualified super when a java default method may be selected", or
• outlaw class C extends T, with: "unable to emit super accessor to `Iterable#spliterator").

Incidentally, that test case crashes in Mixin in 2.11.8

[lrytz]

with scala/scala#5251 merged, we can now also fix this one.

[lrytz]

Bummer, I just realized that we cannot fix the example in this ticket, here it is again:

class A { def m = 1 }
class B extends A { override def m = 2 }
trait T extends A
class C extends B with T {
  override def m = super[T].m // should invoke A.m
}

In bytecode, there are only two implementations of m: one in A, the other in B, both of which are classes (not traits). So the two implementations are ordinary virtual methods, there's no static impl method that could be invoked in super[T].m.

It doesn't matter whether we emit invokespecial A.m or invokespecial B.m, the spec of invokespecial says that lookup starts at the parent of C, which is B, so it will always invoke B.m. The "class named by the symbolic reference", i.e., the receiver class in the bytecode instruction, is ignored.

[retronym]

I've prototyped a @DarkDimius's idea about using reflection +indy to link exactly to the methods we want. This fixes the example above, and also lets us collapse the static-method-and-forwarder pair for traits back to a single method.

But... it requires us to sneakily get a hold of MethodHandle.Lookup.TRUSTED. Which might be a no-go. :(

retronym/scala#24

[DarkDimius]

But... it requires us to sneakily get a hold of MethodHandle.Lookup.TRUSTED. Which might be a no-go. :(

@retronym, why do you need it? Afaik it would only be needed if you are intending to access the method outside of subclass higherarchy. This is why in my scheme I proposed using static fields to obtain the method handles.

Note, that in case of super[X] in traints, as traits are interfaces that only inherit Object, the are outside of the hierarchy. But we create super-accessors in classes when they are mixed in, so we should be fine.

[retronym]

unreflectSpecial requires a MethodHandles.Lookup from the owner of the method, even if the method itself is public and the callsite is in a subclass.

[DarkDimius]

@retronym, can't you use the Lookup.in to create a lookup class that matches superclass? It should allow you to downgrade a subclass Lookup object into a super-class one. You would loose ability to lookup private methods, but public methods should be still accessible.

[retronym]

Assigning to backlog because we don't seem to have a solution in mind that is workable for 2.12.0.

The fundamental problem seems to be the fact that Scala allows traits to extend classes. I wonder if this something that we should deprecate in dotty? A trait could still have the class a a self-type.

[retronym]

@DarkDimius Unfortunately we need Private access capability to bind to a public method with unreflectSpecial.

    public static CallSite bootstrap(MethodHandles.Lookup lookup, String invokedName, MethodType invokedType,
                                     Class<?> fromSite) throws Throwable {
        Method method = fromSite.getMethod(invokedName, invokedType.dropParameterTypes(0, 1).parameterArray());
        System.out.println(lookup.lookupClass());
        System.out.println(fromSite);
        return new ConstantCallSite(lookup.in(fromSite).unreflectSpecial(method, fromSite));
    }

class C
class A
java.lang.BootstrapMethodError: call site initialization exception
    at java.lang.invoke.CallSite.makeSite(CallSite.java:341)
    at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:307)
    at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:297)
    at C.m(sd143.scala:12)
Caused by: java.lang.IllegalAccessException: no private access for invokespecial: class A, from A/package
    at java.lang.invoke.MemberName.makeAccessException(MemberName.java:850)
    at java.lang.invoke.MethodHandles$Lookup.checkSpecialCaller(MethodHandles.java:1567)
    at java.lang.invoke.MethodHandles$Lookup.unreflectSpecial(MethodHandles.java:1226)
    at scala.runtime.InvokeExact.bootstrap(InvokeExact.java:16)
    at java.lang.invoke.CallSite.makeSite(CallSite.java:294)

[retronym]

Alex Buckley pointed out a plausible historical explanation for this latent bug: the semantics of invokespecial for super calls historically was correct, but was changed to use virtual dispatch for class files that use ACC_SUPER (which we did as of https://github.com/scala/commit/7886881b346e01924d95ea2ec1770bf851edaccb). This flag is assumed unconditionally in JVM 1.7 and above.

Background: http://stackoverflow.com/questions/8949933/what-is-the-purpose-of-the-acc-super-access-flag-on-java-class-files

So we sort of stepped into the wrong semantics with by declaring the ACC_SUPER flag, but in any case we can't go backwards now because the JVM doesn't support it.

[lrytz]

scala/scala#5369 for the example shown in #143 (comment)

[lrytz]

See also scala/scala#5377