Throw IllegalAccessError on resolution with inaccessible types #2675
Conversation
pdbain-ibm
force-pushed
the
methodhandle
branch
2 times, most recently
from
fc656ef to
dbad1c2
Compare
| @@ -2503,6 +2510,8 @@ public static MethodHandle varHandleInvoker(VarHandle.AccessMode accessMode, Met | |||
| public static MethodHandle throwException(Class<?> returnType, Class<? extends Throwable> exception) { | |||
| MethodType realType = MethodType.methodType(returnType, exception); | |||
| MethodHandle handle; | |||
| System.out.println("PDB_DEBUG throwException returnType="+returnType | |||
| Map<CacheKey, WeakReference<MethodHandle>> cache = HandleCache.getStaticCache(clazz); | ||
| MethodHandle handle = HandleCache.getMethodFromPerClassCache(cache, methodName, type); | ||
| if (handle == null) { | ||
| initCheck(methodName); | ||
| handle = new DirectHandle(clazz, methodName, type, MethodHandle.KIND_STATIC, null); | ||
| handle = HandleCache.putMethodInPerClassCache(cache, methodName, type, convertToVarargsIfRequired(handle)); | ||
| } | ||
| checkAccess(handle, false); | ||
| checkAccess(handle, true); |
There was a problem hiding this comment.
Pls explain the reason that skipAccessCheckPara need to be changed to true.
There was a problem hiding this comment.
This doesn't seem like the right approach as it applies access checking to the MethodType inconsistently.
The MT being passed in here may have been created somewhere else that did have access to the all the types in that case, the lookup should fail with NoSuchMethodError or similar.
The place to change is the callers of MethodType.vmResolveFromMethodDescriptorString() to do the additional access checking. Most callers - ie: the resolvesupport.cpp code which calls this through sendFromMethodDescriptorString - already do the access checking in native code. There are just 1, mayne 2, places in java code that need to be updated.
pdbain-ibm
force-pushed
the
methodhandle
branch
2 times, most recently
from
e42d49f to
117e994
Compare
|
@DanHeidinga I reworked per |
| /* Can never happen */ | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
| if (!Objects.nonNull(type)) { |
There was a problem hiding this comment.
type can never be null here.
There was a problem hiding this comment.
Similar concern here - the access check should precede the MH lookup
There was a problem hiding this comment.
type can never be null here.: type is initialized to null and cases 1-4 do not set it.
There was a problem hiding this comment.
the access check should precede the MH lookup: moving the checks into the various cases.
| @@ -855,8 +855,16 @@ private static final MethodHandle resolveInvokeDynamic(long j9class, String name | |||
| throw new NullPointerException(Msg.getString("K05cd", classObject.toString(), bsmIndex)); //$NON-NLS-1$ | |||
| } | |||
| Object[] staticArgs = new Object[BSM_OPTIONAL_ARGUMENTS_START_INDEX + bsmArgCount]; | |||
| final MethodHandles.Lookup lookup = new MethodHandles.Lookup(classObject, false); | |||
There was a problem hiding this comment.
The access check should be moved to immediately after the vmResolveFromMethodDescriptorString call so that access exceptions occur before any exceptions getting the BSM
pdbain-ibm
force-pushed
the
methodhandle
branch
from
117e994 to
b45a137
Compare
|
Updated and commits squashed. |
| switch(cpRefKind){ | ||
| case 1: /* getField */ | ||
| return lookup.findGetter(referenceClazz, name, resolveFieldHandleHelper(typeDescriptor, loader)); | ||
| result = lookup.findGetter(referenceClazz, name, resolveFieldHandleHelper(typeDescriptor, loader)); |
There was a problem hiding this comment.
resolveFieldHandleHelper(typeDescriptor, loader) will have the same issue as vmResolveFromMethodDescriptorString and will also need the accessCheck.
There was a problem hiding this comment.
Done. resolveFieldHandleHelper() changed to take a Lookup object to do the test.
private static final Class<?> resolveFieldHandleHelper(String typeDescriptor, Lookup lookup, ClassLoader loader) throws Throwable {
MethodType mt = MethodType.vmResolveFromMethodDescriptorString()...
lookup.accessCheckArgRetTypes(mt);
return mt.parameterType(0);
… inaccessible types Signed-off-by: Peter Bain <peter_bain@ca.ibm.com>
pdbain-ibm
force-pushed
the
methodhandle
branch
from
b45a137 to
be44482
Compare
pdbain-ibm
changed the title
|
@DanHeidinga do you have further comments on this pull request? |
|
Jenkins test sanity xlinux jdk10 |
Make OpenJ9 behaviour compliant to the reference implementation.