--- old/test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java 2019-03-07 15:05:29.000000000 +0000 +++ new/test/jdk/java/lang/invoke/MethodHandlesGeneralTest.java 2019-03-07 15:05:28.000000000 +0000 @@ -634,7 +634,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.cases(testMode)) { boolean positive = (c[1] != Error.class); testGetter(positive, lookup, c[0], c[1], testMode); if (positive) @@ -723,15 +723,19 @@ if (verbosity >= 5) ex.printStackTrace(System.out); } if (verbosity >= 3) - System.out.println("find"+(isStatic?"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)); + if (!positive && noAccess != null) return; if (positive && !testNPE && noAccess != null) throw new RuntimeException(noAccess); assertEquals(positive0 ? "positive test" : "negative test erroneously passed", positive0, mh != null); if (!positive && !testNPE) return; // negative access test failed as expected assertEquals((isStatic ? 0 : 1)+(isGetter ? 0 : 1), mh.type().parameterCount()); - assertSame(mh.type(), expType); //assertNameStringContains(mh, fname); // This does not hold anymore with LFs HasFields fields = new HasFields(); @@ -755,6 +759,8 @@ assertEquals(f.get(fields), value); // clean to start with } Throwable caughtEx = null; + boolean writeAccess = !Modifier.isFinal(f.getModifiers()) || + (!Modifier.isStatic(f.getModifiers()) && f.isAccessible()); if (isGetter) { Object expValue = value; for (int i = 0; i <= 1; i++) { @@ -778,8 +784,7 @@ } } assertEquals(sawValue, expValue); - if (f != null && f.getDeclaringClass() == HasFields.class - && !Modifier.isFinal(f.getModifiers())) { + if (f != null && f.getDeclaringClass() == HasFields.class && writeAccess) { Object random = randomArg(ftype); f.set(fields, random); expValue = random; @@ -813,7 +818,7 @@ } } } - if (f != null && f.getDeclaringClass() == HasFields.class) { + if (f != null && f.getDeclaringClass() == HasFields.class && writeAccess) { f.set(fields, value); // put it back } if (testNPE) { @@ -862,9 +867,17 @@ public void testSetter(int testMode) throws Throwable { Lookup lookup = PRIVATE; // FIXME: test more lookups than this one - startTest("unreflectSetter"); - for (Object[] c : HasFields.CASES) { - boolean positive = (c[1] != Error.class); + startTest("testSetter"); + for (Object[] c : HasFields.cases(testMode)) { + boolean positive = (c[1] != Error.class) && !(c.length == 3 && c[2] == Error.class); + if ((testMode & TEST_UNREFLECT) != 0 && c.length == 3) { + assertTrue(c[0] instanceof Field && c[2] == Error.class); + Field f = (Field)c[0]; + int mods = f.getModifiers(); + // unreflectSetter should only have write access on instance final field if accessible flag is true + positive = !Modifier.isFinal(mods) || + (!Modifier.isStatic(mods) && f.isAccessible()); + } testSetter(positive, lookup, c[0], c[1], testMode); if (positive) testSetter(positive, lookup, c[0], c[1], testMode | TEST_NPE); --- old/test/jdk/java/lang/invoke/MethodHandlesTest.java 2019-03-07 15:05:30.000000000 +0000 +++ new/test/jdk/java/lang/invoke/MethodHandlesTest.java 2019-03-07 15:05:30.000000000 +0000 @@ -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.*; @@ -567,9 +568,32 @@ static Object sL = 'M'; static String sR = "S"; - static final Object[][] CASES; + // final fields + final boolean final_fZ = false; + final byte final_fB = (byte)'B'; + final short final_fS = (short)'S'; + final char final_fC = 'C'; + final int final_fI = 'I'; + final long final_fJ = 'J'; + final float final_fF = 'F'; + final double final_fD = 'D'; + final static boolean final_sZ = true; + final static byte final_sB = 1+(byte)'B'; + final static short final_sS = 1+(short)'S'; + final static char final_sC = 1+'C'; + final static int final_sI = 1+'I'; + final static long final_sJ = 1+'J'; + final static float final_sF = 1+'F'; + final static double final_sD = 1+'D'; + + final Object final_fL = 'L'; + final String final_fR = "R"; + final static Object final_sL = 'M'; + final static String final_sR = "S"; + + static final ArrayList STATIC_FIELD_CASES = new ArrayList<>(); + static final ArrayList INSTANCE_FIELD_CASES = new ArrayList<>(); static { - ArrayList cases = new ArrayList<>(); Object types[][] = { {'L',Object.class}, {'R',String.class}, {'I',int.class}, {'J',long.class}, @@ -581,35 +605,61 @@ for (Object[] t : types) { for (int kind = 0; kind <= 1; kind++) { boolean isStatic = (kind != 0); + ArrayList cases = isStatic ? STATIC_FIELD_CASES : INSTANCE_FIELD_CASES; char btc = (Character)t[0]; - String name = (isStatic ? "s" : "f") + btc; + String fname = (isStatic ? "s" : "f") + btc; + String finalFname = (isStatic ? "final_s" : "final_f") + 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++; assertTrue(value.equals(v)); } - assertTrue(name.equals(field.getName())); - assertTrue(type.equals(field.getType())); - assertTrue(isStatic == (Modifier.isStatic(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); + finalField.setAccessible(true); + assertTrue(isStatic == (Modifier.isStatic(finalField.getModifiers()))); + cases.add(new Object[]{ finalField, value, 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 }); + } + + 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); + } + } + + 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[][] cases(int testMode) { + if ((testMode & TEST_UNREFLECT) != 0) { + return Stream.concat(STATIC_FIELD_CASES.stream(), INSTANCE_FIELD_CASES.stream()) + .toArray(Object[][]::new); + } else if ((testMode & TEST_FIND_STATIC) != 0) { + return STATIC_FIELD_CASES.stream().toArray(Object[][]::new); + } else if ((testMode & TEST_FIND_FIELD) != 0) { + return INSTANCE_FIELD_CASES.stream().toArray(Object[][]::new); + } + throw new InternalError("unexpected test mode: " + testMode); } }