[PATCH] fix: do not use magic number in return address functions

From ea42ba233b9c228f891e0f334970f2e3adc66dae Mon Sep 17 00:00:00 2001
Bug: https://bugs.openjdk.org/browse/JDK-8388170

Gbp-Pq: Name jdk-8388170-proposed.diff

diff --git a/src/hotspot/cpu/aarch64/aarch64.ad b/src/hotspot/cpu/aarch64/aarch64.ad
index 48ed893f2a6f6da6aa94375bece3b36c3e41b178..46d20ed13133453eab6f9266418da2b4e8859fda 100644 (file)
--- a/src/hotspot/cpu/aarch64/aarch64.ad
+++ b/src/hotspot/cpu/aarch64/aarch64.ad
@@ -1628,19 +1628,19 @@ bool needs_acquiring_load_exclusive(const Node *n)
 //       from the start of the call to the point where the return address
 //       will point.
 
-int MachCallStaticJavaNode::ret_addr_offset()
+int MachCallStaticJavaNode::ret_addr_offset() const
 {
   // call should be a simple bl
   int off = 4;
   return off;
 }
 
-int MachCallDynamicJavaNode::ret_addr_offset()
+int MachCallDynamicJavaNode::ret_addr_offset() const
 {
   return 16; // movz, movk, movk, bl
 }
 
-int MachCallRuntimeNode::ret_addr_offset() {
+int MachCallRuntimeNode::ret_addr_offset() const {
   // for generated stubs the call will be
   //   bl(addr)
   // or with far branches

diff --git a/src/hotspot/cpu/arm/arm_32.ad b/src/hotspot/cpu/arm/arm_32.ad
index be4dc825e9c798aa0f53c2e27485103461975243..05359b081b59ee213ea77df939bbc92904c1463f 100644 (file)
--- a/src/hotspot/cpu/arm/arm_32.ad
+++ b/src/hotspot/cpu/arm/arm_32.ad
@@ -430,19 +430,19 @@ OptoRegPair c2::return_value(int ideal_reg) {
 //       from the start of the call to the point where the return address
 //       will point.
 
-int MachCallStaticJavaNode::ret_addr_offset() {
+int MachCallStaticJavaNode::ret_addr_offset() const {
   bool far = (_method == nullptr) ? maybe_far_call(this) : !cache_reachable();
   return ((far ? 3 : 1) + (_method_handle_invoke ? 1 : 0)) *
     NativeInstruction::instruction_size;
 }
 
-int MachCallDynamicJavaNode::ret_addr_offset() {
+int MachCallDynamicJavaNode::ret_addr_offset() const {
   bool far = !cache_reachable();
   // mov_oop is always 2 words
   return (2 + (far ? 3 : 1)) * NativeInstruction::instruction_size;
 }
 
-int MachCallRuntimeNode::ret_addr_offset() {
+int MachCallRuntimeNode::ret_addr_offset() const {
   // bl or movw; movt; blx
   bool far = maybe_far_call(this);
   return (far ? 3 : 1) * NativeInstruction::instruction_size;

diff --git a/src/hotspot/cpu/ppc/ppc.ad b/src/hotspot/cpu/ppc/ppc.ad
index b68632f0b3778fda8bfb0d4917937745fdb936b8..b8e893b4151c0ab29d0650b5ae06442496c039ae 100644 (file)
--- a/src/hotspot/cpu/ppc/ppc.ad
+++ b/src/hotspot/cpu/ppc/ppc.ad
@@ -1171,12 +1171,12 @@ bool followed_by_acquire(const Node *load) {
 
 // PPC port: Removed use of lazy constant construct.
 
-int MachCallStaticJavaNode::ret_addr_offset() {
+int MachCallStaticJavaNode::ret_addr_offset() const {
   // It's only a single branch-and-link instruction.
   return 4;
 }
 
-int MachCallDynamicJavaNode::ret_addr_offset() {
+int MachCallDynamicJavaNode::ret_addr_offset() const {
   // Offset is 4 with postalloc expanded calls (bl is one instruction). We use
   // postalloc expanded calls if we use inline caches and do not update method data.
   if (UseInlineCaches) return 4;
@@ -1191,7 +1191,7 @@ int MachCallDynamicJavaNode::ret_addr_offset() {
   }
 }
 
-int MachCallRuntimeNode::ret_addr_offset() {
+int MachCallRuntimeNode::ret_addr_offset() const {
   if (rule() == CallRuntimeDirect_rule) {
     // CallRuntimeDirectNode uses call_c.
 #if defined(ABI_ELFv2)

diff --git a/src/hotspot/cpu/riscv/riscv.ad b/src/hotspot/cpu/riscv/riscv.ad
index 9b7e7138710063ee7c2c20b3cf4b9e685f007de4..1e2278c7526f351cc9cd09aff76007c4180c36b9 100644 (file)
--- a/src/hotspot/cpu/riscv/riscv.ad
+++ b/src/hotspot/cpu/riscv/riscv.ad
@@ -1218,17 +1218,17 @@ bool needs_acquiring_load_reserved(const Node *n)
 //       from the start of the call to the point where the return address
 //       will point.
 
-int MachCallStaticJavaNode::ret_addr_offset()
+int MachCallStaticJavaNode::ret_addr_offset() const
 {
   return 3 * NativeInstruction::instruction_size; // auipc + ld + jalr
 }
 
-int MachCallDynamicJavaNode::ret_addr_offset()
+int MachCallDynamicJavaNode::ret_addr_offset() const
 {
   return NativeMovConstReg::movptr2_instruction_size + (3 * NativeInstruction::instruction_size); // movptr2, auipc + ld + jal
 }
 
-int MachCallRuntimeNode::ret_addr_offset() {
+int MachCallRuntimeNode::ret_addr_offset() const {
   // For address inside the code cache the call will be:
   //   auipc + jalr
   // For real runtime callouts it will be 8 instructions

diff --git a/src/hotspot/cpu/s390/s390.ad b/src/hotspot/cpu/s390/s390.ad
index c32064be86d879faffbc44197cb548122f8a5c34..35266d11b6095e0463775a07b8078d7f37cb765d 100644 (file)
--- a/src/hotspot/cpu/s390/s390.ad
+++ b/src/hotspot/cpu/s390/s390.ad
@@ -886,29 +886,62 @@ int MachNode::compute_padding(int current_offset) const {
   return 0;
 }
 
-int MachCallStaticJavaNode::ret_addr_offset() {
+int MachCallStaticJavaNode::ret_addr_offset() const {
   if (_method) {
-    return 8;
+    return MacroAssembler::call_far_pcrelative_size();
+
   } else {
     return MacroAssembler::call_far_patchable_ret_addr_offset();
   }
 }
 
-int MachCallDynamicJavaNode::ret_addr_offset() {
+int MachCallDynamicJavaNode::ret_addr_offset() const {
   // Consider size of receiver type profiling (C2 tiers).
   int profile_receiver_type_size = 0;
 
   int vtable_index = this->_vtable_index;
   if (vtable_index == -4) {
-    return 14 + profile_receiver_type_size;
+      return MacroAssembler::load_const_from_toc_size()
+           + MacroAssembler::call_far_pcrelative_size()
+           + profile_receiver_type_size;
   } else {
     assert(!UseInlineCaches, "expect vtable calls only if not using ICs");
-    return 36 + profile_receiver_type_size;
+    // This should return the size of instructions in vtable dispatch
+    // branch of z_enc_java_dynamic_call
+    int offset = 0;
+
+    //     __ load_klass(Z_method, Z_R2);
+    if (UseCompactObjectHeaders) {
+      // load_narrow_klass_compact (z_lg z_srlg)
+      offset += 6 // z_lg
+              + 6; // z_srlg;
+    } else {
+      offset += 6; // z_llgf
+    }
+    offset += MacroAssembler::instr_size_for_decode_klass_not_null();
+
+    // check if displacement is valid, as it will generate different
+    // instructions:
+    int entry_offset = in_bytes(Klass::vtable_start_offset()) +
+                       vtable_index * vtableEntry::size_in_bytes();
+    int v_off = entry_offset + in_bytes(vtableEntry::method_offset());
+    if (!Displacement::is_validDisp(v_off)) {
+        offset += MacroAssembler::load_const_size(); // it also generates load_cost
+    }
+    // both generate z_lg
+    offset += 6; // z_lg (z_method, v_off | Address(Z_method, Z_R1_scratch))
+    // common footer
+    offset += 6; // z_lg(Z_R1_scratch, Method::from_compiled_offset())
+    offset += 2; // z_basr
+
+    return offset + profile_receiver_type_size;
   }
 }
 
-int MachCallRuntimeNode::ret_addr_offset() {
-  return 12 + MacroAssembler::call_far_patchable_ret_addr_offset();
+int MachCallRuntimeNode::ret_addr_offset() const {
+  return 6 // get_PC()  (LARL)
+    + 6 // save_return_pc() (STG)
+    + MacroAssembler::call_far_patchable_ret_addr_offset();
 }
 
 // Compute padding required for nodes which need alignment
@@ -2379,8 +2412,8 @@ encode %{
     // callee doesn't.
     unsigned int start_off = __ offset();
     // Compute size of "larl + stg + call_c_opt".
-    const int size_of_code = 6 + 6 + MacroAssembler::call_far_patchable_size();
-    __ get_PC(Z_R14, size_of_code);
+    const int ret_off = ret_addr_offset();
+    __ get_PC(Z_R14, ret_off);
     __ save_return_pc();
     assert(__ offset() - start_off == 12, "bad prelude len: %d", __ offset() - start_off);
 
@@ -2391,14 +2424,13 @@ encode %{
       return;
     }
 
-#ifdef ASSERT
-    // Plausibility check for size_of_code assumptions.
-    unsigned int actual_ret_off = __ offset();
-    assert(start_off + size_of_code == actual_ret_off, "wrong return_pc");
-#endif
+    assert(__ offset() - start_off == (uint)ret_addr_offset(),
+            "z_enc_java_to_runtime_call return offset mismatch: emitted %d bytes, ret_addr_offset()=%d",
+            __ offset() - start_off, ret_addr_offset());
   %}
 
   enc_class z_enc_java_static_call(method meth) %{
+    unsigned int start_off = __ offset();
     // Call to fixup routine. Fixup routine uses ScopeDesc info to determine
     // whom we intended to call.
     int ret_offset = 0;
@@ -2428,6 +2460,9 @@ encode %{
     }
 
     __ clear_inst_mark();
+    assert(__ offset() - start_off == (uint)ret_addr_offset(),
+            "z_enc_java_static_call return offset mismatch: emitted %d bytes, ret_addr_offset()=%d",
+            __ offset() - start_off, ret_addr_offset());
   %}
 
   // Java dynamic call
@@ -2482,8 +2517,10 @@ encode %{
       __ z_lg(Z_R1_scratch, Address(Z_method, Method::from_compiled_offset()));
       // Call target. Either compiled code or C2I adapter.
       __ z_basr(Z_R14, Z_R1_scratch);
-      unsigned int ret_off = __ offset();
     }
+    assert(__ offset() - start_off == (uint)ret_addr_offset(),
+            "z_enc_java_dynamic_call return offset mismatch: emitted %d bytes, ret_addr_offset()=%d",
+            __ offset() - start_off, ret_addr_offset());
   %}
 
   enc_class z_enc_cmov_reg(cmpOp cmp, iRegI dst, iRegI src) %{

diff --git a/src/hotspot/cpu/x86/x86_64.ad b/src/hotspot/cpu/x86/x86_64.ad
index 7f8b7dbc9f3d622e1d73aa64d9dabf282d3d79cf..f04e339beec2a977050956f0e9a2b6cb957b4ffc 100644 (file)
--- a/src/hotspot/cpu/x86/x86_64.ad
+++ b/src/hotspot/cpu/x86/x86_64.ad
@@ -600,21 +600,21 @@ static int clear_avx_size() {
 // !!!!! Special hack to get all types of calls to specify the byte offset
 //       from the start of the call to the point where the return address
 //       will point.
-int MachCallStaticJavaNode::ret_addr_offset()
+int MachCallStaticJavaNode::ret_addr_offset() const
 {
   int offset = 5; // 5 bytes from start of call to where return address points
   offset += clear_avx_size();
   return offset;
 }
 
-int MachCallDynamicJavaNode::ret_addr_offset()
+int MachCallDynamicJavaNode::ret_addr_offset() const
 {
   int offset = 15; // 15 bytes from start of call to where return address points
   offset += clear_avx_size();
   return offset;
 }
 
-int MachCallRuntimeNode::ret_addr_offset() {
+int MachCallRuntimeNode::ret_addr_offset() const {
   int offset = 13; // movq r10,#addr; callq (r10)
   if (this->ideal_Opcode() != Op_CallLeafVector) {
     offset += clear_avx_size();

diff --git a/src/hotspot/share/opto/machnode.hpp b/src/hotspot/share/opto/machnode.hpp
index 5fd262d54e1b6002601f90646cbb6556d483b8c0..983c4f74d262f5ff648d10467edcfab8b2ef9c61 100644 (file)
--- a/src/hotspot/share/opto/machnode.hpp
+++ b/src/hotspot/share/opto/machnode.hpp
@@ -926,7 +926,7 @@ public:
   virtual bool  pinned() const { return false; }
   virtual const Type* Value(PhaseGVN* phase) const;
   virtual const RegMask &in_RegMask(uint) const;
-  virtual int ret_addr_offset() { return 0; }
+  virtual int ret_addr_offset() const { return 0; }
 
   NOT_LP64(bool return_value_is_used() const;)
 
@@ -988,7 +988,7 @@ public:
   // If this is an uncommon trap, return the request code, else zero.
   int uncommon_trap_request() const;
 
-  virtual int ret_addr_offset();
+  virtual int ret_addr_offset() const;
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
   void dump_trap_args(outputStream *st) const;
@@ -1004,7 +1004,7 @@ public:
     init_class_id(Class_MachCallDynamicJava);
     DEBUG_ONLY(_vtable_index = -99);  // throw an assert if uninitialized
   }
-  virtual int ret_addr_offset();
+  virtual int ret_addr_offset() const;
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
 #endif
@@ -1022,7 +1022,7 @@ public:
   MachCallRuntimeNode() : MachCallNode() {
     init_class_id(Class_MachCallRuntime);
   }
-  virtual int ret_addr_offset();
+  virtual int ret_addr_offset() const;
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
 #endif