Can I please get a review of this test-only change which fixes an issue that
was introduced due to the refactoring that we did in
https://bugs.openjdk.org/browse/JDK-8333130? This PR addresses the failure
reported in https://bugs.openjdk.org/browse/JDK-8333756.
The `NativeMethodPrefixApp` test uses a `javaagent` `NativeMethodPrefixAgent`
which modifies the name of the native methods using the
`java.lang.instrument.Instrumentation` instance:
public static void premain (String agentArgs, Instrumentation instArg) {
inst = instArg;
System.out.println("Premain");
...
instArg.setNativeMethodPrefix(t0, "wrapped_tr0_");
instArg.setNativeMethodPrefix(t1, "wrapped_tr1_");
instArg.setNativeMethodPrefix(t2, "wrapped_tr2_");
The Hotspot VM allows for methods on a class to be annotated with an (VM
internal) `jdk.internal.vm.annotation.IntrinsicCandidate` annotation. When a
class that contains any methods that are annotated with `@IntrinsicCandidate`
is loaded, the VM checks that the corresponding method(s) have an intrinsic
available. It uses the method name to check for the presence of the intrinsic.
In the absence of an intrinsic for a `@IntrinsicCandidate` method, the VM
throws an error and exits. This behaviour is controlled by the
`-XX:+/-CheckIntrinsics` option. By default that option is enabled, implying
that an error will be thrown if the intrinsic isn't found.
In the case where/when this test fails, it so happens that the JVM loads a
class which has a `@IntrinsicCandidate` on a `native` Java method. For example,
on the failing host, I could see this class loading sequence:
tr2: Loading java/util/Date
tr1: Loading java/util/Date
tr0: Loading java/util/Date
tr2: Loading sun/util/calendar/CalendarSystem
tr1: Loading sun/util/calendar/CalendarSystem
tr0: Loading sun/util/calendar/CalendarSystem
tr2: Loading sun/util/calendar/CalendarSystem$GregorianHolder
tr1: Loading sun/util/calendar/CalendarSystem$GregorianHolder
tr0: Loading sun/util/calendar/CalendarSystem$GregorianHolder
...
tr2: Loading sun/util/calendar/ZoneInfoFile$Checksum
tr1: Loading sun/util/calendar/ZoneInfoFile$Checksum
tr0: Loading sun/util/calendar/ZoneInfoFile$Checksum
tr2: Loading java/util/zip/Checksum
tr1: Loading java/util/zip/Checksum
tr0: Loading java/util/zip/Checksum
tr2: Loading java/util/zip/CRC32
tr1: Loading java/util/zip/CRC32
tr0: Loading java/util/zip/CRC32
Method [java.util.zip.CRC32.wrapped_tr2_update(II)I] is annotated with
@IntrinsicCandidate, but no compiler intrinsic is defined for the method.
Exiting.
So a class loading sequence lead to loading of the `java.util.zip.CRC32` class
which has a `native` method annotated with `@IntrinsicCandidate`:
@IntrinsicCandidate
private static native int update(int crc, int b);
Since the `NativeMethodPrefixAgent` modifies the native method names, the VM
thus cannot find a matching intrinsic and thus fails with that error.
The reason why this wasn't uncovered during the testing of changes for
https://bugs.openjdk.org/browse/JDK-8333130 is purely incidental. Even now, the
test fails only intermittently. That's because it's only triggered if some
class with a `native` method with `@IntrinsicCandidate` gets loadded. As an
additional reference, I also found https://bugs.openjdk.org/browse/JDK-8151100
which is when the `-XX:-CheckIntrinsics` was introduced in this test and that
too explains this same issue.
The commit in this PR fixes the issue by reintroducing the
`"-XX:+UnlockDiagnosticVMOptions", "-XX:-CheckIntrinsics"` when launching the
test program. Additionally, the test program now intentionally loads the
`CRC32` class to make the test more deterministic for cases like these and to
prevent any accidental removal of the `-XX:-CheckIntrinsics` in future.
tier testing is currently in progress with this change in our CI.
-------------
Commit messages:
- 8333756: java/lang/instrument/NativeMethodPrefixApp.java failed due to
missing intrinsic
Changes: https://git.openjdk.org/jdk/pull/19595/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19595&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8333756
Stats: 9 lines in 1 file changed: 9 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/19595.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19595/head:pull/19595
PR: https://git.openjdk.org/jdk/pull/19595
