changeset 5655:5abab4a8250f

made RegisterNode fixed which should fix the issue of TLAB values being GVN'ed in the NewInstanceSnippets (bug was found by Thomas)
author Doug Simon <doug.simon@oracle.com>
date Tue, 19 Jun 2012 17:00:24 +0200
parents a4765b93eb96
children c06ee31464c0
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/RegisterNode.java graal/com.oracle.graal.tests/src/com/oracle/graal/compiler/tests/NewInstanceTest.java
diffstat 2 files changed, 38 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/RegisterNode.java	Tue Jun 19 14:09:57 2012 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/nodes/RegisterNode.java	Tue Jun 19 17:00:24 2012 +0200
@@ -26,14 +26,14 @@
 
 import com.oracle.graal.api.code.*;
 import com.oracle.graal.api.meta.*;
-import com.oracle.graal.nodes.calc.*;
+import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.spi.*;
 import com.oracle.graal.nodes.type.*;
 
 /**
  * Access the value of a specific register.
  */
-public final class RegisterNode extends FloatingNode implements LIRLowerable {
+public final class RegisterNode extends FixedWithNextNode implements LIRLowerable {
 
     private final Register register;
 
--- a/graal/com.oracle.graal.tests/src/com/oracle/graal/compiler/tests/NewInstanceTest.java	Tue Jun 19 14:09:57 2012 +0200
+++ b/graal/com.oracle.graal.tests/src/com/oracle/graal/compiler/tests/NewInstanceTest.java	Tue Jun 19 17:00:24 2012 +0200
@@ -53,6 +53,7 @@
         test("newEmptyString");
         test("newString", "value");
         test("newHashMap", 31);
+        test("newRegression", true);
     }
 
     public static Object newObject() {
@@ -149,4 +150,39 @@
         Object f44;
         Object f45;
     }
+
+    /**
+     * Tests that an earlier bug does not occur. The issue was that the loading of the TLAB
+     * 'top' and 'end' values was being GVN'ed from each branch of the 'if' statement.
+     * This meant that the allocated B object in the true branch overwrote the allocated
+     * array. The cause is that RegisterNode was a floating node and the reads from it
+     * were UnsafeLoads which are also floating. The fix was to make RegisterNode a fixed
+     * node (which it should have been in the first place).
+     */
+    public static Object newRegression(boolean condition) {
+        Object result;
+        if (condition) {
+            Object[] arr = {0, 1, 2, 3, 4, 5};
+            result = new B();
+            for (int i = 0; i < arr.length; ++i) {
+                // If the bug exists, the values of arr will now be deadbeef values
+                // and the virtual dispatch will cause a segfault. This can result in
+                // either a VM crash or a spurious NullPointerException.
+                if (arr[i].equals(Integer.valueOf(i))) {
+                    return false;
+                }
+            }
+        } else {
+            result = new B();
+        }
+        return result;
+    }
+
+    static class B {
+        long f1 = 0xdeadbeefdeadbe01L;
+        long f2 = 0xdeadbeefdeadbe02L;
+        long f3 = 0xdeadbeefdeadbe03L;
+        long f4 = 0xdeadbeefdeadbe04L;
+        long f5 = 0xdeadbeefdeadbe05L;
+    }
 }