From 4856344668042fcbc4d15966519d27fb0a4f509f Mon Sep 17 00:00:00 2001 From: Chad Rakoczy Date: Thu, 4 Dec 2025 00:21:53 +0000 Subject: [PATCH] 8371046: Segfault in compiler/whitebox/StressNMethodRelocation.java with -XX:+UseZGC Reviewed-by: kvn, eastigeevich --- src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp | 2 +- src/hotspot/share/asm/codeBuffer.cpp | 2 +- src/hotspot/share/asm/codeBuffer.hpp | 2 +- src/hotspot/share/code/nmethod.cpp | 57 ++++++++++++------- src/hotspot/share/code/nmethod.hpp | 3 + src/hotspot/share/jvmci/vmStructs_jvmci.cpp | 1 + src/hotspot/share/prims/whitebox.cpp | 2 +- 7 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp b/src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp index 94694b58d2f..dbec2d76d4f 100644 --- a/src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/relocInfo_aarch64.cpp @@ -85,7 +85,7 @@ void Relocation::pd_set_call_destination(address x) { } else { MacroAssembler::pd_patch_instruction(addr(), x); } - assert(pd_call_destination(addr()) == x, "fail in reloc"); + guarantee(pd_call_destination(addr()) == x, "fail in reloc"); } void trampoline_stub_Relocation::pd_fix_owner_after_move() { diff --git a/src/hotspot/share/asm/codeBuffer.cpp b/src/hotspot/share/asm/codeBuffer.cpp index 2c6b7d7e96e..7871134e923 100644 --- a/src/hotspot/share/asm/codeBuffer.cpp +++ b/src/hotspot/share/asm/codeBuffer.cpp @@ -90,7 +90,7 @@ typedef CodeBuffer::csize_t csize_t; // file-local definition // External buffer, in a predefined CodeBlob. // Important: The code_start must be taken exactly, and not realigned. -CodeBuffer::CodeBuffer(CodeBlob* blob) DEBUG_ONLY(: Scrubber(this, sizeof(*this))) { +CodeBuffer::CodeBuffer(const CodeBlob* blob) DEBUG_ONLY(: Scrubber(this, sizeof(*this))) { // Provide code buffer with meaningful name initialize_misc(blob->name()); initialize(blob->content_begin(), blob->content_size()); diff --git a/src/hotspot/share/asm/codeBuffer.hpp b/src/hotspot/share/asm/codeBuffer.hpp index 430d4949467..38e151273da 100644 --- a/src/hotspot/share/asm/codeBuffer.hpp +++ b/src/hotspot/share/asm/codeBuffer.hpp @@ -672,7 +672,7 @@ class CodeBuffer: public StackObj DEBUG_ONLY(COMMA private Scrubber) { } // (2) CodeBuffer referring to pre-allocated CodeBlob. - CodeBuffer(CodeBlob* blob); + CodeBuffer(const CodeBlob* blob); // (3) code buffer allocating codeBlob memory for code & relocation // info but with lazy initialization. The name must be something diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp index c2f8b46f00e..edfca5c98ee 100644 --- a/src/hotspot/share/code/nmethod.cpp +++ b/src/hotspot/share/code/nmethod.cpp @@ -1498,6 +1498,40 @@ nmethod::nmethod(const nmethod &nm) : CodeBlob(nm._name, nm._kind, nm._size, nm. // - OOP table memcpy(consts_begin(), nm.consts_begin(), nm.data_end() - nm.consts_begin()); + // Fix relocation + RelocIterator iter(this); + CodeBuffer src(&nm); + CodeBuffer dst(this); + while (iter.next()) { +#ifdef USE_TRAMPOLINE_STUB_FIX_OWNER + // After an nmethod is moved, some direct call sites may end up out of range. + // CallRelocation::fix_relocation_after_move() assumes the target is always + // reachable and does not check branch range. Calling it without range checks + // could cause us to write an offset too large for the instruction. + // + // If a call site has a trampoline, we skip the normal call relocation. The + // associated trampoline_stub_Relocation will handle the call and the + // trampoline, including range checks and updating the branch as needed. + // + // If no trampoline exists, we can assume the call target is always + // reachable and therefore within direct branch range, so calling + // CallRelocation::fix_relocation_after_move() is safe. + if (iter.reloc()->is_call()) { + address trampoline = trampoline_stub_Relocation::get_trampoline_for(iter.reloc()->addr(), this); + if (trampoline != nullptr) { + continue; + } + } +#endif + + iter.reloc()->fix_relocation_after_move(&src, &dst); + } + + { + MutexLocker ml(NMethodState_lock, Mutex::_no_safepoint_check_flag); + clear_inline_caches(); + } + post_init(); } @@ -1521,25 +1555,6 @@ nmethod* nmethod::relocate(CodeBlobType code_blob_type) { return nullptr; } - // Fix relocation - RelocIterator iter(nm_copy); - CodeBuffer src(this); - CodeBuffer dst(nm_copy); - while (iter.next()) { -#ifdef USE_TRAMPOLINE_STUB_FIX_OWNER - // Direct calls may no longer be in range and the use of a trampoline may now be required. - // Instead, allow trampoline relocations to update their owners and perform the necessary checks. - if (iter.reloc()->is_call()) { - address trampoline = trampoline_stub_Relocation::get_trampoline_for(iter.reloc()->addr(), nm_copy); - if (trampoline != nullptr) { - continue; - } - } -#endif - - iter.reloc()->fix_relocation_after_move(&src, &dst); - } - // To make dependency checking during class loading fast, record // the nmethod dependencies in the classes it is dependent on. // This allows the dependency checking code to simply walk the @@ -1569,8 +1584,6 @@ nmethod* nmethod::relocate(CodeBlobType code_blob_type) { if (!is_marked_for_deoptimization() && is_in_use()) { assert(method() != nullptr && method()->code() == this, "should be if is in use"); - nm_copy->clear_inline_caches(); - // Attempt to start using the copy if (nm_copy->make_in_use()) { ICache::invalidate_range(nm_copy->code_begin(), nm_copy->code_size()); @@ -1578,7 +1591,7 @@ nmethod* nmethod::relocate(CodeBlobType code_blob_type) { methodHandle mh(Thread::current(), nm_copy->method()); nm_copy->method()->set_code(mh, nm_copy); - make_not_used(); + make_not_entrant(InvalidationReason::RELOCATED); nm_copy->post_compiled_method_load_event(); diff --git a/src/hotspot/share/code/nmethod.hpp b/src/hotspot/share/code/nmethod.hpp index 0fa9d7fda9e..2391bc6d830 100644 --- a/src/hotspot/share/code/nmethod.hpp +++ b/src/hotspot/share/code/nmethod.hpp @@ -499,6 +499,7 @@ public: UNCOMMON_TRAP, WHITEBOX_DEOPTIMIZATION, ZOMBIE, + RELOCATED, INVALIDATION_REASONS_COUNT }; @@ -543,6 +544,8 @@ public: return "whitebox deoptimization"; case InvalidationReason::ZOMBIE: return "zombie"; + case InvalidationReason::RELOCATED: + return "relocated"; default: { assert(false, "Unhandled reason"); return "Unknown"; diff --git a/src/hotspot/share/jvmci/vmStructs_jvmci.cpp b/src/hotspot/share/jvmci/vmStructs_jvmci.cpp index 7f009eba5f1..77e98db9156 100644 --- a/src/hotspot/share/jvmci/vmStructs_jvmci.cpp +++ b/src/hotspot/share/jvmci/vmStructs_jvmci.cpp @@ -586,6 +586,7 @@ declare_constant(nmethod::InvalidationReason::UNCOMMON_TRAP) \ declare_constant(nmethod::InvalidationReason::WHITEBOX_DEOPTIMIZATION) \ declare_constant(nmethod::InvalidationReason::ZOMBIE) \ + declare_constant(nmethod::InvalidationReason::RELOCATED) \ \ declare_constant(CodeInstaller::VERIFIED_ENTRY) \ declare_constant(CodeInstaller::UNVERIFIED_ENTRY) \ diff --git a/src/hotspot/share/prims/whitebox.cpp b/src/hotspot/share/prims/whitebox.cpp index 5514f7d3260..bbd7b4bf795 100644 --- a/src/hotspot/share/prims/whitebox.cpp +++ b/src/hotspot/share/prims/whitebox.cpp @@ -1678,7 +1678,7 @@ WB_ENTRY(void, WB_RelocateNMethodFromAddr(JNIEnv* env, jobject o, jlong addr, ji CodeBlob* blob = CodeCache::find_blob(address); if (blob != nullptr && blob->is_nmethod()) { nmethod* code = blob->as_nmethod(); - if (code->is_in_use()) { + if (code->is_in_use() && !code->is_unloading()) { CompiledICLocker ic_locker(code); code->relocate(static_cast(blob_type)); }