8194249: SA: G1HeapRegionTable#getByAddress() returns incorrect HeapRegion
authorysuenaga
Wed, 31 Jan 2018 11:35:42 +0100
changeset 48820 9a411a9a17f0
parent 48819 ee513596f3ee
child 48821 8250a10cf60f
8194249: SA: G1HeapRegionTable#getByAddress() returns incorrect HeapRegion Summary: Fix conversion of addresses to HeapRegions. Reviewed-by: jgeorge, tschatzl
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/G1HeapRegionTable.java
test/hotspot/jtreg/serviceability/sa/TestG1HeapRegion.java
--- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/G1HeapRegionTable.java	Tue Jan 30 16:41:40 2018 +0100
+++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/G1HeapRegionTable.java	Wed Jan 31 11:35:42 2018 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2018, 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
@@ -134,11 +134,11 @@
         super(addr);
     }
 
-    public HeapRegion getByAddress(Address addr) {
-        long biasedIndex = addr.asLongValue() >>> shiftBy();
+    public HeapRegion getByAddress(Address target) {
+        Address arrayAddr = biasedBaseField.getValue(addr);
+        long biasedIndex = target.asLongValue() >>> shiftBy();
         long offset = biasedIndex * HeapRegion.getPointerSize();
-        Address result = (addr instanceof OopHandle) ? addr.addOffsetToAsOopHandle(offset)
-                                                     : addr.addOffsetTo(offset);
-        return new HeapRegion(result);
+        Address regionAddr = arrayAddr.getAddressAt(offset);
+        return (HeapRegion)VMObjectFactory.newObject(HeapRegion.class, regionAddr);
     }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/serviceability/sa/TestG1HeapRegion.java	Wed Jan 31 11:35:42 2018 +0100
@@ -0,0 +1,113 @@
+/*
+ * Copyright (c) 2018, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.util.ArrayList;
+import java.util.List;
+
+import sun.jvm.hotspot.gc.g1.G1CollectedHeap;
+import sun.jvm.hotspot.gc.g1.HeapRegion;
+import sun.jvm.hotspot.HotSpotAgent;
+import sun.jvm.hotspot.runtime.VM;
+
+import jdk.test.lib.apps.LingeredApp;
+import jdk.test.lib.Asserts;
+import jdk.test.lib.Platform;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.Utils;
+
+/*
+ * @test
+ * @library /test/lib
+ * @requires os.family != "mac"
+ * @modules jdk.hotspot.agent/sun.jvm.hotspot
+ *          jdk.hotspot.agent/sun.jvm.hotspot.gc.g1
+ *          jdk.hotspot.agent/sun.jvm.hotspot.memory
+ *          jdk.hotspot.agent/sun.jvm.hotspot.runtime
+ * @run main/othervm TestG1HeapRegion
+ */
+
+public class TestG1HeapRegion {
+
+    private static LingeredApp theApp = null;
+
+    private static void checkHeapRegion(String pid) throws Exception {
+        HotSpotAgent agent = new HotSpotAgent();
+
+        try {
+            agent.attach(Integer.parseInt(pid));
+            G1CollectedHeap heap = (G1CollectedHeap)VM.getVM().getUniverse().heap();
+            HeapRegion hr = heap.hrm().heapRegionIterator().next();
+            HeapRegion hrTop = heap.hrm().getByAddress(hr.top());
+
+            Asserts.assertEquals(hr.top(), hrTop.top(),
+                                 "Address of HeapRegion does not match.");
+        } finally {
+            agent.detach();
+        }
+    }
+
+    private static void createAnotherToAttach(long lingeredAppPid)
+                                                         throws Exception {
+        String[] toolArgs = {
+            "--add-modules=jdk.hotspot.agent",
+            "--add-exports=jdk.hotspot.agent/sun.jvm.hotspot=ALL-UNNAMED",
+            "--add-exports=jdk.hotspot.agent/sun.jvm.hotspot.gc.g1=ALL-UNNAMED",
+            "--add-exports=jdk.hotspot.agent/sun.jvm.hotspot.memory=ALL-UNNAMED",
+            "--add-exports=jdk.hotspot.agent/sun.jvm.hotspot.runtime=ALL-UNNAMED",
+            "TestG1HeapRegion",
+            Long.toString(lingeredAppPid)
+        };
+
+        // Start a new process to attach to the lingered app
+        ProcessBuilder processBuilder = ProcessTools.createJavaProcessBuilder(toolArgs);
+        OutputAnalyzer SAOutput = ProcessTools.executeProcess(processBuilder);
+        SAOutput.shouldHaveExitValue(0);
+        System.out.println(SAOutput.getOutput());
+    }
+
+    public static void main (String... args) throws Exception {
+        if (!Platform.shouldSAAttach()) {
+            System.out.println(
+               "SA attach not expected to work - test skipped.");
+            return;
+        }
+
+        if (args == null || args.length == 0) {
+            try {
+                List<String> vmArgs = new ArrayList<String>();
+                vmArgs.add("-XX:+UsePerfData");
+                vmArgs.add("-XX:+UseG1GC");
+                vmArgs.addAll(Utils.getVmOptions());
+
+                theApp = new LingeredApp();
+                LingeredApp.startApp(vmArgs, theApp);
+                createAnotherToAttach(theApp.getPid());
+            } finally {
+                LingeredApp.stopApp(theApp);
+            }
+        } else {
+            checkHeapRegion(args[0]);
+        }
+    }
+}