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

OpenJDK com/sun/jdi/ClassesByName2Test.java fails on 11.0.18 #16604

Closed
pshipton opened this issue Jan 25, 2023 · 5 comments · Fixed by #16613
Closed

OpenJDK com/sun/jdi/ClassesByName2Test.java fails on 11.0.18 #16604

pshipton opened this issue Jan 25, 2023 · 5 comments · Fixed by #16613

Comments

@pshipton
Copy link
Member

pshipton commented Jan 25, 2023

The OpenJDK test can be run manually or via a grinder. The platform doesn't matter. The test passes on jdk17.
https://openj9-jenkins.osuosl.org/view/Test/job/Grinder/1883/

The following should recreate it.
https://openj9-jenkins.osuosl.org/job/Grinder/parambuild/?TARGET=jdk_custom&SDK_RESOURCE=nightly&TEST_FLAG=&UPSTREAM_TEST_JOB_NAME=&DOCKER_REQUIRED=false&VENDOR_TEST_DIRS=&TKG_OWNER_BRANCH=AdoptOpenJDK%3Amaster&OPENJ9_SYSTEMTEST_OWNER_BRANCH=eclipse%3Amaster&PLATFORM=aarch64_linux&GENERATE_JOBS=false&PERSONAL_BUILD=true&ADOPTOPENJDK_REPO=https%3A%2F%2Fgithub.com%2Fadoptium%2Faqa-tests.git&LABEL=&EXTRA_OPTIONS=&CUSTOMIZED_SDK_URL=https%3A%2F%2Fopenj9-artifactory.osuosl.org%2Fartifactory%2Fci-openj9%2FBuild_JDK%24%7BJDK_VERSION%7D_%24%7BPLATFORM%7D_Nightly%2F&DAYS_TO_KEEP=7&ADOPTOPENJDK_BRANCH=master&ARTIFACTORY_SERVER=ci-eclipse-openj9&KEEP_WORKSPACE=false&JDK_VERSION=11&ITERATIONS=1&EXIT_FAILURE=false&EXIT_SUCCESS=false&VENDOR_TEST_REPOS=&JDK_REPO=&OPENJ9_BRANCH=master&OPENJ9_SHA=&VENDOR_TEST_BRANCHES=&OPENJ9_REPO=https%3A%2F%2Fgithub.com%2Feclipse-openj9%2Fopenj9.git&UPSTREAM_JOB_NAME=&CUSTOM_TARGET=com%2Fsun%2Fjdi%2FClassesByName2Test.java&DEBUG_IMAGES_REQUIRED=false&VENDOR_TEST_SHAS=&JDK_BRANCH=&TEST_IMAGES_REQUIRED=true&LABEL_ADDITION=&ARTIFACTORY_REPO=ci-openj9&ARTIFACTORY_ROOT_DIR=Test&UPSTREAM_TEST_JOB_NUMBER=&JDK_IMPL=openj9&AUTO_DETECT=true&DYNAMIC_COMPILE=true&ADOPTOPENJDK_SYSTEMTEST_OWNER_BRANCH=AdoptOpenJDK%3Amaster&TKG_ITERATIONS=1&CUSTOMIZED_SDK_URL_CREDENTIAL_ID=&ARCHIVE_TEST_RESULTS=false&NUM_MACHINES=1&BUILD_LIST=openjdk&USE_TESTENV_PROPERTIES=false&UPSTREAM_JOB_NUMBER=&STF_OWNER_BRANCH=AdoptOpenJDK%3Amaster&JDK_VENDOR=&BUILDS_TO_KEEP=60&JVM_OPTIONS=&PARALLEL=NodesByIterations

Also reported internally

I used amac builds to narrow it down.

Last passing

08:41:27  openjdk version "11.0.17-beta" 2022-10-18
08:41:27  IBM Semeru Runtime Open Edition 11.0.17+8-202210210103 (build 11.0.17-beta+8-202210210103)
08:41:27  Eclipse OpenJ9 VM 11.0.17+8-202210210103 (build master-3ed4e68ca, JRE 11 Mac OS X aarch64-64-Bit 20221020_169 (JIT enabled, AOT enabled)
08:41:27  OpenJ9   - 3ed4e68ca
08:41:27  OMR      - 10c21d5f4
08:41:27  JCL      - 469a7112c0 based on jdk-11.0.17+8)

First failing

08:43:33  openjdk version "11.0.17" 2022-10-18
08:43:33  IBM Semeru Runtime Open Edition 11.0.17+8 (build 11.0.17+8)
08:43:33  Eclipse OpenJ9 VM 11.0.17+8 (build master-1af0a294c, JRE 11 Mac OS X aarch64-64-Bit 20221021_170 (JIT enabled, AOT enabled)
08:43:33  OpenJ9   - 1af0a294c
08:43:33  OMR      - f5970f08f
08:43:33  JCL      - 469a7112c0 based on jdk-11.0.17+8)

3ed4e68...1af0a29
eclipse-openj9/openj9-omr@10c21d5...f5970f0

Given the changes, the failure is caused by #16100

The class name in the failure can vary, but is a hidden class:

08:44:26  java.lang.Exception: CLASS NOT FOUND (by classesByName): java.util.ResourceBundle$$Lambda$23.0x0000000000000000
08:44:26  	at ClassesByName2Test.runTests(ClassesByName2Test.java:174)

The test (test/jdk/com/sun/jdi/ClassesByName2Test.java) is doing the following:

            List all = vm().allClasses();
            for (Iterator it = all.iterator(); it.hasNext(); ) {
                ReferenceType cls = (ReferenceType)it.next();
                String name = cls.name();
                List found = vm().classesByName(name);
                if (found.contains(cls)) {

com.sun.tools.jdi.VirtualMachineImpl implements classesByName() by calling com.sun.tools.jdi.JNITypeParser.typeNameToSignature(className) and then looks for the classes by that signature.

I ran a small test case to test JNITypeParser.typeNameToSignature().

import com.sun.tools.jdi.*;
import java.lang.reflect.*;

public class TypeName {
public static void main(String[] args) throws Exception {
	Method m = JNITypeParser.class.getDeclaredMethod("typeNameToSignature", String.class);
	m.setAccessible(true);
	String sig = (String)m.invoke(JNITypeParser.class, "java.util.ResourceBundle$$Lambda$23.0x0000000000000000");
	System.out.println(sig);
	String sig2 = (String)m.invoke(JNITypeParser.class, "java.util.ResourceBundle$$Lambda$23/0x0000000000000000");
	System.out.println(sig2);
}
}

javac --add-exports jdk.jdi/com.sun.tools.jdi=ALL-UNNAMED TypeName.java

The result is as follows. The . was converted to / in the signature, which I expect explains why the class is not found.

Ljava/util/ResourceBundle$$Lambda$23/0x0000000000000000;
Ljava/util/ResourceBundle$$Lambda$23/0x0000000000000000;
@pshipton
Copy link
Member Author

@tajila @ehrenjulzert

@ehrenjulzert
Copy link

What #16100 does is check if signature[signatureLength - 20] == '/' and then converts the / to a . if that is true. It looks like in your example a signature is passed in that doesn't end in a semicolon, which I believe is necessary for signature[signatureLength - 20] to find the correct character.

@ehrenjulzert
Copy link

Oh nvm I forgot jvmtiGetClassSignature is what builds the signature in the first place so that can't be the issue.

@ehrenjulzert
Copy link

When I run the test locally it passes on jdk19 and on valhalla. It also looks like JNITypeParser.typeNameToSignature is implemented differently in jdk19 and valhalla to take hidden class signatures into account by adding the . into the class signature instead of a / (which is the same behaviour that #16100 adds). I'm guessing this means that there is a difference in spec between JDK 11 and 19 somewhere which says that hidden class signatures need a . on the end, but this isn't required in earlier versions. So the solution is to conditionally add the . in jvmtiGetClassSignature based on what version of java we're running, but I haven't been able to figure out in what version exactly this change takes place (only that it's between 11 and 19).

@pshipton
Copy link
Member Author

pshipton commented Jan 25, 2023

The test passes on jdk17, and running my testcase produces the following on jdk17. We don't support jdk12 - jdk16, so this should be all you need to know.

Ljava/util/ResourceBundle$$Lambda$23/0x0000000000000000;
Ljava/util/ResourceBundle$$Lambda$23.0x0000000000000000;

ehrenjulzert pushed a commit to ehrenjulzert/openj9 that referenced this issue Jan 27, 2023
ehrenjulzert pushed a commit to ehrenjulzert/openj9 that referenced this issue Jan 27, 2023
llxia added a commit to llxia/aqa-tests that referenced this issue Feb 6, 2023
llxia added a commit to llxia/aqa-tests that referenced this issue Feb 6, 2023
llxia added a commit to llxia/aqa-tests that referenced this issue Feb 6, 2023
llxia added a commit to llxia/aqa-tests that referenced this issue Feb 6, 2023
llxia added a commit to llxia/aqa-tests that referenced this issue Feb 6, 2023
llxia added a commit to adoptium/aqa-tests that referenced this issue Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants