Skip to content

Java 1.8.0_131 breaks some super calls inside 2.12 lambdas #10290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SethTisue opened this issue Apr 27, 2017 · 10 comments
Closed

Java 1.8.0_131 breaks some super calls inside 2.12 lambdas #10290

SethTisue opened this issue Apr 27, 2017 · 10 comments
Assignees
Labels
Milestone

Comments

@SethTisue
Copy link
Member

as reported by @milessabin, and reproduced by myself and @dwijnand, test/files/run/t4300.scala (#4300) and test/files/run/t8803.scala (#8803) are failing now on Java 8 Update 131 with e.g.

java.lang.IllegalAccessError: Receiver class Test$$anon$1 must be the current class or a subtype of interface A

I haven't isolated the exact circumstances, but both tests involve super within lambdas

@SethTisue SethTisue added this to the 2.12.3 milestone Apr 27, 2017
@SethTisue
Copy link
Member Author

Update 131 release notes: http://www.oracle.com/technetwork/java/javase/8u131-relnotes-3565278.html, and the full fix list is at http://www.oracle.com/technetwork/java/javase/2col/8u131-bugfixes-3565760.html. surprisingly, I see nothing relevant in either place (nor did Miles)

@SethTisue
Copy link
Member Author

[info] >>> The dbuild result is------------: SUCCESS (project rebuilt ok)

community build is all green, so it seems this won't cause widespread breakage. whew

@lrytz
Copy link
Member

lrytz commented May 3, 2017

Minimized:

trait A {
  def f() = println("A")
}

class B extends A {
  class C {
    def c() = B.super[A].f()
  }
}

object Test {
  def main(args : Array[String]) : Unit = {
    val b = new B()
    new b.C().c()
  }
}

@lrytz
Copy link
Member

lrytz commented May 3, 2017

It seems that check was added here: http://hg.openjdk.java.net/jdk8u/jdk8u/hotspot/rev/0b85ccd62409#l11.66

@lrytz
Copy link
Member

lrytz commented May 3, 2017

We have

public abstract interface A {
  public static synthetic f$(LA;)V
    // parameter final synthetic  $this
   L0
    LINENUMBER 2 L0
    ALOAD 0
    INVOKESPECIAL A.f ()V
}

public class B$C {
  public c()V
   L0
    LINENUMBER 7 L0
    ALOAD 0
    INVOKESTATIC A.f$ (LA;)V
}

I wonder why this worked before: B$C doesn't conform to A, so passing this in B$C.c to A.f$ should have produced a verifier error?

@lrytz
Copy link
Member

lrytz commented May 3, 2017

Indeed, on the older JVMs, a slightly modified version leads to an IncompatibleClassChangeError:

trait A {
  def bar = println("A")
  def f() = bar
}
// otherwise the same as above
java.lang.IncompatibleClassChangeError: Class B$C does not implement the requested interface A
	at A.f(Test.scala:3)
	at A.f$(Test.scala:3)
	at B$C.c(Test.scala:8)

@lrytz
Copy link
Member

lrytz commented May 3, 2017

So in a way that's good news: the code we generate was wrong and would only not crash in the most trivial cases (on old JVMs). The difference on 1.8.0_131 is only for those trivial cases (now they crash as well).

@lrytz
Copy link
Member

lrytz commented May 3, 2017

Also to note: it's a regression in 2.12, 2.11 correctly uses the outer when invoking A.f:

public class B$C {
  public c()V
   L0
    LINENUMBER 7 L0
    ALOAD 0
    INVOKEVIRTUAL B$C.B$C$$$outer ()LB;
    INVOKESTATIC A$class.f (LA;)V

lrytz added a commit to lrytz/scala that referenced this issue Jun 14, 2017
Consider

    class B extends T { class C { B.super[T].f }}

After flatten, that call is ` B$C.this.$outer().super[T].f()`.

In 2.11, mixin translates this to `A$class.f(B$C.this.$outer())`.
In 2.12, the tree is passed unchanged to the backend.

In `genApply` we assumed that in `Apply(Select(Super(qual, ... )))`,
`qual` is a `This` tree, so we just emitted `ALOAD_0`, which caused
the `$outer()` call to get lost. Now we invoke `genLoad(qual)`.

Fixes scala/bug#10290.
@lrytz lrytz added the has PR label Jun 14, 2017
lrytz added a commit to lrytz/scala that referenced this issue Jun 15, 2017
Consider

    class B extends T { class C { B.super[T].f }}

After flatten, that call is ` B$C.this.$outer().super[T].f()`.

In 2.11, mixin translates this to `A$class.f(B$C.this.$outer())`.
In 2.12, the tree is passed unchanged to the backend.

In `genApply` we assumed that in `Apply(Select(Super(qual, ... )))`,
`qual` is a `This` tree, so we just emitted `ALOAD_0`, which caused
the `$outer()` call to get lost. Now we invoke `genLoad(qual)`.

Fixes scala/bug#10290.
lrytz added a commit to lrytz/scala that referenced this issue Jun 15, 2017
Consider

    class B extends T { class C { B.super[T].f }}

After flatten, that call is ` B$C.this.$outer().super[T].f()`.

In 2.11, mixin translates this to `A$class.f(B$C.this.$outer())`.
In 2.12, the tree is passed unchanged to the backend.

In `genApply` we assumed that in `Apply(Select(Super(qual, ... )))`,
`qual` is a `This` tree, so we just emitted `ALOAD_0`, which caused
the `$outer()` call to get lost. Now we invoke `genLoad(qual)`.

Fixes scala/bug#10290.
@milessabin
Copy link

I'm still seeing this on 2.13.x ... is the forward port of the fix still pending?

@lrytz
Copy link
Member

lrytz commented Jul 19, 2017

Yes, it's here: scala/scala#5997

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants