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

Valhalla tests should use migrated value classes #20386

Closed
4 tasks done
theresa-m opened this issue Oct 18, 2024 · 8 comments
Closed
4 tasks done

Valhalla tests should use migrated value classes #20386

theresa-m opened this issue Oct 18, 2024 · 8 comments
Labels
comp:test project:valhalla Used to track Project Valhalla related work

Comments

@theresa-m
Copy link
Contributor

theresa-m commented Oct 18, 2024

Jep 401 migrates existing ValueBased classes (such as java/lang/Integer) to be value classes. Right now these classes are compiled into jdk/lib/valueclasses/java.base-valueclasses.jar and can be used by patching them into the java.base module --patch-module java.base=jdk/lib/valueclasses/java.base-valueclasses.jar

To complete this issue:

@theresa-m theresa-m added comp:test project:valhalla Used to track Project Valhalla related work labels Oct 18, 2024
Copy link

Issue Number: 20386
Status: Open
Recommended Components: comp:test, comp:vm, comp:build
Recommended Assignees: hangshao0, jasonfengj9, theresa-m

@theresa-m
Copy link
Contributor Author

This is currently blocked by #20372

@theresa-m
Copy link
Contributor Author

There is another MethodHandles error when trying to enable the use of classes in java.base-valueclasses.jar:

Exception in thread "main" java.lang.InternalError: java.lang.NoSuchFieldException: no such field: java.util.concurrent.atomic.Striped64.base/long/getField
	at java.base/jdk.internal.invoke.MhUtil.findVarHandle(MhUtil.java:62)
	at java.base/jdk.internal.invoke.MhUtil.findVarHandle(MhUtil.java:52)
	at java.base/java.util.concurrent.atomic.Striped64.<clinit>(Striped64.java:381)
	at java.base/java.util.concurrent.ConcurrentSkipListMap.addCount(ConcurrentSkipListMap.java:439)
	at java.base/java.util.concurrent.ConcurrentSkipListMap.doPut(ConcurrentSkipListMap.java:683)
	at java.base/java.util.concurrent.ConcurrentSkipListMap.putIfAbsent(ConcurrentSkipListMap.java:1789)
	at java.base/java.util.concurrent.ConcurrentSkipListSet.add(ConcurrentSkipListSet.java:240)
	at java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:1193)
	at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1812)
	at java.base/java.net.InetAddress.getLocalHost(InetAddress.java:1925)
	at org.testng.reporters.JUnitXMLReporter.generateReport(JUnitXMLReporter.java:152)
	at org.testng.reporters.JUnitXMLReporter.onFinish(JUnitXMLReporter.java:112)
	at org.testng.TestRunner.fireEvent(TestRunner.java:772)
	at org.testng.TestRunner.afterRun(TestRunner.java:741)
	at org.testng.TestRunner.run(TestRunner.java:509)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:455)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:450)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:415)
	at org.testng.SuiteRunner.run(SuiteRunner.java:364)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:84)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1208)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1137)
	at org.testng.TestNG.runSuites(TestNG.java:1049)
	at org.testng.TestNG.run(TestNG.java:1017)
	at org.testng.TestNG.privateMain(TestNG.java:1354)
	at org.testng.TestNG.main(TestNG.java:1323)
Caused by: java.lang.NoSuchFieldException: no such field: java.util.concurrent.atomic.Striped64.base/long/getField
	at java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:936)
	at java.base/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1013)
	at java.base/java.lang.invoke.MethodHandles$Lookup.resolveOrFail(MethodHandles.java:3744)
	at java.base/java.lang.invoke.MethodHandles$Lookup.findVarHandle(MethodHandles.java:3192)
	at java.base/jdk.internal.invoke.MhUtil.findVarHandle(MhUtil.java:60)
	... 26 more
Caused by: java.lang.NoSuchFieldError
	at java.base/java.lang.invoke.MethodHandleNatives.resolve(Native Method)
	at java.base/java.lang.invoke.MemberName$Factory.resolve(MemberName.java:981)
	at java.base/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1010)
	... 29 more

@hangshao0
Copy link
Contributor

The native implementation of MethodHandleNatives.resolve() is Java_java_lang_invoke_MethodHandleNatives_resolve(). The NoSuchFieldError is likely from:

vmFuncs->setCurrentExceptionUTF(currentThread, J9VMCONSTANTPOOL_JAVALANGNOSUCHFIELDERROR, NULL);
. You can look at why we entered the exception case. May be declaringClass is NULL.

@theresa-m
Copy link
Contributor Author

theresa-m commented Nov 14, 2024

I created a small program to reproduce this and its not related to migrated value classes, and only happens when an abstract class inherits from an abstract value class. Also finding classes seems to work but primitives are failing.

I've confirmed that the field is found but the offset is 0

*offsetOrAddress = result->offset;
.

The result never gets set here because the offset (which eventually sets the value of target) is 0.

result = vmFuncs->j9jni_createLocalRef(env, membernameObject);

The offset is 8 when CAbsVal is not a value class.

import java.lang.invoke.MethodHandles;
import jdk.internal.invoke.MhUtil;
class Test {
        public static void main(String[] args) throws Throwable {
                C m = new C();
        }
}


// error happens when this is a value class
abstract value class CAbsVal {}

abstract class CAbs extends CAbsVal {
        Object o;
        long l;
        static {
                MethodHandles.Lookup l1 = MethodHandles.lookup();
                MhUtil.findVarHandle(l1, "o", Object.class); // this is okay
                MhUtil.findVarHandle(l1, "l", long.class); // this fails
        }
}

class C extends CAbs {}

@hangshao0
Copy link
Contributor

For value classes, there is no lockword between the object header and the first field. So it is possible that the first field starts at offset 0.

MhUtil.findVarHandle(l1, "o", Object.class); // this is okay
MhUtil.findVarHandle(l1, "l", long.class); // this fails

Are you running with -Xnocompressedrefs ?
OpenJ9 will put field long l before Object o in -Xnocompressedrefs mode.
In compressedrefs mode, field Object o will be put before long l, so I expect MhUtil.findVarHandle(l1, "o", Object.class) to fail.

@theresa-m
Copy link
Contributor Author

I see, thanks. Yes I'm running with -Xnocompressedrefs. I tried with a compressed refs build and it does fail on Object o.

@theresa-m
Copy link
Contributor Author

theresa-m commented Dec 13, 2024

I added the option to openjdk test runs and re-enabled passing tests. Some of the passing tests are due to extensions updates and other unrelated changes. ibmruntimes/openj9-openjdk-jdk.valuetypes@79671f4

There are no new openjdk test failures that need addressing so I am closing this issue as resolved.

This is related to #13182 and #20404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:test project:valhalla Used to track Project Valhalla related work
Projects
None yet
Development

No branches or pull requests

2 participants