JDK-8027227 : [asm] generate CONSTANT_InterfaceMethodref for invoke{special/static) of non-abstract methods on ifaces
  • Type: Bug
  • Component: other-libs
  • Affected Version: 8
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2013-10-24
  • Updated: 2014-10-01
  • Resolved: 2013-11-06
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8 b117Fixed port-stage-ppc-aixFixed
Related Reports
Blocks :  
Duplicate :  
Relates :  
Description
The internal ASM (and the external version) does not generate the correct constant pool entry for invokespecial and invokestatic instructions for non-abstract methods on interfaces.

Currently ASM will generate a CONSTANT_Methodref entry, rather than a CONSTANT_InterfaceMethodref. This in turn results in incorrect byte code of generated anon classes for lambda expressions and method references, that may result in possible VM-related errors.

A simple class exercises this aspect:

import java.util.function.*;

class A {
  interface I {
      default Supplier<Integer> a() { return () -> 1; }
      default Supplier<Integer> b(int i) { return () -> i; }
      default Supplier<Integer> c(int i) { return () -> m(i); }

      int m(int i);
      
      static Integer d() {
          return 0;
      }
  }

  static class C implements I {
      public int m(int i) { return i;}
  }

  public static void main(String[] args) {
      I i = new C();

      i.a();
      i.b(1);
      i.c(1);
      I.d();
  }
}

Compile and execute with:

java -Djdk.internal.lambda.dumpProxyClasses=. -cp . A

then analyze the byte code of the generated proxy classes, for example:

  javap -v -p A\$I\$\$Lambda\$1.class

Note this currently executes fine, even though the constant pool entry is incorrect (possibly indicating a byte code validation issue with the VM).
Comments
8-critical-sqe: sqe agrees to fix 8.
05-11-2013

Agree to get these changes pushed, testing covered by existing tests.
05-11-2013

The webrev for this is here: http://cr.openjdk.java.net/~ksrini/8027227/webrev.0/ There are no unit tests for this changeset, all existing tests for nashorn, lambda and jfr tests must pass.
05-11-2013

Two questions: - Where can I see the unit test that was written by [~ksrini]? Will that unit test be pushed with this fix even though existing regression tests are being used as a way to confirm the fix? - Also, where can I get the changeset for this fix?
04-11-2013

8-critical-dev: justification, this fixes ASM code base for JSR-335 compliant byte code generation, this patch also contains fixes for LocalVariableTable needed for improved debugging capabilities, and fixes to StackMapTable frame generation required for byte-code verification, not doing this will cause regressions in lambda generated byte code as well serviceability related issues.
01-11-2013

There are no specific regression tests, all existing regression tests for java/lang/invoke, java/util, nashorn, jrunscript, and jfr must pass.
01-11-2013

Ok I got a unit test for the first case which tests for maj version 52 and InterfaceMethodRef for the invokestatic and invokespecial. I also have now a classfile creator which creates a class and validates it using javap via APIs in the same vm.
28-10-2013

I was thinking along the same lines, and yes it is easy these days to run javap via an API, we are currently using this in the pack200 verifier tests, as of the indy work in pack200. Here is an example: X.....\langtools\test\tools\javap\classfile\6888367\T6888367.java Also I think from what I am hearing is that the ASM and j.l.i patch needs to pushed together, but definitely as two separate patches.
25-10-2013

I think a strict unit test is enough; no lambdas are needed. Use ASM itself to assemble a trivial test class with the new visit* method (and itf=true). Then use "javap -c" and a string search to find evidence that the proper kind of CP entry was emitted. If you assemble a call to java.util.function.Function.identity, the evidence will look like: 2: invokestatic #4 // InterfaceMethod java/util/function/Function.identity:()Ljava/util/function/Function; (For extra points, find a way to run javap not as a command line but from in the same JVM that runs ASM.)
25-10-2013

As I am getting ready to go on this, I realized something, what happens if we decouple the ASM and j.l.i fixes in the asm-intfc.patch ? will something break in the JDK ? should we have synchronized pushes wrt. ASM and j.l.i fixes ?
25-10-2013

I am not aware of it affecting any known failures, it is just one aspect that *might* be affecting some things (since the lambda proxy class bytecode is incorrect).
25-10-2013

I also tested this, "With or without you" -U2 asm-intfc.patch, DefaultMethodsTest has the same behavior. So what/which existing failure does this change affect ?
25-10-2013

I think you are observing JDK-8026735 (Stream tests throw java.lang.IncompatibleClassChangeError) for the failures in ReduceByOpTest. Yes, a test should be associated with JDK-8027232 (i believe for lambdas it should be possible to run jtreg with the dumpProxyClasses property set and then parse the dumped class files, not quite sure how to test for the change to InvokerBytecodeGenerator).
25-10-2013

Also don't we need some sort of an automated test for this ? We need a test, and the test needs to be co-located and pushed with j.l.i changes, here is why, if this issue is fixed in a newer drop of ASM then we can cleanly apply an anti-delta and remove this patch in the future without having to worry about the test.
25-10-2013

Testing with the asm-intfc.patch, the good news is most of the tests pass, ie. nashorn tests, lambda tests, j.l.i and jfr. The bad news is that there is one test failure with java/util/stream/test/org/openjdk/tests/java/util/stream/ReduceByOpTest.java with the good old ICCE. The changes in j.l.i.{InnerClassLambdaMetafactory, InvokeByteCodeGenerator} are supposed to fix this right ? Attached the .jtr file for your reference.
24-10-2013

Two patches are attached that resolve this issue. The first, asm-intf.patch, is a minimal patch that enhances the internal version of ASM and additionally patches the lambda meta factory code to use the enhancement (including updating the version of the generated class files, otherwise the hotspot byte code verifier will fail with the example presented in the description). The second, asm-intf-more.patch, attempts to complete enhancement to other areas of the internal ASM code base. In the interim of this enhancement being implemented in the open ASM code base (the ASM developers have been kept in the loop on this) it would be useful to apply the minimal patch internally so as to remove one variable from the equation of potential VM bugs. To do that it has to be determined that the minimal patch is sufficient for the usages of the internal ASM. This is looking likely for the case of the lambda meta factory (stream and lang.invoke tests pass) but needs to be determined for other areas to increase confidence this will not cause any regression.
24-10-2013