Skip to content

Commit

Permalink
Improve replacement injection point locator
Browse files Browse the repository at this point in the history
  • Loading branch information
@Su5eD
Su5eD committed Jul 14, 2024
1 parent 07891c4 commit e8f6c7e
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 20 deletions.
65 changes: 45 additions & 20 deletions ...main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixArbitraryInjectionPoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,19 @@
import org.sinytra.adapter.patch.api.MixinConstants;
import org.sinytra.adapter.patch.api.Patch;
import org.sinytra.adapter.patch.transformer.ModifyInjectionPoint;
import org.sinytra.adapter.patch.transformer.ModifyInjectionTarget;
import org.sinytra.adapter.patch.util.AdapterUtil;

import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import java.util.function.UnaryOperator;

public class DynFixArbitraryInjectionPoint implements DynamicFixer<DynFixArbitraryInjectionPoint.Data> {
private static final Set<String> ACCEPTED_ANNOTATIONS = Set.of(MixinConstants.INJECT, MixinConstants.MODIFY_EXPR_VAL);
private static final Set<String> ACCEPTED_ANNOTATIONS = Set.of(MixinConstants.INJECT, MixinConstants.MODIFY_ARG, MixinConstants.MODIFY_EXPR_VAL);

public record Data(MethodContext.TargetPair dirtyTarget, AbstractInsnNode cleanInjectionInsn) {
}
public record Data(MethodContext.TargetPair dirtyTarget, AbstractInsnNode cleanInjectionInsn) {}

@Nullable
@Override
Expand All @@ -30,7 +32,8 @@ public Data prepare(MethodContext methodContext) {
List<AbstractInsnNode> cleanInsns = methodContext.findInjectionTargetInsns(cleanInjectionTarget);
if (cleanInsns.size() == 1 && methodContext.failsDirtyInjectionCheck()) {
MethodContext.TargetPair dirtyInjectionTarget = methodContext.findDirtyInjectionTarget();
return new Data(dirtyInjectionTarget, cleanInsns.getFirst());
AbstractInsnNode cleanInjectionInsn = cleanInsns.getFirst();
return new Data(dirtyInjectionTarget, cleanInjectionInsn);
}
}
return null;
Expand All @@ -41,10 +44,41 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont
MethodNode dirtyTargetMethod = data.dirtyTarget().methodNode();
AbstractInsnNode cleanInjectionInsn = data.cleanInjectionInsn();

AbstractInsnNode nextCallCandidate = findCandidates(MethodCallAnalyzer.findBackwardsInstructions(cleanInjectionInsn, 5).inverse(), dirtyTargetMethod, List::getLast);
MethodInsnNode targetMethodCall;
if (nextCallCandidate != null) {
targetMethodCall = findReplacementInjectionPoint(nextCallCandidate, AbstractInsnNode::getNext, methodContext);
} else {
AbstractInsnNode previousCallCandidate = findCandidates(MethodCallAnalyzer.findForwardInstructions(cleanInjectionInsn, 5), dirtyTargetMethod, List::getFirst);
if (previousCallCandidate != null) {
targetMethodCall = findReplacementInjectionPoint(previousCallCandidate, AbstractInsnNode::getPrevious, methodContext);
} else {
return Patch.Result.PASS;
}
}

if (targetMethodCall != null) {
// New method is in the same class? It's possible our target injection point was moved there
if (targetMethodCall.owner.equals(data.dirtyTarget().classNode().name) && !methodContext.methodAnnotation().matchesDesc(MixinConstants.INJECT)) {
return tryMoveTargetMethod(classNode, methodNode, targetMethodCall, methodContext);
}

String newInjectionPoint = Type.getObjectType(targetMethodCall.owner).getDescriptor() + targetMethodCall.name + targetMethodCall.desc;
return new ModifyInjectionPoint("INVOKE", newInjectionPoint, true, false).apply(classNode, methodNode, methodContext);
}

return Patch.Result.PASS;
}

private static Patch.Result tryMoveTargetMethod(ClassNode classNode, MethodNode methodNode, MethodInsnNode insn, MethodContext methodContext) {
String newTarget = insn.name + insn.desc;
return new ModifyInjectionTarget(List.of(newTarget)).apply(classNode, methodNode, methodContext);
}

private static AbstractInsnNode findCandidates(InstructionMatcher cleanMatcher, MethodNode dirtyTargetMethod, Function<List<AbstractInsnNode>, AbstractInsnNode> selector) {
// Find an common instruction in the clean and dirty target methods
InstructionMatcher cleanMatcher = MethodCallAnalyzer.findBackwardsInstructions(cleanInjectionInsn, 5).inverse();
if (cleanMatcher.after().isEmpty()) {
return Patch.Result.PASS;
return null;
}
int firstOpcode = cleanMatcher.after().getFirst().getOpcode();

Expand All @@ -59,32 +93,23 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont
InstructionMatcher dirtyMatcher = MethodCallAnalyzer.findForwardInstructionsDirect(insn, 5);
if (cleanMatcher.test(dirtyMatcher, InsnComparator.IGNORE_VAR_INDEX)) {
// Find first method call past matched instruction
AbstractInsnNode lastInsn = dirtyMatcher.after().getLast();
AbstractInsnNode lastInsn = selector.apply(dirtyMatcher.after());
if (lastInsn != null) {
candidates.add(lastInsn);
}
}
}

if (candidates.size() == 1) {
AbstractInsnNode lastInsn = candidates.getFirst();
MethodInsnNode nextMethodCall = findReplacementInjectionPoint(lastInsn, methodContext);
if (nextMethodCall != null) {
String newInjectionPoint = Type.getObjectType(nextMethodCall.owner).getDescriptor() + nextMethodCall.name + nextMethodCall.desc;
return new ModifyInjectionPoint("INVOKE", newInjectionPoint, true, false).apply(classNode, methodNode, methodContext);
}
}

return Patch.Result.PASS;
return !candidates.isEmpty() ? candidates.getFirst() : null;
}

private static MethodInsnNode findReplacementInjectionPoint(AbstractInsnNode lastInsn, MethodContext methodContext) {
private static MethodInsnNode findReplacementInjectionPoint(AbstractInsnNode lastInsn, UnaryOperator<AbstractInsnNode> flow, MethodContext methodContext) {
// Require matching return types for ModifyExpressionValue mixins
if (methodContext.methodAnnotation().matchesDesc(MixinConstants.MODIFY_EXPR_VAL)) {
Type desiredReturnType = Type.getReturnType(methodContext.getInjectionPointMethodQualifier().desc());
return (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, AbstractInsnNode::getNext, v -> v instanceof MethodInsnNode minsn && Type.getReturnType(minsn.desc).equals(desiredReturnType));
return (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, flow, v -> v instanceof MethodInsnNode minsn && Type.getReturnType(minsn.desc).equals(desiredReturnType));
} else {
return (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, AbstractInsnNode::getNext, v -> v instanceof MethodInsnNode);
return (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, flow, v -> v instanceof MethodInsnNode);
}
}
}
10 changes: 10 additions & 0 deletions test/src/test/java/org/sinytra/adapter/patch/test/mixin/DynamicMixinPatchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ void testMovedInjectionPointToMethod3() throws Exception {
);
}

@Test
void testMovedInjectionPointToMethod4() throws Exception {
assertSameCode(
"org/sinytra/adapter/test/mixin/AbstractMinecartMixin",
"modifiedMovement",
assertTargetMethod(),
assertInjectionPoint()
);
}

@Test
void testUpdatedInjectionPointFieldToMethod() throws Exception {
assertSameCode(
Expand Down
20 changes: 20 additions & 0 deletions test/src/testClasses/java/org/sinytra/adapter/test/mixin/AbstractMinecartMixin.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package org.sinytra.adapter.test.mixin;

import net.minecraft.world.entity.vehicle.AbstractMinecart;
import net.minecraft.world.phys.Vec3;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.ModifyArg;

@Mixin(AbstractMinecart.class)
public class AbstractMinecartMixin {
@ModifyArg(method = "moveAlongTrack", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/vehicle/AbstractMinecart;move(Lnet/minecraft/world/entity/MoverType;Lnet/minecraft/world/phys/Vec3;)V", ordinal = 0))
private Vec3 modifiedMovement(Vec3 movement) {
return movement;
}

@ModifyArg(method = "moveMinecartOnRail(Lnet/minecraft/core/BlockPos;)V", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/vehicle/AbstractMinecart;move(Lnet/minecraft/world/entity/MoverType;Lnet/minecraft/world/phys/Vec3;)V", ordinal = 0))
private Vec3 modifiedMovementExpected(Vec3 movement) {
return movement;
}
}

0 comments on commit e8f6c7e

Please sign in to comment.