Skip to content

Commit 7867711

Browse files
LlamaLad7LlamaLad7
committed
Fix: Overhaul initialiser merging and targeting for new mixins.
We introduce a new compatibility level to gate the changes, and we achieve the following properties: - New <clinit>s have their try-catch blocks, local variables and line numbers preserved - New <clinit>s cannot be skipped by a theoretical early return in the target or in another mixin - New initialisers cannot be targeted by any injectors - New injectors cannot target any initialisers - Old injectors still target old initialisers (bad behaviour but must be preserved)
1 parent 7e5a603 commit 7867711

File tree

3 files changed

+155
-18
lines changed

3 files changed

+155
-18
lines changed

‎src/main/java/org/spongepowered/asm/mixin/FabricUtil.java‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
package org.spongepowered.asm.mixin;
2727

2828
import org.spongepowered.asm.mixin.extensibility.IMixinConfig;
29+
import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
2930
import org.spongepowered.asm.mixin.injection.selectors.ISelectorContext;
31+
import org.spongepowered.asm.mixin.refmap.IMixinContext;
3032

3133
public final class FabricUtil {
3234
public static final String KEY_MOD_ID = "fabric-modId";
@@ -59,10 +61,15 @@ public final class FabricUtil {
5961
*/
6062
public static final int COMPATIBILITY_0_17_0 = 17000; // 0.17.0+mixin.0.8.7
6163

64+
/**
65+
* Fabric compatibility version 0.17.0
66+
*/
67+
public static final int COMPATIBILITY_0_17_1 = 17001; // 0.17.1+mixin.0.8.7
68+
6269
/**
6370
* Latest compatibility version
6471
*/
65-
public static final int COMPATIBILITY_LATEST = COMPATIBILITY_0_17_0;
72+
public static final int COMPATIBILITY_LATEST = COMPATIBILITY_0_17_1;
6673

6774
public static String getModId(IMixinConfig config) {
6875
return getModId(config, "(unknown)");
@@ -77,7 +84,15 @@ public static String getModId(ISelectorContext context) {
7784
}
7885

7986
public static int getCompatibility(ISelectorContext context) {
80-
return getDecoration(getConfig(context), KEY_COMPATIBILITY, COMPATIBILITY_LATEST);
87+
return getCompatibility(getConfig(context));
88+
}
89+
90+
public static int getCompatibility(IMixinContext context) {
91+
return getCompatibility(context.getMixin().getConfig());
92+
}
93+
94+
private static int getCompatibility(IMixinConfig config) {
95+
return getDecoration(config, KEY_COMPATIBILITY, COMPATIBILITY_LATEST);
8196
}
8297

8398
private static IMixinConfig getConfig(ISelectorContext context) {

‎src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorStandard.java‎

Lines changed: 63 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,17 @@
2828
import java.util.*;
2929
import java.util.Map.Entry;
3030
import java.util.function.BiConsumer;
31+
import java.util.function.Supplier;
3132

33+
import com.google.common.base.Suppliers;
3234
import org.spongepowered.asm.logging.ILogger;
33-
import org.objectweb.asm.Label;
3435
import org.objectweb.asm.Opcodes;
3536
import org.objectweb.asm.Type;
3637
import org.objectweb.asm.signature.SignatureReader;
3738
import org.objectweb.asm.signature.SignatureVisitor;
3839
import org.objectweb.asm.tree.*;
39-
import org.spongepowered.asm.mixin.Final;
40-
import org.spongepowered.asm.mixin.Intrinsic;
41-
import org.spongepowered.asm.mixin.MixinEnvironment;
40+
import org.spongepowered.asm.mixin.*;
4241
import org.spongepowered.asm.mixin.MixinEnvironment.Option;
43-
import org.spongepowered.asm.mixin.Overwrite;
4442
import org.spongepowered.asm.mixin.extensibility.IActivityContext.IActivity;
4543
import org.spongepowered.asm.mixin.gen.Accessor;
4644
import org.spongepowered.asm.mixin.gen.Invoker;
@@ -51,29 +49,31 @@
5149
import org.spongepowered.asm.mixin.injection.ModifyVariable;
5250
import org.spongepowered.asm.mixin.injection.Redirect;
5351
import org.spongepowered.asm.mixin.injection.struct.Constructor;
52+
import org.spongepowered.asm.mixin.injection.struct.InjectionNodes.InjectionNode;
53+
import org.spongepowered.asm.mixin.injection.struct.Target;
5454
import org.spongepowered.asm.mixin.throwables.MixinError;
5555
import org.spongepowered.asm.mixin.transformer.ClassInfo.Field;
5656
import org.spongepowered.asm.mixin.transformer.ext.extensions.ExtensionClassExporter;
5757
import org.spongepowered.asm.mixin.transformer.meta.MixinMerged;
5858
import org.spongepowered.asm.mixin.transformer.meta.MixinRenamed;
59+
import org.spongepowered.asm.mixin.transformer.struct.Clinit;
5960
import org.spongepowered.asm.mixin.transformer.struct.Initialiser;
6061
import org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException;
6162
import org.spongepowered.asm.mixin.transformer.throwables.MixinApplicatorException;
6263
import org.spongepowered.asm.service.IMixinAuditTrail;
6364
import org.spongepowered.asm.service.MixinService;
6465
import org.spongepowered.asm.util.Annotations;
6566
import org.spongepowered.asm.util.Bytecode;
66-
import org.spongepowered.asm.util.Bytecode.DelegateInitialiser;
6767
import org.spongepowered.asm.util.Constants;
6868
import org.spongepowered.asm.util.ConstraintParser;
6969
import org.spongepowered.asm.util.ConstraintParser.Constraint;
70+
import org.spongepowered.asm.util.asm.ASM;
7071
import org.spongepowered.asm.util.perf.Profiler;
7172
import org.spongepowered.asm.util.perf.Profiler.Section;
7273
import org.spongepowered.asm.util.throwables.ConstraintViolationException;
7374
import org.spongepowered.asm.util.throwables.InvalidConstraintException;
7475

7576
import com.google.common.collect.ImmutableList;
76-
import com.google.common.collect.ImmutableSet;
7777

7878
/**
7979
* Applies mixins to a target class
@@ -277,14 +277,46 @@ private void runApplicatorPass(ApplicatorPass pass, List<MixinTargetContext> mix
277277
this.applyFields(mixin);
278278
activity.next("Apply Methods");
279279
this.applyMethods(mixin);
280-
activity.next("Apply Initialisers");
281-
this.applyInitialisers(mixin);
282280
});
283281
break;
284282
case INJECT_PREPARE:
285283
this.processMixins(mixinContexts, (activity, mixin) -> {
286-
activity.next("Prepare Injections");
287-
this.prepareInjections(mixin);
284+
if (FabricUtil.getCompatibility(mixin) >= FabricUtil.COMPATIBILITY_0_17_1) {
285+
activity.next("Prepare Injections");
286+
this.prepareInjections(mixin);
287+
}
288+
});
289+
break;
290+
case INITIALISER_APPLY_LEGACY:
291+
this.processMixins(mixinContexts, (activity, mixin) -> {
292+
if (FabricUtil.getCompatibility(mixin) < FabricUtil.COMPATIBILITY_0_17_1) {
293+
activity.next("Apply Legacy Initialisers");
294+
this.applyInitialisers(mixin);
295+
MethodNode clinit = Bytecode.findMethod(mixin.getClassNode(), Constants.CLINIT, "()V");
296+
if (clinit != null) {
297+
activity.next("Apply Legacy CLINIT");
298+
this.appendInsns(mixin, clinit);
299+
}
300+
}
301+
});
302+
break;
303+
case INJECT_PREPARE_LEGACY:
304+
this.processMixins(mixinContexts, (activity, mixin) -> {
305+
if (FabricUtil.getCompatibility(mixin) < FabricUtil.COMPATIBILITY_0_17_1) {
306+
activity.next("Prepare Legacy Injections");
307+
this.prepareInjections(mixin);
308+
}
309+
});
310+
break;
311+
case INITIALISER_APPLY:
312+
Supplier<Clinit> targetClinit = Suppliers.memoize(this::prepareOrCreateClinit);
313+
this.processMixins(mixinContexts, (activity, mixin) -> {
314+
if (FabricUtil.getCompatibility(mixin) >= FabricUtil.COMPATIBILITY_0_17_1) {
315+
activity.next("Apply Initialisers");
316+
this.applyInitialisers(mixin);
317+
activity.next("Apply CLINIT");
318+
this.applyClinit(mixin, targetClinit);
319+
}
288320
});
289321
break;
290322
case ACCESSOR:
@@ -467,11 +499,6 @@ protected void applyNormalMethod(MixinTargetContext mixin, MethodNode mixinMetho
467499
this.checkMethodVisibility(mixin, mixinMethod);
468500
this.checkMethodConstraints(mixin, mixinMethod);
469501
this.mergeMethod(mixin, mixinMethod);
470-
} else if (Constants.CLINIT.equals(mixinMethod.name)) {
471-
// Class initialiser insns get appended
472-
IActivity activity = this.activities.begin("Merge CLINIT insns");
473-
this.appendInsns(mixin, mixinMethod);
474-
activity.end();
475502
}
476503
}
477504

@@ -723,6 +750,26 @@ protected void applyInitialisers(MixinTargetContext mixin) {
723750
}
724751
}
725752

753+
private Clinit prepareOrCreateClinit() {
754+
MethodNode clinit = Bytecode.findMethod(this.targetClass, Constants.CLINIT, "()V");
755+
if (clinit != null) {
756+
return Clinit.prepare(this.context.getTargetMethod(clinit));
757+
}
758+
clinit = new MethodNode(ASM.API_VERSION, Opcodes.ACC_STATIC, Constants.CLINIT, "()V", null, null);
759+
InsnNode finalReturn = new InsnNode(Opcodes.RETURN);
760+
clinit.instructions.add(finalReturn);
761+
this.targetClass.methods.add(clinit);
762+
return new Clinit(clinit, finalReturn);
763+
}
764+
765+
protected void applyClinit(MixinTargetContext mixin, Supplier<Clinit> clinit) {
766+
MethodNode mixinClinit = Bytecode.findMethod(mixin.getClassNode(), Constants.CLINIT, "()V");
767+
if (mixinClinit == null) {
768+
return;
769+
}
770+
clinit.get().append(mixinClinit);
771+
}
772+
726773
/**
727774
* Scan for injector methods and injection points
728775
*

‎src/main/java/org/spongepowered/asm/mixin/transformer/struct/Clinit.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package org.spongepowered.asm.mixin.transformer.struct;
2+
3+
import org.objectweb.asm.Opcodes;
4+
import org.objectweb.asm.tree.*;
5+
import org.spongepowered.asm.mixin.injection.struct.InjectionNodes;
6+
import org.spongepowered.asm.mixin.injection.struct.Target;
7+
import org.spongepowered.asm.util.Bytecode;
8+
9+
import java.util.ListIterator;
10+
import java.util.Map;
11+
12+
public class Clinit {
13+
private final MethodNode clinit;
14+
private final AbstractInsnNode finalReturn;
15+
16+
public Clinit(MethodNode clinit, AbstractInsnNode finalReturn) {
17+
this.clinit = clinit;
18+
this.finalReturn = finalReturn;
19+
}
20+
21+
public void append(MethodNode mixinClinit) {
22+
prepareClinit(mixinClinit, null);
23+
Map<LabelNode, LabelNode> labels = Bytecode.cloneLabels(mixinClinit.instructions);
24+
for (AbstractInsnNode insn : mixinClinit.instructions) {
25+
if (insn.getOpcode() == Opcodes.RETURN) {
26+
continue;
27+
}
28+
this.clinit.instructions.insertBefore(this.finalReturn, insn.clone(labels));
29+
}
30+
this.clinit.maxLocals = Math.max(this.clinit.maxLocals, mixinClinit.maxLocals);
31+
this.clinit.maxStack = Math.max(this.clinit.maxStack, mixinClinit.maxStack);
32+
for (TryCatchBlockNode tryCatch : mixinClinit.tryCatchBlocks) {
33+
this.clinit.tryCatchBlocks.add(new TryCatchBlockNode(labels.get(tryCatch.start), labels.get(tryCatch.end), labels.get(tryCatch.handler), tryCatch.type));
34+
}
35+
for (LocalVariableNode local : mixinClinit.localVariables) {
36+
this.clinit.localVariables.add(new LocalVariableNode(local.name, local.desc, local.signature, labels.get(local.start), labels.get(local.end), local.index));
37+
}
38+
}
39+
40+
public static Clinit prepare(Target clinit) {
41+
return new Clinit(clinit.method, Clinit.prepareClinit(clinit.method, clinit));
42+
}
43+
44+
private static AbstractInsnNode prepareClinit(MethodNode clinit, Target target) {
45+
LabelNode endLabel = new LabelNode();
46+
AbstractInsnNode existingFinalReturn = null;
47+
for (ListIterator<AbstractInsnNode> iter = clinit.instructions.iterator(); iter.hasNext(); ) {
48+
AbstractInsnNode insn = iter.next();
49+
50+
if (insn.getOpcode() == Opcodes.RETURN) {
51+
if (insn.getNext() == null) {
52+
existingFinalReturn = insn;
53+
break;
54+
}
55+
AbstractInsnNode newInsn = new JumpInsnNode(Opcodes.GOTO, endLabel);
56+
iter.set(newInsn);
57+
58+
if (target != null) {
59+
InjectionNodes.InjectionNode injectionNode = target.getInjectionNode(insn);
60+
if (injectionNode != null) {
61+
injectionNode.replace(newInsn);
62+
}
63+
}
64+
}
65+
}
66+
if (existingFinalReturn != null) {
67+
clinit.instructions.insertBefore(existingFinalReturn, endLabel);
68+
return existingFinalReturn;
69+
}
70+
clinit.instructions.add(endLabel);
71+
InsnNode finalReturn = new InsnNode(Opcodes.RETURN);
72+
clinit.instructions.add(finalReturn);
73+
return finalReturn;
74+
}
75+
}

0 commit comments

Comments
 (0)