From feb72b8ae122e0b7088a1ee8c6127069ca038269 Mon Sep 17 00:00:00 2001 From: Jan Date: Thu, 20 Jan 2022 20:54:57 +0100 Subject: [PATCH] Update ASM EventExecutor generator patch to respect event handler return types. Fixes #7311 (#7317) Co-authored-by: Jan Boerman --- .../api/0026-Use-ASM-for-event-executors.patch | 16 ++++++++++------ ...rn-on-strange-EventHandler-return-types.patch | 6 +++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/patches/api/0026-Use-ASM-for-event-executors.patch b/patches/api/0026-Use-ASM-for-event-executors.patch index bec520ef22..d9b5f4b494 100644 --- a/patches/api/0026-Use-ASM-for-event-executors.patch +++ b/patches/api/0026-Use-ASM-for-event-executors.patch @@ -6,7 +6,7 @@ Subject: [PATCH] Use ASM for event executors. Uses method handles for private or static methods. diff --git a/build.gradle.kts b/build.gradle.kts -index 0e8e827b5a05ac9423386eaea4188d132cc7f954..70d6503745942ea8637146369aeab8f9357c42d5 100644 +index 80fdd05dd593455ca89b66636ed30f1d9facf4ed..d8d459561cc75935136f8f9888ff27b45ad98f9e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -38,6 +38,9 @@ dependencies { @@ -118,10 +118,10 @@ index 0000000000000000000000000000000000000000..c83672427324bd068ed52916f700b684 +} diff --git a/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java b/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java new file mode 100644 -index 0000000000000000000000000000000000000000..b6e7d8ee8d903ebf975d60bec0e08603d9a49fdb +index 0000000000000000000000000000000000000000..b8d5c13980858dc27fb5383726b7ebcaf14adcb8 --- /dev/null +++ b/src/main/java/com/destroystokyo/paper/event/executor/asm/ASMEventExecutorGenerator.java -@@ -0,0 +1,47 @@ +@@ -0,0 +1,51 @@ +package com.destroystokyo.paper.event.executor.asm; + +import java.lang.reflect.Method; @@ -147,14 +147,18 @@ index 0000000000000000000000000000000000000000..b6e7d8ee8d903ebf975d60bec0e08603 + methodGenerator.returnValue(); + methodGenerator.endMethod(); + // Generate the execute method -+ methodGenerator = new GeneratorAdapter(writer.visitMethod(ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Event;)V", null, null), ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Listener;)V");; ++ methodGenerator = new GeneratorAdapter(writer.visitMethod(ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Event;)V", null, null), ACC_PUBLIC, "execute", "(Lorg/bukkit/event/Listener;Lorg/bukkit/event/Listener;)V"); + methodGenerator.loadArg(0); + methodGenerator.checkCast(Type.getType(m.getDeclaringClass())); + methodGenerator.loadArg(1); + methodGenerator.checkCast(Type.getType(m.getParameterTypes()[0])); + methodGenerator.visitMethodInsn(m.getDeclaringClass().isInterface() ? INVOKEINTERFACE : INVOKEVIRTUAL, Type.getInternalName(m.getDeclaringClass()), m.getName(), Type.getMethodDescriptor(m), m.getDeclaringClass().isInterface()); -+ if (m.getReturnType() != void.class) { -+ methodGenerator.pop(); ++ // The only purpose of this switch statement is to generate the correct pop instruction, should the event handler method return something other than void. ++ // Non-void event handlers will be unsupported in a future release. ++ switch (Type.getType(m.getReturnType()).getSize()) { ++ // case 0 is omitted because the only type that has size 0 is void - no pop instruction needed. ++ case 1 -> methodGenerator.pop(); // handles reference types and most primitives ++ case 2 -> methodGenerator.pop2(); // handles long and double + } + methodGenerator.returnValue(); + methodGenerator.endMethod(); diff --git a/patches/api/0357-Warn-on-strange-EventHandler-return-types.patch b/patches/api/0357-Warn-on-strange-EventHandler-return-types.patch index 1ac1378f26..29026c1725 100644 --- a/patches/api/0357-Warn-on-strange-EventHandler-return-types.patch +++ b/patches/api/0357-Warn-on-strange-EventHandler-return-types.patch @@ -5,7 +5,7 @@ Subject: [PATCH] Warn on strange @EventHandler return types diff --git a/src/main/java/org/bukkit/plugin/EventExecutor.java b/src/main/java/org/bukkit/plugin/EventExecutor.java -index 9026e108ccd3a88aee1267ee275137befa646455..e5f10ed4312529c082acf7de44af3639f77c8b8e 100644 +index 9026e108ccd3a88aee1267ee275137befa646455..e1860322ae0f3c35097d16767628744034941749 100644 --- a/src/main/java/org/bukkit/plugin/EventExecutor.java +++ b/src/main/java/org/bukkit/plugin/EventExecutor.java @@ -51,6 +51,12 @@ public interface EventExecutor { @@ -15,8 +15,8 @@ index 9026e108ccd3a88aee1267ee275137befa646455..e5f10ed4312529c082acf7de44af3639 + if (m.getReturnType() != Void.TYPE) { + final org.bukkit.plugin.java.JavaPlugin plugin = org.bukkit.plugin.java.JavaPlugin.getProvidingPlugin(m.getDeclaringClass()); + org.bukkit.Bukkit.getLogger().warning("@EventHandler method " + m.getDeclaringClass().getName() + (Modifier.isStatic(m.getModifiers()) ? '.' : '#') + m.getName() -+ + " returns non-void type " + m.getReturnType().getName() + ". This is an unsupported behavior which will be removed in a future version of Paper." -+ + "This should be reported to the developers of " + plugin.getDescription().getFullName() + ", (" + String.join(",", plugin.getDescription().getAuthors()) + ')'); ++ + " returns non-void type " + m.getReturnType().getName() + ". This is unsupported behavior and will no longer work in a future version of Paper." ++ + " This should be reported to the developers of " + plugin.getDescription().getFullName() + " (" + String.join(",", plugin.getDescription().getAuthors()) + ')'); + } if (Modifier.isStatic(m.getModifiers())) { return new StaticMethodHandleEventExecutor(eventClass, m);