revert disallow of j.l.Object redefine

src/hotspot/cpu/x86/continuationFreezeThaw_x86.inline.hpp

@@ -283,7 +283,7 @@ inline void ThawBase::patch_pd(frame& f, intptr_t* caller_sp) {
 
 inline void ThawBase::fix_native_wrapper_return_pc_pd(frame& top) {
   bool from_interpreted = top.is_interpreted_frame();
-  address resume_address = from_interpreted ? Interpreter::native_frame_resume_entry() : SharedRuntime::native_frame_resume_entry();
+  address resume_address = from_interpreted ? Interpreter::native_frame_resume_entry() : (top.pc() + SharedRuntime::object_wait_resume_offset());
   DEBUG_ONLY(Method* method = from_interpreted ? top.interpreter_frame_method() : CodeCache::find_blob(resume_address)->as_nmethod()->method();)
   assert(method->is_object_wait0(), "");
   ContinuationHelper::Frame::patch_pc(top, resume_address);

src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp

@@ -2374,7 +2374,6 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
   __ movl(Address(r15_thread, JavaThread::thread_state_offset()), _thread_in_Java);
   __ bind(after_transition);
 
-  int resume_wait_offset = 0;
   if (LockingMode != LM_LEGACY && method->is_object_wait0()) {
     // Check preemption for Object.wait()
     Label not_preempted;
@@ -2384,7 +2383,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
     __ movptr(Address(r15_thread, JavaThread::preempt_alternate_return_offset()), NULL_WORD);
     __ jmp(rscratch1);
     __ bind(not_preempted);
-    resume_wait_offset = ((intptr_t)__ pc()) - start;
+    SharedRuntime::set_object_wait_resume_offset(((intptr_t)__ pc()) - the_pc);
   }
 
   Label reguard;
@@ -2606,10 +2605,6 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
                                             in_ByteSize(lock_slot_offset*VMRegImpl::stack_slot_size),
                                             oop_maps);
 
-  if (LockingMode != LM_LEGACY && nm != nullptr && method->is_object_wait0()) {
-    SharedRuntime::set_native_frame_resume_entry(nm->code_begin() + resume_wait_offset);
-  }
-
   return nm;
 }
 

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -347,9 +347,6 @@ bool VM_RedefineClasses::is_modifiable_class(oop klass_mirror) {
   if (InstanceKlass::cast(k)->is_hidden()) {
     return false;
   }
-  if (InstanceKlass::cast(k) == vmClasses::Object_klass()) {
-    return false;
-  }
   if (InstanceKlass::cast(k) == vmClasses::Continuation_klass()) {
     // Don't redefine Continuation class. See 8302779.
     return false;

src/hotspot/share/runtime/continuationFreezeThaw.cpp

@@ -3000,15 +3000,16 @@ static void log_frames_after_thaw(JavaThread* thread, ContinuationWrapper& cont,
         sp0 += frame::metadata_words;
       } else {
         // Compiled case:
+        CodeBlob* cb = CodeCache::find_blob(pc0);
+        assert(cb != nullptr, "");
 #if defined (AMD64)
-        if (pc0 == SharedRuntime::native_frame_resume_entry()) {
+        if (cb->is_nmethod()) {
+          assert(cb->as_nmethod()->method()->is_object_wait0(), "");
           // For x64, when top is the compiled native wrapper (Object.wait())
           // the pc would have been modified from its original value to return
           // to the correct place. But that means we won't find the oopMap for
           // that fixed pc when getting the sender which will trigger asserts.
           // So just start walking the frames from the sender instead.
-          CodeBlob* cb = CodeCache::find_blob(pc0);
-          assert(cb->as_nmethod()->method()->is_object_wait0(), "");
           sp0 += cb->frame_size();
           if (sp0 == cont.entrySP()) {
             // sp0[-1] will be the return barrier pc. This is a stub, i.e. associated
@@ -3018,9 +3019,8 @@ static void log_frames_after_thaw(JavaThread* thread, ContinuationWrapper& cont,
           }
         }
 #elif defined (AARCH64) || defined (RISCV64)
-        CodeBlob* cb = CodeCache::find_blob(pc0);
-        assert(cb != nullptr, "should be either c1 or c2 runtime stub");
         if (cb->frame_size() == 2) {
+          assert(cb->is_runtime_stub(), "");
           // Returning to c2 runtime stub requires extra adjustment on aarch64
           // and riscv64 (see push_resume_adapter()).
           sp0 += frame::metadata_words;

src/hotspot/share/runtime/sharedRuntime.cpp

@@ -99,7 +99,7 @@
   SHARED_STUBS_DO(SHARED_STUB_FIELD_DEFINE)
 #undef SHARED_STUB_FIELD_DEFINE
 
-address             SharedRuntime::_native_frame_resume_entry = nullptr;
+int                 SharedRuntime::_object_wait_resume_offset = 0;
 
 nmethod*            SharedRuntime::_cont_doYield_stub;
 

src/hotspot/share/runtime/sharedRuntime.hpp

@@ -88,7 +88,7 @@ class SharedRuntime: AllStatic {
 #endif
 
 
-  static address             _native_frame_resume_entry;
+  static int _object_wait_resume_offset;
 
   // cont_doYieldStub is not yet folded into the general model for
   // shared stub/blob handling. It is actually a specially generated
@@ -243,10 +243,10 @@ class SharedRuntime: AllStatic {
                                                      address faulting_pc,
                                                      ImplicitExceptionKind exception_kind);
 
-  static address native_frame_resume_entry() { return _native_frame_resume_entry; }
-  static void set_native_frame_resume_entry(address val) {
-    assert(_native_frame_resume_entry == nullptr, "");
-    _native_frame_resume_entry = val;
+  static int object_wait_resume_offset() { return _object_wait_resume_offset; }
+  static void set_object_wait_resume_offset(int offset) {
+    assert(_object_wait_resume_offset == 0, "");
+    _object_wait_resume_offset = offset;
   }
 
   // Post-slow-path-allocation, pre-initializing-stores step for

test/hotspot/jtreg/TEST.quick-groups

@@ -1185,6 +1185,7 @@ vmTestbase_nsk_jvmti_quick = \
   vmTestbase/nsk/jvmti/scenarios/bcinstr/BI02/bi02t002/TestDescription.java \
   vmTestbase/nsk/jvmti/scenarios/bcinstr/BI03/bi03t001/TestDescription.java \
   vmTestbase/nsk/jvmti/scenarios/bcinstr/BI03/bi03t002/TestDescription.java \
+  vmTestbase/nsk/jvmti/scenarios/bcinstr/BI04/bi04t002/TestDescription.java \
   vmTestbase/nsk/jvmti/scenarios/capability/CM01/cm01t001/TestDescription.java \
   vmTestbase/nsk/jvmti/scenarios/capability/CM01/cm01t002/TestDescription.java \
   vmTestbase/nsk/jvmti/scenarios/capability/CM01/cm01t003/TestDescription.java \

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineObject.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2024, 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
@@ -23,22 +23,37 @@
 
 /*
  * @test
- * @summary Tests that java.lang.Object cannot be redefined/retransformed
+ * @bug 8232613
+ * @summary Ensure Object natives stay registered after redefinition
  * @requires vm.jvmti
  * @library /test/lib
- * @modules java.instrument
+ * @modules java.base/jdk.internal.misc
+ *          java.base/jdk.internal.org.objectweb.asm
+ *          java.compiler
+ *          java.instrument
  *          jdk.jartool/sun.tools.jar
  * @run main RedefineObject buildagent
  * @run main/othervm -javaagent:redefineagent.jar RedefineObject
  */
 
+import static jdk.test.lib.Asserts.assertTrue;
 import jdk.test.lib.helpers.ClassFileInstaller;
 import java.io.FileNotFoundException;
 import java.io.PrintWriter;
+import java.lang.RuntimeException;
 import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.IllegalClassFormatException;
 import java.lang.instrument.Instrumentation;
 import java.lang.instrument.UnmodifiableClassException;
 import java.security.ProtectionDomain;
+import java.util.Arrays;
+
+import jdk.internal.org.objectweb.asm.ClassReader;
+import jdk.internal.org.objectweb.asm.ClassVisitor;
+import jdk.internal.org.objectweb.asm.ClassWriter;
+
+import static jdk.internal.org.objectweb.asm.Opcodes.ASM6;
+import static jdk.internal.org.objectweb.asm.Opcodes.V1_8;
 
 public class RedefineObject {
 
@@ -49,23 +64,56 @@ public static void premain(String agentArgs, Instrumentation inst) {
     }
 
     static class Transformer implements ClassFileTransformer {
-        // set to true if transform method called to transform java.lang.Object
-        private boolean transformObjectInvoked;
 
-        @Override
-        public byte[] transform(ClassLoader loader,
-                                String className,
-                                Class<?> classBeingRedefined,
-                                ProtectionDomain protectionDomain,
-                                byte[] classfileBuffer) {
-            if (className.contains("java/lang/Object")) {
-                transformObjectInvoked = true;
+        public byte[] asm(ClassLoader loader, String className,
+                          Class<?> classBeingRedefined,
+                          ProtectionDomain protectionDomain, byte[] classfileBuffer)
+                throws IllegalClassFormatException {
+            ClassWriter cw = new ClassWriter(0);
+            // Force an older ASM to force a bytecode update
+            ClassVisitor cv = new DummyClassVisitor(ASM6, cw) { };
+            ClassReader cr = new ClassReader(classfileBuffer);
+            cr.accept(cv, 0);
+            byte[] bytes = cw.toByteArray();
+            return bytes;
+        }
+
+        public class DummyClassVisitor extends ClassVisitor {
+
+            public DummyClassVisitor(int api, ClassVisitor cv) {
+                super(api, cv);
+            }
+
+            public void visit(
+                    final int version,
+                    final int access,
+                    final String name,
+                    final String signature,
+                    final String superName,
+                    final String[] interfaces) {
+                // Artificially lower to JDK 8 version to force a redefine
+                cv.visit(V1_8, access, name, signature, superName, interfaces);
             }
-            return null;
         }
 
-        boolean transformObjectInvoked() {
-            return transformObjectInvoked;
+        @Override public byte[] transform(ClassLoader loader, String className,
+                                          Class<?> classBeingRedefined,
+                                          ProtectionDomain protectionDomain, byte[] classfileBuffer)
+                throws IllegalClassFormatException {
+
+            if (className.contains("java/lang/Object")) {
+                try {
+                    // Here we remove and re-add the dummy fields. This shuffles the constant pool
+                    return asm(loader, className, classBeingRedefined, protectionDomain, classfileBuffer);
+                } catch (Throwable e) {
+                    // The retransform native code that called this method does not propagate
+                    // exceptions. Instead of getting an uninformative generic error, catch
+                    // problems here and print it, then exit.
+                    e.printStackTrace();
+                    System.exit(1);
+                }
+            }
+            return null;
         }
     }
 
@@ -76,10 +124,12 @@ private static void buildAgent() {
             throw new RuntimeException("Could not write agent classfile", e);
         }
 
-        try (PrintWriter pw = new PrintWriter("MANIFEST.MF")) {
+        try {
+            PrintWriter pw = new PrintWriter("MANIFEST.MF");
             pw.println("Premain-Class: RedefineObject");
             pw.println("Agent-Class: RedefineObject");
             pw.println("Can-Retransform-Classes: true");
+            pw.close();
         } catch (FileNotFoundException e) {
             throw new RuntimeException("Could not write manifest file for the agent", e);
         }
@@ -91,6 +141,9 @@ private static void buildAgent() {
     }
 
     public static void main(String[] args) throws Exception {
+
+        int objHash = System.identityHashCode(Object.class);
+        System.out.println("Object hashCode: " + objHash);
         if (args.length == 1 && args[0].equals("buildagent")) {
             buildAgent();
             return;
@@ -100,20 +153,27 @@ public static void main(String[] args) throws Exception {
             throw new RuntimeException("Instrumentation object was null");
         }
 
-        if (inst.isModifiableClass(Object.class)) {
-            throw new RuntimeException("java.lang.Object should not be modifable");
-        }
-
-        var transformer = new Transformer();
-        inst.addTransformer(transformer, true);
         try {
+            inst.addTransformer(new RedefineObject.Transformer(), true);
             inst.retransformClasses(Object.class);
-            throw new RuntimeException("UnmodifiableClassException not thrown by retransformClasses");
         } catch (UnmodifiableClassException e) {
-            // expected
+            throw new RuntimeException(e);
         }
-        if (transformer.transformObjectInvoked()) {
-            throw new RuntimeException();
+
+        // Exercise native methods on Object after transform
+        Object b = new Object();
+        b.hashCode();
+
+        C c = new C();
+        assertTrue(c.hashCode() != c.clone().hashCode() || c != c.clone());
+        assertTrue(c.clone() instanceof C);
+        c = (C)c.clone(); // native method on new Object
+    }
+
+    private static class C implements Cloneable {
+        @Override
+        protected Object clone() throws CloneNotSupportedException {
+            return super.clone();
         }
     }
 }