RFR 8174954: Parameter target type can yield successful access after a module read edge or a package export has occurred after failed resolution

[Lois Foltan]

Hi Harold,

I like your fix but I think there may be an issue, would like to know 
your opinion.  If I add another identical but distinct invokedynamic 
call in p5/c5.java, something like

  29 public class c5 {
          static final String printDescription = "In c5's method5 with param = ";
   30     public void method5(p2.c2 param,Module c2Module) { System.out.println(printDescription  + param);
                                                       methodAddReadEdge(c2Module); System.out.println(PrintDescription + param);  }
   31
   32     public void methodAddReadEdge(Module m) {
   33         // Add a read edge from p5/c5's module (first_mod) to second_mod
   34         c5.class.getModule().addReads(m);
   35     }
   36 }
   37

c5's method5 should end up with 2 invokedynamic bytecodes pointing at 
the same constant pool index

          4: invokedynamic #3,  0              // InvokeDynamic 
#0:makeConcatWithConstants:(Lp2/c2;)Ljava/lang/String;
          9: invokevirtual #4                  // Method 
java/io/PrintStream.println:(Ljava/lang/String;)V
         12: aload_0
         13: aload_2
         14: invokevirtual #5                  // Method 
methodAddReadEdge:(Ljava/lang/Module;)V
         17: getstatic     #2                  // Field 
java/lang/System.out:Ljava/io/PrintStream;
         20: aload_1
         21: invokedynamic #3,  0              // InvokeDynamic 
#0:makeConcatWithConstants:(Lp2/c2;)Ljava/lang/String;


The invokedynamic at #4 should fail due to an IAE and continue to fail, 
but the invokedynamic at #21 should succeed.  With your fix the 
invokedynamic at #21 fails with the cached exception.  I suspect what is 
happening is that the exception is stored at the original 
invokedynamic's CP index and not the rewritten invokedynamic's cpCache 
index.  Can you take a look at this and confirm?  I could be wrong, but 
if I am I think this is worth adding to the test.

Thanks,
Lois

On 6/8/2017 9:33 AM, harold seigel wrote:
> Hi,
>
> Please review this JDK-10 fix for JDK-8174954.  For invokedynamic, the 
> JVM currently does not conform to the following module related 
> upcoming JVM Spec change to 5.4.3 Resolution:
>
>    This means that a class in one module that attempts to access, via
>    resolution of a symbolic reference in its run-time constant pool, an
>    unexported public type in a different module will always receive the
>    same error indicating an inaccessible type (§5.4.4), even if the
>    Java SE Platform API is used to dynamically export the public type's
>    package at some time after the class's first attempt.
>
> This RFR fixes this issue by caching the LinkageError exception from 
> the initial resolution failure.  Subsequent resolution attempts will 
> then just throw the cached exception instead of actually re-doing the 
> resolution, and so will get the same failure even if the module graph 
> has been changed.
>
> Note that this fix also fixes JDK-8174942 
> <https://bugs.openjdk.java.net/browse/JDK-8174942>.
>
> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8174954/webrev/
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8174954
>
> The fix was tested with JCK Lang, VM, and API/java_lang tests, the 
> JTreg hotspot, java/io, java/lang, java/util and other tests, the 
> co-located NSK tests, RBT tests, and with JPRT.  The new tests (based 
> on Lois's test cases) were run with and without -Xcomp.
>
> Thanks, Harold
>