On Fri, 7 Jun 2024 10:31:22 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> 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... test/jdk/java/lang/instrument/NativeMethodPrefixApp.java line 103: > 101: // the native method names, which then causes a failure > in the VM check > 102: // for the presence of an intrinsic on a > @IntrinsicCandidate native method > 103: "-XX:+UnlockDiagnosticVMOptions", "-XX:-CheckIntrinsics", Can you update both comments to begin with a capital letter and end with a period. Thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19595#discussion_r1633711272
