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

[Karen Kinnear]

Harold,

Running behind so I have not looked at your fix. I do agree with Lois on the interpretation of the expected behavior and probable way to track this.

thanks for tackling this,
Karen

> On Jun 8, 2017, at 1:21 PM, Lois Foltan <lois.foltan at oracle.com> wrote:
> 
> 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
>> 
>