8358821: patch_verified_entry causes problems, use nmethod entry barriers instead

Co-authored-by: Martin Doerr <[email protected]>
Co-authored-by: Amit Kumar <[email protected]>
Reviewed-by: mdoerr, eosterlund

Author: 3 people

src/hotspot/cpu/aarch64/aarch64.ad

@@ -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");
 

src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp

@@ -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());
 
@@ -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
 }
@@ -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() {  }
 
 // MT-safe patching of a long jump instruction.

src/hotspot/cpu/aarch64/nativeInst_aarch64.hpp

@@ -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:
@@ -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) {

src/hotspot/cpu/arm/gc/shared/barrierSetAssembler_arm.cpp

@@ -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");

src/hotspot/cpu/arm/nativeInst_arm_32.cpp

@@ -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");

src/hotspot/cpu/arm/nativeInst_arm_32.hpp

@@ -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
@@ -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;
@@ -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) {

src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp

@@ -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);
 

src/hotspot/cpu/ppc/nativeInst_ppc.cpp

@@ -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.
@@ -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);
@@ -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

src/hotspot/cpu/ppc/nativeInst_ppc.hpp

@@ -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.
  *
@@ -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() {
@@ -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

src/hotspot/cpu/ppc/ppc.ad

@@ -1493,12 +1493,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);
   }