From c8a0fb5057a0188945d494b6721e976e71be6c77 Mon Sep 17 00:00:00 2001 From: LlamaLad7 Date: Mon, 26 Aug 2024 22:08:58 +0100 Subject: [PATCH] Fix: Rewrite argMap allocation algorithm. The old one did not properly account for discontinuities in the list, leading to double-width types clobbering meaningful locals and causing mysterious VerifyErrors. --- .../asm/mixin/injection/struct/Target.java | 69 +++++++++++-------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/struct/Target.java b/src/main/java/org/spongepowered/asm/mixin/injection/struct/Target.java index 912b833e..87f44b56 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/struct/Target.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/struct/Target.java @@ -44,6 +44,7 @@ import org.spongepowered.asm.mixin.transformer.ClassInfo; import org.spongepowered.asm.util.Bytecode; import org.spongepowered.asm.util.Constants; +import org.spongepowered.asm.util.Counter; import org.spongepowered.asm.util.Locals.SyntheticLocalVariableNode; /** @@ -460,12 +461,17 @@ public int[] generateArgMap(Type[] args, int start, boolean fresh) { if (this.argMapVars == null) { this.argMapVars = new ArrayList(); } - + int[] argMap = new int[args.length]; - for (int arg = start, index = 0; arg < args.length; arg++) { + Counter index = new Counter(); + for (int arg = start; arg < args.length; arg++) { int size = args[arg].getSize(); - argMap[arg] = fresh ? this.allocateLocals(size) : this.allocateArgMapLocal(index, size); - index += size; + if (fresh) { + argMap[arg] = this.allocateLocals(size); + index.value += size; + } else { + argMap[arg] = this.allocateArgMapLocal(index, size); + } } return argMap; } @@ -481,35 +487,40 @@ public int[] generateArgMap(Type[] args, int start, boolean fresh) { * @param size Size of variable * @return local offset to use */ - private int allocateArgMapLocal(int index, int size) { - // Allocate extra space if we've reached the end - if (index >= this.argMapVars.size()) { - int base = this.allocateLocals(size); - for (int offset = 0; offset < size; offset++) { - this.argMapVars.add(Integer.valueOf(base + offset)); + private int allocateArgMapLocal(Counter index, int size) { + boolean isLastSlotFree = index.value < this.argMapVars.size(); + while (index.value < this.argMapVars.size()) { + int local = this.argMapVars.get(index.value); + if (size == 1) { + index.value++; + return local; } - return base; - } - - int local = this.argMapVars.get(index); - - // Allocate extra space if the variable is oversize - if (size > 1 && index + size > this.argMapVars.size()) { - int nextLocal = this.allocateLocals(1); - if (nextLocal == local + 1) { - // Next allocated was contiguous, so we can continue - this.argMapVars.add(Integer.valueOf(nextLocal)); + int nextIndex = index.value + 1; + if (nextIndex < this.argMapVars.size() && local + 1 == this.argMapVars.get(nextIndex)) { + // We own the next slot so this is a safe place to store a double-width type. + // We've consumed the next slot so increment the next available index. + index.value += 2; return local; } - - // Next allocated was not contiguous, allocate a new local slot so - // that indexes are contiguous - this.argMapVars.set(index, Integer.valueOf(nextLocal)); - this.argMapVars.add(Integer.valueOf(this.allocateLocals(1))); - return nextLocal; + index.value++; } - - return local; + // We don't have anywhere to store the arg. + int newLocal = this.allocateLocal(); + this.argMapVars.add(newLocal); + index.value++; + if (size == 1) { + return newLocal; + } + if (isLastSlotFree && newLocal == this.argMapVars.get(this.argMapVars.size() - 2) + 1) { + // We own the preceding local as well, and it is not yet used by this argMap, so we can store our + // double-width type there. + return newLocal - 1; + } + // Allocate 1 more local so we can fit the double-width type. We know it will follow directly from the + // previous one. + this.argMapVars.add(this.allocateLocal()); + index.value++; + return newLocal; } /**