[GR-60879] Revert "Define both native and Java breakpoints to trace Class#getSigners."

PullRequest: graal/19746

Author: DSouzaM

substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/BreakpointInterceptor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -317,13 +317,9 @@ private static boolean getSigners(JNIEnvironment jni, JNIObjectHandle thread, Br
         return handleGetClasses(jni, thread, bp, state);
     }
 
-    private static boolean handleGetClasses(JNIEnvironment jni, JNIObjectHandle thread, AbstractBreakpoint<?> bp, InterceptedState state) {
-        JNIObjectHandle self = getReceiver(thread);
-        return handleGetClassesWithReceiver(jni, self, bp, state);
-    }
-
-    private static boolean handleGetClassesWithReceiver(JNIEnvironment jni, JNIObjectHandle self, AbstractBreakpoint<?> bp, InterceptedState state) {
+    private static boolean handleGetClasses(JNIEnvironment jni, JNIObjectHandle thread, Breakpoint bp, InterceptedState state) {
         JNIObjectHandle callerClass = state.getDirectCallerClass();
+        JNIObjectHandle self = getReceiver(thread);
         traceReflectBreakpoint(jni, self, nullHandle(), callerClass, bp.specification.methodName, null, state.getFullStackTraceOrNull());
         return true;
     }
@@ -370,10 +366,10 @@ private interface AllocateInstanceFunctionPointer extends CFunctionPointer {
     @CEntryPoint
     @CEntryPointOptions(prologue = AgentIsolate.Prologue.class)
     static long nativeAllocateInstance(JNIEnvironment jni, JNIObjectHandle self, JNIObjectHandle clazz) {
-        NativeBreakpoint breakpoint = NATIVE_ALLOCATE_INSTANCE_BREAKPOINT_SPEC.installed;
-        VMError.guarantee(breakpoint != null && breakpoint.replacedFunction.isNonNull(), "incompletely installed");
+        VMError.guarantee(NATIVE_ALLOCATE_INSTANCE_BREAKPOINT_SPEC.installed != null &&
+                        NATIVE_ALLOCATE_INSTANCE_BREAKPOINT_SPEC.installed.replacedFunction.isNonNull(), "incompletely installed");
 
-        AllocateInstanceFunctionPointer original = (AllocateInstanceFunctionPointer) breakpoint.replacedFunction;
+        AllocateInstanceFunctionPointer original = (AllocateInstanceFunctionPointer) NATIVE_ALLOCATE_INSTANCE_BREAKPOINT_SPEC.installed.replacedFunction;
         long result = original.invoke(jni, self, clazz);
         if (!Support.isInitialized()) { // in case of a (very) late call
             return result;
@@ -396,36 +392,6 @@ private static void traceAllocateInstance(JNIEnvironment jni, JNIObjectHandle cl
         }
     }
 
-    private static final CEntryPointLiteral<AllocateInstanceFunctionPointer> nativeGetSigners = CEntryPointLiteral.create(
-                    BreakpointInterceptor.class, "nativeGetSigners", JNIEnvironment.class, JNIObjectHandle.class);
-    private static final NativeBreakpointSpecification NATIVE_GET_SIGNERS_BREAKPOINT_SPEC = new NativeBreakpointSpecification(
-                    "java/lang/Class", "getSigners", "()[Ljava/lang/Object;", nativeGetSigners);
-
-    private interface GetSignersFunctionPointer extends CFunctionPointer {
-        @InvokeCFunctionPointer
-        JNIObjectHandle invoke(JNIEnvironment jni, JNIObjectHandle self);
-    }
-
-    /**
-     * Before Java 24, {@code java/lang/Class#getSigners} was declared as a native method, so we use
-     * a native breakpoint to trace it. Either this breakpoint or the Java breakpoint will be set
-     * (but not both).
-     */
-    @CEntryPoint
-    @CEntryPointOptions(prologue = AgentIsolate.Prologue.class)
-    static JNIObjectHandle nativeGetSigners(JNIEnvironment jni, JNIObjectHandle self) {
-        NativeBreakpoint breakpoint = NATIVE_GET_SIGNERS_BREAKPOINT_SPEC.installed;
-        VMError.guarantee(breakpoint != null && breakpoint.replacedFunction.isNonNull(), "incompletely installed");
-        GetSignersFunctionPointer original = (GetSignersFunctionPointer) breakpoint.replacedFunction;
-        JNIObjectHandle result = original.invoke(jni, self);
-        if (!Support.isInitialized()) {
-            return result;
-        }
-        InterceptedState state = interceptedStateSupplier.get();
-        handleGetClassesWithReceiver(jni, self, breakpoint, state);
-        return result;
-    }
-
     private static boolean objectFieldOffsetByName(JNIEnvironment jni, JNIObjectHandle thread, Breakpoint bp, InterceptedState state) {
         JNIObjectHandle callerClass = state.getDirectCallerClass();
         JNIObjectHandle self = getReceiver(thread);
@@ -1360,13 +1326,17 @@ public static void onLoad(JvmtiEnv jvmti, JvmtiEventCallbacks callbacks, Tracer
         BreakpointInterceptor.experimentalClassDefineSupport = exptlClassDefineSupport;
         BreakpointInterceptor.experimentalUnsafeAllocationSupport = exptlUnsafeAllocationSupport;
         BreakpointInterceptor.trackReflectionMetadata = trackReflectionData;
-        BreakpointInterceptor.boundNativeMethods = new HashMap<>();
 
         JvmtiCapabilities capabilities = UnmanagedMemory.calloc(SizeOf.get(JvmtiCapabilities.class));
         check(jvmti.getFunctions().GetCapabilities().invoke(jvmti, capabilities));
         capabilities.setCanGenerateBreakpointEvents(1);
         capabilities.setCanAccessLocalVariables(1);
-        capabilities.setCanGenerateNativeMethodBindEvents(1);
+
+        if (exptlUnsafeAllocationSupport) {
+            capabilities.setCanGenerateNativeMethodBindEvents(1);
+            callbacks.setNativeMethodBind(onNativeMethodBindLiteral.getFunctionPointer());
+            BreakpointInterceptor.boundNativeMethods = new HashMap<>();
+        }
 
         if (exptlClassLoaderSupport) {
             capabilities.setCanGetBytecodes(1);
@@ -1378,10 +1348,8 @@ public static void onLoad(JvmtiEnv jvmti, JvmtiEventCallbacks callbacks, Tracer
         }
         check(jvmti.getFunctions().AddCapabilities().invoke(jvmti, capabilities));
         UnmanagedMemory.free(capabilities);
-        check(jvmti.getFunctions().SetEventNotificationMode().invoke(jvmti, JvmtiEventMode.JVMTI_ENABLE, JVMTI_EVENT_NATIVE_METHOD_BIND, nullHandle()));
 
         callbacks.setBreakpoint(onBreakpointLiteral.getFunctionPointer());
-        callbacks.setNativeMethodBind(onNativeMethodBindLiteral.getFunctionPointer());
 
         if (exptlClassDefineSupport) {
             callbacks.setClassFileLoadHook(onClassFileLoadHookLiteral.getFunctionPointer());
@@ -1391,6 +1359,9 @@ public static void onLoad(JvmtiEnv jvmti, JvmtiEventCallbacks callbacks, Tracer
             callbacks.setClassPrepare(onClassPrepareLiteral.getFunctionPointer());
         }
 
+        if (exptlUnsafeAllocationSupport) {
+            Support.check(jvmti.getFunctions().SetEventNotificationMode().invoke(jvmti, JvmtiEventMode.JVMTI_ENABLE, JVMTI_EVENT_NATIVE_METHOD_BIND, nullHandle()));
+        }
     }
 
     public static void onVMInit(JvmtiEnv jvmti, JNIEnvironment jni) {
@@ -1441,7 +1412,9 @@ public static void onVMInit(JvmtiEnv jvmti, JNIEnvironment jni) {
         }
         installedBreakpoints = breakpoints;
 
-        setupNativeBreakpoints(jni, lastClass, lastClassName);
+        if (experimentalUnsafeAllocationSupport) {
+            setupNativeBreakpoints(jni, lastClass, lastClassName);
+        }
 
         if (experimentalClassDefineSupport) {
             setupClassLoadEvent(jvmti, jni);
@@ -1458,12 +1431,8 @@ private static void setupNativeBreakpoints(JNIEnvironment jni, JNIObjectHandle p
         String lastClassName = previousClassName;
         nativeBreakpointsInitLock.lock();
         try {
-            List<NativeBreakpointSpecification> nativeBreakpointsList = new ArrayList<>(Arrays.asList(NATIVE_BREAKPOINT_SPECIFICATIONS));
-            if (experimentalUnsafeAllocationSupport) {
-                nativeBreakpointsList.addAll(Arrays.asList(EXPERIMENTAL_UNSAFE_ALLOCATION_NATIVE_BREAKPOINT_SPECIFICATIONS));
-            }
-            nativeBreakpoints = new HashMap<>(nativeBreakpointsList.size());
-            for (NativeBreakpointSpecification br : nativeBreakpointsList) {
+            nativeBreakpoints = new HashMap<>(NATIVE_BREAKPOINT_SPECIFICATIONS.length);
+            for (NativeBreakpointSpecification br : NATIVE_BREAKPOINT_SPECIFICATIONS) {
                 JNIObjectHandle clazz;
                 if (lastClassName != null && lastClassName.equals(br.className)) {
                     clazz = lastClass;
@@ -1477,8 +1446,7 @@ private static void setupNativeBreakpoints(JNIEnvironment jni, JNIObjectHandle p
                     NativeBreakpoint bp = new NativeBreakpoint(br, clazz, method);
                     nativeBreakpoints.put(method.rawValue(), bp);
                     Long original = boundNativeMethods.get(method.rawValue());
-                    // only bind a native breakpoint if method was bound as a native method
-                    if (original != null) {
+                    if (original != null) { // already bound, replace
                         bindNativeBreakpoint(jni, bp, WordFactory.pointer(original), nullPointer());
                     }
                 }
@@ -1742,7 +1710,7 @@ private interface BreakpointHandler {
                     optionalBrk("java/lang/Class", "getNestMembers", "()[Ljava/lang/Class;",
                                     BreakpointInterceptor::getNestMembers),
                     optionalBrk("java/lang/Class", "getSigners", "()[Ljava/lang/Object;",
-                                    BreakpointInterceptor::getSigners),
+                                    BreakpointInterceptor::getSigners)
     };
 
     private static boolean allocateInstance(JNIEnvironment jni, JNIObjectHandle thread, @SuppressWarnings("unused") Breakpoint bp, InterceptedState state) {
@@ -1756,10 +1724,6 @@ private static boolean allocateInstance(JNIEnvironment jni, JNIObjectHandle thre
                     "(Ljava/lang/String;)Ljava/lang/Class;", BreakpointInterceptor::loadClass);
 
     private static final NativeBreakpointSpecification[] NATIVE_BREAKPOINT_SPECIFICATIONS = {
-                    NATIVE_GET_SIGNERS_BREAKPOINT_SPEC,
-    };
-
-    private static final NativeBreakpointSpecification[] EXPERIMENTAL_UNSAFE_ALLOCATION_NATIVE_BREAKPOINT_SPECIFICATIONS = {
                     NATIVE_ALLOCATE_INSTANCE_BREAKPOINT_SPEC
     };