[9] RFR(L) 8013267 : move MemberNameTable from native code to Java heap, use to intern MemberNames

[David Chase]

bug: https://bugs.openjdk.java.net/browse/JDK-8013267

webrevs:
http://cr.openjdk.java.net/~drchase/8013267/jdk.03/
http://cr.openjdk.java.net/~drchase/8013267/hotspot.03/

related bug: https://bugs.openjdk.java.net/browse/JDK-8042235

The “bug” is a cleanup/space-saving RFE that enables fixes for other bugs related
to method name tables — e.g., failure to properly update two method handles
referring to the same method after the method is redefined.  The patch is months
old and needed to be brought up to date.

In addition, the patch needed the following changes beyond what was seen in
other reviews:
1) on the hotspot side, code to adjust the membername table needed to check
    that the data for the membername table was actually allocated (one goal of this
    rfe was to avoid allocating that table) else it would null deref and crash.

2) on the jdk side, fencepost error in the membername table insertion; the first
    entry could be duplicated (  “if index > 0” wrong, should be “if index >= 0”).

3) on the jdk side, several of the constructors for MemberNames needed to be
    replaced with factories that would consult the MemberNameTable and avoid
    creating duplicates; again, that was the entire point of this fix.  *NOT ALL
    CONSTRUCTORS WERE MODIFIED LIKE THIS”, and there was one curious
    piece of code where it seemed that this might not be what was desired, but it
    was unclear how this would behave in the face of method redefinition.  From 

http://cr.openjdk.java.net/~drchase/8013267/jdk.03/src/java.base/share/classes/java/lang/invoke/MemberName.java.html

1047         private MemberName resolve(byte refKind, MemberName ref, Class<?> lookupClass) {
1048             MemberName m = ref.clone();  // JVM will side-effect the ref
1049             assert(refKind == m.getReferenceKind());
1050             try {
1051                 m = MethodHandleNatives.resolve(m, lookupClass);
1052                 m.checkForTypeAlias();
1053                 m.resolution = null;
1054                 m.intern();

Note that this performs an “intern” that ignores what is already found in the table
and may not place m in the table if a duplicate name is already there. ?????????


testing:
jtreg:
  hotspot/{compiler,runtime,gc}
  jdk/{vm,jdk,sun/{invoke,misc,reflect}
by-hand: verified that the crash example for 8042235 still crash in an unmodified VM and does not crash  in a modified VM.

Modified and retested
https://bugs.openjdk.java.net/secure/attachment/20184/RedefineMethodUsedByMultipleMethodHandles.java
Old:
        // Calling fooMH1.vmtarget crashes the VM
        System.out.println("fooMH1.invoke = " + fooMH1.invokeExact());

New:
        // Calling fooMH1.vmtarget crashes the VM
        System.out.println("fooMH1.invoke = " + fooMH1.invokeExact());
        System.out.println("fooMH2.invoke = " + fooMH2.invokeExact());

This turned out to be a reasonable thing to do, since the added line caused a crash till further corrections were made.
(See fixes 2 and 3 above).

Should RedefineMethodUsedByMultipleMethodHandles.java also be added as a test?

Are there other tests that would be good to run ?(and if they are ute tests, the more
detailed the instructions for how to run them, the better.)

David



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141017/7a27bb42/signature.asc>