Skip to content

Commit

Permalink
Properly handle resolve-time invokeDynamic errors for OJDK MHs
Browse files Browse the repository at this point in the history 
This patch fixes #14990.
For OJDK MHs invokeDynamic resolve-time errors need to be wrapped in
BootstrapMethodErrors and thrown during invoke-time. However, prior
to Java 11, if a CallSite initially binds to null the error needs
to be thrown during resolution to ensure that resolution is reattempted
by the interpreter.

This patch also updates the IndyTest.java test case to conform to the
spec for OJDK MHs.

Issues: #14990
Signed-off-by: Nathan Henderson <[email protected]>
  • Loading branch information
@ThanHenderson
ThanHenderson committed Nov 21, 2023
1 parent df46709 commit d60b284
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 64 deletions.
50 changes: 31 additions & 19 deletions jcl/src/java.base/share/classes/java/lang/invoke/MethodHandleResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ private static final Object resolveInvokeDynamic(long j9class, String name, Stri
/*[ELSE] JAVA_SPEC_VERSION >= 11*/
try {
type = MethodTypeHelper.vmResolveFromMethodDescriptorString(methodDescriptor, access.getClassloader(classObject), null);
/*[ENDIF] JAVA_SPEC_VERSION >= 11 */
/*[ENDIF] JAVA_SPEC_VERSION >= 11*/

int bsmIndex = UNSAFE.getShort(bsmData);
int bsmArgCount = UNSAFE.getShort(bsmData + BSM_ARGUMENT_COUNT_OFFSET);
Expand All @@ -265,13 +265,10 @@ private static final Object resolveInvokeDynamic(long j9class, String name, Stri

Object[] appendixResult = new Object[1];

/* result[0] stores a MemberName object, which specifies the caller method (generated bytecodes), or a Throwable.
*
* This leads to a type check in the interpreter for the object stored in result[0].
*
* TODO: Investigate if the Throwable can be wrapped in a MemberName. This will help prevent type checks in the
* interpreter since result[0] will always be a MemberName. This will improve performance.
*/
/*[IF JAVA_SPEC_VERSION >= 11]*/
try {
/*[ENDIF] JAVA_SPEC_VERSION >= 11*/
/* result[0] stores a MemberName object, which specifies the caller method (generated bytecodes). */
result[0] = MethodHandleNatives.linkCallSite(classObject,
/* The second parameter is not used in Java 8 and Java 18+ (JDK bug: 8272614). */
/*[IF (JAVA_SPEC_VERSION > 8) & (JAVA_SPEC_VERSION < 18)]*/
Expand All @@ -281,23 +278,39 @@ private static final Object resolveInvokeDynamic(long j9class, String name, Stri

/* result[1] stores a MethodHandle object, which is used as the last argument to the caller method. */
result[1] = appendixResult[0];
/*[IF JAVA_SPEC_VERSION < 11]*/
} catch (Throwable e) {
if (type == null) {
throw new BootstrapMethodError(e);
}

/* linkCallSite may correctly throw a BootstrapMethodError. */
/*[IF JAVA_SPEC_VERSION < 11]*/
/* Before Java 11, if linkCallSite throws a BootstrapMethodError due to the CallSite binding
* resolving to null, throw it at resolution time. This ensures that resolution will be
* reattempted on the subsequent visit to the invokedynamic instruction in the interpreter.
*/
if (e instanceof BootstrapMethodError) {
throw e;
}

/* Any other throwables are wrapped in an invoke-time BootstrapMethodError exception throw. */
/*[ENDIF] JAVA_SPEC_VERSION < 11*/
/* Any throwables are wrapped in an invoke-time BootstrapMethodError exception throw. */
try {
MethodHandle resultHandle;
MethodHandle thrower = MethodHandles.throwException(type.returnType(), BootstrapMethodError.class);
MethodHandle constructor = IMPL_LOOKUP.findConstructor(BootstrapMethodError.class, MethodType.methodType(void.class, Throwable.class));
MethodHandle resultHandle = MethodHandles.foldArguments(thrower, constructor.bindTo(e));

MethodHandle combiner = null;
if (e instanceof BootstrapMethodError) {
Throwable cause = e.getCause();
if (cause == null) {
combiner = constructor.bindTo(e.getMessage());
} else {
combiner = constructor.bindTo(cause);
}
} else {
combiner = constructor.bindTo(e);
}
resultHandle = MethodHandles.foldArguments(thrower, combiner);
/*[IF JAVA_SPEC_VERSION >= 11]*/
MemberName memberName = resultHandle.internalForm().vmentry;
/*[ELSE] JAVA_SPEC_VERSION >= 11*/
MemberName memberName = resultHandle.internalForm().compileToBytecode();
/*[ENDIF] JAVA_SPEC_VERSION >= 11*/
result[0] = memberName;
result[1] = resultHandle;
} catch (IllegalAccessException iae) {
Expand All @@ -306,7 +319,6 @@ private static final Object resolveInvokeDynamic(long j9class, String name, Stri
throw new Error(nsme);
}
}
/*[ENDIF] JAVA_SPEC_VERSION < 11 */
return (Object)result;
/*[ELSE] OPENJDK_METHODHANDLES*/
MethodHandle result = null;
Expand All @@ -332,7 +344,7 @@ private static final Object resolveInvokeDynamic(long j9class, String name, Stri
int bsmArgCount = UNSAFE.getShort(bsmData + BSM_ARGUMENT_COUNT_OFFSET);
long bsmArgs = bsmData + BSM_ARGUMENTS_OFFSET;
MethodHandle bsm = getCPMethodHandleAt(internalConstantPool, bsmIndex);
if (null == bsm) {
if (bsm == null) {
/*[MSG "K05cd", "unable to resolve 'bootstrap_method_ref' in '{0}' at index {1}"]*/
throw new NullPointerException(Msg.getString("K05cd", classObject.toString(), bsmIndex)); //$NON-NLS-1$
}
Expand Down
38 changes: 16 additions & 22 deletions runtime/vm/BytecodeInterpreter.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8865,29 +8865,23 @@ class INTERPRETER_CLASS
j9object_t invokeCacheArray = (ramConstantPool->ramClass->callSites)[index];

if (J9_EXPECTED(NULL != invokeCacheArray)) {
J9Class *clazz = J9OBJECT_CLAZZ(_currentThread, invokeCacheArray);
if (J9CLASS_IS_ARRAY(clazz)) {
/* Fetch target method and appendix from invokeCacheArray (2 element array)
* Stack transitions from:
* args <- SP
* MH
* To:
* invokeCacheArray[1] "appendix" <- SP
* args
* MH
*
* and sendMethod is ((J9Method *)((j.l.MemberName)invokeCacheArray[0]) + vmtargetOffset)
*/
j9object_t memberName = (j9object_t)J9JAVAARRAYOFOBJECT_LOAD(_currentThread, invokeCacheArray, 0);
_sendMethod = (J9Method *)(UDATA)J9OBJECT_U64_LOAD(_currentThread, memberName, _vm->vmtargetOffset);
/* Fetch target method and appendix from invokeCacheArray (2 element array)
* Stack transitions from:
* args <- SP
* MH
* To:
* invokeCacheArray[1] "appendix" <- SP
* args
* MH
*
* and sendMethod is ((J9Method *)((j.l.MemberName)invokeCacheArray[0]) + vmtargetOffset)
*/
j9object_t memberName = (j9object_t)J9JAVAARRAYOFOBJECT_LOAD(_currentThread, invokeCacheArray, 0);
_sendMethod = (J9Method *)(UDATA)J9OBJECT_U64_LOAD(_currentThread, memberName, _vm->vmtargetOffset);

j9object_t appendix = (j9object_t)J9JAVAARRAYOFOBJECT_LOAD(_currentThread, invokeCacheArray, 1);
if (NULL != appendix) {
*--_sp = (UDATA)appendix;
}
} else {
VM_VMHelpers::setExceptionPending(_currentThread, invokeCacheArray);
rc = GOTO_THROW_CURRENT_EXCEPTION;
j9object_t appendix = (j9object_t)J9JAVAARRAYOFOBJECT_LOAD(_currentThread, invokeCacheArray, 1);
if (NULL != appendix) {
*--_sp = (UDATA)appendix;
}
} else {
buildGenericSpecialStackFrame(REGISTER_ARGS, 0);
Expand Down
29 changes: 14 additions & 15 deletions runtime/vm/resolvesupport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2239,24 +2239,23 @@ resolveInvokeDynamic(J9VMThread *vmThread, J9ConstantPool *ramCP, UDATA callSite
/* Check if an exception is already pending */
if (vmThread->currentException != NULL) {
/* Already a pending exception */
result = vmThread->currentException;
result = NULL;
} else if (NULL == result) {
setCurrentExceptionUTF(vmThread, J9VMCONSTANTPOOL_JAVALANGNULLPOINTEREXCEPTION, NULL);
result = vmThread->currentException;
}

/* The result can be an array or exception. Ensure that the result and its elements are
* written/published before the result reference is stored.
*/
VM_AtomicSupport::writeBarrier();

J9MemoryManagerFunctions *gcFuncs = vmThread->javaVM->memoryManagerFunctions;
J9Class *j9class = J9_CLASS_FROM_CP(ramCP);
if (0 == gcFuncs->j9gc_objaccess_staticCompareAndSwapObject(vmThread, j9class, callSite, NULL, result)) {
/* Another thread beat this thread to updating the call site, ensure both threads
* return the same method handle.
} else {
/* The result is an array. Ensure that the result and its elements are
* written/published before the result reference is stored.
*/
result = *callSite;
VM_AtomicSupport::writeBarrier();

J9MemoryManagerFunctions *gcFuncs = vmThread->javaVM->memoryManagerFunctions;
J9Class *j9class = J9_CLASS_FROM_CP(ramCP);
if (0 == gcFuncs->j9gc_objaccess_staticCompareAndSwapObject(vmThread, j9class, callSite, NULL, result)) {
/* Another thread beat this thread to updating the call site, ensure both threads
* return the same method handle.
*/
result = *callSite;
}
}
#else /* defined(J9VM_OPT_OPENJDK_METHODHANDLE) */
/* Check if an exception is already pending */
Expand Down
19 changes: 11 additions & 8 deletions test/functional/Jsr292/src/com/ibm/j9/jsr292/indyn/IndyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,30 +373,33 @@ public void test_boostrap_return_constant_MethodType() {
AssertJUnit.assertTrue(expected == mt);
}

// test that if resolved CallSite is null, the same error is rethrown
// Test to verify behaviour if a CallSite initially resolves to null
@Test(groups = { "level.extended" })
public void test_CallSiteNullErrorRethrown () {
public void test_CallSiteNullErrorRethrown() {
/* The bootstrap method associated with the indy call in test_CallSiteNullErrorRethrown
* will return null the first time its called, and a valid CallSite for all repeat calls.
*/

// Java 8: NullPointerException is expected on the first run
// Java 11: BootstrapMethodError is expected on the first run
try {
com.ibm.j9.jsr292.indyn.GenIndyn.test_CallSiteNullErrorRethrown();
Assert.fail("BootstrapMethodError or NullPointerException should be thrown.");
} catch(BootstrapMethodError e) {
Assert.assertTrue(VersionCheck.major() >= 11);
} catch (BootstrapMethodError e) {
// Java 8 (with OJDK MHs): BoostrapMethodError is expected on the first run
// Java 11: BootstrapMethodError is expected on the first run
Assert.assertTrue(
(VersionCheck.major() >= 11)
|| "true".equals(System.getProperty("openjdk.methodhandles", "false")));
} catch (NullPointerException e) {
// Java 8 (with OJ9 MHs): NullPointerException is expected on the first run
Assert.assertTrue(VersionCheck.major() == 8);
}

// Java 8: CallSite resolution is expected to succeed
// Java 11 :The same BSME is expected on the second run
// Java 11: The same BSME is expected on the second run
try {
com.ibm.j9.jsr292.indyn.GenIndyn.test_CallSiteNullErrorRethrown();
Assert.assertTrue(VersionCheck.major() == 8);
} catch ( java.lang.BootstrapMethodError e ) {
} catch (java.lang.BootstrapMethodError e) {
Assert.assertTrue(VersionCheck.major() >= 11);
}
}
Expand Down

0 comments on commit d60b284

Please sign in to comment.