8220282: Add MethodHandle tests on accessing final fields
authormchung
Sat, 06 Apr 2019 21:05:58 +0800
changeset 54445 a5da0277d9bb
parent 54444 259b40b4d473
child 54446 b16e8a886fc3
8220282: Add MethodHandle tests on accessing final fields Reviewed-by: lancea
test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java
test/jdk/java/lang/invoke/MethodHandlesTest.java
--- a/test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java	Fri Apr 05 15:57:33 2019 -0700
+++ b/test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java	Sat Apr 06 21:05:58 2019 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -635,7 +635,7 @@
 
     public void testGetter(int testMode) throws Throwable {
         Lookup lookup = PRIVATE;  // FIXME: test more lookups than this one
-        for (Object[] c : HasFields.CASES) {
+        for (Object[] c : HasFields.testCasesFor(testMode)) {
             boolean positive = (c[1] != Error.class);
             testGetter(positive, lookup, c[0], c[1], testMode);
             if (positive)
@@ -665,7 +665,6 @@
         boolean testNPE  = ((testMode0 & TEST_NPE) != 0);
         int testMode = testMode0 & ~(TEST_SETTER | TEST_BOUND | TEST_NPE);
         boolean positive = positive0 && !testNPE;
-        boolean isFinal;
         boolean isStatic;
         Class<?> fclass;
         String   fname;
@@ -673,14 +672,12 @@
         Field f = (fieldRef instanceof Field ? (Field)fieldRef : null);
         if (f != null) {
             isStatic = Modifier.isStatic(f.getModifiers());
-            isFinal  = Modifier.isFinal(f.getModifiers());
             fclass   = f.getDeclaringClass();
             fname    = f.getName();
             ftype    = f.getType();
         } else {
             Object[] scnt = (Object[]) fieldRef;
             isStatic = (Boolean)  scnt[0];
-            isFinal  = false;
             fclass   = (Class<?>) scnt[1];
             fname    = (String)   scnt[2];
             ftype    = (Class<?>) scnt[3];
@@ -724,19 +721,21 @@
                 ?   NoSuchFieldException.class
                 :   IllegalAccessException.class,
                 noAccess);
-            if (((testMode0 & TEST_SETTER) != 0) && (isFinal && isStatic)) return; // Final static field setter test failed as intended.
             if (verbosity >= 5)  ex.printStackTrace(System.out);
         }
         if (verbosity >= 3)
-            System.out.println((((testMode0 & TEST_UNREFLECT) != 0)?"unreflect":"find")
-                              +(((testMode0 & TEST_FIND_STATIC) != 0)?"Static":"")
-                              +(isGetter?"Getter":"Setter")
-                              +" "+fclass.getName()+"."+fname+"/"+ftype
-                              +" => "+mh
-                              +(noAccess == null ? "" : " !! "+noAccess));
+            System.out.format("%s%s %s.%s/%s => %s %s%n",
+                              (testMode0 & TEST_UNREFLECT) != 0
+                                  ? "unreflect"
+                                  : "find" + ((testMode0 & TEST_FIND_STATIC) != 0 ? "Static" : ""),
+                              (isGetter ? "Getter" : "Setter"),
+                              fclass.getName(), fname, ftype, mh,
+                              (noAccess == null ? "" : " !! "+noAccess));
+        // negative test case and expected noAccess, then done.
+        if (!positive && noAccess != null) return;
+        // positive test case but found noAccess, then error
         if (positive && !testNPE && noAccess != null)  throw new RuntimeException(noAccess);
         assertEquals(positive0 ? "positive test" : "negative test erroneously passed", positive0, mh != null);
-        assertFalse("Setter methods should throw an exception if passed a final static field.", ((testMode0 & TEST_SETTER) != 0) && (isFinal && isStatic));
         if (!positive && !testNPE)  return; // negative access test failed as expected
         assertEquals((isStatic ? 0 : 1)+(isGetter ? 0 : 1), mh.type().parameterCount());
         assertSame(mh.type(), expType);
@@ -762,6 +761,9 @@
             assertEquals(f.get(fields), value);  // clean to start with
         }
         Throwable caughtEx = null;
+        // non-final field and setAccessible(true) on instance field will have write access
+        boolean writeAccess = !Modifier.isFinal(f.getModifiers()) ||
+                              (!Modifier.isStatic(f.getModifiers()) && f.isAccessible());
         if (isGetter) {
             Object expValue = value;
             for (int i = 0; i <= 1; i++) {
@@ -785,7 +787,7 @@
                     }
                 }
                 assertEquals(sawValue, expValue);
-                if (f != null && f.getDeclaringClass() == HasFields.class && !isFinal) {
+                if (f != null && f.getDeclaringClass() == HasFields.class && writeAccess) {
                     Object random = randomArg(ftype);
                     f.set(fields, random);
                     expValue = random;
@@ -819,8 +821,8 @@
                 }
             }
         }
-        if ((f != null) && (f.getDeclaringClass() == HasFields.class) && !isFinal) {
-            f.set(fields, value);  // put it back if we changed it.
+        if (f != null && f.getDeclaringClass() == HasFields.class && writeAccess) {
+            f.set(fields, value);  // put it back if it has write access
         }
         if (testNPE) {
             if (caughtEx == null || !(caughtEx instanceof NullPointerException))
@@ -868,7 +870,8 @@
 
     public void testSetter(int testMode) throws Throwable {
         Lookup lookup = PRIVATE;  // FIXME: test more lookups than this one
-        for (Object[] c : HasFields.CASES) {
+        startTest("testSetter");
+        for (Object[] c : HasFields.testCasesFor(testMode|TEST_SETTER)) {
             boolean positive = (c[1] != Error.class);
             testSetter(positive, lookup, c[0], c[1], testMode);
             if (positive)
--- a/test/jdk/java/lang/invoke/MethodHandlesTest.java	Fri Apr 05 15:57:33 2019 -0700
+++ b/test/jdk/java/lang/invoke/MethodHandlesTest.java	Sat Apr 06 21:05:58 2019 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2019, 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
@@ -38,6 +38,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Stream;
 
 import static org.junit.Assert.*;
 
@@ -561,25 +562,37 @@
         static long sJ = 1+'J';
         static float sF = 1+'F';
         static double sD = 1+'D';
+
+        // final fields
+        final boolean fiZ = false;
+        final byte fiB = 2+(byte)'B';
+        final short fiS = 2+(short)'S';
+        final char fiC = 2+'C';
+        final int fiI = 2+'I';
+        final long fiJ = 2+'J';
+        final float fiF = 2+'F';
+        final double fiD = 2+'D';
         final static boolean fsZ = false;
-        final static byte fsB = 2+(byte)'B';
-        final static short fsS = 2+(short)'S';
-        final static char fsC = 2+'C';
-        final static int fsI = 2+'I';
-        final static long fsJ = 2+'J';
-        final static float fsF = 2+'F';
-        final static double fsD = 2+'D';
+        final static byte fsB = 3+(byte)'B';
+        final static short fsS = 3+(short)'S';
+        final static char fsC = 3+'C';
+        final static int fsI = 3+'I';
+        final static long fsJ = 3+'J';
+        final static float fsF = 3+'F';
+        final static double fsD = 3+'D';
 
         Object iL = 'L';
-        String iR = "R";
-        static Object sL = 'M';
-        static String sR = "S";
-        final static Object fsL = 'N';
-        final static String fsR = "T";
+        String iR = "iR";
+        static Object sL = 1+'L';
+        static String sR = "sR";
+        final Object fiL = 2+'L';
+        final String fiR = "fiR";
+        final static Object fsL = 3+'L';
+        final static String fsR = "fsR";
 
-        static final Object[][] CASES;
+        static final ArrayList<Object[]> STATIC_FIELD_CASES = new ArrayList<>();
+        static final ArrayList<Object[]> INSTANCE_FIELD_CASES = new ArrayList<>();
         static {
-            ArrayList<Object[]> cases = new ArrayList<>();
             Object types[][] = {
                 {'L',Object.class}, {'R',String.class},
                 {'I',int.class}, {'J',long.class},
@@ -589,42 +602,91 @@
             };
             HasFields fields = new HasFields();
             for (Object[] t : types) {
-                for (int kind = 0; kind <= 2; kind++) {
+                for (int kind = 0; kind <= 1; kind++) {
                     boolean isStatic = (kind != 0);
-                    boolean isFinal  = (kind == 2);
+                    ArrayList<Object[]> cases = isStatic ? STATIC_FIELD_CASES : INSTANCE_FIELD_CASES;
                     char btc = (Character)t[0];
-                    String name = (isStatic ? "s" : "i") + btc;
-                    if (isFinal) name = "f" + name;
+                    String fname = (isStatic ? "s" : "i") + btc;
+                    String finalFname = (isStatic ? "fs" : "fi") + btc;
                     Class<?> type = (Class<?>) t[1];
-                    Object value;
-                    Field field;
-                    try {
-                        field = HasFields.class.getDeclaredField(name);
-                    } catch (NoSuchFieldException | SecurityException ex) {
-                        throw new InternalError("no field HasFields."+name);
-                    }
-                    try {
-                        value = field.get(fields);
-                    } catch (IllegalArgumentException | IllegalAccessException ex) {
-                        throw new InternalError("cannot fetch field HasFields."+name);
-                    }
+                    // non-final field
+                    Field nonFinalField = getField(fname, type);
+                    Object value = getValue(fields, nonFinalField);
                     if (type == float.class) {
                         float v = 'F';
                         if (isStatic)  v++;
-                        if (isFinal)   v++;
                         assertTrue(value.equals(v));
                     }
-                    if (isFinal && isStatic) field.setAccessible(true);
-                    assertTrue(name.equals(field.getName()));
-                    assertTrue(type.equals(field.getType()));
-                    assertTrue(isStatic == (Modifier.isStatic(field.getModifiers())));
-                    assertTrue(isFinal  == (Modifier.isFinal(field.getModifiers())));
-                    cases.add(new Object[]{ field, value });
+                    assertTrue(isStatic == (Modifier.isStatic(nonFinalField.getModifiers())));
+                    cases.add(new Object[]{ nonFinalField, value });
+
+                    // setAccessible(true) on final field but static final field only has read access
+                    Field finalField = getField(finalFname, type);
+                    Object fvalue = getValue(fields, finalField);
+                    finalField.setAccessible(true);
+                    assertTrue(isStatic == (Modifier.isStatic(finalField.getModifiers())));
+                    cases.add(new Object[]{ finalField, fvalue, Error.class});
                 }
             }
-            cases.add(new Object[]{ new Object[]{ false, HasFields.class, "bogus_fD", double.class }, Error.class });
-            cases.add(new Object[]{ new Object[]{ true,  HasFields.class, "bogus_sL", Object.class }, Error.class });
-            CASES = cases.toArray(new Object[0][]);
+            INSTANCE_FIELD_CASES.add(new Object[]{ new Object[]{ false, HasFields.class, "bogus_fD", double.class }, Error.class });
+            STATIC_FIELD_CASES.add(new Object[]{ new Object[]{ true,  HasFields.class, "bogus_sL", Object.class }, Error.class });
+        }
+
+        private static Field getField(String name, Class<?> type) {
+            try {
+                Field field = HasFields.class.getDeclaredField(name);
+                assertTrue(name.equals(field.getName()));
+                assertTrue(type.equals(field.getType()));
+                return field;
+            } catch (NoSuchFieldException | SecurityException ex) {
+                throw new InternalError("no field HasFields."+name);
+            }
+        }
+
+        private static Object getValue(Object o, Field field) {
+            try {
+                return field.get(o);
+            } catch (IllegalArgumentException | IllegalAccessException ex) {
+                throw new InternalError("cannot fetch field HasFields."+field.getName());
+            }
+        }
+
+        static Object[][] testCasesFor(int testMode) {
+            Stream<Object[]> cases;
+            if ((testMode & TEST_UNREFLECT) != 0) {
+                cases = Stream.concat(STATIC_FIELD_CASES.stream(), INSTANCE_FIELD_CASES.stream());
+            } else if ((testMode & TEST_FIND_STATIC) != 0) {
+                cases = STATIC_FIELD_CASES.stream();
+            } else if ((testMode & TEST_FIND_FIELD) != 0) {
+                cases = INSTANCE_FIELD_CASES.stream();
+            } else {
+                throw new InternalError("unexpected test mode: " + testMode);
+            }
+            return cases.map(c -> mapTestCase(testMode, c)).toArray(Object[][]::new);
+
+        }
+
+        private static Object[] mapTestCase(int testMode, Object[] c) {
+            // non-final fields (2-element) and final fields (3-element) if not TEST_SETTER
+            if (c.length == 2 || (testMode & TEST_SETTER) == 0)
+                return c;
+
+            // final fields (3-element)
+            assertTrue((testMode & TEST_SETTER) != 0 && c[0] instanceof Field && c[2] == Error.class);
+            if ((testMode & TEST_UNREFLECT) == 0)
+                return new Object[]{ c[0], c[2]};   // negative test case; can't set on final fields
+
+            // unreflectSetter grants write access on instance final field if accessible flag is true
+            // hence promote the negative test case to positive test case
+            Field f = (Field) c[0];
+            int mods = f.getModifiers();
+            if (!Modifier.isFinal(mods) || (!Modifier.isStatic(mods) && f.isAccessible())) {
+                // positive test case
+                return new Object[]{ c[0], c[1] };
+            } else {
+                // otherwise, negative test case
+                return new Object[]{ c[0], c[2]};
+            }
         }
     }