RFR(S): 8228649: [PPC64] SA reads wrong slots from interpreter frames

[Doerr, Martin]

Hi Gustavo,

thanks for reviewing and for running the tests again.

I have removed ClhsdbCDSJstackPrintAll.java and ClhsdbFindPC.java from the ProblemList.
Tests have passed. Webrev is updated in place.
I’ll push it after some more testing time.

Best regards,
Martin


From: Gustavo Bueno Romero <gromero at br.ibm.com>
Sent: Dienstag, 30. Juli 2019 15:09
To: hotspot-runtime-dev at openjdk.java.net; Doerr, Martin <martin.doerr at sap.com>
Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; gustavo.romero at protonmail.com
Subject: Re: RFR(S): 8228649: [PPC64] SA reads wrong slots from interpreter frames

Hi Martin,

Change looks good!

In fact I see it fixed way more than just 'DebugdConnectTest.java'.

On fastdebug, only two tests have issues now:

1. FAILED: serviceability/sa/TestInstanceKlassSize.java
2. Error:  serviceability/sa/TestJmapCoreMetaspace.java

On release, only one:

2. FAILED: serviceability/sa/TestInstanceKlassSize.java

1. is succeeds on exploded image files tho and 2. looks to fail
only in fastdebug due to added asserts related to barriers
debugging.

So I think we could remove the following two tests too from the
problem list (besides the ones you already removed)

$ egrep "ClhsdbCDSJstackPrintAll.java|ClhsdbFindPC.java"  test/hotspot/jtreg/ProblemList.txt
serviceability/sa/ClhsdbCDSJstackPrintAll.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64
serviceability/sa/ClhsdbFindPC.java 8193639,8211767 solaris-all,linux-ppc64le,linux-ppc64

since both pass on fastdebug and release builds.

Would you mind to remove them and test the change again against
SAP CI to double check if the looks ok there too? (no new webrev needed)

So, I wasn't smart enough to figure out:

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

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


-    Address result = addressOfStackSlot(INTERPRETER_FRAME_MONITOR_BLOCK_TOP_OFFSET).getAddressAt(0);

+    Address result = addressOfStackSlot(INTERPRETER_FRAME_MONITORS_OFFSET).getAddressAt(0);
when investigating JDK-8228649 [0] .  Hence I closed [0] as a duplicate.

The ablation of reserved slot intended for debugging looks good too.

Thumps up from my side.

Thank you.

Best regards,
Gustavo

[0] https://bugs.openjdk.java.net/browse/JDK-8211767

--

For the records, the current SA status I see on PPC64 LE when this change is applied:

** fastdebug **

$ sudo JT_JAVA=/usr/lib/jvm/java-1.8.0-openjdk-ppc64el /home/gromero/jtreg/bin/jtreg -nativepath:/home/gromero/hg/jdk/jdk/build/linux-ppc64le-server-fastdebug/support/test/hotspot/jtreg/native/lib -v1 -conc:1 -jdk:./build/linux-ppc64le-server-fastdebug/images/jdk/ ./test/hotspot/jtreg/serviceability/sa
Passed: serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
Passed: serviceability/sa/sadebugd/DebugdConnectTest.java
Passed: serviceability/sa/sadebugd/SADebugDTest.java
Passed: serviceability/sa/CDSJMapClstats.java
Passed: serviceability/sa/ClhsdbAttach.java
Passed: serviceability/sa/ClhsdbCDSCore.java
Passed: serviceability/sa/ClhsdbCDSJstackPrintAll.java
Passed: serviceability/sa/ClhsdbField.java
Passed: serviceability/sa/ClhsdbFindPC.java
Passed: serviceability/sa/ClhsdbFlags.java
Passed: serviceability/sa/ClhsdbInspect.java
Passed: serviceability/sa/ClhsdbJdis.java
Passed: serviceability/sa/ClhsdbJhisto.java
Passed: serviceability/sa/ClhsdbJstack.java
Passed: serviceability/sa/ClhsdbLongConstant.java
Passed: serviceability/sa/ClhsdbPmap.java
Passed: serviceability/sa/ClhsdbPrintAll.java
Passed: serviceability/sa/ClhsdbPrintAs.java
Passed: serviceability/sa/ClhsdbPrintStatics.java
Passed: serviceability/sa/ClhsdbPstack.java
Passed: serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java
Passed: serviceability/sa/ClhsdbScanOops.java
Passed: serviceability/sa/ClhsdbSource.java
Passed: serviceability/sa/ClhsdbThread.java
Passed: serviceability/sa/ClhsdbVmStructsDump.java
Passed: serviceability/sa/ClhsdbWhere.java
Passed: serviceability/sa/DeadlockDetectionTest.java
Passed: serviceability/sa/JhsdbThreadInfoTest.java
Passed: serviceability/sa/TestClassDump.java
Passed: serviceability/sa/TestClhsdbJstackLock.java
Passed: serviceability/sa/TestCpoolForInvokeDynamic.java
Passed: serviceability/sa/TestDefaultMethods.java
Passed: serviceability/sa/TestG1HeapRegion.java
Passed: serviceability/sa/TestHeapDumpForInvokeDynamic.java
Passed: serviceability/sa/TestHeapDumpForLargeArray.java
FAILED: serviceability/sa/TestInstanceKlassSize.java
Passed: serviceability/sa/TestInstanceKlassSizeForInterface.java
Passed: serviceability/sa/TestIntConstant.java
Passed: serviceability/sa/TestJhsdbJstackLock.java
Passed: serviceability/sa/TestJhsdbJstackMixed.java
Passed: serviceability/sa/TestJmapCore.java
Error:  serviceability/sa/TestJmapCoreMetaspace.java
Passed: serviceability/sa/TestPrintMdo.java
Passed: serviceability/sa/TestRevPtrsForInvokeDynamic.java
Passed: serviceability/sa/TestType.java
Passed: serviceability/sa/TestUniverse.java
Test results: passed: 44; failed: 1; error: 1


** release **

$ sudo JT_JAVA=/usr/lib/jvm/java-1.8.0-openjdk-ppc64el /home/gromero/jtreg/bin/jtreg -nativepath:/home/gromero/hg/jdk/jdk/build/linux-ppc64le-server-release/support/test/hotspot/jtreg/native/lib -v1 -conc:1 -jdk:./build/linux-ppc64le-server-release/images/jdk/ ./test/hotspot/jtreg/serviceability/sa
Passed: serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
Passed: serviceability/sa/sadebugd/DebugdConnectTest.java
Passed: serviceability/sa/sadebugd/SADebugDTest.java
Passed: serviceability/sa/CDSJMapClstats.java
Passed: serviceability/sa/ClhsdbAttach.java
Passed: serviceability/sa/ClhsdbCDSCore.java
Passed: serviceability/sa/ClhsdbCDSJstackPrintAll.java
Passed: serviceability/sa/ClhsdbField.java
Passed: serviceability/sa/ClhsdbFindPC.java
Passed: serviceability/sa/ClhsdbFlags.java
Passed: serviceability/sa/ClhsdbInspect.java
Passed: serviceability/sa/ClhsdbJdis.java
Passed: serviceability/sa/ClhsdbJhisto.java
Passed: serviceability/sa/ClhsdbJstack.java
Passed: serviceability/sa/ClhsdbLongConstant.java
Passed: serviceability/sa/ClhsdbPmap.java
Passed: serviceability/sa/ClhsdbPrintAll.java
Passed: serviceability/sa/ClhsdbPrintAs.java
Passed: serviceability/sa/ClhsdbPrintStatics.java
Passed: serviceability/sa/ClhsdbPstack.java
Passed: serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java
Passed: serviceability/sa/ClhsdbScanOops.java
Passed: serviceability/sa/ClhsdbSource.java
Passed: serviceability/sa/ClhsdbThread.java
Passed: serviceability/sa/ClhsdbVmStructsDump.java
Passed: serviceability/sa/ClhsdbWhere.java
Passed: serviceability/sa/DeadlockDetectionTest.java
Passed: serviceability/sa/JhsdbThreadInfoTest.java
Passed: serviceability/sa/TestClassDump.java
Passed: serviceability/sa/TestClhsdbJstackLock.java
Passed: serviceability/sa/TestCpoolForInvokeDynamic.java
Passed: serviceability/sa/TestDefaultMethods.java
Passed: serviceability/sa/TestG1HeapRegion.java
Passed: serviceability/sa/TestHeapDumpForInvokeDynamic.java
Passed: serviceability/sa/TestHeapDumpForLargeArray.java
FAILED: serviceability/sa/TestInstanceKlassSize.java
Passed: serviceability/sa/TestInstanceKlassSizeForInterface.java
Passed: serviceability/sa/TestIntConstant.java
Passed: serviceability/sa/TestJhsdbJstackLock.java
Passed: serviceability/sa/TestJhsdbJstackMixed.java
Passed: serviceability/sa/TestJmapCore.java
Passed: serviceability/sa/TestJmapCoreMetaspace.java
Passed: serviceability/sa/TestPrintMdo.java
Passed: serviceability/sa/TestRevPtrsForInvokeDynamic.java
Passed: serviceability/sa/TestType.java
Passed: serviceability/sa/TestUniverse.java
Test results: passed: 45; failed: 1