Fix: Rework bridge method merging.

The previous implementation did not properly handle pseudo-instructions, sometimes leading to unnecessary mismatches.
FabricMC · Nov 24, 2024 · 1c69523 · 1c69523
1 parent c516fb7
commit 1c69523
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 14 deletions.
diff --git a/src/main/java/org/spongepowered/asm/util/Bytecode.java b/src/main/java/org/spongepowered/asm/util/Bytecode.java
@@ -30,9 +30,9 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
-import java.util.ListIterator;
 import java.util.Map;
 
+import com.google.common.collect.Iterators;
 import org.objectweb.asm.Attribute;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassWriter;
@@ -1256,16 +1256,13 @@ public static String getUnboxingMethod(Type type) {
      * @param b Incoming method
      */
     public static void compareBridgeMethods(MethodNode a, MethodNode b) {
-        ListIterator<AbstractInsnNode> ia = a.instructions.iterator();
-        ListIterator<AbstractInsnNode> ib = b.instructions.iterator();
+        Iterator<AbstractInsnNode> ia = Iterators.filter(a.instructions.iterator(), Bytecode::isRealInsn);
+        Iterator<AbstractInsnNode> ib = Iterators.filter(b.instructions.iterator(), Bytecode::isRealInsn);
 
         int index = 0;
         for (; ia.hasNext() && ib.hasNext(); index++) {
             AbstractInsnNode na = ia.next();
             AbstractInsnNode nb = ib.next();
-            if (na instanceof LabelNode) {
-                continue;
-            } 
 
             if (na instanceof MethodInsnNode) {
                 MethodInsnNode ma = (MethodInsnNode)na;
@@ -1292,9 +1289,16 @@ public static void compareBridgeMethods(MethodNode a, MethodNode b) {
             }
         }
 
-        if (ia.hasNext() || ib.hasNext()) {
-            throw new SyntheticBridgeException(Problem.BAD_LENGTH, a.name, a.desc, index, null, null);
+        if (ia.hasNext()) {
+            throw new SyntheticBridgeException(Problem.BAD_LENGTH, a.name, a.desc, index, ia.next(), null);
         }
+        if (ib.hasNext()) {
+            throw new SyntheticBridgeException(Problem.BAD_LENGTH, a.name, a.desc, index, null, ib.next());
+        }
+    }
+
+    private static boolean isRealInsn(AbstractInsnNode insn) {
+        return insn.getOpcode() != -1;
     }
 
     /**

diff --git a/src/main/java/org/spongepowered/asm/util/throwables/SyntheticBridgeException.java b/src/main/java/org/spongepowered/asm/util/throwables/SyntheticBridgeException.java
@@ -125,15 +125,15 @@ public void printAnalysis(IMixinContext context, MethodNode mda, MethodNode mdb)
         printer.add().kv("Method", this.name + this.desc).kv("Problem Type", this.problem).add().hr();
         String merged = Annotations.<String>getValue(Annotations.getVisible(mda, MixinMerged.class), "mixin");
         String owner = merged != null ? merged : context.getTargetClassRef().replace('/', '.');
-        this.printMethod(printer.add("Existing method").add().kv("Owner", owner).add(), mda).hr();
-        this.printMethod(printer.add("Incoming method").add().kv("Owner", context.getClassRef().replace('/', '.')).add(), mdb).hr();
+        this.printMethod(printer.add("Existing method").add().kv("Owner", owner).add(), mda, a).hr();
+        this.printMethod(printer.add("Incoming method").add().kv("Owner", context.getClassRef().replace('/', '.')).add(), mdb, b).hr();
         this.printProblem(printer, context, mda, mdb).print(System.err);
     }
 
-    private PrettyPrinter printMethod(PrettyPrinter printer, MethodNode method) {
-        int index = 0;
-        for (Iterator<AbstractInsnNode> iter = method.instructions.iterator(); iter.hasNext(); index++) {
-            printer.kv(index == this.index ? ">>>>" : "", Bytecode.describeNode(iter.next()));
+    private PrettyPrinter printMethod(PrettyPrinter printer, MethodNode method, AbstractInsnNode marker) {
+        for (Iterator<AbstractInsnNode> iter = method.instructions.iterator(); iter.hasNext();) {
+            AbstractInsnNode insn = iter.next();
+            printer.kv(insn == marker ? ">>>>" : "", Bytecode.describeNode(insn));
         }
         return printer.add();
     }