Mercurial > hg > icedtea9-forest > jdk
changeset 5874:beeb1d5ecd9e
7129034: VM crash with a field setter method with a filterArguments
Summary: add null checks before unsafe calls that take a variable base reference; update unit tests
Reviewed-by: kvn, twisti
| author | jrose |
|---|---|
| date | Thu, 12 Jul 2012 00:11:35 -0700 |
| parents | 05e5ce861a58 |
| children | 556141c6326c |
| files | src/share/classes/java/lang/invoke/MethodHandleImpl.java test/java/lang/invoke/MethodHandlesTest.java |
| diffstat | 2 files changed, 178 insertions(+), 65 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/java/lang/invoke/MethodHandleImpl.java Thu Jul 12 00:10:53 2012 -0700 +++ b/src/share/classes/java/lang/invoke/MethodHandleImpl.java Thu Jul 12 00:11:35 2012 -0700 @@ -190,32 +190,36 @@ @Override String debugString() { return addTypeString(name, this); } - int getFieldI(Object /*C*/ obj) { return unsafe.getInt(obj, offset); } - void setFieldI(Object /*C*/ obj, int x) { unsafe.putInt(obj, offset, x); } - long getFieldJ(Object /*C*/ obj) { return unsafe.getLong(obj, offset); } - void setFieldJ(Object /*C*/ obj, long x) { unsafe.putLong(obj, offset, x); } - float getFieldF(Object /*C*/ obj) { return unsafe.getFloat(obj, offset); } - void setFieldF(Object /*C*/ obj, float x) { unsafe.putFloat(obj, offset, x); } - double getFieldD(Object /*C*/ obj) { return unsafe.getDouble(obj, offset); } - void setFieldD(Object /*C*/ obj, double x) { unsafe.putDouble(obj, offset, x); } - boolean getFieldZ(Object /*C*/ obj) { return unsafe.getBoolean(obj, offset); } - void setFieldZ(Object /*C*/ obj, boolean x) { unsafe.putBoolean(obj, offset, x); } - byte getFieldB(Object /*C*/ obj) { return unsafe.getByte(obj, offset); } - void setFieldB(Object /*C*/ obj, byte x) { unsafe.putByte(obj, offset, x); } - short getFieldS(Object /*C*/ obj) { return unsafe.getShort(obj, offset); } - void setFieldS(Object /*C*/ obj, short x) { unsafe.putShort(obj, offset, x); } - char getFieldC(Object /*C*/ obj) { return unsafe.getChar(obj, offset); } - void setFieldC(Object /*C*/ obj, char x) { unsafe.putChar(obj, offset, x); } - Object /*V*/ getFieldL(Object /*C*/ obj) { return unsafe.getObject(obj, offset); } - void setFieldL(Object /*C*/ obj, Object /*V*/ x) { unsafe.putObject(obj, offset, x); } - // cast (V) is OK here, since we wrap convertArguments around the MH. + private static Object nullCheck(Object obj) { + obj.getClass(); // NPE + return obj; + } + + int getFieldI(Object /*C*/ obj) { return unsafe.getInt(nullCheck(obj), offset); } + void setFieldI(Object /*C*/ obj, int x) { unsafe.putInt(nullCheck(obj), offset, x); } + long getFieldJ(Object /*C*/ obj) { return unsafe.getLong(nullCheck(obj), offset); } + void setFieldJ(Object /*C*/ obj, long x) { unsafe.putLong(nullCheck(obj), offset, x); } + float getFieldF(Object /*C*/ obj) { return unsafe.getFloat(nullCheck(obj), offset); } + void setFieldF(Object /*C*/ obj, float x) { unsafe.putFloat(nullCheck(obj), offset, x); } + double getFieldD(Object /*C*/ obj) { return unsafe.getDouble(nullCheck(obj), offset); } + void setFieldD(Object /*C*/ obj, double x) { unsafe.putDouble(nullCheck(obj), offset, x); } + boolean getFieldZ(Object /*C*/ obj) { return unsafe.getBoolean(nullCheck(obj), offset); } + void setFieldZ(Object /*C*/ obj, boolean x) { unsafe.putBoolean(nullCheck(obj), offset, x); } + byte getFieldB(Object /*C*/ obj) { return unsafe.getByte(nullCheck(obj), offset); } + void setFieldB(Object /*C*/ obj, byte x) { unsafe.putByte(nullCheck(obj), offset, x); } + short getFieldS(Object /*C*/ obj) { return unsafe.getShort(nullCheck(obj), offset); } + void setFieldS(Object /*C*/ obj, short x) { unsafe.putShort(nullCheck(obj), offset, x); } + char getFieldC(Object /*C*/ obj) { return unsafe.getChar(nullCheck(obj), offset); } + void setFieldC(Object /*C*/ obj, char x) { unsafe.putChar(nullCheck(obj), offset, x); } + Object /*V*/ getFieldL(Object /*C*/ obj) { return unsafe.getObject(nullCheck(obj), offset); } + void setFieldL(Object /*C*/ obj, Object /*V*/ x) { unsafe.putObject(nullCheck(obj), offset, x); } static Object staticBase(final MemberName field) { if (!field.isStatic()) return null; return AccessController.doPrivileged(new PrivilegedAction<Object>() { public Object run() { try { - Class c = field.getDeclaringClass(); + Class<?> c = field.getDeclaringClass(); // FIXME: Should not have to create 'f' to get this value. java.lang.reflect.Field f = c.getDeclaredField(field.getName()); return unsafe.staticFieldBase(f); @@ -242,7 +246,6 @@ void setStaticS(short x) { unsafe.putShort(base, offset, x); } char getStaticC() { return unsafe.getChar(base, offset); } void setStaticC(char x) { unsafe.putChar(base, offset, x); } - @SuppressWarnings("unchecked") // (V) is for internal clarity but triggers warning Object /*V*/ getStaticL() { return unsafe.getObject(base, offset); } void setStaticL(Object /*V*/ x) { unsafe.putObject(base, offset, x); }
--- a/test/java/lang/invoke/MethodHandlesTest.java Thu Jul 12 00:10:53 2012 -0700 +++ b/test/java/lang/invoke/MethodHandlesTest.java Thu Jul 12 00:11:35 2012 -0700 @@ -280,6 +280,8 @@ { param = c; break; } } } + if (param.isInterface() && param.isAssignableFrom(List.class)) + return Arrays.asList("#"+nextArg()); if (param.isInterface() || param.isAssignableFrom(String.class)) return "#"+nextArg(); else @@ -457,6 +459,7 @@ @Override public String toString() { return name; } } } + static interface SubIntExample extends IntExample { } static final Object[][][] ACCESS_CASES = { { { false, PUBLIC }, { false, PACKAGE }, { false, PRIVATE }, { false, EXAMPLE } }, //[0]: all false @@ -960,7 +963,7 @@ } } - static final int TEST_UNREFLECT = 1, TEST_FIND_FIELD = 2, TEST_FIND_STATIC = 3, TEST_SETTER = 0x10; + static final int TEST_UNREFLECT = 1, TEST_FIND_FIELD = 2, TEST_FIND_STATIC = 3, TEST_SETTER = 0x10, TEST_NPE = 0x20; static boolean testModeMatches(int testMode, boolean isStatic) { switch (testMode) { case TEST_FIND_STATIC: return isStatic; @@ -990,6 +993,8 @@ for (Object[] c : HasFields.CASES) { boolean positive = (c[1] != Error.class); testGetter(positive, lookup, c[0], c[1], testMode); + if (positive) + testGetter(positive, lookup, c[0], c[1], testMode | TEST_NPE); } testGetter(true, lookup, new Object[]{ true, System.class, "out", java.io.PrintStream.class }, @@ -1005,12 +1010,14 @@ testAccessor(positive, lookup, fieldRef, value, testMode); } - public void testAccessor(boolean positive, MethodHandles.Lookup lookup, + public void testAccessor(boolean positive0, MethodHandles.Lookup lookup, Object fieldRef, Object value, int testMode0) throws Throwable { if (verbosity >= 4) - System.out.println("testAccessor"+Arrays.asList(positive, lookup, fieldRef, value, testMode0)); + System.out.println("testAccessor"+Arrays.deepToString(new Object[]{positive0, lookup, fieldRef, value, testMode0})); boolean isGetter = ((testMode0 & TEST_SETTER) == 0); - int testMode = testMode0 & ~TEST_SETTER; + boolean testNPE = ((testMode0 & TEST_NPE) != 0); + int testMode = testMode0 & ~(TEST_SETTER | TEST_NPE); + boolean positive = positive0 && !testNPE; boolean isStatic; Class<?> fclass; String fname; @@ -1035,6 +1042,7 @@ } if (!testModeMatches(testMode, isStatic)) return; if (f == null && testMode == TEST_UNREFLECT) return; + if (testNPE && isStatic) return; countTest(positive); MethodType expType; if (isGetter) @@ -1045,7 +1053,7 @@ Exception noAccess = null; MethodHandle mh; try { - switch (testMode0) { + switch (testMode0 & ~TEST_NPE) { case TEST_UNREFLECT: mh = lookup.unreflectGetter(f); break; case TEST_FIND_FIELD: mh = lookup.findGetter(fclass, fname, ftype); break; case TEST_FIND_STATIC: mh = lookup.findStaticGetter(fclass, fname, ftype); break; @@ -1070,15 +1078,17 @@ System.out.println("find"+(isStatic?"Static":"")+(isGetter?"Getter":"Setter")+" "+fclass.getName()+"."+fname+"/"+ftype +" => "+mh +(noAccess == null ? "" : " !! "+noAccess)); - if (positive && noAccess != null) throw new RuntimeException(noAccess); - assertEquals(positive ? "positive test" : "negative test erroneously passed", positive, mh != null); - if (!positive) return; // negative test failed as expected + 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); HasFields fields = new HasFields(); + HasFields fieldsForMH = fields; + if (testNPE) fieldsForMH = null; // perturb MH argument to elicit expected error Object sawValue; Class<?> vtype = ftype; if (ftype != int.class) vtype = Object.class; @@ -1094,6 +1104,7 @@ if (f != null && f.getDeclaringClass() == HasFields.class) { assertEquals(f.get(fields), value); // clean to start with } + Throwable caughtEx = null; if (isGetter) { Object expValue = value; for (int i = 0; i <= 1; i++) { @@ -1103,10 +1114,18 @@ else sawValue = mh.invokeExact(); } else { - if (ftype == int.class) - sawValue = (int) mh.invokeExact((Object) fields); - else - sawValue = mh.invokeExact((Object) fields); + sawValue = null; // make DA rules happy under try/catch + try { + if (ftype == int.class) + sawValue = (int) mh.invokeExact((Object) fieldsForMH); + else + sawValue = mh.invokeExact((Object) fieldsForMH); + } catch (RuntimeException ex) { + if (ex instanceof NullPointerException && testNPE) { + caughtEx = ex; + break; + } + } } assertEquals(sawValue, expValue); if (f != null && f.getDeclaringClass() == HasFields.class @@ -1127,10 +1146,17 @@ else mh.invokeExact(putValue); } else { - if (ftype == int.class) - mh.invokeExact((Object) fields, (int)putValue); - else - mh.invokeExact((Object) fields, putValue); + try { + if (ftype == int.class) + mh.invokeExact((Object) fieldsForMH, (int)putValue); + else + mh.invokeExact((Object) fieldsForMH, putValue); + } catch (RuntimeException ex) { + if (ex instanceof NullPointerException && testNPE) { + caughtEx = ex; + break; + } + } } if (f != null && f.getDeclaringClass() == HasFields.class) { assertEquals(f.get(fields), putValue); @@ -1140,6 +1166,14 @@ if (f != null && f.getDeclaringClass() == HasFields.class) { f.set(fields, value); // put it back } + if (testNPE) { + if (caughtEx == null || !(caughtEx instanceof NullPointerException)) + throw new RuntimeException("failed to catch NPE exception"+(caughtEx == null ? " (caughtEx=null)" : ""), caughtEx); + caughtEx = null; // nullify expected exception + } + if (caughtEx != null) { + throw new RuntimeException("unexpected exception", caughtEx); + } } @@ -1164,6 +1198,8 @@ for (Object[] c : HasFields.CASES) { boolean positive = (c[1] != Error.class); testSetter(positive, lookup, c[0], c[1], testMode); + if (positive) + testSetter(positive, lookup, c[0], c[1], testMode | TEST_NPE); } for (int isStaticN = 0; isStaticN <= 1; isStaticN++) { testSetter(false, lookup, @@ -1188,24 +1224,71 @@ testArrayElementGetterSetter(true); } + private static final int TEST_ARRAY_NONE = 0, TEST_ARRAY_NPE = 1, TEST_ARRAY_OOB = 2, TEST_ARRAY_ASE = 3; + public void testArrayElementGetterSetter(boolean testSetter) throws Throwable { - testArrayElementGetterSetter(new Object[10], testSetter); - testArrayElementGetterSetter(new String[10], testSetter); - testArrayElementGetterSetter(new boolean[10], testSetter); - testArrayElementGetterSetter(new byte[10], testSetter); - testArrayElementGetterSetter(new char[10], testSetter); - testArrayElementGetterSetter(new short[10], testSetter); - testArrayElementGetterSetter(new int[10], testSetter); - testArrayElementGetterSetter(new float[10], testSetter); - testArrayElementGetterSetter(new long[10], testSetter); - testArrayElementGetterSetter(new double[10], testSetter); + testArrayElementGetterSetter(testSetter, TEST_ARRAY_NONE); + } + + @Test + public void testArrayElementErrors() throws Throwable { + startTest("arrayElementErrors"); + testArrayElementGetterSetter(false, TEST_ARRAY_NPE); + testArrayElementGetterSetter(true, TEST_ARRAY_NPE); + testArrayElementGetterSetter(false, TEST_ARRAY_OOB); + testArrayElementGetterSetter(true, TEST_ARRAY_OOB); + testArrayElementGetterSetter(new Object[10], true, TEST_ARRAY_ASE); + testArrayElementGetterSetter(new Example[10], true, TEST_ARRAY_ASE); + testArrayElementGetterSetter(new IntExample[10], true, TEST_ARRAY_ASE); } - public void testArrayElementGetterSetter(Object array, boolean testSetter) throws Throwable { - countTest(true); - if (verbosity > 2) System.out.println("array type = "+array.getClass().getComponentType().getName()+"["+Array.getLength(array)+"]"); + public void testArrayElementGetterSetter(boolean testSetter, int negTest) throws Throwable { + testArrayElementGetterSetter(new String[10], testSetter, negTest); + testArrayElementGetterSetter(new Iterable<?>[10], testSetter, negTest); + testArrayElementGetterSetter(new Example[10], testSetter, negTest); + testArrayElementGetterSetter(new IntExample[10], testSetter, negTest); + testArrayElementGetterSetter(new Object[10], testSetter, negTest); + testArrayElementGetterSetter(new boolean[10], testSetter, negTest); + testArrayElementGetterSetter(new byte[10], testSetter, negTest); + testArrayElementGetterSetter(new char[10], testSetter, negTest); + testArrayElementGetterSetter(new short[10], testSetter, negTest); + testArrayElementGetterSetter(new int[10], testSetter, negTest); + testArrayElementGetterSetter(new float[10], testSetter, negTest); + testArrayElementGetterSetter(new long[10], testSetter, negTest); + testArrayElementGetterSetter(new double[10], testSetter, negTest); + } + + public void testArrayElementGetterSetter(Object array, boolean testSetter, int negTest) throws Throwable { + boolean positive = (negTest == TEST_ARRAY_NONE); + int length = java.lang.reflect.Array.getLength(array); Class<?> arrayType = array.getClass(); Class<?> elemType = arrayType.getComponentType(); + Object arrayToMH = array; + // this stanza allows negative tests to make argument perturbations: + switch (negTest) { + case TEST_ARRAY_NPE: + arrayToMH = null; + break; + case TEST_ARRAY_OOB: + assert(length > 0); + arrayToMH = java.lang.reflect.Array.newInstance(elemType, 0); + break; + case TEST_ARRAY_ASE: + assert(testSetter && !elemType.isPrimitive()); + if (elemType == Object.class) + arrayToMH = new StringBuffer[length]; // very random subclass of Object! + else if (elemType == Example.class) + arrayToMH = new SubExample[length]; + else if (elemType == IntExample.class) + arrayToMH = new SubIntExample[length]; + else + return; // can't make an ArrayStoreException test + assert(arrayType.isInstance(arrayToMH)) + : Arrays.asList(arrayType, arrayToMH.getClass(), testSetter, negTest); + break; + } + countTest(positive); + if (verbosity > 2) System.out.println("array type = "+array.getClass().getComponentType().getName()+"["+length+"]"+(positive ? "" : " negative test #"+negTest+" using "+Arrays.deepToString(new Object[]{arrayToMH}))); MethodType expType = !testSetter ? MethodType.methodType(elemType, arrayType, int.class) : MethodType.methodType(void.class, arrayType, int.class, elemType); @@ -1214,25 +1297,29 @@ : MethodHandles.arrayElementSetter(arrayType); assertSame(mh.type(), expType); if (elemType != int.class && elemType != boolean.class) { - // FIXME: change Integer.class and (Integer) below to int.class and (int) below. - MethodType gtype = mh.type().generic().changeParameterType(1, Integer.class); + MethodType gtype = mh.type().generic().changeParameterType(1, int.class); if (testSetter) gtype = gtype.changeReturnType(void.class); mh = mh.asType(gtype); } Object sawValue, expValue; List<Object> model = array2list(array); - int length = Array.getLength(array); + Throwable caughtEx = null; for (int i = 0; i < length; i++) { // update array element Object random = randomArg(elemType); model.set(i, random); if (testSetter) { - if (elemType == int.class) - mh.invokeExact((int[]) array, i, (int)random); - else if (elemType == boolean.class) - mh.invokeExact((boolean[]) array, i, (boolean)random); - else - mh.invokeExact(array, (Integer)i, random); + try { + if (elemType == int.class) + mh.invokeExact((int[]) arrayToMH, i, (int)random); + else if (elemType == boolean.class) + mh.invokeExact((boolean[]) arrayToMH, i, (boolean)random); + else + mh.invokeExact(arrayToMH, i, random); + } catch (RuntimeException ex) { + caughtEx = ex; + break; + } assertEquals(model, array2list(array)); } else { Array.set(array, i, random); @@ -1247,16 +1334,39 @@ sawValue = Array.get(array, i); if (!testSetter) { expValue = sawValue; - if (elemType == int.class) - sawValue = (int) mh.invokeExact((int[]) array, i); - else if (elemType == boolean.class) - sawValue = (boolean) mh.invokeExact((boolean[]) array, i); - else - sawValue = mh.invokeExact(array, (Integer)i); + try { + if (elemType == int.class) + sawValue = (int) mh.invokeExact((int[]) arrayToMH, i); + else if (elemType == boolean.class) + sawValue = (boolean) mh.invokeExact((boolean[]) arrayToMH, i); + else + sawValue = mh.invokeExact(arrayToMH, i); + } catch (RuntimeException ex) { + caughtEx = ex; + break; + } assertEquals(sawValue, expValue); assertEquals(model, array2list(array)); } } + if (!positive) { + if (caughtEx == null) + throw new RuntimeException("failed to catch exception for negTest="+negTest); + // test the kind of exception + Class<?> reqType = null; + switch (negTest) { + case TEST_ARRAY_ASE: reqType = ArrayStoreException.class; break; + case TEST_ARRAY_OOB: reqType = ArrayIndexOutOfBoundsException.class; break; + case TEST_ARRAY_NPE: reqType = NullPointerException.class; break; + default: assert(false); + } + if (reqType.isInstance(caughtEx)) { + caughtEx = null; // nullify expected exception + } + } + if (caughtEx != null) { + throw new RuntimeException("unexpected exception", caughtEx); + } } List<Object> array2list(Object array) {