8321509: False positive in get_trampoline fast path causes crash

src/hotspot/cpu/aarch64/globalDefinitions_aarch64.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2024, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2014, 2015, Red Hat Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -67,4 +67,6 @@ const bool CCallingConventionRequiresIntsAsLongs = false;
 #define NOT_R18_RESERVED(code) code
 #endif
 
+#define USE_TRAMPOLINE_STUB_FIX_OWNER
+
 #endif // CPU_AARCH64_GLOBALDEFINITIONS_AARCH64_HPP

src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2014, 2020, Red Hat Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -158,13 +158,18 @@ void NativeGotJump::verify() const {
 }
 
 address NativeCall::destination() const {
-  address addr = (address)this;
-  address destination = instruction_address() + displacement();
+  address addr = instruction_address();
+  address destination = addr + displacement();
+
+  // Performance optimization: no need to call find_blob() if it is a self-call
+  if (destination == addr) {
+    return destination;
+  }
 
   // Do we use a trampoline stub for this call?
   CodeBlob* cb = CodeCache::find_blob_unsafe(addr);   // Else we get assertion if nmethod is zombie.
-  assert(cb && cb->is_nmethod(), "sanity");
-  nmethod *nm = (nmethod *)cb;
+  assert(cb != nullptr && cb->is_nmethod(), "nmethod expected");
+  nmethod *nm = cb->as_nmethod();
   if (nm->stub_contains(destination) && is_NativeCallTrampolineStub_at(destination)) {
     // Yes we do, so get the destination from the trampoline stub.
     const address trampoline_stub_addr = destination;
@@ -179,12 +184,8 @@ address NativeCall::destination() const {
 // call instruction at all times.
 //
 // Used in the runtime linkage of calls; see class CompiledIC.
-//
-// Add parameter assert_lock to switch off assertion
-// during code generation, where no patching lock is needed.
-void NativeCall::set_destination_mt_safe(address dest, bool assert_lock) {
-  assert(!assert_lock ||
-         (Patching_lock->is_locked() || SafepointSynchronize::is_at_safepoint()) ||
+void NativeCall::set_destination_mt_safe(address dest) {
+  assert((Patching_lock->is_locked() || SafepointSynchronize::is_at_safepoint()) ||
          CompiledICLocker::is_safe(addr_at(0)),
          "concurrent code patching");
 
@@ -211,22 +212,18 @@ void NativeCall::set_destination_mt_safe(address dest, bool assert_lock) {
 }
 
 address NativeCall::get_trampoline() {
-  address call_addr = addr_at(0);
+  address call_addr = instruction_address();
 
   CodeBlob *code = CodeCache::find_blob(call_addr);
-  assert(code != NULL, "Could not find the containing code blob");
+  assert(code != nullptr && code->is_nmethod(), "nmethod expected");
+  nmethod* nm = code->as_nmethod();
 
-  address bl_destination
-    = MacroAssembler::pd_call_destination(call_addr);
-  if (code->contains(bl_destination) &&
+  address bl_destination = call_addr + displacement();
+  if (nm->stub_contains(bl_destination) &&
       is_NativeCallTrampolineStub_at(bl_destination))
     return bl_destination;
 
-  if (code->is_nmethod()) {
-    return trampoline_stub_Relocation::get_trampoline_for(call_addr, (nmethod*)code);
-  }
-
-  return NULL;
+  return trampoline_stub_Relocation::get_trampoline_for(call_addr, nm);
 }
 
 // Inserts a native call instruction at a given pc

src/hotspot/cpu/aarch64/nativeInst_aarch64.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2014, 2108, Red Hat Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -212,6 +212,7 @@ class NativeCall: public NativeInstruction {
   int displacement() const { return (int_at(displacement_offset) << 6) >> 4; }
   address displacement_address() const { return addr_at(displacement_offset); }
   address return_address() const { return addr_at(return_address_offset); }
+  address raw_destination() const { return instruction_address() + displacement(); }
   address destination() const;
 
   void set_destination(address dest) {
@@ -251,9 +252,7 @@ class NativeCall: public NativeInstruction {
   //
   // Used in the runtime linkage of calls; see class CompiledIC.
   // (Cf. 4506997 and 4479829, where threads witnessed garbage displacements.)
-
-  // The parameter assert_lock disables the assertion during code generation.
-  void set_destination_mt_safe(address dest, bool assert_lock = true);
+  void set_destination_mt_safe(address dest);
 
   address get_trampoline();
 #if INCLUDE_JVMCI

src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp

@@ -60,13 +60,12 @@ void Relocation::pd_set_data_value(address x, intptr_t o, bool verify_only) {
 
 address Relocation::pd_call_destination(address orig_addr) {
   assert(is_call(), "should be a call here");
-  if (NativeCall::is_call_at(addr())) {
-    address trampoline = nativeCall_at(addr())->get_trampoline();
-    if (trampoline) {
-      return nativeCallTrampolineStub_at(trampoline)->destination();
+  if (orig_addr == nullptr) {
+    if (NativeCall::is_call_at(addr())) {
+      NativeCall* call = nativeCall_at(addr());
+      return call->destination();
     }
-  }
-  if (orig_addr != NULL) {
+  } else {
     address new_addr = MacroAssembler::pd_call_destination(orig_addr);
     // If call is branch to self, don't try to relocate it, just leave it
     // as branch to self. This happens during code generation if the code
@@ -82,16 +81,26 @@ address Relocation::pd_call_destination(address orig_addr) {
 void Relocation::pd_set_call_destination(address x) {
   assert(is_call(), "should be a call here");
   if (NativeCall::is_call_at(addr())) {
-    address trampoline = nativeCall_at(addr())->get_trampoline();
-    if (trampoline) {
-      nativeCall_at(addr())->set_destination_mt_safe(x, /* assert_lock */false);
-      return;
-    }
+    NativeCall* call = nativeCall_at(addr());
+    call->set_destination(x);
+  } else {
+    MacroAssembler::pd_patch_instruction(addr(), x);
   }
-  MacroAssembler::pd_patch_instruction(addr(), x);
   assert(pd_call_destination(addr()) == x, "fail in reloc");
 }
 
+void trampoline_stub_Relocation::pd_fix_owner_after_move() {
+  NativeCall* call = nativeCall_at(owner());
+  assert(call->raw_destination() == owner(), "destination should be empty");
+  address trampoline = addr();
+  address dest = nativeCallTrampolineStub_at(trampoline)->destination();
+  if (!Assembler::reachable_from_branch_at(owner(), dest)) {
+    dest = trampoline;
+  }
+  call->set_destination(dest);
+}
+
+
 address* Relocation::pd_address_in_code() {
   return (address*)(addr() + 8);
 }

src/hotspot/share/code/relocInfo.cpp

@@ -359,6 +359,14 @@ void CallRelocation::fix_relocation_after_move(const CodeBuffer* src, CodeBuffer
 }
 
 
+#ifdef USE_TRAMPOLINE_STUB_FIX_OWNER
+void trampoline_stub_Relocation::fix_relocation_after_move(const CodeBuffer* src, CodeBuffer* dest) {
+  // Finalize owner destination only for nmethods
+  if (dest->blob() != nullptr) return;
+  pd_fix_owner_after_move();
+}
+#endif
+
 //// pack/unpack methods
 
 void oop_Relocation::pack_data_to(CodeSection* dest) {

src/hotspot/share/code/relocInfo.hpp

@@ -1183,6 +1183,11 @@ class runtime_call_w_cp_Relocation : public CallRelocation {
 // in the code, it can patch it to jump to the trampoline where is
 // sufficient space for a far branch. Needed on PPC.
 class trampoline_stub_Relocation : public Relocation {
+#ifdef USE_TRAMPOLINE_STUB_FIX_OWNER
+  void pd_fix_owner_after_move();
+  void fix_relocation_after_move(const CodeBuffer* src, CodeBuffer* dest) override;
+#endif
+
  public:
   static RelocationHolder spec(address static_call) {
     RelocationHolder rh = newHolder();
@@ -1204,8 +1209,8 @@ class trampoline_stub_Relocation : public Relocation {
   // Return the address of the NativeCall that owns the trampoline.
   address owner() { return _owner; }
 
-  void pack_data_to(CodeSection * dest);
-  void unpack_data();
+  void pack_data_to(CodeSection * dest) override;
+  void unpack_data() override;
 
   // Find the trampoline stub for a call.
   static address get_trampoline_for(address call, nmethod* code);