8228649: [PPC64] SA reads wrong slots from interpreter frames

Summary: Make frame layout consistent between dbg and product build and implement offsets accordingly.
Reviewed-by: goetz, gromero

src/hotspot/cpu/ppc/frame_ppc.hpp

@@ -250,9 +250,6 @@
         (offset_of(frame::top_ijava_frame_abi, _component))
 
   struct ijava_state {
-#ifdef ASSERT
-    uint64_t ijava_reserved; // Used for assertion.
-#endif
     uint64_t method;
     uint64_t mirror;
     uint64_t locals;
@@ -409,12 +406,6 @@
   // The size of a cInterpreter object.
   static inline int interpreter_frame_cinterpreterstate_size_in_bytes();
 
- private:
-
-  ConstantPoolCache** interpreter_frame_cpoolcache_addr() const;
-
- public:
-
   // Additional interface for entry frames:
   inline entry_frame_locals* get_entry_frame_locals() const {
     return (entry_frame_locals*) (((address) fp()) - entry_frame_locals_size);

src/hotspot/cpu/ppc/frame_ppc.inline.hpp

@@ -130,19 +130,22 @@ inline frame::ijava_state* frame::get_ijava_state() const {
 inline intptr_t** frame::interpreter_frame_locals_addr() const {
   return (intptr_t**) &(get_ijava_state()->locals);
 }
+
 inline intptr_t* frame::interpreter_frame_bcp_addr() const {
   return (intptr_t*) &(get_ijava_state()->bcp);
 }
+
 inline intptr_t* frame::interpreter_frame_mdp_addr() const {
   return (intptr_t*) &(get_ijava_state()->mdx);
 }
+
 // Pointer beyond the "oldest/deepest" BasicObjectLock on stack.
 inline BasicObjectLock* frame::interpreter_frame_monitor_end() const {
-  return (BasicObjectLock *) get_ijava_state()->monitors;
+  return (BasicObjectLock*) get_ijava_state()->monitors;
 }
 
 inline BasicObjectLock* frame::interpreter_frame_monitor_begin() const {
-  return (BasicObjectLock *) get_ijava_state();
+  return (BasicObjectLock*) get_ijava_state();
 }
 
 // Return register stack slot addr at which currently interpreted method is found.
@@ -154,23 +157,21 @@ inline oop* frame::interpreter_frame_mirror_addr() const {
   return (oop*) &(get_ijava_state()->mirror);
 }
 
-inline ConstantPoolCache** frame::interpreter_frame_cpoolcache_addr() const {
-  return (ConstantPoolCache**) &(get_ijava_state()->cpoolCache);
-}
 inline ConstantPoolCache** frame::interpreter_frame_cache_addr() const {
   return (ConstantPoolCache**) &(get_ijava_state()->cpoolCache);
 }
 
 inline oop* frame::interpreter_frame_temp_oop_addr() const {
-  return (oop *) &(get_ijava_state()->oop_tmp);
+  return (oop*) &(get_ijava_state()->oop_tmp);
 }
+
 inline intptr_t* frame::interpreter_frame_esp() const {
   return (intptr_t*) get_ijava_state()->esp;
 }
 
 // Convenient setters
 inline void frame::interpreter_frame_set_monitor_end(BasicObjectLock* end)    { get_ijava_state()->monitors = (intptr_t) end;}
-inline void frame::interpreter_frame_set_cpcache(ConstantPoolCache* cp)       { *frame::interpreter_frame_cpoolcache_addr() = cp; }
+inline void frame::interpreter_frame_set_cpcache(ConstantPoolCache* cp)       { *interpreter_frame_cache_addr() = cp; }
 inline void frame::interpreter_frame_set_esp(intptr_t* esp)                   { get_ijava_state()->esp = (intptr_t) esp; }
 inline void frame::interpreter_frame_set_top_frame_sp(intptr_t* top_frame_sp) { get_ijava_state()->top_frame_sp = (intptr_t) top_frame_sp; }
 inline void frame::interpreter_frame_set_sender_sp(intptr_t* sender_sp)       { get_ijava_state()->sender_sp = (intptr_t) sender_sp; }

src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp

@@ -756,16 +756,6 @@ void InterpreterMacroAssembler::merge_frames(Register Rsender_sp, Register retur
   ld(Rscratch1, 0, R1_SP); // *SP
   ld(Rsender_sp, _ijava_state_neg(sender_sp), Rscratch1); // top_frame_sp
   ld(Rscratch2, 0, Rscratch1); // **SP
-#ifdef ASSERT
-  {
-    Label Lok;
-    ld(R0, _ijava_state_neg(ijava_reserved), Rscratch1);
-    cmpdi(CCR0, R0, 0x5afe);
-    beq(CCR0, Lok);
-    stop("frame corrupted (remove activation)", 0x5afe);
-    bind(Lok);
-  }
-#endif
   if (return_pc!=noreg) {
     ld(return_pc, _abi(lr), Rscratch1); // LR
   }
@@ -2251,14 +2241,6 @@ void InterpreterMacroAssembler::restore_interpreter_state(Register scratch, bool
     stop("frame too small (restore istate)", 0x5432);
     bind(Lok);
   }
-  {
-    Label Lok;
-    ld(R0, _ijava_state_neg(ijava_reserved), scratch);
-    cmpdi(CCR0, R0, 0x5afe);
-    beq(CCR0, Lok);
-    stop("frame corrupted (restore istate)", 0x5afe);
-    bind(Lok);
-  }
 #endif
 }
 

src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp

@@ -2690,10 +2690,6 @@ static void push_skeleton_frame(MacroAssembler* masm, bool deopt,
   __ ld(frame_size_reg, 0, frame_sizes_reg);
   __ std(pc_reg, _abi(lr), R1_SP);
   __ push_frame(frame_size_reg, R0/*tmp*/);
-#ifdef ASSERT
-  __ load_const_optimized(pc_reg, 0x5afe);
-  __ std(pc_reg, _ijava_state_neg(ijava_reserved), R1_SP);
-#endif
   __ std(R1_SP, _ijava_state_neg(sender_sp), R1_SP);
   __ addi(number_of_frames_reg, number_of_frames_reg, -1);
   __ addi(frame_sizes_reg, frame_sizes_reg, wordSize);
@@ -2766,10 +2762,6 @@ static void push_skeleton_frames(MacroAssembler* masm, bool deopt,
   __ std(R12_scratch2, _abi(lr), R1_SP);
 
   // Initialize initial_caller_sp.
-#ifdef ASSERT
- __ load_const_optimized(pc_reg, 0x5afe);
- __ std(pc_reg, _ijava_state_neg(ijava_reserved), R1_SP);
-#endif
  __ std(frame_size_reg, _ijava_state_neg(sender_sp), R1_SP);
 
 #ifdef ASSERT

src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp

@@ -1050,17 +1050,14 @@ void TemplateInterpreterGenerator::generate_fixed_frame(bool native_call, Regist
   // Get mirror and store it in the frame as GC root for this Method*.
   __ load_mirror_from_const_method(R12_scratch2, Rconst_method);
 
-  __ addi(R26_monitor, R1_SP, - frame::ijava_state_size);
-  __ addi(R15_esp, R26_monitor, - Interpreter::stackElementSize);
+  __ addi(R26_monitor, R1_SP, -frame::ijava_state_size);
+  __ addi(R15_esp, R26_monitor, -Interpreter::stackElementSize);
 
   // Store values.
-  // R15_esp, R14_bcp, R26_monitor, R28_mdx are saved at java calls
-  // in InterpreterMacroAssembler::call_from_interpreter.
   __ std(R19_method, _ijava_state_neg(method), R1_SP);
   __ std(R12_scratch2, _ijava_state_neg(mirror), R1_SP);
-  __ std(R21_sender_SP, _ijava_state_neg(sender_sp), R1_SP);
-  __ std(R27_constPoolCache, _ijava_state_neg(cpoolCache), R1_SP);
   __ std(R18_locals, _ijava_state_neg(locals), R1_SP);
+  __ std(R27_constPoolCache, _ijava_state_neg(cpoolCache), R1_SP);
 
   // Note: esp, bcp, monitor, mdx live in registers. Hence, the correct version can only
   // be found in the frame after save_interpreter_state is done. This is always true
@@ -1068,31 +1065,20 @@ void TemplateInterpreterGenerator::generate_fixed_frame(bool native_call, Regist
   // because e.g. frame::interpreter_frame_bcp() will not access the correct value
   // (Enhanced Stack Trace).
   // The signal handler does not save the interpreter state into the frame.
+
+  // We have to initialize some of these frame slots for native calls (accessed by GC).
+  // Also initialize them for non-native calls for better tool support (even though
+  // you may not get the most recent version as described above).
   __ li(R0, 0);
-#ifdef ASSERT
-  // Fill remaining slots with constants.
-  __ load_const_optimized(R11_scratch1, 0x5afe);
-  __ load_const_optimized(R12_scratch2, 0xdead);
-#endif
-  // We have to initialize some frame slots for native calls (accessed by GC).
-  if (native_call) {
-    __ std(R26_monitor, _ijava_state_neg(monitors), R1_SP);
-    __ std(R14_bcp, _ijava_state_neg(bcp), R1_SP);
-    if (ProfileInterpreter) { __ std(R28_mdx, _ijava_state_neg(mdx), R1_SP); }
-  }
-#ifdef ASSERT
-  else {
-    __ std(R12_scratch2, _ijava_state_neg(monitors), R1_SP);
-    __ std(R12_scratch2, _ijava_state_neg(bcp), R1_SP);
-    __ std(R12_scratch2, _ijava_state_neg(mdx), R1_SP);
-  }
-  __ std(R11_scratch1, _ijava_state_neg(ijava_reserved), R1_SP);
-  __ std(R12_scratch2, _ijava_state_neg(esp), R1_SP);
-  __ std(R12_scratch2, _ijava_state_neg(lresult), R1_SP);
-  __ std(R12_scratch2, _ijava_state_neg(fresult), R1_SP);
-#endif
+  __ std(R26_monitor, _ijava_state_neg(monitors), R1_SP);
+  __ std(R14_bcp, _ijava_state_neg(bcp), R1_SP);
+  if (ProfileInterpreter) { __ std(R28_mdx, _ijava_state_neg(mdx), R1_SP); }
+  __ std(R15_esp, _ijava_state_neg(esp), R1_SP);
+  __ std(R0, _ijava_state_neg(oop_tmp), R1_SP); // only used for native_call
+
+  // Store sender's SP and this frame's top SP.
   __ subf(R12_scratch2, top_frame_size, R1_SP);
-  __ std(R0, _ijava_state_neg(oop_tmp), R1_SP);
+  __ std(R21_sender_SP, _ijava_state_neg(sender_sp), R1_SP);
   __ std(R12_scratch2, _ijava_state_neg(top_frame_sp), R1_SP);
 
   // Push top frame.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ppc64/PPC64Frame.java

@@ -51,14 +51,12 @@ public class PPC64Frame extends Frame {
   private static final int INTERPRETER_FRAME_MDX_OFFSET = INTERPRETER_FRAME_LAST_SP_OFFSET -1;
   private static final int INTERPRETER_FRAME_ESP_OFFSET = INTERPRETER_FRAME_MDX_OFFSET - 1;
   private static final int INTERPRETER_FRAME_BCX_OFFSET = INTERPRETER_FRAME_ESP_OFFSET - 1;
-  private static final int INTERPRETER_FRAME_CACHE_OFFSET =INTERPRETER_FRAME_BCX_OFFSET - 1;
+  private static final int INTERPRETER_FRAME_CACHE_OFFSET = INTERPRETER_FRAME_BCX_OFFSET - 1;
   private static final int INTERPRETER_FRAME_MONITORS_OFFSET = INTERPRETER_FRAME_CACHE_OFFSET - 1;
   private static final int INTERPRETER_FRAME_LOCALS_OFFSET = INTERPRETER_FRAME_MONITORS_OFFSET - 1;
   private static final int INTERPRETER_FRAME_MIRROR_OFFSET = INTERPRETER_FRAME_LOCALS_OFFSET - 1;
   private static final int INTERPRETER_FRAME_METHOD_OFFSET = INTERPRETER_FRAME_MIRROR_OFFSET - 1;
-  private static final int INTERPRETER_FRAME_INITIAL_SP_OFFSET = INTERPRETER_FRAME_BCX_OFFSET - 1; // FIXME: probably wrong, but unused anyway
-  private static final int INTERPRETER_FRAME_MONITOR_BLOCK_TOP_OFFSET = INTERPRETER_FRAME_INITIAL_SP_OFFSET;
-  private static final int INTERPRETER_FRAME_MONITOR_BLOCK_BOTTOM_OFFSET = INTERPRETER_FRAME_INITIAL_SP_OFFSET;
+  private static final int INTERPRETER_FRAME_MONITOR_BLOCK_BOTTOM_OFFSET = INTERPRETER_FRAME_METHOD_OFFSET - 1;
 
   // Entry frames
   private static int ENTRY_FRAME_CALL_WRAPPER_OFFSET;
@@ -444,7 +442,7 @@ public class PPC64Frame extends Frame {
   }
 
   public BasicObjectLock interpreterFrameMonitorEnd() {
-    Address result = addressOfStackSlot(INTERPRETER_FRAME_MONITOR_BLOCK_TOP_OFFSET).getAddressAt(0);
+    Address result = addressOfStackSlot(INTERPRETER_FRAME_MONITORS_OFFSET).getAddressAt(0);
     if (Assert.ASSERTS_ENABLED) {
       // make sure the pointer points inside the frame
       Assert.that(AddressOps.gt(getFP(), result), "result must <  than frame pointer");

test/hotspot/jtreg/ProblemList.txt

@@ -88,34 +88,33 @@ runtime/jni/terminatedThread/TestTerminatedThread.java 8219652 aix-ppc64
 
 serviceability/sa/ClhsdbAttach.java 8193639 solaris-all
 serviceability/sa/ClhsdbCDSCore.java 8207832 linux-x64
-serviceability/sa/ClhsdbCDSJstackPrintAll.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/ClhsdbCDSJstackPrintAll.java 8193639 solaris-all
 serviceability/sa/CDSJMapClstats.java 8193639 solaris-all
 serviceability/sa/ClhsdbField.java 8193639 solaris-all
-serviceability/sa/ClhsdbFindPC.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/ClhsdbFindPC.java 8193639 solaris-all
 serviceability/sa/ClhsdbFlags.java 8193639 solaris-all
-serviceability/sa/ClhsdbInspect.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
-serviceability/sa/ClhsdbJdis.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/ClhsdbInspect.java 8193639 solaris-all
+serviceability/sa/ClhsdbJdis.java 8193639 solaris-all
 serviceability/sa/ClhsdbJhisto.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
-serviceability/sa/ClhsdbJstack.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/ClhsdbJstack.java 8193639 solaris-all
 serviceability/sa/ClhsdbLongConstant.java 8193639 solaris-all
 serviceability/sa/ClhsdbPmap.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
 serviceability/sa/ClhsdbPrintAll.java 8193639 solaris-all
-serviceability/sa/ClhsdbPrintAs.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/ClhsdbPrintAs.java 8193639 solaris-all
 serviceability/sa/ClhsdbPrintStatics.java 8193639 solaris-all
 serviceability/sa/ClhsdbPstack.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
 serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java 8193639 solaris-all
 serviceability/sa/ClhsdbScanOops.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
-serviceability/sa/ClhsdbSource.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/ClhsdbSource.java 8193639 solaris-all
 serviceability/sa/ClhsdbSymbol.java 8193639 solaris-all
 serviceability/sa/ClhsdbSymbolTable.java 8193639 solaris-all
 serviceability/sa/ClhsdbThread.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
 serviceability/sa/ClhsdbVmStructsDump.java 8193639 solaris-all
-serviceability/sa/ClhsdbWhere.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/ClhsdbWhere.java 8193639 solaris-all
 serviceability/sa/DeadlockDetectionTest.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
-serviceability/sa/JhsdbThreadInfoTest.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/JhsdbThreadInfoTest.java 8193639 solaris-all
 serviceability/sa/sadebugd/SADebugDTest.java 8163805 generic-all
 serviceability/sa/TestClassDump.java 8193639 solaris-all
-serviceability/sa/TestClhsdbJstackLock.java 8211767 linux-ppc64le,linux-ppc64
 serviceability/sa/TestCpoolForInvokeDynamic.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
 serviceability/sa/TestDefaultMethods.java 8193639 solaris-all
 serviceability/sa/TestG1HeapRegion.java 8193639 solaris-all
@@ -123,7 +122,7 @@ serviceability/sa/TestHeapDumpForInvokeDynamic.java 8193639 solaris-all
 serviceability/sa/TestHeapDumpForLargeArray.java 8193639 solaris-all
 serviceability/sa/TestInstanceKlassSizeForInterface.java 8193639 solaris-all
 serviceability/sa/TestIntConstant.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
-serviceability/sa/TestJhsdbJstackLock.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/TestJhsdbJstackLock.java 8193639 solaris-all
 serviceability/sa/TestRevPtrsForInvokeDynamic.java 8191270 generic-all
 serviceability/sa/TestType.java 8193639 solaris-all
 serviceability/sa/TestUniverse.java#id0 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64