[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR 8174954: Parameter target type can yield successful access after a module read edge or a pac
From:       Karen Kinnear <karen.kinnear () oracle ! com>
Date:       2017-06-09 18:46:08
Message-ID: B21D6B25-D5E3-475A-B544-EE7EE03A1624 () oracle ! com
[Download RAW message or body]

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@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
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]