Skip to content

Commit f0f4cb2

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 f0f4cb2

File tree

4 files changed

+203
-24
lines changed

4 files changed

+203
-24
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/MixinApplicatorInterface.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,16 @@
2525
package org.spongepowered.asm.mixin.transformer;
2626

2727
import java.util.Map.Entry;
28+
import java.util.function.Supplier;
2829

2930
import org.objectweb.asm.tree.FieldNode;
30-
import org.objectweb.asm.tree.MethodNode;
3131
import org.spongepowered.asm.mixin.MixinEnvironment.Feature;
3232
import org.spongepowered.asm.mixin.injection.struct.InjectionInfo;
3333
import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException;
3434
import org.spongepowered.asm.mixin.transformer.ClassInfo.Field;
35+
import org.spongepowered.asm.mixin.transformer.struct.Clinit;
3536
import org.spongepowered.asm.mixin.transformer.throwables.InvalidInterfaceMixinException;
3637
import org.spongepowered.asm.util.Annotations;
37-
import org.spongepowered.asm.util.Constants;
3838

3939
/**
4040
* Applicator for interface mixins, mainly just disables things which aren't
@@ -97,11 +97,13 @@ protected void mergeNewFields(MixinTargetContext mixin) {
9797
}
9898

9999
@Override
100-
protected void applyNormalMethod(MixinTargetContext mixin, MethodNode mixinMethod) {
100+
protected void applyClinitLegacy(MixinTargetContext mixin) {
101+
//Skip merging static blocks, the only contents will be setting shadowed fields
102+
}
103+
104+
@Override
105+
protected void applyClinit(MixinTargetContext mixin, Supplier<Clinit> clinit) {
101106
//Skip merging static blocks, the only contents will be setting shadowed fields
102-
if (!Constants.CLINIT.equals(mixinMethod.name)) {
103-
super.applyNormalMethod(mixin, mixinMethod);
104-
}
105107
}
106108

107109
/* (non-Javadoc)

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

Lines changed: 79 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
@@ -103,12 +103,24 @@ enum ApplicatorPass {
103103
*/
104104
MAIN,
105105

106+
/**
107+
* Enumerate injectors and scan for injection points
108+
*/
106109
INJECT_PREPARE,
107110

111+
/**
112+
* Apply instance field initialisers and {@code <clinit>} methods in legacy mixins (compat level < 0.17.1)
113+
*/
108114
INITIALISER_APPLY_LEGACY,
109115

116+
/**
117+
* Enumerate injectors and scan for injection points in legacy mixins (compat level < 0.17.1)
118+
*/
110119
INJECT_PREPARE_LEGACY,
111120

121+
/**
122+
* Apply instance field initialisers and {@code <clinit>} methods
123+
*/
112124
INITIALISER_APPLY,
113125

114126
/**
@@ -277,14 +289,43 @@ private void runApplicatorPass(ApplicatorPass pass, List<MixinTargetContext> mix
277289
this.applyFields(mixin);
278290
activity.next("Apply Methods");
279291
this.applyMethods(mixin);
280-
activity.next("Apply Initialisers");
281-
this.applyInitialisers(mixin);
282292
});
283293
break;
284294
case INJECT_PREPARE:
285295
this.processMixins(mixinContexts, (activity, mixin) -> {
286-
activity.next("Prepare Injections");
287-
this.prepareInjections(mixin);
296+
if (FabricUtil.getCompatibility(mixin) >= FabricUtil.COMPATIBILITY_0_17_1) {
297+
activity.next("Prepare Injections");
298+
this.prepareInjections(mixin);
299+
}
300+
});
301+
break;
302+
case INITIALISER_APPLY_LEGACY:
303+
this.processMixins(mixinContexts, (activity, mixin) -> {
304+
if (FabricUtil.getCompatibility(mixin) < FabricUtil.COMPATIBILITY_0_17_1) {
305+
activity.next("Apply Legacy Initialisers");
306+
this.applyInitialisers(mixin);
307+
activity.next("Apply Legacy CLINIT");
308+
this.applyClinitLegacy(mixin);
309+
}
310+
});
311+
break;
312+
case INJECT_PREPARE_LEGACY:
313+
this.processMixins(mixinContexts, (activity, mixin) -> {
314+
if (FabricUtil.getCompatibility(mixin) < FabricUtil.COMPATIBILITY_0_17_1) {
315+
activity.next("Prepare Legacy Injections");
316+
this.prepareInjections(mixin);
317+
}
318+
});
319+
break;
320+
case INITIALISER_APPLY:
321+
Supplier<Clinit> targetClinit = Suppliers.memoize(this::prepareOrCreateClinit);
322+
this.processMixins(mixinContexts, (activity, mixin) -> {
323+
if (FabricUtil.getCompatibility(mixin) >= FabricUtil.COMPATIBILITY_0_17_1) {
324+
activity.next("Apply Initialisers");
325+
this.applyInitialisers(mixin);
326+
activity.next("Apply CLINIT");
327+
this.applyClinit(mixin, targetClinit);
328+
}
288329
});
289330
break;
290331
case ACCESSOR:
@@ -467,11 +508,6 @@ protected void applyNormalMethod(MixinTargetContext mixin, MethodNode mixinMetho
467508
this.checkMethodVisibility(mixin, mixinMethod);
468509
this.checkMethodConstraints(mixin, mixinMethod);
469510
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();
475511
}
476512
}
477513

@@ -723,6 +759,33 @@ protected void applyInitialisers(MixinTargetContext mixin) {
723759
}
724760
}
725761

762+
protected void applyClinitLegacy(MixinTargetContext mixin) {
763+
MethodNode clinit = Bytecode.findMethod(mixin.getClassNode(), Constants.CLINIT, "()V");
764+
if (clinit != null) {
765+
this.appendInsns(mixin, clinit);
766+
}
767+
}
768+
769+
private Clinit prepareOrCreateClinit() {
770+
MethodNode clinit = Bytecode.findMethod(this.targetClass, Constants.CLINIT, "()V");
771+
if (clinit != null) {
772+
return Clinit.prepare(this.context.getTargetMethod(clinit));
773+
}
774+
clinit = new MethodNode(ASM.API_VERSION, Opcodes.ACC_STATIC, Constants.CLINIT, "()V", null, null);
775+
InsnNode finalReturn = new InsnNode(Opcodes.RETURN);
776+
clinit.instructions.add(finalReturn);
777+
this.targetClass.methods.add(clinit);
778+
return new Clinit(clinit, finalReturn);
779+
}
780+
781+
protected void applyClinit(MixinTargetContext mixin, Supplier<Clinit> clinit) {
782+
MethodNode mixinClinit = Bytecode.findMethod(mixin.getClassNode(), Constants.CLINIT, "()V");
783+
if (mixinClinit == null) {
784+
return;
785+
}
786+
clinit.get().append(mixinClinit);
787+
}
788+
726789
/**
727790
* Scan for injector methods and injection points
728791
*

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* This file is part of Mixin, licensed under the MIT License (MIT).
3+
*
4+
* Copyright (c) SpongePowered <https://www.spongepowered.org>
5+
* Copyright (c) contributors
6+
*
7+
* Permission is hereby granted, free of charge, to any person obtaining a copy
8+
* of this software and associated documentation files (the "Software"), to deal
9+
* in the Software without restriction, including without limitation the rights
10+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
11+
* copies of the Software, and to permit persons to whom the Software is
12+
* furnished to do so, subject to the following conditions:
13+
*
14+
* The above copyright notice and this permission notice shall be included in
15+
* all copies or substantial portions of the Software.
16+
*
17+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
18+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
19+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
20+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
21+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
22+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
23+
* THE SOFTWARE.
24+
*/
25+
package org.spongepowered.asm.mixin.transformer.struct;
26+
27+
import org.objectweb.asm.Opcodes;
28+
import org.objectweb.asm.tree.*;
29+
import org.spongepowered.asm.mixin.injection.struct.InjectionNodes;
30+
import org.spongepowered.asm.mixin.injection.struct.Target;
31+
import org.spongepowered.asm.util.Bytecode;
32+
33+
import java.util.ListIterator;
34+
import java.util.Map;
35+
36+
public class Clinit {
37+
private final MethodNode clinit;
38+
private final AbstractInsnNode finalReturn;
39+
40+
public Clinit(MethodNode clinit, AbstractInsnNode finalReturn) {
41+
this.clinit = clinit;
42+
this.finalReturn = finalReturn;
43+
}
44+
45+
public void append(MethodNode mixinClinit) {
46+
prepareClinit(mixinClinit, null);
47+
Map<LabelNode, LabelNode> labels = Bytecode.cloneLabels(mixinClinit.instructions);
48+
for (AbstractInsnNode insn : mixinClinit.instructions) {
49+
if (insn.getOpcode() == Opcodes.RETURN) {
50+
continue;
51+
}
52+
this.clinit.instructions.insertBefore(this.finalReturn, insn.clone(labels));
53+
}
54+
this.clinit.maxLocals = Math.max(this.clinit.maxLocals, mixinClinit.maxLocals);
55+
this.clinit.maxStack = Math.max(this.clinit.maxStack, mixinClinit.maxStack);
56+
for (TryCatchBlockNode tryCatch : mixinClinit.tryCatchBlocks) {
57+
this.clinit.tryCatchBlocks.add(new TryCatchBlockNode(labels.get(tryCatch.start), labels.get(tryCatch.end), labels.get(tryCatch.handler), tryCatch.type));
58+
}
59+
for (LocalVariableNode local : mixinClinit.localVariables) {
60+
this.clinit.localVariables.add(new LocalVariableNode(local.name, local.desc, local.signature, labels.get(local.start), labels.get(local.end), local.index));
61+
}
62+
}
63+
64+
public static Clinit prepare(Target clinit) {
65+
return new Clinit(clinit.method, Clinit.prepareClinit(clinit.method, clinit));
66+
}
67+
68+
private static AbstractInsnNode prepareClinit(MethodNode clinit, Target target) {
69+
LabelNode endLabel = new LabelNode();
70+
AbstractInsnNode existingFinalReturn = null;
71+
for (ListIterator<AbstractInsnNode> iter = clinit.instructions.iterator(); iter.hasNext(); ) {
72+
AbstractInsnNode insn = iter.next();
73+
74+
if (insn.getOpcode() == Opcodes.RETURN) {
75+
if (insn.getNext() == null) {
76+
existingFinalReturn = insn;
77+
break;
78+
}
79+
AbstractInsnNode newInsn = new JumpInsnNode(Opcodes.GOTO, endLabel);
80+
iter.set(newInsn);
81+
82+
if (target != null) {
83+
InjectionNodes.InjectionNode injectionNode = target.getInjectionNode(insn);
84+
if (injectionNode != null) {
85+
injectionNode.replace(newInsn);
86+
}
87+
}
88+
}
89+
}
90+
if (existingFinalReturn != null) {
91+
clinit.instructions.insertBefore(existingFinalReturn, endLabel);
92+
return existingFinalReturn;
93+
}
94+
clinit.instructions.add(endLabel);
95+
InsnNode finalReturn = new InsnNode(Opcodes.RETURN);
96+
clinit.instructions.add(finalReturn);
97+
return finalReturn;
98+
}
99+
}

0 commit comments

Comments
 (0)