changeset 53729:af83087aefcf lworld

[lworld] toString throws NPE on uninitialized default value containing null field
author mchung
date Mon, 14 Jan 2019 11:58:22 -0800
parents 2e0d2eda25c1
children 3148b1bc8aec
files src/hotspot/share/prims/unsafe.cpp src/java.base/share/classes/jdk/internal/misc/Unsafe.java test/jdk/valhalla/valuetypes/ObjectMethods.java test/jdk/valhalla/valuetypes/Value.java test/langtools/tools/javac/valhalla/lworld-values/CheckSeparateCompile.java
diffstat 5 files changed, 97 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/prims/unsafe.cpp	Mon Jan 14 11:55:25 2019 -0800
+++ b/src/hotspot/share/prims/unsafe.cpp	Mon Jan 14 11:58:22 2019 -0800
@@ -350,6 +350,13 @@
   return k->is_valueArray_klass();
 } UNSAFE_END
 
+UNSAFE_ENTRY(jobject, Unsafe_UninitializedDefaultValue(JNIEnv *env, jobject unsafe, jclass vc)) {
+  Klass* k = java_lang_Class::as_Klass(JNIHandles::resolve_non_null(vc));
+  ValueKlass* vk = ValueKlass::cast(k);
+  oop v = vk->default_value();
+  return JNIHandles::make_local(env, v);
+} UNSAFE_END
+
 UNSAFE_ENTRY(jobject, Unsafe_GetValue(JNIEnv *env, jobject unsafe, jobject obj, jlong offset, jclass vc)) {
   oop base = JNIHandles::resolve(obj);
   Klass* k = java_lang_Class::as_Klass(JNIHandles::resolve_non_null(vc));
@@ -1181,6 +1188,7 @@
     {CC "isFlattenedArray", CC "(" CLS ")Z",                     FN_PTR(Unsafe_IsFlattenedArray)},
     {CC "getValue",         CC "(" OBJ "J" CLS ")" OBJ,          FN_PTR(Unsafe_GetValue)},
     {CC "putValue",         CC "(" OBJ "J" CLS OBJ ")V",         FN_PTR(Unsafe_PutValue)},
+    {CC "uninitializedDefaultValue", CC "(" CLS ")" OBJ,         FN_PTR(Unsafe_UninitializedDefaultValue)},
     {CC "makePrivateBuffer",     CC "(" OBJ ")" OBJ,             FN_PTR(Unsafe_MakePrivateBuffer)},
     {CC "finishPrivateBuffer",   CC "(" OBJ ")" OBJ,             FN_PTR(Unsafe_FinishPrivateBuffer)},
     {CC "valueHeaderSize",       CC "(" CLS ")J",                FN_PTR(Unsafe_ValueHeaderSize)},
--- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java	Mon Jan 14 11:55:25 2019 -0800
+++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java	Mon Jan 14 11:58:22 2019 -0800
@@ -261,6 +261,38 @@
     public native <V> void putValue(Object o, long offset, Class<?> vc, V v);
 
     /**
+     * Fetches a reference value of type {@code vc} from a given Java variable.
+     * This method can return a reference to a value or a null reference
+     * for a boxed value type.
+     *
+     * @param vc value class
+     */
+    public Object getReference(Object o, long offset, Class<?> vc) {
+        Object ref = getReference(o, offset);
+        if (ref == null && isValueType(vc)) {
+            // If the type of the returned reference is a normal value type
+            // return an uninitialized default value if null
+            ref = uninitializedDefaultValue(vc);
+        }
+        return ref;
+    }
+
+    public Object getReferenceVolatile(Object o, long offset, Class<?> vc) {
+        Object ref = getReferenceVolatile(o, offset);
+        if (ref == null && isValueType(vc)) {
+            // If the type of the returned reference is a normal value type
+            // return an uninitialized default value if null
+            ref = uninitializedDefaultValue(vc);
+        }
+        return ref;
+    }
+
+    /**
+     * Returns an uninitialized default value of the given value class.
+     */
+    public native <V> V uninitializedDefaultValue(Class<?> vc);
+
+    /**
      * Returns an object instance with a private buffered value whose layout
      * and contents is exactly the given value instance.  The return object
      * is in the larval state that can be updated using the unsafe put operation.
--- a/test/jdk/valhalla/valuetypes/ObjectMethods.java	Mon Jan 14 11:55:25 2019 -0800
+++ b/test/jdk/valhalla/valuetypes/ObjectMethods.java	Mon Jan 14 11:58:22 2019 -0800
@@ -27,10 +27,14 @@
  * @summary test Object methods on value types
  * @compile -XDallowWithFieldOperator ObjectMethods.java
  * @run testng/othervm -XX:+EnableValhalla -Dvalue.bsm.salt=1 ObjectMethods
+ * @run testng/othervm -XX:+EnableValhalla -Dvalue.bsm.salt=1 -XX:ValueFieldMaxFlatSize=0 ObjectMethods
  */
 
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
+import java.util.stream.Stream;
 
 import org.testng.annotations.BeforeTest;
 import org.testng.annotations.DataProvider;
@@ -38,6 +42,7 @@
 import static org.testng.Assert.*;
 
 public class ObjectMethods {
+    static final int SALT = 1;
     static final Point P1 = Point.makePoint(1, 2);
     static final Point P2 = Point.makePoint(30, 40);
     static final Line LINE1 = Line.makeLine(1, 2, 3, 4);
@@ -77,6 +82,10 @@
             { MUTABLE_PATH, MutablePath.makePath(10, 20, 30, 40), false},
             { MIXED_VALUES, MIXED_VALUES, true},
             { MIXED_VALUES, new MixedValues(P1, LINE1, MUTABLE_PATH, "value"), false},
+            // uninitialized default value
+            { MyValue1.default, MyValue1.default, true},
+            { MyValue1.default, MyValue1.make(0,0, null), true},
+            { MyValue1.make(10, 20, P1), MyValue1.make(10, 20, Point.makePoint(1,2)), true},
         };
     }
 
@@ -104,13 +113,17 @@
                 .setReference(List.of("ref"))
                 .setNumber(new Value.IntNumber(99)).build(),
               "[Value char_v=\u0000 byte_v=0 boolean_v=false int_v=0 short_v=0 long_v=0 double_v=0.0 " +
-              "float_v=0.0 number_v=99 point_v=[Point x=0 y=0] ref_v=[ref]]" }
+              "float_v=0.0 number_v=99 point_v=[Point x=0 y=0] ref_v=[ref]]" },
+            // enclosing instance field `this$0` should be filtered
+            { MyValue1.default, "[ObjectMethods$MyValue1 p=[Point x=0 y=0] box=null]" },
+            { MyValue1.make(0,0, null), "[ObjectMethods$MyValue1 p=[Point x=0 y=0] box=null]" },
+            { MyValue1.make(0,0, P1), "[ObjectMethods$MyValue1 p=[Point x=0 y=0] box=[Point x=1 y=2]]" },
         };
     }
 
     @Test(dataProvider="toStringTests")
     public void testToString(Object o, String s) {
-        assertTrue(o.toString().equals(s));
+        assertTrue(o.toString().equals(s), o.toString());
     }
 
     @DataProvider(name="hashcodeTests")
@@ -126,25 +139,55 @@
                            .setPoint(Point.makePoint(200, 200))
                            .setNumber(Value.Number.intValue(10))
                            .setReference(new Object()).build();
-
+        // this is sensitive to the order of the returned fields from Class::getDeclaredFields
         return new Object[][]{
-            { P1,           hash(Point.class.asValueType(), 100, 200) },
-            { LINE1,        hash(Line.class.asValueType(), 1, 2, 3, 4) },
-            { v, hash(Value.class.asValueType(), v.char_v, v.boolean_v, v.byte_v, v.short_v,
-                      v.long_v, v.float_v, v.double_v, v.int_v, v.point_v, v.number_v, v.ref_v) }
+            { P1,                   hash(Point.class.asValueType(), 1, 2) },
+            { LINE1,                hash(Line.class.asValueType(), Point.makePoint(1, 2), Point.makePoint(3, 4)) },
+            { v,                    hash(hashCodeComponents(v))},
+            { Point.makePoint(0,0), hash(Point.class.asValueType(), 0, 0) },
+            { Point.default,        hash(Point.class.asValueType(), 0, 0) },
+            { MyValue1.default,     hash(MyValue1.class.asValueType(), Point.default, null) },
+            { MyValue1.make(0,0, null), hash(MyValue1.class.asValueType(), Point.makePoint(0,0), null) },
         };
     }
 
     @Test(dataProvider="hashcodeTests")
     public void testHashCode(Object o, int hash) {
-        assertTrue(o.hashCode() != hash);
+        assertEquals(o.hashCode(), hash);
+    }
+
+    private static Object[] hashCodeComponents(Object o) {
+        Class<?> type = o.getClass().asValueType();
+        // filter static fields and synthetic fields
+        Stream<Object> fields = Arrays.stream(type.getDeclaredFields())
+            .filter(f -> !Modifier.isStatic(f.getModifiers()) && !f.isSynthetic())
+            .map(f -> {
+                try {
+                    return f.get(o);
+                } catch (IllegalAccessException e) {
+                    throw new RuntimeException(e);
+                }
+            });
+        return Stream.concat(Stream.of(type), fields).toArray(Object[]::new);
     }
 
     private static int hash(Object... values) {
-        int hc = 1;
+        int hc = SALT;
         for (Object o : values) {
-            hc = 31 * hc + o.hashCode();
+            hc = 31 * hc + (o != null ? o.hashCode() : 0);
         }
         return hc;
     }
+
+    static value class MyValue1 {
+        Point p = Point.default;
+        Point.box box = Point.default;
+
+        static MyValue1 make(int x, int y, Point.box box) {
+            MyValue1 v = MyValue1.default;
+            v = __WithField(v.p, Point.makePoint(x, y));
+            v = __WithField(v.box, box);
+            return v;
+        }
+    }
 }
--- a/test/jdk/valhalla/valuetypes/Value.java	Mon Jan 14 11:55:25 2019 -0800
+++ b/test/jdk/valhalla/valuetypes/Value.java	Mon Jan 14 11:58:22 2019 -0800
@@ -36,15 +36,15 @@
 
     Value() {
         char_v = 'z';
+        byte_v = 0;
         boolean_v = true;
-        byte_v = 0;
         int_v = 1;
         short_v = 2;
         long_v = 3;
         float_v = 0.1f;
         double_v = 0.2d;
+        number_v = null;
         point_v = Point.makePoint(0,0);
-        number_v = null;
         ref_v = null;
     }
     static Value makeValue(char c, boolean z, byte b, int x, short y, long l, float f, double d, Number number, Point p, Object o) {
--- a/test/langtools/tools/javac/valhalla/lworld-values/CheckSeparateCompile.java	Mon Jan 14 11:55:25 2019 -0800
+++ b/test/langtools/tools/javac/valhalla/lworld-values/CheckSeparateCompile.java	Mon Jan 14 11:58:22 2019 -0800
@@ -32,7 +32,7 @@
 public class CheckSeparateCompile {
 	public static void main(String[] args) {
 		String s = new CheckSeparateCompile0().new O().new M().new I().foo();
-        if (!s.equals("[CheckSeparateCompile0$O$M$I i=0 this$2=o = 456 m = 789 x = 123]"))
+        if (!s.equals("[CheckSeparateCompile0$O$M$I i=0]"))
             throw new AssertionError(s);
 	}
-}
\ No newline at end of file
+}