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
--- a/src/hotspot/cpu/ppc/frame_ppc.hpp Fri Aug 02 11:21:41 2019 +0200
+++ b/src/hotspot/cpu/ppc/frame_ppc.hpp Fri Aug 02 11:28:58 2019 +0200
@@ -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);
--- a/src/hotspot/cpu/ppc/frame_ppc.inline.hpp Fri Aug 02 11:21:41 2019 +0200
+++ b/src/hotspot/cpu/ppc/frame_ppc.inline.hpp Fri Aug 02 11:28:58 2019 +0200
@@ -130,19 +130,22 @@
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 @@
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; }
--- a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp Fri Aug 02 11:21:41 2019 +0200
+++ b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp Fri Aug 02 11:28:58 2019 +0200
@@ -768,16 +768,6 @@
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
}
@@ -2263,14 +2253,6 @@
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
}
--- a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp Fri Aug 02 11:21:41 2019 +0200
+++ b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp Fri Aug 02 11:28:58 2019 +0200
@@ -2719,10 +2719,6 @@
__ 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);
@@ -2795,10 +2791,6 @@
__ 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
--- a/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp Fri Aug 02 11:21:41 2019 +0200
+++ b/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp Fri Aug 02 11:28:58 2019 +0200
@@ -1050,17 +1050,14 @@
// 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(R18_locals, _ijava_state_neg(locals), R1_SP);
__ std(R27_constPoolCache, _ijava_state_neg(cpoolCache), R1_SP);
- __ std(R18_locals, _ijava_state_neg(locals), 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 @@
// 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.
--- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ppc64/PPC64Frame.java Fri Aug 02 11:21:41 2019 +0200
+++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ppc64/PPC64Frame.java Fri Aug 02 11:28:58 2019 +0200
@@ -51,14 +51,12 @@
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 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");
--- a/test/hotspot/jtreg/ProblemList.txt Fri Aug 02 11:21:41 2019 +0200
+++ b/test/hotspot/jtreg/ProblemList.txt Fri Aug 02 11:28:58 2019 +0200
@@ -94,32 +94,32 @@
serviceability/sa/ClhsdbAttach.java 8193639 solaris-all
serviceability/sa/ClhsdbCDSCore.java 8193639 solaris-all
-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/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/jmap-hprof/JMapHProfLargeHeapTest.java 8193639 solaris-all
serviceability/sa/TestClassDump.java 8193639 solaris-all
-serviceability/sa/TestClhsdbJstackLock.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
+serviceability/sa/TestClhsdbJstackLock.java 8193639 solaris-all
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
@@ -128,7 +128,7 @@
serviceability/sa/TestInstanceKlassSize.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/TestJmapCore.java 8193639 solaris-all
serviceability/sa/TestJmapCoreMetaspace.java 8193639 solaris-all
serviceability/sa/TestPrintMdo.java 8193639 solaris-all