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

[harold seigel]

Hi Lois,

Thanks for your comments.  I'll  look into this and let you know what I 
find.

Harold


On 6/8/2017 1:21 PM, Lois Foltan 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
>>
>