# HG changeset patch # User jvernee # Date 1573725346 -3600 # Node ID 355f4f42dda5b1d0167e0f14d1a1cb6f0f070226 # Parent b987ea528c21997ebb11b48079c672786f8f9b0c 8233920: MethodHandles::tryFinally generates illegal bytecode for long/double return type Reviewed-by: redestad, vlivanov, jrose diff -r b987ea528c21 -r 355f4f42dda5 src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java --- 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); - * }} + * }} */ 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) * } + * * = 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. *

diff -r b987ea528c21 -r 355f4f42dda5 test/jdk/java/lang/invoke/TryFinallyTest.java --- 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);