Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need JVM support for nestmates #2269

Closed
keithc-ca opened this issue Jun 26, 2018 · 15 comments
Closed

Need JVM support for nestmates #2269

keithc-ca opened this issue Jun 26, 2018 · 15 comments

Comments

@keithc-ca
Copy link
Contributor

The latest merge from jdk-11+19 expects three new JVM functions; builds will fail with link errors until these are defined:

JNIEXPORT jboolean JNICALL
JVM_AreNestMates(JNIEnv *env, jclass current, jclass member);

JNIEXPORT jclass JNICALL
JVM_GetNestHost(JNIEnv *env, jclass current);

JNIEXPORT jobjectArray JNICALL
JVM_GetNestMembers(JNIEnv *env, jclass current);
@theresa-m
Copy link
Contributor

Working on stubs to unblock builds.

@JasonFengJ9
Copy link
Member

JasonFengJ9 commented Jun 26, 2018

I have a change to add these three methods however adding them alone still won't make raw build -version work due to a j.l.IllegalAccessError.

@tajila
Copy link
Contributor

tajila commented Jun 26, 2018

@JasonFengJ9 we need to turn on the spec as well, #2270

We also need to do some refactoring for the JVM_* methods so we don't duplicate the code, but that can be done after the stubs are added.

@JasonFengJ9
Copy link
Member

Enabled the nestmates spec as per #2270, now get following error:

Exception in thread "main" java/lang/IncompatibleClassChangeError: java/lang/ThreadLocal$ThreadLocalMap.getEntry(Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;
	at java/lang/ThreadLocal.get (java.base@9/ThreadLocal.java:165)
	at java/lang/StringCoding.decodeASCII (java.base@9/StringCoding.java:520)
	at java/lang/StringCoding.decode (java.base@9/StringCoding.java:235)
	at java/lang/String.<init> (java.base@9/String.java:586)
	at java/lang/String.<init> (java.base@9/String.java:627)
	at java/lang/System.initProperties (java.base@9/NativeMethod:4294967295)
	at java/lang/System.ensureProperties (java.base@9/System.java:284)
	at java/lang/System.afterClinitInitialization (java.base@9/System.java:128)
	at java/lang/Thread.initialize (java.base@9/Thread.java:371)
	at java/lang/Thread.<init> (java.base@9/Thread.java:153)

@JasonFengJ9
Copy link
Member

Discussed with Tobi, this IncompatibleClassChangeError doesn't happen in a CCM so it is raw build specific only.

@JasonFengJ9
Copy link
Member

Raw build with nestmates enabled without JVM_ stub methods shows symbol JVM_GetNestHost, version SUNWprivate_1.1 not defined in file libjvm.so
It appears JVM_ stub methods are required for raw builds.
If CCM doesn't require these JVM_ methods, we might have to re-think how to make raw builds.

@tajila
Copy link
Contributor

tajila commented Jun 27, 2018

@JasonFengJ9 turns out we will need the stubs.

They are used in Java_jdk_internal_reflect_Reflection_*

@keithc-ca
Copy link
Contributor Author

I suggest that 'raw builds' are no longer useful: there's no point expending effort to update them.

@JasonFengJ9
Copy link
Member

When the automated build starts merging latest OpenJDK tip into extension repo, it will be straightforward to discover VM/JCL incompatible issue with CCM than raw builds.

@pshipton
Copy link
Member

When the automated build starts merging latest OpenJDK tip into extension repo

This is already occurring, and is how we got the jdk-11+19 update.

@tajila
Copy link
Contributor

tajila commented Jun 27, 2018

Update: The version I tested with was b19 (394c091d02f906cbed0a783c06f464cf7dba1dc8) but slightly older than the one Jason was using. When I updated I ran into the same ICCE issue that Jason encountered.

The nestmates feature allows private access between nestmates. Because of this javac no longer has to generate access bridges for inner classes, it simply puts the inner classes in the same nest has the outer class. The outer class can then access private fields/methods in the inner classes directly.

Here is an example:

With last weeks javac:

 public T get();
    Code:
       0: invokestatic  #10                 // Method java/lang/Thread.currentThread:()Ljava/lang/Thread;
       3: astore_1
       4: aload_0
       5: aload_1
       6: invokevirtual #11                 // Method getMap:(Ljava/lang/Thread;)Ljava/lang/ThreadLocal$ThreadLocalMap;
       9: astore_2
      10: aload_2
      11: ifnull        33
      14: aload_2
      15: aload_0
      16: invokestatic  #12                 // Method java/lang/ThreadLocal$ThreadLocalMap.access$000:(Ljava/lang/ThreadLocal$ThreadLocalMap;Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;
      19: astore_3
      20: aload_3
      21: ifnull        33
      24: aload_3
      25: getfield      #13                 // Field java/lang/ThreadLocal$ThreadLocalMap$Entry.value:Ljava/lang/Object;
      28: astore        4
      30: aload         4
      32: areturn
      33: aload_0
      34: invokespecial #14                 // Method setInitialValue:()Ljava/lang/Object;
      37: areturn

With this weeks javac however:

  public T get();
    descriptor: ()Ljava/lang/Object;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=2, locals=5, args_size=1
         0: invokestatic  #10                 // Method java/lang/Thread.currentThread:()Ljava/lang/Thread;
         3: astore_1
         4: aload_0
         5: aload_1
         6: invokevirtual #11                 // Method getMap:(Ljava/lang/Thread;)Ljava/lang/ThreadLocal$ThreadLocalMap;
         9: astore_2
        10: aload_2
        11: ifnull        33
        14: aload_2
        15: aload_0
        16: invokevirtual #12                 // Method java/lang/ThreadLocal$ThreadLocalMap.getEntry:(Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;
        19: astore_3
        20: aload_3
        21: ifnull        33
        24: aload_3
        25: getfield      #13                 // Field java/lang/ThreadLocal$ThreadLocalMap$Entry.value:Ljava/lang/Object;
        28: astore        4
        30: aload         4
        32: areturn

The big change is :
16: invokestatic #12 // Method java/lang/ThreadLocal$ThreadLocalMap.access$000:(Ljava/lang/ThreadLocal$ThreadLocalMap;Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;

to:

16: invokevirtual #12 // Method java/lang/ThreadLocal$ThreadLocalMap.getEntry:(Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;

Our invokevirtual resolution doesn't currently deal with private methods which is causing the ICCE.

@tajila
Copy link
Contributor

tajila commented Jun 27, 2018

If we can turn off the javac nestmates changes for the CCM builds we should be fine, until we fix the rest of the issues on the interpreter side

@JasonFengJ9
Copy link
Member

As per Tobi suggested above, changing -source 11 -target 11 to -source 10 -target 10 within https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/openj9-jdk/make/common/SetupJavaCompilers.gmk#L75 (&L85) fixed the ICCE.
Ideally the change need to be made within https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/openj9-jdk/closed/make/common/SetupJavaCompilers.gmk instead.

@pshipton
Copy link
Member

pshipton commented Jun 28, 2018

I suppose we can do that temporarily, just to ensure we can continue to build/run this hybrid Java 11 while work on Nestmates changes are in progress. Anything using Java 11, such as compiling the tests, is going to use the default (-target 11) and not be able to run. Not sure we want to change everything only to have to revert it once Nestmates are working.

@pshipton
Copy link
Member

fyi #2897 which reverts to target 11. I don't believe we need this issue open any more, the remaining work is covered by other issues and PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants