8161720: Better byte behavior for off-heap data

Normalize boolean values read with Unsafe.

Reviewed-by: aph, simonis, jrose, psandoz

hotspot/src/share/vm/c1/c1_LIRGenerator.cpp

@@ -2410,6 +2410,15 @@ void LIRGenerator::do_UnsafeGetObject(UnsafeGetObject* x) {
 #endif // INCLUDE_ALL_GCS
 
   if (x->is_volatile() && os::is_MP()) __ membar_acquire();
+
+  /* Normalize boolean value returned by unsafe operation, i.e., value  != 0 ? value = true : value false. */
+  if (type == T_BOOLEAN) {
+    LabelObj* equalZeroLabel = new LabelObj();
+    __ cmp(lir_cond_equal, value, 0);
+    __ branch(lir_cond_equal, T_BOOLEAN, equalZeroLabel->label());
+    __ move(LIR_OprFact::intConst(1), value);
+    __ branch_destination(equalZeroLabel->label());
+  }
 }
 
 

hotspot/src/share/vm/opto/library_call.cpp

@@ -2471,6 +2471,28 @@ bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, c
       // load value
       switch (type) {
       case T_BOOLEAN:
+      {
+        // Normalize the value returned by getBoolean in the following cases
+        if (mismatched ||
+            heap_base_oop == top() ||                            // - heap_base_oop is NULL or
+            (can_access_non_heap && alias_type->field() == NULL) // - heap_base_oop is potentially NULL
+                                                                 //   and the unsafe access is made to large offset
+                                                                 //   (i.e., larger than the maximum offset necessary for any
+                                                                 //   field access)
+            ) {
+          IdealKit ideal = IdealKit(this);
+#define __ ideal.
+          IdealVariable normalized_result(ideal);
+          __ declarations_done();
+          __ set(normalized_result, p);
+          __ if_then(p, BoolTest::ne, ideal.ConI(0));
+          __ set(normalized_result, ideal.ConI(1));
+          ideal.end_if();
+          final_sync(ideal);
+          p = __ value(normalized_result);
+#undef __
+        }
+      }
       case T_CHAR:
       case T_BYTE:
       case T_SHORT:

hotspot/src/share/vm/prims/unsafe.cpp

@@ -150,14 +150,23 @@ class MemoryAccess : StackObj {
   }
 
   template <typename T>
-  T normalize(T x) {
+  T normalize_for_write(T x) {
     return x;
   }
 
-  jboolean normalize(jboolean x) {
+  jboolean normalize_for_write(jboolean x) {
     return x & 1;
   }
 
+  template <typename T>
+  T normalize_for_read(T x) {
+    return x;
+  }
+
+  jboolean normalize_for_read(jboolean x) {
+    return x != 0;
+  }
+
   /**
    * Helper class to wrap memory accesses in JavaThread::doing_unsafe_access()
    */
@@ -196,7 +205,7 @@ public:
 
     T* p = (T*)addr();
 
-    T x = *p;
+    T x = normalize_for_read(*p);
 
     return x;
   }
@@ -207,7 +216,7 @@ public:
 
     T* p = (T*)addr();
 
-    *p = normalize(x);
+    *p = normalize_for_write(x);
   }
 
 
@@ -223,7 +232,7 @@ public:
 
     T x = OrderAccess::load_acquire((volatile T*)p);
 
-    return x;
+    return normalize_for_read(x);
   }
 
   template <typename T>
@@ -232,7 +241,7 @@ public:
 
     T* p = (T*)addr();
 
-    OrderAccess::release_store_fence((volatile T*)p, normalize(x));
+    OrderAccess::release_store_fence((volatile T*)p, normalize_for_write(x));
   }
 
 
@@ -256,7 +265,7 @@ public:
 
     jlong* p = (jlong*)addr();
 
-    Atomic::store(normalize(x),  p);
+    Atomic::store(normalize_for_write(x),  p);
   }
 #endif
 };

hotspot/test/compiler/unsafe/UnsafeOffHeapBooleanTest.java

@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2016, 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.
+ */
+
+/*
+ * @test
+ * @bug 8161720
+ * @modules java.base/jdk.internal.misc
+ * @run main/othervm -Xint UnsafeOffHeapBooleanTest 1
+ * @run main/othervm -XX:+TieredCompilation -XX:TieredStopAtLevel=3 -Xbatch  UnsafeOffHeapBooleanTest 20000
+ * @run main/othervm -XX:-TieredCompilation -Xbatch UnsafeOffHeapBooleanTest 20000
+ */
+
+
+import java.lang.reflect.Field;
+import jdk.internal.misc.Unsafe;
+
+public class UnsafeOffHeapBooleanTest {
+    static boolean bool0 = false, bool1 = false, result = false;
+    static Unsafe UNSAFE = Unsafe.getUnsafe();
+    static long offHeapMemory;
+
+    public static void test() {
+        // Write two bytes to the off-heap memory location, both
+        // bytes correspond to the boolean value 'true'.
+        UNSAFE.putShort(null, offHeapMemory, (short)0x0204);
+
+        // Read two bytes from the storage allocated above (as booleans).
+        bool0 = UNSAFE.getBoolean(null, offHeapMemory + 0);
+        bool1 = UNSAFE.getBoolean(null, offHeapMemory + 1);
+        result = bool0 & bool1;
+    }
+
+    public static void main(String args[]) {
+        System.out.println("### Test started");
+
+        if (args.length != 1) {
+            throw new RuntimeException("### Test failure: test called with incorrect number of arguments");
+        }
+
+        // Allocate two bytes of storage.
+        offHeapMemory = UNSAFE.allocateMemory(2);
+
+        try {
+            for (int i = 0; i < Integer.parseInt(args[0]); i++) {
+                test();
+            }
+
+            // Check if the two 'true' boolean values were normalized
+            // (i.e., reduced from the range 1...255 to 1).
+            if (!bool0 || !bool1 || !result) {
+                System.out.println("Some of the results below are wrong");
+                System.out.println("bool0 is: " + bool0);
+                System.out.println("bool1 is: " + bool1);
+                System.out.println("bool0 & bool1 is: " + result);
+                System.out.println("===================================");
+                throw new RuntimeException("### Test failed");
+            } else {
+                System.out.println("Test generated correct results");
+                System.out.println("bool0 is: " + bool0);
+                System.out.println("bool1 is: " + bool1);
+                System.out.println("bool0 & bool1 is: " + result);
+                System.out.println("===================================");
+            }
+        } catch (NumberFormatException e) {
+            throw new RuntimeException("### Test failure: test called with incorrectly formatted parameter");
+        }
+
+        UNSAFE.freeMemory(offHeapMemory);
+
+        System.out.println("### Test passed");
+    }
+}

hotspot/test/compiler/unsafe/UnsafeOnHeapBooleanTest.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2016, 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.
+ */
+
+/*
+ * @test
+ * @bug 8161720
+ * @modules java.base/jdk.internal.misc
+ * @run main/othervm -Xint UnsafeOnHeapBooleanTest 1
+ * @run main/othervm -XX:-UseOnStackReplacement -XX:+TieredCompilation -XX:TieredStopAtLevel=3 -Xbatch UnsafeOnHeapBooleanTest 20000
+ * @run main/othervm -XX:-UseOnStackReplacement -XX:-TieredCompilation -Xbatch UnsafeOnHeapBooleanTest 20000
+ */
+
+import java.lang.reflect.Field;
+import jdk.internal.misc.Unsafe;
+
+public class UnsafeOnHeapBooleanTest {
+    static short static_v;
+    static boolean bool0 = false, bool1 = false, result = false;
+    static Unsafe UNSAFE = Unsafe.getUnsafe();
+
+    public static void test() {
+        try {
+            // Write two bytes into the static field
+            // UnsafeOnHeapBooleanTest.static_v write two values. Both
+            // bytes correspond to the boolean value 'true'.
+            Field staticVField = UnsafeOnHeapBooleanTest.class.getDeclaredField("static_v");
+            Object base = UNSAFE.staticFieldBase(staticVField);
+            long offset = UNSAFE.staticFieldOffset(staticVField);
+            UNSAFE.putShort(base, offset, (short)0x0204);
+
+            // Read two bytes from the static field
+            // UnsafeOnHeapBooleanTest.static_v (as booleans).
+            bool0 = UNSAFE.getBoolean(base, offset + 0);
+            bool1 = UNSAFE.getBoolean(base, offset + 1);
+            result = bool0 & bool1;
+        } catch (NoSuchFieldException e) {
+            throw new RuntimeException("### Test failure: static field UnsafeOnHeapBooleanTest.static_v was not found");
+        }
+    }
+
+    public static void main(String args[]) {
+        System.out.println("### Test started");
+
+        if (args.length != 1) {
+            throw new RuntimeException("### Test failure: test called with incorrect number of arguments");
+        }
+
+        try {
+            for (int i = 0; i < Integer.parseInt(args[0]); i++) {
+                test();
+            }
+
+            // Check if the two 'true' boolean values were normalized
+            // (i.e., reduced from the range 1...255 to 1).
+            if (!bool0 || !bool1 || !result) {
+                System.out.println("Some of the results below are wrong");
+                System.out.println("bool0 is: " + bool0);
+                System.out.println("bool1 is: " + bool1);
+                System.out.println("bool0 & bool1 is: " + result);
+                System.out.println("===================================");
+                throw new RuntimeException("### Test failed");
+            } else {
+                System.out.println("Test generated correct results");
+                System.out.println("bool0 is: " + bool0);
+                System.out.println("bool1 is: " + bool1);
+                System.out.println("bool0 & bool1 is: " + result);
+                System.out.println("===================================");
+            }
+        } catch (NumberFormatException e) {
+            throw new RuntimeException("### Test failure: test called with incorrectly formatted parameter");
+        }
+
+        System.out.println("### Test passed");
+    }
+}

hotspot/test/compiler/unsafe/UnsafeSmallOffsetBooleanAccessTest.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2016, 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.
+ */
+
+/*
+ * @test
+ * @bug 8161720
+ * @modules java.base/jdk.internal.misc
+ * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -Xbatch -XX:-TieredCompilation UnsafeSmallOffsetBooleanAccessTest
+ * @run main/othervm -Xbatch UnsafeSmallOffsetBooleanAccessTest
+ */
+
+import java.util.Random;
+import jdk.internal.misc.Unsafe;
+
+public class UnsafeSmallOffsetBooleanAccessTest {
+    static final Unsafe UNSAFE = Unsafe.getUnsafe();
+    static final long F_OFFSET;
+    static final Random random = new Random();
+
+    static {
+        try {
+            F_OFFSET = UNSAFE.objectFieldOffset(T.class.getDeclaredField("f"));
+            System.out.println("The offset is: " + F_OFFSET);
+        } catch (Exception e) {
+            throw new Error(e);
+        }
+    }
+
+    static class T {
+        boolean f;
+    }
+
+    // Always return false in a way that is not obvious to the compiler.
+    public static boolean myRandom() {
+        if (random.nextInt(101) > 134) {
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    public static boolean test(T t) {
+        boolean result = false;
+        for (int i = 0; i < 20000; i++) {
+            boolean random = myRandom();
+            // If myRandom() returns false, access t.f.
+            //
+            // If myRandom() returns true, access virtual address
+            // F_OFFSET. That address is most likely not mapped,
+            // therefore the access will most likely cause a
+            // crash. We're not concerned about that, though, because
+            // myRandom() always returns false. However, the C2
+            // compiler avoids normalization of the value returned by
+            // getBoolean in this case.
+            result = UNSAFE.getBoolean(myRandom() ? null : t, F_OFFSET);
+        }
+        return result;
+    }
+
+    public static void main(String[] args) {
+        T t = new T();
+        UNSAFE.putBoolean(t, F_OFFSET, true);
+        System.out.println("The result for t is: " + test(t));
+    }
+}