Skip to content

8358821: patch_verified_entry causes problems, use nmethod entry barriers instead #25764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
4 changes: 0 additions & 4 deletions src/hotspot/cpu/aarch64/aarch64.ad
Original file line number Diff line number Diff line change
Expand Up @@ -1765,10 +1765,6 @@ void MachPrologNode::emit(C2_MacroAssembler *masm, PhaseRegAlloc *ra_) const {
// n.b. frame size includes space for return pc and rfp
const int framesize = C->output()->frame_size_in_bytes();

// insert a nop at the start of the prolog so we can patch in a
// branch if we need to invalidate the method later
__ nop();

if (C->clinit_barrier_on_entry()) {
assert(!C->method()->holder()->is_not_initialized(), "initialization should have been started");

Expand Down
34 changes: 0 additions & 34 deletions src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,6 @@ void NativeMovRegMem::verify() {

void NativeJump::verify() { ; }


void NativeJump::check_verified_entry_alignment(address entry, address verified_entry) {
}


address NativeJump::jump_destination() const {
address dest = MacroAssembler::target_addr_for_insn_or_null(instruction_address());

Expand Down Expand Up @@ -345,10 +340,6 @@ bool NativeInstruction::is_movk() {
return Instruction_aarch64::extract(int_at(0), 30, 23) == 0b11100101;
}

bool NativeInstruction::is_sigill_not_entrant() {
return uint_at(0) == 0xd4bbd5a1; // dcps1 #0xdead
}

void NativeIllegalInstruction::insert(address code_pos) {
*(juint*)code_pos = 0xd4bbd5a1; // dcps1 #0xdead
}
Expand All @@ -359,31 +350,6 @@ bool NativeInstruction::is_stop() {

//-------------------------------------------------------------------

// MT-safe inserting of a jump over a jump or a nop (used by
// nmethod::make_not_entrant)

void NativeJump::patch_verified_entry(address entry, address verified_entry, address dest) {

assert(dest == SharedRuntime::get_handle_wrong_method_stub(), "expected fixed destination of patch");
assert(nativeInstruction_at(verified_entry)->is_jump_or_nop()
|| nativeInstruction_at(verified_entry)->is_sigill_not_entrant(),
"Aarch64 cannot replace non-jump with jump");

// Patch this nmethod atomically.
if (Assembler::reachable_from_branch_at(verified_entry, dest)) {
ptrdiff_t disp = dest - verified_entry;
guarantee(disp < 1 << 27 && disp > - (1 << 27), "branch overflow");

unsigned int insn = (0b000101 << 26) | ((disp >> 2) & 0x3ffffff);
*(unsigned int*)verified_entry = insn;
} else {
// We use an illegal instruction for marking a method as not_entrant.
NativeIllegalInstruction::insert(verified_entry);
}

ICache::invalidate_range(verified_entry, instruction_size);
}

void NativeGeneralJump::verify() { }

void NativeGeneralJump::insert_unconditional(address code_pos, address entry) {
Expand Down
6 changes: 1 addition & 5 deletions src/hotspot/cpu/aarch64/nativeInst_aarch64.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2025, 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.
*
Expand Down Expand Up @@ -83,7 +83,6 @@ class NativeInstruction {
bool is_safepoint_poll();
bool is_movz();
bool is_movk();
bool is_sigill_not_entrant();
bool is_stop();

protected:
Expand Down Expand Up @@ -360,9 +359,6 @@ class NativeJump: public NativeInstruction {

// Insertion of native jump instruction
static void insert(address code_pos, address entry);
// MT-safe insertion of native jump at verified method entry
static void check_verified_entry_alignment(address entry, address verified_entry);
static void patch_verified_entry(address entry, address verified_entry, address dest);
};

inline NativeJump* nativeJump_at(address address) {
Expand Down
5 changes: 1 addition & 4 deletions src/hotspot/cpu/arm/gc/shared/barrierSetAssembler_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,7 @@ void BarrierSetAssembler::nmethod_entry_barrier(MacroAssembler* masm) {
__ bind(guard);

// nmethod guard value. Skipped over in common case.
//
// Put a debug value to make any offsets skew
// clearly visible in coredump
__ emit_int32(0xDEADBEAF);
__ emit_int32(0); // initial armed value, will be reset later

__ bind(skip);
__ block_comment("nmethod_barrier end");
Expand Down
10 changes: 0 additions & 10 deletions src/hotspot/cpu/arm/nativeInst_arm_32.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,16 +282,6 @@ void NativeMovConstReg::set_pc_relative_offset(address addr, address pc) {
}
}

void RawNativeJump::check_verified_entry_alignment(address entry, address verified_entry) {
}

void RawNativeJump::patch_verified_entry(address entry, address verified_entry, address dest) {
assert(dest == SharedRuntime::get_handle_wrong_method_stub(), "should be");
int *a = (int *)verified_entry;
a[0] = not_entrant_illegal_instruction; // always illegal
ICache::invalidate_range((address)&a[0], sizeof a[0]);
}

void NativeGeneralJump::insert_unconditional(address code_pos, address entry) {
int offset = (int)(entry - code_pos - 8);
assert(offset < 0x2000000 && offset > -0x2000000, "encoding constraint");
Expand Down
10 changes: 1 addition & 9 deletions src/hotspot/cpu/arm/nativeInst_arm_32.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2008, 2025, 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
Expand Down Expand Up @@ -61,10 +61,6 @@ class RawNativeInstruction {
instr_fld_fst = 0xd0
};

// illegal instruction used by NativeJump::patch_verified_entry
// permanently undefined (UDF): 0xe << 28 | 0b1111111 << 20 | 0b1111 << 4
static const int not_entrant_illegal_instruction = 0xe7f000f0;

static int decode_rotated_imm12(int encoding) {
int base = encoding & 0xff;
int right_rotation = (encoding & 0xf00) >> 7;
Expand Down Expand Up @@ -274,10 +270,6 @@ class RawNativeJump: public NativeInstruction {
}
}

static void check_verified_entry_alignment(address entry, address verified_entry);

static void patch_verified_entry(address entry, address verified_entry, address dest);

};

inline RawNativeJump* rawNativeJump_at(address address) {
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ void C1_MacroAssembler::explicit_null_check(Register base) {


void C1_MacroAssembler::build_frame(int frame_size_in_bytes, int bang_size_in_bytes) {
// Avoid stack bang as first instruction. It may get overwritten by patch_verified_entry.
const Register return_pc = R20;
mflr(return_pc);

Expand Down
35 changes: 1 addition & 34 deletions src/hotspot/cpu/ppc/nativeInst_ppc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,6 @@
#include "c1/c1_Runtime1.hpp"
#endif

// We use an illtrap for marking a method as not_entrant
// Work around a C++ compiler bug which changes 'this'
bool NativeInstruction::is_sigill_not_entrant_at(address addr) {
if (!Assembler::is_illtrap(addr)) return false;
CodeBlob* cb = CodeCache::find_blob(addr);
if (cb == nullptr || !cb->is_nmethod()) return false;
nmethod *nm = (nmethod *)cb;
// This method is not_entrant iff the illtrap instruction is
// located at the verified entry point.
return nm->verified_entry_point() == addr;
}

#ifdef ASSERT
void NativeInstruction::verify() {
// Make sure code pattern is actually an instruction address.
Expand Down Expand Up @@ -331,25 +319,6 @@ void NativeMovConstReg::verify() {
}
#endif // ASSERT

void NativeJump::patch_verified_entry(address entry, address verified_entry, address dest) {
ResourceMark rm;
int code_size = 1 * BytesPerInstWord;
CodeBuffer cb(verified_entry, code_size + 1);
MacroAssembler* a = new MacroAssembler(&cb);
#ifdef COMPILER2
assert(dest == SharedRuntime::get_handle_wrong_method_stub(), "expected fixed destination of patch");
#endif
// Patch this nmethod atomically. Always use illtrap/trap in debug build.
if (DEBUG_ONLY(false &&) a->is_within_range_of_b(dest, a->pc())) {
a->b(dest);
} else {
// The signal handler will continue at dest=OptoRuntime::handle_wrong_method_stub().
// We use an illtrap for marking a method as not_entrant.
a->illtrap();
}
ICache::ppc64_flush_icache_bytes(verified_entry, code_size);
}

#ifdef ASSERT
void NativeJump::verify() {
address addr = addr_at(0);
Expand Down Expand Up @@ -462,9 +431,7 @@ bool NativeDeoptInstruction::is_deopt_at(address code_pos) {
if (!Assembler::is_illtrap(code_pos)) return false;
CodeBlob* cb = CodeCache::find_blob(code_pos);
if (cb == nullptr || !cb->is_nmethod()) return false;
nmethod *nm = (nmethod *)cb;
// see NativeInstruction::is_sigill_not_entrant_at()
return nm->verified_entry_point() != code_pos;
return true;
}

// Inserts an instruction which is specified to cause a SIGILL at a given pc
Expand Down
17 changes: 1 addition & 16 deletions src/hotspot/cpu/ppc/nativeInst_ppc.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024 SAP SE. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
Expand Down Expand Up @@ -69,13 +69,6 @@ class NativeInstruction {
return MacroAssembler::tdi_get_si16(long_at(0), Assembler::traptoUnconditional, 0);
}

// We use an illtrap for marking a method as not_entrant.
bool is_sigill_not_entrant() {
// Work around a C++ compiler bug which changes 'this'.
return NativeInstruction::is_sigill_not_entrant_at(addr_at(0));
}
static bool is_sigill_not_entrant_at(address addr);

#ifdef COMPILER2
// SIGTRAP-based implicit range checks
bool is_sigtrap_range_check() {
Expand Down Expand Up @@ -328,15 +321,7 @@ class NativeJump: public NativeInstruction {
}
}

// MT-safe insertion of native jump at verified method entry
static void patch_verified_entry(address entry, address verified_entry, address dest);

void verify() NOT_DEBUG_RETURN;

static void check_verified_entry_alignment(address entry, address verified_entry) {
// We just patch one instruction on ppc64, so the jump doesn't have to
// be aligned. Nothing to do here.
}
};

// Instantiates a NativeJump object starting at the given instruction
Expand Down
7 changes: 1 addition & 6 deletions src/hotspot/cpu/ppc/ppc.ad
Original file line number Diff line number Diff line change
Expand Up @@ -1686,12 +1686,7 @@ void MachPrologNode::emit(C2_MacroAssembler *masm, PhaseRegAlloc *ra_) const {
const Register toc_temp = R23;
assert_different_registers(R11, return_pc, callers_sp, push_frame_temp, toc_temp);

if (method_is_frameless) {
// Add nop at beginning of all frameless methods to prevent any
// oop instructions from getting overwritten by make_not_entrant
// (patching attempt would fail).
__ nop();
} else {
if (!method_is_frameless) {
// Get return pc.
__ mflr(return_pc);
}
Expand Down
57 changes: 0 additions & 57 deletions src/hotspot/cpu/riscv/nativeInst_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,18 +356,6 @@ void NativeMovRegMem::verify() {

void NativeJump::verify() { }


void NativeJump::check_verified_entry_alignment(address entry, address verified_entry) {
// Patching to not_entrant can happen while activations of the method are
// in use. The patching in that instance must happen only when certain
// alignment restrictions are true. These guarantees check those
// conditions.

// Must be 4 bytes aligned
MacroAssembler::assert_alignment(verified_entry);
}


address NativeJump::jump_destination() const {
address dest = MacroAssembler::target_addr_for_insn(instruction_address());

Expand Down Expand Up @@ -420,12 +408,6 @@ bool NativeInstruction::is_safepoint_poll() {
return MacroAssembler::is_lwu_to_zr(address(this));
}

// A 16-bit instruction with all bits ones is permanently reserved as an illegal instruction.
bool NativeInstruction::is_sigill_not_entrant() {
// jvmci
return uint_at(0) == 0xffffffff;
}

void NativeIllegalInstruction::insert(address code_pos) {
assert_cond(code_pos != nullptr);
Assembler::sd_instr(code_pos, 0xffffffff); // all bits ones is permanently reserved as an illegal instruction
Expand All @@ -437,45 +419,6 @@ bool NativeInstruction::is_stop() {

//-------------------------------------------------------------------

// MT-safe inserting of a jump over a jump or a nop (used by
// nmethod::make_not_entrant)

void NativeJump::patch_verified_entry(address entry, address verified_entry, address dest) {

assert(dest == SharedRuntime::get_handle_wrong_method_stub(), "expected fixed destination of patch");

assert(nativeInstruction_at(verified_entry)->is_jump_or_nop() ||
nativeInstruction_at(verified_entry)->is_sigill_not_entrant(),
"riscv cannot replace non-jump with jump");

check_verified_entry_alignment(entry, verified_entry);

// Patch this nmethod atomically.
if (Assembler::reachable_from_branch_at(verified_entry, dest)) {
ptrdiff_t offset = dest - verified_entry;
guarantee(Assembler::is_simm21(offset) && ((offset % 2) == 0),
"offset is too large to be patched in one jal instruction."); // 1M

uint32_t insn = 0;
address pInsn = (address)&insn;
Assembler::patch(pInsn, 31, 31, (offset >> 20) & 0x1);
Assembler::patch(pInsn, 30, 21, (offset >> 1) & 0x3ff);
Assembler::patch(pInsn, 20, 20, (offset >> 11) & 0x1);
Assembler::patch(pInsn, 19, 12, (offset >> 12) & 0xff);
Assembler::patch(pInsn, 11, 7, 0); // zero, no link jump
Assembler::patch(pInsn, 6, 0, 0b1101111); // j, (jal x0 offset)
Assembler::sd_instr(verified_entry, insn);
} else {
// We use an illegal instruction for marking a method as
// not_entrant.
NativeIllegalInstruction::insert(verified_entry);
}

ICache::invalidate_range(verified_entry, instruction_size);
}

//-------------------------------------------------------------------

void NativeGeneralJump::insert_unconditional(address code_pos, address entry) {
CodeBuffer cb(code_pos, instruction_size);
MacroAssembler a(&cb);
Expand Down
6 changes: 1 addition & 5 deletions src/hotspot/cpu/riscv/nativeInst_riscv.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2018, Red Hat Inc. All rights reserved.
* Copyright (c) 2020, 2023, Huawei Technologies Co., Ltd. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
Expand Down Expand Up @@ -74,7 +74,6 @@ class NativeInstruction {
bool is_nop() const;
bool is_jump_or_nop();
bool is_safepoint_poll();
bool is_sigill_not_entrant();
bool is_stop();

protected:
Expand Down Expand Up @@ -274,9 +273,6 @@ class NativeJump: public NativeInstruction {

// Insertion of native jump instruction
static void insert(address code_pos, address entry);
// MT-safe insertion of native jump at verified method entry
static void check_verified_entry_alignment(address entry, address verified_entry);
static void patch_verified_entry(address entry, address verified_entry, address dest);
};

inline NativeJump* nativeJump_at(address addr) {
Expand Down
11 changes: 1 addition & 10 deletions src/hotspot/cpu/riscv/riscv.ad
Original file line number Diff line number Diff line change
Expand Up @@ -1368,14 +1368,6 @@ void MachPrologNode::emit(C2_MacroAssembler *masm, PhaseRegAlloc *ra_) const {
// n.b. frame size includes space for return pc and fp
const int framesize = C->output()->frame_size_in_bytes();

// insert a nop at the start of the prolog so we can patch in a
// branch if we need to invalidate the method later
{
Assembler::IncompressibleScope scope(masm); // keep the nop as 4 bytes for patching.
MacroAssembler::assert_alignment(__ pc());
__ nop(); // 4 bytes
}

assert_cond(C != nullptr);

if (C->clinit_barrier_on_entry()) {
Expand Down Expand Up @@ -1804,7 +1796,6 @@ void MachUEPNode::emit(C2_MacroAssembler* masm, PhaseRegAlloc* ra_) const
// This is the unverified entry point.
__ ic_check(CodeEntryAlignment);

// Verified entry point must be properly 4 bytes aligned for patching by NativeJump::patch_verified_entry().
// ic_check() aligns to CodeEntryAlignment >= InteriorEntryAlignment(min 16) > NativeInstruction::instruction_size(4).
assert(((__ offset()) % CodeEntryAlignment) == 0, "Misaligned verified entry point");
}
Expand Down Expand Up @@ -8199,7 +8190,7 @@ instruct unnecessary_membar_volatile_rvtso() %{
ins_cost(0);

size(0);

format %{ "#@unnecessary_membar_volatile_rvtso (unnecessary so empty encoding)" %}
ins_encode %{
__ block_comment("unnecessary_membar_volatile_rvtso");
Expand Down
Loading