8233920: MethodHandles::tryFinally generates illegal bytecode for long/double return type
Reviewed-by: redestad, vlivanov, jrose
--- a/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java Wed Nov 13 19:55:11 2019 -0800
+++ b/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java Thu Nov 14 10:55:46 2019 +0100
@@ -1168,7 +1168,7 @@
* } catch (Throwable e) {
* if (!a2.isInstance(e)) throw e;
* return a3.invokeBasic(ex, a6, a7);
- * }}
+ * }}</pre></blockquote>
*/
private Name emitGuardWithCatch(int pos) {
Name args = lambdaForm.names[pos];
@@ -1263,26 +1263,27 @@
* load target (-- target)
* load args (-- args... target)
* INVOKEVIRTUAL MethodHandle.invokeBasic (depends)
- * FINALLY_NORMAL: (-- r)
- * load cleanup (-- cleanup r)
- * SWAP (-- r cleanup)
- * ACONST_NULL (-- t r cleanup)
- * SWAP (-- r t cleanup)
- * load args (-- args... r t cleanup)
- * INVOKEVIRTUAL MethodHandle.invokeBasic (-- r)
+ * FINALLY_NORMAL: (-- r_2nd* r)
+ * store returned value (--)
+ * load cleanup (-- cleanup)
+ * ACONST_NULL (-- t cleanup)
+ * load returned value (-- r_2nd* r t cleanup)
+ * load args (-- args... r_2nd* r t cleanup)
+ * INVOKEVIRTUAL MethodHandle.invokeBasic (-- r_2nd* r)
* GOTO DONE
* CATCH: (-- t)
* DUP (-- t t)
* FINALLY_EXCEPTIONAL: (-- t t)
* load cleanup (-- cleanup t t)
* SWAP (-- t cleanup t)
- * load default for r (-- r t cleanup t)
- * load args (-- args... r t cleanup t)
- * INVOKEVIRTUAL MethodHandle.invokeBasic (-- r t)
- * POP (-- t)
+ * load default for r (-- r_2nd* r t cleanup t)
+ * load args (-- args... r_2nd* r t cleanup t)
+ * INVOKEVIRTUAL MethodHandle.invokeBasic (-- r_2nd* r t)
+ * POP/POP2* (-- t)
* ATHROW
* DONE: (-- r)
* }</pre></blockquote>
+ * * = depends on whether the return type takes up 2 stack slots.
*/
private Name emitTryFinally(int pos) {
Name args = lambdaForm.names[pos];
@@ -1295,7 +1296,9 @@
Label lDone = new Label();
Class<?> returnType = result.function.resolvedHandle().type().returnType();
+ BasicType basicReturnType = BasicType.basicType(returnType);
boolean isNonVoid = returnType != void.class;
+
MethodType type = args.function.resolvedHandle().type()
.dropParameterTypes(0,1)
.changeReturnType(returnType);
@@ -1316,13 +1319,14 @@
mv.visitLabel(lTo);
// FINALLY_NORMAL:
- emitPushArgument(invoker, 1); // load cleanup
+ int index = extendLocalsMap(new Class<?>[]{ returnType });
if (isNonVoid) {
- mv.visitInsn(Opcodes.SWAP);
+ emitStoreInsn(basicReturnType, index);
}
+ emitPushArgument(invoker, 1); // load cleanup
mv.visitInsn(Opcodes.ACONST_NULL);
if (isNonVoid) {
- mv.visitInsn(Opcodes.SWAP);
+ emitLoadInsn(basicReturnType, index);
}
emitPushArguments(args, 1); // load args (skip 0: method handle)
mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, MH, "invokeBasic", cleanupDesc, false);
@@ -1341,7 +1345,7 @@
emitPushArguments(args, 1); // load args (skip 0: method handle)
mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, MH, "invokeBasic", cleanupDesc, false);
if (isNonVoid) {
- mv.visitInsn(Opcodes.POP);
+ emitPopInsn(basicReturnType);
}
mv.visitInsn(Opcodes.ATHROW);
@@ -1351,6 +1355,24 @@
return result;
}
+ private void emitPopInsn(BasicType type) {
+ mv.visitInsn(popInsnOpcode(type));
+ }
+
+ private static int popInsnOpcode(BasicType type) {
+ switch (type) {
+ case I_TYPE:
+ case F_TYPE:
+ case L_TYPE:
+ return Opcodes.POP;
+ case J_TYPE:
+ case D_TYPE:
+ return Opcodes.POP2;
+ default:
+ throw new InternalError("unknown type: " + type);
+ }
+ }
+
/**
* Emit bytecode for the loop idiom.
* <p>
--- a/test/jdk/java/lang/invoke/TryFinallyTest.java Wed Nov 13 19:55:11 2019 -0800
+++ b/test/jdk/java/lang/invoke/TryFinallyTest.java Thu Nov 14 10:55:46 2019 +0100
@@ -24,8 +24,8 @@
*/
/* @test
- * @bug 8139885 8150824 8150825 8194238
- * @run testng/othervm -ea -esa test.java.lang.invoke.TryFinallyTest
+ * @bug 8139885 8150824 8150825 8194238 8233920
+ * @run testng/othervm -ea -esa -Xverify:all test.java.lang.invoke.TryFinallyTest
*/
package test.java.lang.invoke;
@@ -55,6 +55,41 @@
assertEquals("Hello, world!", hello.invoke("world"));
}
+ @DataProvider
+ static Object[][] tryFinallyArgs() {
+ return new Object[][] {
+ { boolean.class, true },
+ { byte.class, (byte) 2 },
+ { short.class, (short) 2 },
+ { char.class, (char) 2 },
+ { int.class, 2 },
+ { long.class, 2L },
+ { float.class, 2f },
+ { double.class, 2D },
+ { Object.class, new Object() }
+ };
+ }
+
+ @Test(dataProvider = "tryFinallyArgs")
+ public static void testTryFinally(Class<?> argType, Object arg) throws Throwable {
+ MethodHandle identity = MethodHandles.identity(argType);
+ MethodHandle tryFinally = MethodHandles.tryFinally(
+ identity,
+ MethodHandles.dropArguments(identity, 0, Throwable.class));
+ assertEquals(methodType(argType, argType), tryFinally.type());
+ assertEquals(arg, tryFinally.invoke(arg));
+ }
+
+ @Test(dataProvider = "tryFinallyArgs", expectedExceptions = TryFinally.T1.class)
+ public static void testTryFinallyException(Class<?> argType, Object arg) throws Throwable {
+ MethodHandle identity = TryFinally.MH_throwingTargetIdentity.asType(methodType(argType, argType));
+ MethodHandle tryFinally = MethodHandles.tryFinally(
+ identity,
+ MethodHandles.dropArguments(identity, 0, TryFinally.T1.class));
+ assertEquals(methodType(argType, argType), tryFinally.type());
+ tryFinally.invoke(arg); // should throw
+ }
+
@Test
public static void testTryFinallyVoid() throws Throwable {
MethodHandle tfVoid = MethodHandles.tryFinally(TryFinally.MH_print, TryFinally.MH_printMore);
@@ -175,6 +210,10 @@
throw new T1();
}
+ static Object throwingTargetIdentity(Object o) throws Throwable {
+ throw new T1();
+ }
+
static void catchingCleanup(T2 t) throws Throwable {
}
@@ -189,6 +228,7 @@
static final MethodType MT_voidTarget = methodType(void.class);
static final MethodType MT_voidCleanup = methodType(void.class, Throwable.class);
static final MethodType MT_throwingTarget = methodType(void.class);
+ static final MethodType MT_throwingTargetIdentity = methodType(Object.class, Object.class);
static final MethodType MT_catchingCleanup = methodType(void.class, T2.class);
static final MethodHandle MH_greet;
@@ -200,6 +240,7 @@
static final MethodHandle MH_voidTarget;
static final MethodHandle MH_voidCleanup;
static final MethodHandle MH_throwingTarget;
+ static final MethodHandle MH_throwingTargetIdentity;
static final MethodHandle MH_catchingCleanup;
static final MethodHandle MH_dummyTarget;
@@ -219,6 +260,7 @@
MH_voidTarget = LOOKUP.findStatic(TRY_FINALLY, "voidTarget", MT_voidTarget);
MH_voidCleanup = LOOKUP.findStatic(TRY_FINALLY, "voidCleanup", MT_voidCleanup);
MH_throwingTarget = LOOKUP.findStatic(TRY_FINALLY, "throwingTarget", MT_throwingTarget);
+ MH_throwingTargetIdentity = LOOKUP.findStatic(TRY_FINALLY, "throwingTargetIdentity", MT_throwingTargetIdentity);
MH_catchingCleanup = LOOKUP.findStatic(TRY_FINALLY, "catchingCleanup", MT_catchingCleanup);
MH_dummyTarget = MethodHandles.dropArguments(MH_voidTarget, 0, int.class, long.class, Object.class,
int.class, long.class, Object.class);