From b09c98e3d2ec766ae151b449fe89bebbc445a4aa Mon Sep 17 00:00:00 2001 From: Su5eD Date: Fri, 12 Jul 2024 17:54:34 +0200 Subject: [PATCH] Dynamic fix for ModifyExpressionValue mixins --- .../patch/analysis/MethodCallAnalyzer.java | 6 +++ .../dynfix/DynFixArbitraryInjectionPoint.java | 38 ++++++++++++++----- .../dynfix/DynFixAtVariableAssignStore.java | 2 - .../test/mixin/DynamicMixinPatchTest.java | 21 +++++++++- .../test/mixin/FarmLandBlockMixin.java | 29 ++++++++++++++ .../adapter/test/mixin/HoeItemMixin.java | 2 +- .../test/mixin/MilkBucketItemMixin.java | 24 ++++++++++++ 7 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 test/src/testClasses/java/org/sinytra/adapter/test/mixin/FarmLandBlockMixin.java create mode 100644 test/src/testClasses/java/org/sinytra/adapter/test/mixin/MilkBucketItemMixin.java diff --git a/definition/src/main/java/org/sinytra/adapter/patch/analysis/MethodCallAnalyzer.java b/definition/src/main/java/org/sinytra/adapter/patch/analysis/MethodCallAnalyzer.java index 3ab924c..2c486f5 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/analysis/MethodCallAnalyzer.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/analysis/MethodCallAnalyzer.java @@ -54,6 +54,12 @@ public static InstructionMatcher findForwardInstructions(AbstractInsnNode insn, return new InstructionMatcher(insn, List.of(), nextInsns); } + public static InstructionMatcher findForwardInstructionsDirect(AbstractInsnNode insn, int range) { + List nextInsns = getInsns(insn, range, FORWARD); + + return new InstructionMatcher(insn, List.of(), nextInsns); + } + private static List getInsns(AbstractInsnNode root, int range, UnaryOperator operator) { return Stream.iterate(root, Objects::nonNull, operator) .filter(insn -> !(insn instanceof FrameNode) && !(insn instanceof LineNumberNode)) diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixArbitraryInjectionPoint.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixArbitraryInjectionPoint.java index ff898d0..7b64098 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixArbitraryInjectionPoint.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixArbitraryInjectionPoint.java @@ -2,10 +2,7 @@ import org.jetbrains.annotations.Nullable; import org.objectweb.asm.Type; -import org.objectweb.asm.tree.AbstractInsnNode; -import org.objectweb.asm.tree.ClassNode; -import org.objectweb.asm.tree.MethodInsnNode; -import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.*; import org.sinytra.adapter.patch.analysis.InsnComparator; import org.sinytra.adapter.patch.analysis.InstructionMatcher; import org.sinytra.adapter.patch.analysis.MethodCallAnalyzer; @@ -15,16 +12,20 @@ import org.sinytra.adapter.patch.transformer.ModifyInjectionPoint; import org.sinytra.adapter.patch.util.AdapterUtil; +import java.util.ArrayList; import java.util.List; +import java.util.Set; public class DynFixArbitraryInjectionPoint implements DynamicFixer { + private static final Set ACCEPTED_ANNOTATIONS = Set.of(MixinConstants.INJECT, MixinConstants.MODIFY_EXPR_VAL); + public record Data(MethodContext.TargetPair dirtyTarget, AbstractInsnNode cleanInjectionInsn) { } @Nullable @Override public Data prepare(MethodContext methodContext) { - if (methodContext.methodAnnotation().matchesDesc(MixinConstants.INJECT)) { + if (methodContext.methodAnnotation().matchesAny(ACCEPTED_ANNOTATIONS)) { MethodContext.TargetPair cleanInjectionTarget = methodContext.findCleanInjectionTarget(); List cleanInsns = methodContext.findInjectionTargetInsns(cleanInjectionTarget); if (cleanInsns.size() == 1 && methodContext.failsDirtyInjectionCheck()) { @@ -47,24 +48,41 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont } int firstOpcode = cleanMatcher.after().getFirst().getOpcode(); + List candidates = new ArrayList<>(); + for (int i = 0; i < dirtyTargetMethod.instructions.size(); i++) { AbstractInsnNode insn = dirtyTargetMethod.instructions.get(i); - if (insn.getOpcode() != firstOpcode) { + if (insn instanceof FrameNode || insn instanceof LineNumberNode || insn.getOpcode() != firstOpcode) { continue; } - InstructionMatcher dirtyMatcher = MethodCallAnalyzer.findForwardInstructions(insn, 5); + 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(); if (lastInsn != null) { - MethodInsnNode nextMethodCall = (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, AbstractInsnNode::getNext, v -> v instanceof MethodInsnNode); - String newInjectionPoint = Type.getObjectType(nextMethodCall.owner).getDescriptor() + nextMethodCall.name + nextMethodCall.desc; - return new ModifyInjectionPoint("INVOKE", newInjectionPoint, true, false).apply(classNode, methodNode, methodContext); + candidates.add(lastInsn); } } } + if (candidates.size() == 1) { + AbstractInsnNode lastInsn = candidates.getFirst(); + MethodInsnNode nextMethodCall = findReplacementInjectionPoint(lastInsn, methodContext); + 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; } + + private static MethodInsnNode findReplacementInjectionPoint(AbstractInsnNode lastInsn, 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)); + } else { + return (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, AbstractInsnNode::getNext, v -> v instanceof MethodInsnNode); + } + } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixAtVariableAssignStore.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixAtVariableAssignStore.java index a72747d..90c4697 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixAtVariableAssignStore.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixAtVariableAssignStore.java @@ -14,8 +14,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -import java.util.function.Predicate; -import java.util.function.UnaryOperator; /** * Find our new injection point with relation to variable assignments diff --git a/test/src/test/java/org/sinytra/adapter/patch/test/mixin/DynamicMixinPatchTest.java b/test/src/test/java/org/sinytra/adapter/patch/test/mixin/DynamicMixinPatchTest.java index f2c61db..59f8860 100644 --- a/test/src/test/java/org/sinytra/adapter/patch/test/mixin/DynamicMixinPatchTest.java +++ b/test/src/test/java/org/sinytra/adapter/patch/test/mixin/DynamicMixinPatchTest.java @@ -58,7 +58,26 @@ void testUpdatedInjectionTargetSamePoint() throws Exception { void testUpdatedInjectionPointFieldToMethod() throws Exception { assertSameCode( "org/sinytra/adapter/test/mixin/HoeItemMixin", - "injectUseOn" + "injectUseOn", + assertInjectionPoint() + ); + } + + @Test + void testUpdatedInjectionPoint2() throws Exception { + assertSameCode( + "org/sinytra/adapter/test/mixin/MilkBucketItemMixin", + "onClearStatusEffect", + assertInjectionPoint() + ); + } + + @Test + void testUpdatedInjectionPointModifyExprVal() throws Exception { + assertSameCode( + "org/sinytra/adapter/test/mixin/FarmLandBlockMixin", + "isFarmlandNearWater", + assertInjectionPoint() ); } diff --git a/test/src/testClasses/java/org/sinytra/adapter/test/mixin/FarmLandBlockMixin.java b/test/src/testClasses/java/org/sinytra/adapter/test/mixin/FarmLandBlockMixin.java new file mode 100644 index 0000000..3e47fab --- /dev/null +++ b/test/src/testClasses/java/org/sinytra/adapter/test/mixin/FarmLandBlockMixin.java @@ -0,0 +1,29 @@ +package org.sinytra.adapter.test.mixin; + +import com.llamalad7.mixinextras.injector.ModifyExpressionValue; +import com.llamalad7.mixinextras.sugar.Local; +import net.minecraft.core.BlockPos; +import net.minecraft.world.level.LevelReader; +import net.minecraft.world.level.block.FarmBlock; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; + +@Mixin(FarmBlock.class) +public class FarmLandBlockMixin { + // https://github.com/warior456/Sculk-Depths/blob/06870f1ab93b8d500bbbea285eebbfa7db6a5f89/src/main/java/net/ugi/sculk_depths/mixin/FarmLandBlockMixin.java#L23 + @ModifyExpressionValue( + method = "isNearWater", + at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/material/FluidState;is(Lnet/minecraft/tags/TagKey;)Z") + ) + private static boolean isFarmlandNearWater(boolean original, @Local(ordinal = 0) LevelReader world, @Local(ordinal = 1) BlockPos blockPos) { + return original; + } + + @ModifyExpressionValue( + method = "isNearWater", + at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/BlockState;canBeHydrated(Lnet/minecraft/world/level/BlockGetter;Lnet/minecraft/core/BlockPos;Lnet/minecraft/world/level/material/FluidState;Lnet/minecraft/core/BlockPos;)Z") + ) + private static boolean isFarmlandNearWaterExpected(boolean original, @Local(ordinal = 0) LevelReader world, @Local(ordinal = 1) BlockPos blockPos) { + return original; + } +} diff --git a/test/src/testClasses/java/org/sinytra/adapter/test/mixin/HoeItemMixin.java b/test/src/testClasses/java/org/sinytra/adapter/test/mixin/HoeItemMixin.java index 9f97b22..8e3befc 100644 --- a/test/src/testClasses/java/org/sinytra/adapter/test/mixin/HoeItemMixin.java +++ b/test/src/testClasses/java/org/sinytra/adapter/test/mixin/HoeItemMixin.java @@ -26,7 +26,7 @@ public void injectUseOn(UseOnContext itemUsageContext, CallbackInfoReturnable cir) { + // Noop + } + + @Inject(method = "finishUsingItem", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/LivingEntity;removeEffectsCuredBy(Lnet/neoforged/neoforge/common/EffectCure;)Z")) + private void onClearStatusEffectExpected(ItemStack stack, Level level, LivingEntity user, CallbackInfoReturnable cir) { + // Noop + } +}