Skip to content

Commit

Permalink
Fix: Rewrite argMap allocation algorithm.
Browse files Browse the repository at this point in the history
The old one did not properly account for discontinuities in the list, leading to double-width types clobbering meaningful locals and causing mysterious VerifyErrors.
  • Loading branch information
LlamaLad7 committed Aug 26, 2024
1 parent 023e393 commit c8a0fb5
Showing 1 changed file with 40 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -460,12 +461,17 @@ public int[] generateArgMap(Type[] args, int start, boolean fresh) {
if (this.argMapVars == null) {
this.argMapVars = new ArrayList<Integer>();
}

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;
}
Expand All @@ -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;
}

/**
Expand Down

0 comments on commit c8a0fb5

Please sign in to comment.