From ab5ba82dd9f51dc3739cddcd071cb258eb5333ca Mon Sep 17 00:00:00 2001 From: Su5eD Date: Wed, 24 Jul 2024 21:02:40 +0200 Subject: [PATCH] Implement patch audit trail --- .../adapter/patch/MethodContextImpl.java | 10 +- .../adapter/patch/PatchAuditTrailImpl.java | 132 ++++++++++++++++++ .../adapter/patch/PatchEnvironmentImpl.java | 16 ++- .../sinytra/adapter/patch/PatchInstance.java | 3 + .../adapter/patch/api/MethodContext.java | 4 +- .../org/sinytra/adapter/patch/api/Patch.java | 4 - .../adapter/patch/api/PatchAuditTrail.java | 33 +++++ .../adapter/patch/api/PatchEnvironment.java | 2 + .../adapter/patch/fixes/MethodUpgrader.java | 47 ++++++- .../DynamicAnonymousShadowFieldTypePatch.java | 8 +- .../DynamicInheritedInjectionPointPatch.java | 4 +- .../dynamic/DynamicInjectorOrdinalPatch.java | 12 +- .../transformer/dynamic/DynamicLVTPatch.java | 13 +- .../DynamicModifyVarAtReturnPatch.java | 9 +- .../dynfix/DynFixArbitraryInjectionPoint.java | 15 +- .../dynfix/DynFixAtVariableAssignStore.java | 34 ++--- .../dynfix/DynFixMethodComparison.java | 56 ++++---- .../dynfix/DynFixParameterTypeAdapter.java | 8 +- .../dynfix/DynFixResolveAmbigousTarget.java | 20 +-- .../dynfix/DynFixSliceBoundary.java | 14 +- .../transformer/dynfix/DynFixSplitMethod.java | 20 ++- .../transformer/dynfix/DynamicFixer.java | 19 ++- .../dynfix/DynamicInjectionPointPatch.java | 22 +-- .../transformer/operation/DisableMixin.java | 7 +- .../transformer/operation/ExtractMixin.java | 2 + .../operation/ModifyInjectionPoint.java | 11 +- .../operation/ModifyInjectionTarget.java | 19 +-- .../operation/ModifyMethodAccess.java | 11 +- .../operation/ModifyMethodParams.java | 23 ++- .../operation/ModifyMixinType.java | 1 + .../operation/ModifyTargetClasses.java | 1 + .../operation/param/TransformParameters.java | 2 +- .../adapter/patch/util/AdapterUtil.java | 1 - .../test/mixin/DynamicMixinPatchTest.java | 56 +++++--- .../test/mixin/MinecraftMixinPatchTest.java | 2 +- 35 files changed, 436 insertions(+), 205 deletions(-) create mode 100644 definition/src/main/java/org/sinytra/adapter/patch/PatchAuditTrailImpl.java create mode 100644 definition/src/main/java/org/sinytra/adapter/patch/api/PatchAuditTrail.java diff --git a/definition/src/main/java/org/sinytra/adapter/patch/MethodContextImpl.java b/definition/src/main/java/org/sinytra/adapter/patch/MethodContextImpl.java index 6ea1bef..826e46d 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/MethodContextImpl.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/MethodContextImpl.java @@ -9,6 +9,7 @@ import org.objectweb.asm.tree.*; import org.sinytra.adapter.patch.analysis.LocalVariableLookup; import org.sinytra.adapter.patch.api.MethodContext; +import org.sinytra.adapter.patch.api.MethodTransform; import org.sinytra.adapter.patch.api.MixinConstants; import org.sinytra.adapter.patch.api.PatchContext; import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; @@ -127,10 +128,10 @@ public List findInjectionTargetInsns(@Nullable TargetPair targ } @Override - public void updateDescription(List parameters) { + public void updateDescription(MethodTransform transform, List parameters) { Type returnType = Type.getReturnType(this.methodNode.desc); String newDesc = Type.getMethodDescriptor(returnType, parameters.toArray(Type[]::new)); - LOGGER.info(PatchInstance.MIXINPATCH, "Changing descriptor of method {}.{}{} to {}", this.classNode.name, this.methodNode.name, this.methodNode.desc, newDesc); + recordAudit(transform, "Change descriptor to %s", newDesc); this.methodNode.desc = newDesc; this.methodNode.signature = null; } @@ -224,6 +225,11 @@ public boolean hasInjectionPointValue(String value) { return this.injectionPointAnnotation != null && this.injectionPointAnnotation.getValue("value").map(v -> value.equals(v.get())).orElse(false); } + @Override + public void recordAudit(Object transform, String message, Object... args) { + this.patchContext.environment().auditTrail().recordAudit(transform, this, message, args); + } + private InsnList getSlicedInsns(AnnotationHandle parentAnnotation, ClassNode classNode, MethodNode injectorMethod, ClassNode targetClass, MethodNode targetMethod, PatchContext context, Target mixinTarget) { return parentAnnotation.getValue("slice") .map(handle -> { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/PatchAuditTrailImpl.java b/definition/src/main/java/org/sinytra/adapter/patch/PatchAuditTrailImpl.java new file mode 100644 index 0000000..8cd6514 --- /dev/null +++ b/definition/src/main/java/org/sinytra/adapter/patch/PatchAuditTrailImpl.java @@ -0,0 +1,132 @@ +package org.sinytra.adapter.patch; + +import com.mojang.logging.LogUtils; +import it.unimi.dsi.fastutil.Pair; +import org.jetbrains.annotations.Nullable; +import org.objectweb.asm.tree.ClassNode; +import org.objectweb.asm.tree.MethodNode; +import org.sinytra.adapter.patch.api.MethodContext; +import org.sinytra.adapter.patch.api.PatchAuditTrail; +import org.slf4j.Logger; + +import java.text.DecimalFormat; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; + +public class PatchAuditTrailImpl implements PatchAuditTrail { + private static final DecimalFormat FORMAT = new DecimalFormat("##.00"); + private static final Logger LOGGER = LogUtils.getLogger(); + private final Map auditTrail = new LinkedHashMap<>(); + private final Map candidates = new ConcurrentHashMap<>(); + + public void prepareMethod(MethodContext methodContext) { + Candidate candidate = new Candidate(methodContext.getMixinClass(), methodContext.getMixinMethod()); + synchronized (this.auditTrail) { + this.auditTrail.put(candidate, AuditLog.create(methodContext)); + } + } + + public void recordAudit(Object transform, ClassNode classNode, String message, Object... args) { + Candidate candidate = new Candidate(classNode, null); + recordAudit(transform, null, candidate, message.formatted(args)); + } + + public void recordAudit(Object transform, MethodContext methodContext, String message, Object... args) { + Candidate candidate = new Candidate(methodContext.getMixinClass(), methodContext.getMixinMethod()); + recordAudit(transform, methodContext.getMixinMethod(), candidate, message.formatted(args)); + } + + private void recordAudit(Object transform, @Nullable MethodNode methodNode, Candidate candidate, String message) { + synchronized (this.auditTrail) { + AuditLog auditLog = this.auditTrail.computeIfAbsent(candidate, k -> new AuditLog(methodNode != null ? methodNode.name + methodNode.desc : null, new ArrayList<>())); + List> entries = auditLog.entries(); + + StringBuilder builder; + if (entries.isEmpty() || entries.getLast().left() != transform) { + entries.add(Pair.of(transform, builder = new StringBuilder())); + builder.append("\n >> Using ").append(transform.getClass().getName()); + } else { + builder = entries.getLast().right(); + } + + builder.append("\n - ").append(message); + LOGGER.info(MIXINPATCH, "Applying [{}] {}", transform.getClass().getSimpleName(), message); + } + } + + public void recordResult(MethodContext methodContext, Match match) { + Candidate candidate = new Candidate(methodContext.getMixinClass(), methodContext.getMixinMethod()); + this.candidates.compute(candidate, (key, prev) -> prev == null ? match : prev.or(match)); + } + + public String getCompleteReport() { + StringBuilder builder = new StringBuilder(); + + getSummaryLines().forEach(l -> builder.append(l).append("\n")); + + List> failed = this.candidates.entrySet().stream().filter(m -> m.getValue() == Match.NONE).toList(); + if (!failed.isEmpty()) { + builder.append("\n=============== Failed mixins ==============="); + failed.forEach(e -> builder.append("\n").append(e.getKey().classNode().name).append("\t").append(e.getKey().methodNode().name).append(e.getKey().methodNode().desc)); + builder.append("\n=============================================\n\n"); + } else { + builder.append("\n"); + } + + this.auditTrail.forEach((candidate, auditLog) -> { + if (auditLog.entries().isEmpty()) { + return; + } + + if (auditLog.originalMethod() == null) { + builder.append("Mixin class ").append(candidate.classNode().name); + } else { + builder.append("Mixin method ").append(candidate.classNode().name).append(" ").append(auditLog.originalMethod()); + } + + for (Pair record : auditLog.entries()) { + builder.append(record.right()); + } + + builder.append("\n\n"); + }); + + return builder.toString(); + } + + private List getSummaryLines() { + int total = this.candidates.size(); + int successful = (int) this.candidates.values().stream().filter(m -> m == Match.FULL).count(); + int partial = (int) this.candidates.values().stream().filter(m -> m == Match.PARTIAL).count(); + int failed = (int) this.candidates.values().stream().filter(m -> m == Match.NONE).count(); + double rate = (successful + partial) / (double) total * 100; + double accuracy = (successful / (double) total + partial / (double) total / 2.0) * 100; + + return List.of( + "==== Connector Mixin Patch Audit Summary ====", + "Successful: %s".formatted(successful), + "Partial: %s".formatted(partial), + "Failed: %s".formatted(failed), + "Success rate: %s%% Accuracy: %s%%".formatted(FORMAT.format(rate), FORMAT.format(accuracy)), + "=============================================" + ); + } + + public boolean hasFailingMixins() { + return this.candidates.containsValue(Match.NONE); + } + + record AuditLog(@Nullable String originalMethod, List> entries) { + public static AuditLog create(MethodContext methodContext) { + return new AuditLog(methodContext.getMixinMethod().name + methodContext.getMixinMethod().desc, new ArrayList<>()); + } + } + + record Candidate(ClassNode classNode, MethodNode methodNode) { + } +} diff --git a/definition/src/main/java/org/sinytra/adapter/patch/PatchEnvironmentImpl.java b/definition/src/main/java/org/sinytra/adapter/patch/PatchEnvironmentImpl.java index 3084136..5af8f64 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/PatchEnvironmentImpl.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/PatchEnvironmentImpl.java @@ -3,20 +3,28 @@ import org.jetbrains.annotations.Nullable; import org.sinytra.adapter.patch.analysis.InheritanceHandler; import org.sinytra.adapter.patch.api.MixinClassGenerator; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.api.PatchEnvironment; import org.sinytra.adapter.patch.api.RefmapHolder; import org.sinytra.adapter.patch.fixes.BytecodeFixerUpper; import org.sinytra.adapter.patch.util.provider.ClassLookup; import org.sinytra.adapter.patch.util.provider.MixinClassLookup; -public record PatchEnvironmentImpl(RefmapHolder refmapHolder, ClassLookup cleanClassLookup, ClassLookup dirtyClassLookup, @Nullable BytecodeFixerUpper bytecodeFixerUpper, - MixinClassGenerator classGenerator, InheritanceHandler inheritanceHandler, int fabricLVTCompatibility) implements PatchEnvironment { +public record PatchEnvironmentImpl( + RefmapHolder refmapHolder, + ClassLookup cleanClassLookup, ClassLookup dirtyClassLookup, + @Nullable BytecodeFixerUpper bytecodeFixerUpper, + MixinClassGenerator classGenerator, + InheritanceHandler inheritanceHandler, + int fabricLVTCompatibility, + PatchAuditTrail auditTrail +) implements PatchEnvironment { public PatchEnvironmentImpl(RefmapHolder refmapHolder, ClassLookup cleanClassLookup, @Nullable BytecodeFixerUpper bytecodeFixerUpper, int fabricLVTCompatibility) { - this(refmapHolder, cleanClassLookup, MixinClassLookup.INSTANCE, bytecodeFixerUpper, new MixinClassGeneratorImpl(), new InheritanceHandler(MixinClassLookup.INSTANCE), fabricLVTCompatibility); + this(refmapHolder, cleanClassLookup, MixinClassLookup.INSTANCE, bytecodeFixerUpper, new MixinClassGeneratorImpl(), new InheritanceHandler(MixinClassLookup.INSTANCE), fabricLVTCompatibility, new PatchAuditTrailImpl()); } public PatchEnvironmentImpl(RefmapHolder refmapHolder, ClassLookup cleanClassLookup, ClassLookup dirtyClassLookup, @Nullable BytecodeFixerUpper bytecodeFixerUpper, int fabricLVTCompatibility) { - this(refmapHolder, cleanClassLookup, dirtyClassLookup, bytecodeFixerUpper, new MixinClassGeneratorImpl(), new InheritanceHandler(MixinClassLookup.INSTANCE), fabricLVTCompatibility); + this(refmapHolder, cleanClassLookup, dirtyClassLookup, bytecodeFixerUpper, new MixinClassGeneratorImpl(), new InheritanceHandler(MixinClassLookup.INSTANCE), fabricLVTCompatibility, new PatchAuditTrailImpl()); } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/PatchInstance.java b/definition/src/main/java/org/sinytra/adapter/patch/PatchInstance.java index da57adc..e9d9a71 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/PatchInstance.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/PatchInstance.java @@ -63,6 +63,9 @@ public Result apply(ClassNode classNode, PatchEnvironment environment) { for (MethodNode method : classNode.methods) { MethodContext methodContext = checkMethodTarget(classAnnotation, classNode, method, environment, classTarget.targetTypes(), context); if (methodContext != null) { + if (!this.transforms.isEmpty()) { + environment.auditTrail().prepareMethod(methodContext); + } for (MethodTransform transform : this.transforms) { Collection accepted = transform.getAcceptedAnnotations(); if (accepted.isEmpty() || accepted.contains(methodContext.methodAnnotation().getDesc())) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/api/MethodContext.java b/definition/src/main/java/org/sinytra/adapter/patch/api/MethodContext.java index 7c8dc76..52e8e4b 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/api/MethodContext.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/api/MethodContext.java @@ -57,7 +57,7 @@ public interface MethodContext { @Nullable Pair> findInjectionTargetCandidates(ClassLookup lookup, boolean ignoreDesc); - void updateDescription(List parameters); + void updateDescription(MethodTransform transform, List parameters); boolean isStatic(); @@ -80,6 +80,8 @@ default List getTargetMethodLocals(TargetPair target, int startPo boolean hasInjectionPointValue(String value); + void recordAudit(Object transform, String message, Object... args); + record LocalVariable(int index, Type type) {} record TargetPair(ClassNode classNode, MethodNode methodNode) {} diff --git a/definition/src/main/java/org/sinytra/adapter/patch/api/Patch.java b/definition/src/main/java/org/sinytra/adapter/patch/api/Patch.java index e5f20a5..c59d789 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/api/Patch.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/api/Patch.java @@ -42,10 +42,6 @@ public Result or(Result other) { } return this; } - - public Result orElseGet(Supplier other) { - return this == PASS ? other.get() : this; - } } interface Builder> extends MethodTransformBuilder { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/api/PatchAuditTrail.java b/definition/src/main/java/org/sinytra/adapter/patch/api/PatchAuditTrail.java new file mode 100644 index 0000000..b63090b --- /dev/null +++ b/definition/src/main/java/org/sinytra/adapter/patch/api/PatchAuditTrail.java @@ -0,0 +1,33 @@ +package org.sinytra.adapter.patch.api; + +import org.objectweb.asm.tree.ClassNode; + +public interface PatchAuditTrail { + void prepareMethod(MethodContext methodContext); + + void recordAudit(Object transform, ClassNode classNode, String message, Object... args); + + void recordAudit(Object transform, MethodContext methodContext, String message, Object... args); + + void recordResult(MethodContext methodContext, Match match); + + String getCompleteReport(); + + boolean hasFailingMixins(); + + enum Match { + NONE, + PARTIAL, + FULL; + + public Match or(Match other) { + if (this == NONE && other != NONE) { + return other; + } + if (this == PARTIAL && other == FULL) { + return FULL; + } + return this; + } + } +} diff --git a/definition/src/main/java/org/sinytra/adapter/patch/api/PatchEnvironment.java b/definition/src/main/java/org/sinytra/adapter/patch/api/PatchEnvironment.java index 2ebf822..fd9867d 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/api/PatchEnvironment.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/api/PatchEnvironment.java @@ -29,4 +29,6 @@ static PatchEnvironment create(RefmapHolder refmapHolder, ClassLookup cleanClass RefmapHolder refmapHolder(); int fabricLVTCompatibility(); + + PatchAuditTrail auditTrail(); } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java b/definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java index 92c16b6..a929fd8 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java @@ -1,24 +1,28 @@ package org.sinytra.adapter.patch.fixes; import com.google.common.collect.ImmutableList; +import com.mojang.datafixers.util.Pair; import org.objectweb.asm.Type; import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.AnnotationNode; import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; import org.sinytra.adapter.patch.analysis.LocalVarAnalyzer; import org.sinytra.adapter.patch.analysis.params.EnhancedParamsDiff; import org.sinytra.adapter.patch.analysis.params.LayeredParamsDiffSnapshot; import org.sinytra.adapter.patch.analysis.params.SimpleParamsDiffSnapshot; +import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; +import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; import org.sinytra.adapter.patch.api.MethodContext; import org.sinytra.adapter.patch.api.MethodTransform; import org.sinytra.adapter.patch.api.MixinConstants; -import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; import org.sinytra.adapter.patch.transformer.operation.param.ParamTransformTarget; import org.sinytra.adapter.patch.transformer.operation.param.ParameterTransformer; import org.sinytra.adapter.patch.transformer.operation.param.TransformParameters; import org.sinytra.adapter.patch.util.AdapterUtil; import org.sinytra.adapter.patch.util.MethodQualifier; +import java.util.Comparator; import java.util.List; public final class MethodUpgrader { @@ -32,10 +36,47 @@ public static void upgradeMethod(MethodNode methodNode, MethodContext methodCont if (dirtyQualifier == null) { return; } - if (methodContext.methodAnnotation().matchesDesc(MixinConstants.MODIFY_ARGS)) { + AnnotationHandle annotation = methodContext.methodAnnotation(); + if (annotation.matchesDesc(MixinConstants.MODIFY_ARGS)) { ModifyArgsOffsetTransformer.handleModifiedDesc(methodNode, cleanQualifier.desc(), dirtyQualifier.desc()); - } else if (methodContext.methodAnnotation().matchesDesc(MixinConstants.WRAP_OPERATION)) { + } else if (annotation.matchesDesc(MixinConstants.WRAP_OPERATION)) { upgradeWrapOperation(methodNode, methodContext, cleanQualifier, dirtyQualifier); + } else if (annotation.matchesDesc(MixinConstants.MODIFY_EXPR_VAL)) { + upgradeModifyExpValue(methodNode, methodContext, cleanQualifier, dirtyQualifier); + } + } + + private static void upgradeModifyExpValue(MethodNode methodNode, MethodContext methodContext, MethodQualifier cleanQualifier, MethodQualifier dirtyQualifier) { + if (dirtyQualifier.desc() == null || cleanQualifier.desc() == null) { + return; + } + List originalTargetDesc = List.of(Type.getArgumentTypes(cleanQualifier.desc())); + List modifiedTargetDesc = List.of(Type.getArgumentTypes(dirtyQualifier.desc())); + List originalDesc = List.of(Type.getArgumentTypes(methodNode.desc)); + final List originalDescRef = originalDesc; + List> localAnnotations = AdapterUtil.getAnnotatedParameters(methodNode, originalDesc.toArray(Type[]::new), MixinConstants.LOCAL, Pair::of).stream() + .sorted(Comparator.comparingInt(i -> originalDescRef.indexOf(i.getSecond()))) + .toList(); + if (!localAnnotations.isEmpty()) { + originalDesc = originalDesc.subList(0, originalDesc.indexOf(localAnnotations.getFirst().getSecond())); + } + if (originalDesc.size() == 1) { + return; + } + + int capturedParams = Math.min(originalTargetDesc.size(), originalDesc.size() - 1); + int popParams = originalTargetDesc.size() - capturedParams; + List modifiedDesc = ImmutableList.builder() + // Add return object parameter + .add(originalDesc.getFirst()) + // Add target parameters + .addAll(modifiedTargetDesc.subList(0, modifiedTargetDesc.size() - popParams)) + .build(); + // Create diff + SimpleParamsDiffSnapshot diff = EnhancedParamsDiff.create(originalDesc, modifiedDesc); + if (!diff.isEmpty()) { + MethodTransform patch = diff.asParameterTransformer(ParamTransformTarget.ALL, false, false); + patch.apply(methodContext); } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicAnonymousShadowFieldTypePatch.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicAnonymousShadowFieldTypePatch.java index 2c43afe..c0d79cc 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicAnonymousShadowFieldTypePatch.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicAnonymousShadowFieldTypePatch.java @@ -2,16 +2,14 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; -import com.mojang.logging.LogUtils; import org.jetbrains.annotations.Nullable; import org.objectweb.asm.tree.*; +import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; import org.sinytra.adapter.patch.api.ClassTransform; import org.sinytra.adapter.patch.api.MixinConstants; import org.sinytra.adapter.patch.api.Patch; import org.sinytra.adapter.patch.api.PatchContext; -import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; import org.sinytra.adapter.patch.util.AdapterUtil; -import org.slf4j.Logger; import java.util.Collection; import java.util.HashMap; @@ -19,8 +17,6 @@ import java.util.Map; public class DynamicAnonymousShadowFieldTypePatch implements ClassTransform { - private static final Logger LOGGER = LogUtils.getLogger(); - @Override public Patch.Result apply(ClassNode classNode, @Nullable AnnotationValueHandle annotation, PatchContext context) { if (annotation == null || !annotation.getKey().equals("targets")) { @@ -64,7 +60,7 @@ public Patch.Result apply(ClassNode classNode, @Nullable AnnotationValueHandle LOGGER.info("Renaming anonymous class field {}.{} to {}", classNode.name, from, to)); + renames.forEach((from, to) -> context.environment().auditTrail().recordAudit(this, classNode, "Rename anonymous class field %s to %s", from, to)); for (MethodNode method : classNode.methods) { for (AbstractInsnNode insn : method.instructions) { if (insn instanceof FieldInsnNode finsn && finsn.owner.equals(classNode.name)) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInheritedInjectionPointPatch.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInheritedInjectionPointPatch.java index 53c8332..d4108a9 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInheritedInjectionPointPatch.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInheritedInjectionPointPatch.java @@ -13,6 +13,8 @@ import java.util.List; +import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; + public class DynamicInheritedInjectionPointPatch implements MethodTransform { private static final Logger LOGGER = LogUtils.getLogger(); @@ -20,7 +22,7 @@ public class DynamicInheritedInjectionPointPatch implements MethodTransform { public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchContext context) { AnnotationHandle atNode = methodContext.injectionPointAnnotation(); if (atNode == null) { - LOGGER.debug("Target @At annotation not found in method {}.{}{}", classNode.name, methodNode.name, methodNode.desc); + LOGGER.debug(MIXINPATCH, "Target @At annotation not found in method {}.{}{}", classNode.name, methodNode.name, methodNode.desc); return Patch.Result.PASS; } if (atNode.getValue("value").map(v -> !v.get().equals("INVOKE")).orElse(true)) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInjectorOrdinalPatch.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInjectorOrdinalPatch.java index e15a924..4fbcf31 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInjectorOrdinalPatch.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicInjectorOrdinalPatch.java @@ -2,26 +2,22 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Multimap; -import com.mojang.logging.LogUtils; import org.jetbrains.annotations.Nullable; import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; import org.objectweb.asm.tree.*; -import org.sinytra.adapter.patch.PatchInstance; import org.sinytra.adapter.patch.analysis.*; -import org.sinytra.adapter.patch.api.*; import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; +import org.sinytra.adapter.patch.api.*; import org.sinytra.adapter.patch.util.AdapterUtil; import org.sinytra.adapter.patch.util.GeneratedVariables; import org.sinytra.adapter.patch.util.SingleValueHandle; -import org.slf4j.Logger; import java.util.*; import java.util.function.Consumer; public class DynamicInjectorOrdinalPatch implements MethodTransform { - private static final Logger LOGGER = LogUtils.getLogger(); private static final Map OFFSET_HANDLERS = Map.of( "INVOKE", InvokeOffsetHandler.INSTANCE, "RETURN", ReturnOffsetHandler.INSTANCE @@ -50,7 +46,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont boolean applied = false; for (HandlerInstance instance : offsetHandlers) { - applied |= instance.apply(methodContext, classNode, methodNode, cleanTarget, dirtyTarget); + applied |= instance.apply(this, methodContext, classNode, methodNode, cleanTarget, dirtyTarget); } return applied ? Patch.Result.APPLY : Patch.Result.PASS; } @@ -105,11 +101,11 @@ public LocalVar(LocalVariableNode lvn, int ordinal) { } private record HandlerInstance(UpdateHandler handler, T context, Consumer applicator) { - public boolean apply(MethodContext methodContext, ClassNode classNode, MethodNode methodNode, MethodContext.TargetPair cleanTarget, MethodContext.TargetPair dirtyTarget) { + public boolean apply(MethodTransform transform, MethodContext methodContext, ClassNode classNode, MethodNode methodNode, MethodContext.TargetPair cleanTarget, MethodContext.TargetPair dirtyTarget) { Optional updatedValue = this.handler.apply(methodContext, classNode, methodNode, cleanTarget, dirtyTarget, this.context); if (updatedValue.isPresent()) { U value = updatedValue.get(); - LOGGER.info(PatchInstance.MIXINPATCH, "Updating injection point ordinal of {}.{} from {} to {}", classNode.name, methodNode.name, this.context, value); + methodContext.recordAudit(transform, "Update injection point ordinal from %s to %s", this.context, value); this.applicator.accept(value); return true; } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicLVTPatch.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicLVTPatch.java index fb2e643..0288fb2 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicLVTPatch.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicLVTPatch.java @@ -13,14 +13,13 @@ import org.objectweb.asm.Type; import org.objectweb.asm.tree.*; import org.sinytra.adapter.patch.LVTOffsets; -import org.sinytra.adapter.patch.PatchInstance; import org.sinytra.adapter.patch.analysis.LocalVariableLookup; import org.sinytra.adapter.patch.analysis.params.EnhancedParamsDiff; import org.sinytra.adapter.patch.analysis.params.ParamsDiffSnapshot; import org.sinytra.adapter.patch.analysis.params.SimpleParamsDiffSnapshot; -import org.sinytra.adapter.patch.api.*; import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; +import org.sinytra.adapter.patch.api.*; import org.sinytra.adapter.patch.transformer.operation.param.ParamTransformTarget; import org.sinytra.adapter.patch.util.AdapterUtil; import org.slf4j.Logger; @@ -30,6 +29,8 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; + public record DynamicLVTPatch(Supplier lvtOffsets) implements MethodTransform { private static final Logger LOGGER = LogUtils.getLogger(); private static final Set ANNOTATIONS = Set.of(MixinConstants.INJECT, MixinConstants.MODIFY_EXPR_VAL, MixinConstants.MODIFY_VAR); @@ -114,7 +115,7 @@ private Patch.Result offsetParameterIndex(ClassNode classNode, MethodNode method if (sameType.size() == 1) { int index = sameType.getFirst().index(); annotation.appendValue("index", index); - LOGGER.info(PatchInstance.MIXINPATCH, "Fixing @Local annotation target on {}.{} using index {}", classNode.name, methodNode.name, index); + methodContext.recordAudit(this, "Fix @Local annotation using index %s", index); return Patch.Result.APPLY; } } @@ -142,7 +143,7 @@ private Patch.Result offsetVariableIndex(ClassNode classNode, MethodNode methodN OptionalInt reorder = this.lvtOffsets.get().findReorder(targetClass.name, targetMethod.name, targetMethod.desc, index); if (reorder.isPresent()) { int newIndex = reorder.getAsInt(); - LOGGER.info(PatchInstance.MIXINPATCH, "Swapping {} index in {}.{} from {} for {}", annotation.getDesc(), classNode.name, methodNode.name, index, newIndex); + methodContext.recordAudit(this, "Swap %s index from %s to %s", annotation.getDesc(), index, newIndex); handle.set(newIndex); return Patch.Result.APPLY; } @@ -174,7 +175,7 @@ private ParamsDiffSnapshot compareParameters(ClassNode classNode, MethodNode met // Check if we can rearrange parameters SimpleParamsDiffSnapshot rearrange = rearrangeParameters(capturedLocals.expected(), availableTypes); if (rearrange == null) { - LOGGER.debug("Tried to replace local variables in mixin method {}.{} using {}", classNode.name, methodNode.name + methodNode.desc, diff.replacements()); + LOGGER.debug(MIXINPATCH, "Tried to replace local variables in mixin method {}.{} using {}", classNode.name, methodNode.name + methodNode.desc, diff.replacements()); return null; } diff = rearrange; @@ -191,7 +192,7 @@ private ParamsDiffSnapshot compareParameters(ClassNode classNode, MethodNode met int removalIndex = lvt.get(removalLocal).index; for (AbstractInsnNode insn : methodNode.instructions) { if (insn instanceof VarInsnNode varInsn && varInsn.var == removalIndex) { - LOGGER.debug("Cannot remove parameter {} in mixin method {}.{}", removal, classNode.name, methodNode.name + methodNode.desc); + LOGGER.debug(MIXINPATCH, "Cannot remove parameter {} in mixin method {}.{}", removal, classNode.name, methodNode.name + methodNode.desc); return null; } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicModifyVarAtReturnPatch.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicModifyVarAtReturnPatch.java index dba1cf4..0e55e33 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicModifyVarAtReturnPatch.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynamic/DynamicModifyVarAtReturnPatch.java @@ -1,20 +1,17 @@ package org.sinytra.adapter.patch.transformer.dynamic; import com.mojang.datafixers.util.Pair; -import com.mojang.logging.LogUtils; import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; import org.objectweb.asm.tree.*; import org.objectweb.asm.tree.analysis.SourceInterpreter; import org.objectweb.asm.tree.analysis.SourceValue; -import org.sinytra.adapter.patch.PatchInstance; import org.sinytra.adapter.patch.analysis.MethodCallAnalyzer; -import org.sinytra.adapter.patch.api.*; import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; +import org.sinytra.adapter.patch.api.*; import org.sinytra.adapter.patch.transformer.operation.ModifyMixinType; import org.sinytra.adapter.patch.util.MockMixinRuntime; -import org.slf4j.Logger; import org.spongepowered.asm.mixin.injection.modify.LocalVariableDiscriminator; import org.spongepowered.asm.mixin.injection.struct.InjectionInfo; import org.spongepowered.asm.mixin.injection.struct.Target; @@ -73,8 +70,6 @@ * } */ public class DynamicModifyVarAtReturnPatch implements MethodTransform { - private static final Logger LOGGER = LogUtils.getLogger(); - @Override public Collection getAcceptedAnnotations() { return Set.of(MixinConstants.MODIFY_VAR); @@ -127,7 +122,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont } String qualifier = MethodCallAnalyzer.getCallQualifier(dirtyMinsn); final int index = i; - LOGGER.info(PatchInstance.MIXINPATCH, "Redirecting RETURN variable modifier to parameter {} of method call to {}", i, qualifier); + methodContext.recordAudit(this, "Redirect RETURN variable modifier to parameter %s of method call to %s", i, qualifier); MethodTransform transform = new ModifyMixinType(MixinConstants.MODIFY_ARG, b -> b.sameTarget() .injectionPoint("INVOKE", qualifier) .putValue("index", index)); 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 de01d4a..a6093fd 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 @@ -8,7 +8,7 @@ import org.sinytra.adapter.patch.analysis.MethodCallAnalyzer; import org.sinytra.adapter.patch.api.MethodContext; import org.sinytra.adapter.patch.api.MixinConstants; -import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.transformer.operation.ModifyInjectionPoint; import org.sinytra.adapter.patch.transformer.operation.ModifyInjectionTarget; import org.sinytra.adapter.patch.util.AdapterUtil; @@ -43,7 +43,8 @@ public Data prepare(MethodContext methodContext) { } @Override - public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, Data data) { + @Nullable + public FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, Data data) { MethodNode dirtyTargetMethod = data.dirtyTarget().methodNode(); AbstractInsnNode cleanInjectionInsn = data.cleanInjectionInsn(); @@ -56,7 +57,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont if (previousCallCandidate != null) { targetMethodCall = findReplacementInjectionPoint(previousCallCandidate, AbstractInsnNode::getPrevious, methodContext); } else { - return Patch.Result.PASS; + return null; } } @@ -67,15 +68,15 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont } String newInjectionPoint = Type.getObjectType(targetMethodCall.owner).getDescriptor() + targetMethodCall.name + targetMethodCall.desc; - return new ModifyInjectionPoint("INVOKE", newInjectionPoint, true, false).apply(methodContext); + return FixResult.of(new ModifyInjectionPoint("INVOKE", newInjectionPoint, true, false).apply(methodContext), PatchAuditTrail.Match.PARTIAL); } - return Patch.Result.PASS; + return null; } - private static Patch.Result tryMoveTargetMethod(MethodInsnNode insn, MethodContext methodContext) { + private static FixResult tryMoveTargetMethod(MethodInsnNode insn, MethodContext methodContext) { String newTarget = insn.name + insn.desc; - return new ModifyInjectionTarget(List.of(newTarget)).apply(methodContext); + return FixResult.of(new ModifyInjectionTarget(List.of(newTarget)).apply(methodContext), PatchAuditTrail.Match.PARTIAL); } private static AbstractInsnNode findCandidates(InstructionMatcher cleanMatcher, MethodNode dirtyTargetMethod, Function, AbstractInsnNode> selector) { 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 e4213c4..359c49c 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 @@ -5,7 +5,7 @@ import org.objectweb.asm.tree.*; import org.sinytra.adapter.patch.api.MethodContext; import org.sinytra.adapter.patch.api.MixinConstants; -import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.transformer.operation.ModifyInjectionPoint; import org.sinytra.adapter.patch.transformer.operation.ModifyInjectionTarget; import org.sinytra.adapter.patch.util.AdapterUtil; @@ -20,7 +20,7 @@ */ public class DynFixAtVariableAssignStore implements DynamicFixer { private static final Set ACCEPTED_ANNOTATIONS = Set.of(MixinConstants.INJECT, MixinConstants.WRAP_OPERATION); - + public record Data(MethodNode cleanTargetMethod, MethodContext.TargetPair dirtyTarget, AbstractInsnNode cleanInjectionInsn) {} @Nullable @@ -42,7 +42,8 @@ public Data prepare(MethodContext methodContext) { } @Override - public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, Data data) { + @Nullable + public FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, Data data) { MethodNode cleanTargetMethod = data.cleanTargetMethod(); MethodNode dirtyTargetMethod = data.dirtyTarget().methodNode(); AbstractInsnNode cleanInjectionInsn = data.cleanInjectionInsn(); @@ -50,53 +51,54 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont // Check that the following instruction is a store operation AbstractInsnNode next = findNextUsefulInsn(cleanInjectionInsn); if (!(next instanceof VarInsnNode varInsn) || !OpcodeUtil.isStoreOpcode(varInsn.getOpcode())) { - return Patch.Result.PASS; + return null; } // Find matching local in dirty target method LocalVariableNode cleanLocal = methodContext.cleanLocalsTable().getByIndexOrNull(varInsn.var); if (cleanLocal == null) { - return Patch.Result.PASS; + return null; } List cleanLocals = methodContext.cleanLocalsTable().getForType(cleanLocal); List dirtyLocals = methodContext.dirtyLocalsTable().getForType(cleanLocal); if (cleanLocals.size() != dirtyLocals.size()) { - return Patch.Result.PASS; + return null; } LocalVariableNode dirtyLocal = dirtyLocals.get(cleanLocals.indexOf(cleanLocal)); // Find store insns List cleanStoreInsns = findStoreInsns(cleanTargetMethod.instructions, cleanLocal.index); int cleanStoreInsnIndex = cleanStoreInsns.indexOf(varInsn); if (cleanStoreInsnIndex == -1) { - return Patch.Result.PASS; + return null; } List dirtyStoreInsns = findStoreInsns(dirtyTargetMethod.instructions, dirtyLocal.index); if (cleanStoreInsns.size() != dirtyStoreInsns.size()) { - return Patch.Result.PASS; + return null; } AbstractInsnNode dirtyStoreInsn = dirtyStoreInsns.get(cleanStoreInsnIndex); // Find first method call before dirty store MethodInsnNode previousMethodCall = (MethodInsnNode) AdapterUtil.iterateInsns(dirtyStoreInsn, AbstractInsnNode::getPrevious, i -> i instanceof MethodInsnNode); if (previousMethodCall == null) { - return Patch.Result.PASS; + return null; } - + if (methodContext.methodAnnotation().matchesDesc(MixinConstants.WRAP_OPERATION)) { return handleWrapAnnotation(methodContext, data, previousMethodCall); } - + // All checks have passed, proceed to patch method String newInjectionPoint = Type.getObjectType(previousMethodCall.owner).getDescriptor() + previousMethodCall.name + previousMethodCall.desc; - return new ModifyInjectionPoint((String) null, newInjectionPoint, true, true) - .apply(methodContext); + return FixResult.of(new ModifyInjectionPoint((String) null, newInjectionPoint, true, true) + .apply(methodContext), PatchAuditTrail.Match.FULL); } // In case the mixin is call-sensitive, we try to keep the orignal injection point if the method was moved - private static Patch.Result handleWrapAnnotation(MethodContext methodContext, Data data, MethodInsnNode previousMethodCall) { + @Nullable + private static FixResult handleWrapAnnotation(MethodContext methodContext, Data data, MethodInsnNode previousMethodCall) { if (previousMethodCall.owner.equals(data.dirtyTarget().classNode().name)) { String newTarget = previousMethodCall.name + previousMethodCall.desc; - return new ModifyInjectionTarget(List.of(newTarget)).apply(methodContext); + return FixResult.of(new ModifyInjectionTarget(List.of(newTarget)).apply(methodContext), PatchAuditTrail.Match.FULL); } - return Patch.Result.PASS; + return null; } private static List findStoreInsns(InsnList insns, int index) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixMethodComparison.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixMethodComparison.java index 9bed36e..e89ffc1 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixMethodComparison.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixMethodComparison.java @@ -10,11 +10,12 @@ import org.sinytra.adapter.patch.analysis.LocalVariableLookup; import org.sinytra.adapter.patch.analysis.MethodCallAnalyzer; import org.sinytra.adapter.patch.analysis.MethodLabelComparator; +import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.api.MethodContext; import org.sinytra.adapter.patch.api.MixinConstants; import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.fixes.MethodUpgrader; -import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.transformer.BundledMethodTransform; import org.sinytra.adapter.patch.transformer.operation.ModifyInjectionPoint; import org.sinytra.adapter.patch.transformer.operation.param.*; @@ -23,6 +24,7 @@ import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; public class DynFixMethodComparison implements DynamicFixer { @@ -47,18 +49,20 @@ public Data prepare(MethodContext methodContext) { } @Override - public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, Data data) { + @Nullable + public FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, Data data) { AbstractInsnNode cleanInjectionInsn = data.cleanInjectionInsn(); MethodLabelComparator.ComparisonResult comparisonResult = MethodLabelComparator.findPatchedLabels(cleanInjectionInsn, methodContext); if (comparisonResult == null) { - return Patch.Result.PASS; + return null; } List> hunkLabels = comparisonResult.patchedLabels(); if (methodContext.methodAnnotation().matchesDesc(MixinConstants.WRAP_OPERATION)) { return handleWrapOperationToInstanceOf(cleanInjectionInsn, comparisonResult.cleanLabel(), hunkLabels, methodContext) - .orElseGet(() -> handleWrapOpertationNewInjectionPoint(cleanInjectionInsn, comparisonResult.cleanLabel(), hunkLabels, methodContext)) - .orElseGet(() -> handleTargetModification(hunkLabels, methodContext)); + .or(() -> handleWrapOpertationNewInjectionPoint(cleanInjectionInsn, comparisonResult.cleanLabel(), hunkLabels, methodContext)) + .or(() -> handleTargetModification(hunkLabels, methodContext)) + .orElse(null); } ClassNode cleanTargetClass = methodContext.findCleanInjectionTarget().classNode(); @@ -67,7 +71,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont if (insn instanceof MethodInsnNode minsn && minsn.getOpcode() == Opcodes.INVOKESTATIC && !minsn.owner.equals(cleanTargetClass.name)) { Patch.Result result = attemptExtractMixin(minsn, methodContext); if (result != Patch.Result.PASS) { - return result; + return FixResult.of(result, PatchAuditTrail.Match.FULL); } } } @@ -78,17 +82,17 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont for (AbstractInsnNode insn : insns) { if (insn instanceof MethodInsnNode minsn) { String newInjectionPoint = Type.getObjectType(minsn.owner).getDescriptor() + minsn.name + minsn.desc; - return new ModifyInjectionPoint("INVOKE", newInjectionPoint, true, false).apply(methodContext); + return FixResult.of(new ModifyInjectionPoint("INVOKE", newInjectionPoint, true, false).apply(methodContext), PatchAuditTrail.Match.PARTIAL); } } } - return Patch.Result.PASS; + return null; } - private static Patch.Result handleWrapOpertationNewInjectionPoint(AbstractInsnNode cleanInjectionInsn, List cleanLabel, List> hunkLabels, MethodContext methodContext) { + private static Optional handleWrapOpertationNewInjectionPoint(AbstractInsnNode cleanInjectionInsn, List cleanLabel, List> hunkLabels, MethodContext methodContext) { if (!(cleanInjectionInsn instanceof MethodInsnNode minsn) || hunkLabels.size() != 1) { - return Patch.Result.PASS; + return Optional.empty(); } Type cleanReturnType = Type.getReturnType(minsn.desc); List dirtyLabel = hunkLabels.getFirst(); @@ -98,25 +102,25 @@ private static Patch.Result handleWrapOpertationNewInjectionPoint(AbstractInsnNo .map(i -> (MethodInsnNode) i) .toList(); if (methodCalls.size() != 1) { - return Patch.Result.PASS; + return Optional.empty(); } MethodInsnNode dirtyMinsn = methodCalls.getFirst(); - return BundledMethodTransform.builder() + return Optional.of(FixResult.of(BundledMethodTransform.builder() .modifyInjectionPoint("INVOKE", MethodCallAnalyzer.getCallQualifier(dirtyMinsn)) - .apply(methodContext); + .apply(methodContext), PatchAuditTrail.Match.FULL)); } - private static Patch.Result handleWrapOperationToInstanceOf(AbstractInsnNode cleanInjectionInsn, List cleanLabel, List> hunkLabels, MethodContext methodContext) { + private static Optional handleWrapOperationToInstanceOf(AbstractInsnNode cleanInjectionInsn, List cleanLabel, List> hunkLabels, MethodContext methodContext) { if (!(cleanInjectionInsn instanceof MethodInsnNode minsn) || hunkLabels.size() != 1 || !(cleanLabel.getLast() instanceof JumpInsnNode) || cleanLabel.stream().anyMatch(i -> i instanceof TypeInsnNode)) { - return Patch.Result.PASS; + return Optional.empty(); } List dirtyLabel = hunkLabels.getFirst(); if (!(dirtyLabel.getLast() instanceof JumpInsnNode)) { - return Patch.Result.PASS; + return Optional.empty(); } List instanceOfCalls = dirtyLabel.stream().filter(i -> i instanceof TypeInsnNode).map(i -> (TypeInsnNode) i).toList(); if (instanceOfCalls.size() != 1) { - return Patch.Result.PASS; + return Optional.empty(); } TypeInsnNode instanceOfCall = instanceOfCalls.getFirst(); MethodNode methodNode = methodContext.getMixinMethod(); @@ -152,11 +156,11 @@ private static Patch.Result handleWrapOperationToInstanceOf(AbstractInsnNode cle if (paramVar == instanceLocal.index) { int cleanOrdinal = methodContext.cleanLocalsTable().getTypedOrdinal(methodContext.cleanLocalsTable().getByIndex(((VarInsnNode) originalCallArgs.getFirst()).var)).orElse(-1); if (cleanOrdinal == -1) { - return Patch.Result.PASS; + return Optional.empty(); } LocalVariableNode dirtyLocal = methodContext.dirtyLocalsTable().getByTypedOrdinal(Type.getType(instanceLocal.desc), cleanOrdinal).orElse(null); if (dirtyLocal == null) { - return Patch.Result.PASS; + return Optional.empty(); } TransformParameters.builder() .transform(new InjectParameterTransform(argsTypes.length, Type.getType(dirtyLocal.desc), false)) @@ -172,7 +176,7 @@ private static Patch.Result handleWrapOperationToInstanceOf(AbstractInsnNode cle String loadedType = instanceOfCall.getPrevious() instanceof MethodInsnNode m ? Type.getReturnType(m.desc).getDescriptor() : null; LocalVariableNode node = mixinLocals.getByIndex(paramVar); if (loadedType == null || !loadedType.equals(node.desc)) { - return Patch.Result.PASS; + return Optional.empty(); } usedVars.get(paramVar).forEach(varInsn -> varInsn.var = instanceLocal.index); } @@ -188,31 +192,31 @@ private static Patch.Result handleWrapOperationToInstanceOf(AbstractInsnNode cle .build() .apply(methodContext); if (result == Patch.Result.PASS) { - return Patch.Result.PASS; + return Optional.empty(); } - AnnotationHandle ann = methodContext.methodAnnotation(); + AnnotationHandle ann = methodContext.methodAnnotation(); ann.removeValues("at"); AnnotationVisitor visitor = ann.unwrap().visitAnnotation("constant", MixinConstants.CONSTANT); visitor.visit("classValue", Type.getObjectType(instanceOfCall.desc)); visitor.visitEnd(); - return Patch.Result.APPLY; + return Optional.of(FixResult.of(Patch.Result.APPLY, PatchAuditTrail.Match.FULL)); } - private static Patch.Result handleTargetModification(List> hunkLabels, MethodContext methodContext) { + private static Optional handleTargetModification(List> hunkLabels, MethodContext methodContext) { ClassNode dirtyTarget = methodContext.findDirtyInjectionTarget().classNode(); for (List insns : hunkLabels) { for (AbstractInsnNode insn : insns) { if (insn instanceof MethodInsnNode minsn && minsn.owner.equals(dirtyTarget.name)) { MethodNode method = MethodCallAnalyzer.findMethodByUniqueName(dirtyTarget, minsn.name).orElse(null); if (method != null && !methodContext.findInjectionTargetInsns(new MethodContext.TargetPair(dirtyTarget, method)).isEmpty()) { - return BundledMethodTransform.builder().modifyTarget(minsn.name + minsn.desc).apply(methodContext); + return Optional.of(FixResult.of(BundledMethodTransform.builder().modifyTarget(minsn.name + minsn.desc).apply(methodContext), PatchAuditTrail.Match.FULL)); } } } } - return Patch.Result.PASS; + return Optional.empty(); } private static Patch.Result attemptExtractMixin(MethodInsnNode minsn, MethodContext methodContext) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixParameterTypeAdapter.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixParameterTypeAdapter.java index f89a0fc..86255d4 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixParameterTypeAdapter.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixParameterTypeAdapter.java @@ -12,6 +12,7 @@ import org.sinytra.adapter.patch.api.MethodContext; import org.sinytra.adapter.patch.api.MixinConstants; import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.transformer.BundledMethodTransform; import org.sinytra.adapter.patch.transformer.operation.param.ParamTransformTarget; import org.sinytra.adapter.patch.util.MethodQualifier; @@ -56,13 +57,14 @@ public Data prepare(MethodContext methodContext) { } @Override - public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, Data data) { + @Nullable + public FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, Data data) { Patch.Result result = BundledMethodTransform.builder() .modifyInjectionPoint(MethodCallAnalyzer.getCallQualifier(data.newCall())) .transform(data.diff().asParameterTransformer(ParamTransformTarget.INJECTION_POINT, false)) .apply(methodContext); if (result == Patch.Result.PASS) { - return Patch.Result.PASS; + return null; } MethodQualifier qualifier = data.qualifier(); @@ -74,7 +76,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont } } - return result.or(Patch.Result.APPLY); + return FixResult.of(result.or(Patch.Result.APPLY), PatchAuditTrail.Match.FULL); } @Nullable diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixResolveAmbigousTarget.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixResolveAmbigousTarget.java index d06f09c..d26d6f0 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixResolveAmbigousTarget.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixResolveAmbigousTarget.java @@ -1,28 +1,21 @@ package org.sinytra.adapter.patch.transformer.dynfix; import com.mojang.datafixers.util.Pair; -import com.mojang.logging.LogUtils; import org.jetbrains.annotations.Nullable; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodNode; import org.sinytra.adapter.patch.api.MethodContext; -import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.transformer.BundledMethodTransform; -import org.slf4j.Logger; import java.util.List; -import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; - /** * Handle cases where a mixin with no descriptor in its target tmethod selector has come to have multiple candidate injection methods * as a result of Forge adding one with the same name. */ public class DynFixResolveAmbigousTarget implements DynamicFixer { - private static final Logger LOGGER = LogUtils.getLogger(); - - public record Data(Pair> candidates) { - } + public record Data(Pair> candidates) {} @Nullable @Override @@ -42,15 +35,16 @@ public Data prepare(MethodContext methodContext) { } @Override - public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, Data data) { + @Nullable + public FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, Data data) { List candidates = data.candidates().getSecond(); for (MethodNode target : candidates) { if (candidates.size() == 1 || !methodContext.findInjectionTargetInsns(new MethodContext.TargetPair(data.candidates().getFirst(), target)).isEmpty()) { String newTarget = target.name + target.desc; - LOGGER.debug(MIXINPATCH, "Resolving ambigous method selector of {}.{} to {}", classNode.name, methodNode.name, newTarget); - return BundledMethodTransform.builder().modifyTarget(newTarget).apply(methodContext); + methodContext.recordAudit(this, "Resolve ambigous method selector to %s", newTarget); + return FixResult.of(BundledMethodTransform.builder().modifyTarget(newTarget).apply(methodContext), PatchAuditTrail.Match.FULL); } } - return Patch.Result.PASS; + return null; } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSliceBoundary.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSliceBoundary.java index b325d36..262d9b6 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSliceBoundary.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSliceBoundary.java @@ -3,10 +3,11 @@ import org.jetbrains.annotations.Nullable; import org.objectweb.asm.Type; import org.objectweb.asm.tree.*; -import org.sinytra.adapter.patch.api.MethodContext; -import org.sinytra.adapter.patch.api.Patch; import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; +import org.sinytra.adapter.patch.api.MethodContext; +import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.util.MethodQualifier; import org.sinytra.adapter.patch.util.MockMixinRuntime; import org.spongepowered.asm.mixin.injection.code.ISliceContext; @@ -40,7 +41,8 @@ public DynFixSliceBoundary.Data prepare(MethodContext methodContext) { } @Override - public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, Data data) { + @Nullable + public FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, Data data) { MethodContext.TargetPair dirtyTarget = methodContext.findDirtyInjectionTarget(); Target mixinTarget = MockMixinRuntime.createMixinTarget(dirtyTarget); AnnotationHandle slice = data.slice(); @@ -51,11 +53,11 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont InsnList insns = methodSlice.getSlice(mixinTarget); if (insns.size() != dirtyTarget.methodNode().instructions.size()) { - return Patch.Result.PASS; + return null; } - return data.slices().stream() - .reduce(Patch.Result.PASS, (a, b) -> a.or(fixSlideInjectionPoint(b, dirtyTarget.methodNode())), Patch.Result::or); + return FixResult.of(data.slices().stream() + .reduce(Patch.Result.PASS, (a, b) -> a.or(fixSlideInjectionPoint(b, dirtyTarget.methodNode())), Patch.Result::or), PatchAuditTrail.Match.FULL); } private static Patch.Result fixSlideInjectionPoint(AnnotationHandle annotation, MethodNode dirtyMethod) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSplitMethod.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSplitMethod.java index 7ab5383..8b973fe 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSplitMethod.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynFixSplitMethod.java @@ -1,28 +1,23 @@ package org.sinytra.adapter.patch.transformer.dynfix; -import com.mojang.logging.LogUtils; import org.jetbrains.annotations.Nullable; import org.objectweb.asm.tree.*; import org.sinytra.adapter.patch.analysis.MethodCallAnalyzer; import org.sinytra.adapter.patch.api.MethodContext; -import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; import org.sinytra.adapter.patch.transformer.operation.ModifyInjectionTarget; import org.sinytra.adapter.patch.util.AdapterUtil; import org.sinytra.adapter.patch.util.OpcodeUtil; -import org.slf4j.Logger; import java.util.ArrayList; import java.util.List; -import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; - /** * Handle cases where a single method is split into multiple smaller pieces. * For an example, see net.minecraft.client.gui.Gui#renderPlayerHealth */ public class DynFixSplitMethod implements DynamicFixer { private static final String DEPRECATED = "Ljava/lang/Deprecated;"; - private static final Logger LOGGER = LogUtils.getLogger(); public record Data() {} @@ -36,14 +31,15 @@ public DynFixSplitMethod.Data prepare(MethodContext methodContext) { } @Override - public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, Data data) { + @Nullable + public FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, Data data) { MethodNode cleanTargetMethod = methodContext.findCleanInjectionTarget().methodNode(); ClassNode dirtyTargetClass = methodContext.findDirtyInjectionTarget().classNode(); MethodNode dirtyTargetMethod = methodContext.findDirtyInjectionTarget().methodNode(); // Check that a Deprecated annotation was added to the dirty method if (AdapterUtil.hasAnnotation(cleanTargetMethod.visibleAnnotations, DEPRECATED) || !AdapterUtil.hasAnnotation(dirtyTargetMethod.visibleAnnotations, DEPRECATED)) { - return Patch.Result.PASS; + return null; } // Iterate over isns, leave out first and last elements @@ -58,7 +54,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont MethodNode method = dirtyTargetClass.methods.stream().filter(m -> m.name.equals(methodInsn.name) && m.desc.equals(methodInsn.desc)).findFirst().orElseThrow(); invocations.add(method); } else if (previous == null || !OpcodeUtil.isReturnOpcode(previous.getOpcode())) { - return Patch.Result.PASS; + return null; } } } @@ -77,11 +73,11 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont if (candidates.size() == 1) { MethodNode method = candidates.getFirst(); String newTarget = method.name + method.desc; - LOGGER.debug(MIXINPATCH, "Adjusting split method target of {}.{} to {}", classNode.name, methodNode.name, newTarget); - return new ModifyInjectionTarget(List.of(newTarget)).apply(methodContext); + methodContext.recordAudit(this, "Adjusting split method target to %s", newTarget); + return FixResult.of(new ModifyInjectionTarget(List.of(newTarget)).apply(methodContext), PatchAuditTrail.Match.FULL); } - return Patch.Result.PASS; + return null; } private static List findInsnsCalls(List methods, MethodContext methodContext) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicFixer.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicFixer.java index c965877..c67ecc5 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicFixer.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicFixer.java @@ -5,10 +5,25 @@ import org.objectweb.asm.tree.MethodNode; import org.sinytra.adapter.patch.api.MethodContext; import org.sinytra.adapter.patch.api.Patch; +import org.sinytra.adapter.patch.api.PatchAuditTrail; public interface DynamicFixer { @Nullable DATA prepare(MethodContext methodContext); - - Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, DATA data); + + @Nullable + FixResult apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchAuditTrail auditTrail, DATA data); + + record FixResult(Patch.Result result, PatchAuditTrail.Match match) { + public FixResult { + if (result == Patch.Result.PASS) { + throw new IllegalArgumentException("Result must be non-PASS"); + } + } + + @Nullable + public static FixResult of(Patch.Result result, PatchAuditTrail.Match match) { + return result == Patch.Result.PASS ? null : new FixResult(result, match); + } + } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicInjectionPointPatch.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicInjectionPointPatch.java index 9ef11a2..66c0b7c 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicInjectionPointPatch.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/dynfix/DynamicInjectionPointPatch.java @@ -3,10 +3,7 @@ import com.mojang.logging.LogUtils; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodNode; -import org.sinytra.adapter.patch.api.MethodContext; -import org.sinytra.adapter.patch.api.MethodTransform; -import org.sinytra.adapter.patch.api.Patch; -import org.sinytra.adapter.patch.api.PatchContext; +import org.sinytra.adapter.patch.api.*; import org.slf4j.Logger; import java.util.List; @@ -32,22 +29,29 @@ public class DynamicInjectionPointPatch implements MethodTransform { @Override public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchContext context) { if (methodContext.failsDirtyInjectionCheck() && methodContext.findCleanInjectionTarget() != null) { - // TODO Only show in tests LOGGER.debug(MIXINPATCH, "Considering method {}.{}", classNode.name, methodNode.name); + PatchAuditTrail auditTrail = context.environment().auditTrail(); + auditTrail.recordResult(methodContext, PatchAuditTrail.Match.NONE); + Patch.Result result = Patch.Result.PASS; for (DynamicFixer fix : PREPATCH) { Object data = fix.prepare(methodContext); if (data != null) { - result = result.or(fix.apply(classNode, methodNode, methodContext, data)); + DynamicFixer.FixResult fixResult = fix.apply(classNode, methodNode, methodContext, auditTrail, data); + if (fixResult != null) { + auditTrail.recordResult(methodContext, fixResult.match()); + result = result.or(fixResult.result()); + } } } for (DynamicFixer fix : FIXES) { Object data = fix.prepare(methodContext); if (data != null) { - Patch.Result patchResult = fix.apply(classNode, methodNode, methodContext, data); - if (patchResult != Patch.Result.PASS) { - return patchResult.or(result); + DynamicFixer.FixResult fixResult = fix.apply(classNode, methodNode, methodContext, auditTrail, data); + if (fixResult != null) { + auditTrail.recordResult(methodContext, fixResult.match()); + return result.or(fixResult.result()); } } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/DisableMixin.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/DisableMixin.java index af82c08..bfdbed7 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/DisableMixin.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/DisableMixin.java @@ -1,6 +1,5 @@ package org.sinytra.adapter.patch.transformer.operation; -import com.mojang.logging.LogUtils; import com.mojang.serialization.Codec; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodNode; @@ -8,12 +7,8 @@ import org.sinytra.adapter.patch.api.MethodTransform; import org.sinytra.adapter.patch.api.Patch; import org.sinytra.adapter.patch.api.PatchContext; -import org.slf4j.Logger; - -import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; public class DisableMixin implements MethodTransform { - private static final Logger LOGGER = LogUtils.getLogger(); public static final DisableMixin INSTANCE = new DisableMixin(); public static final Codec CODEC = Codec.unit(INSTANCE); @@ -24,7 +19,7 @@ public Codec codec() { @Override public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchContext context) { - LOGGER.debug(MIXINPATCH, "Removing mixin method {}.{}{}", classNode.name, methodNode.name, methodNode.desc); + methodContext.recordAudit(this, "Remove mixin method"); context.postApply(() -> classNode.methods.remove(methodNode)); return Patch.Result.APPLY; } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ExtractMixin.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ExtractMixin.java index ec10989..038f1d2 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ExtractMixin.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ExtractMixin.java @@ -70,8 +70,10 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont result = result.or(recreateLocalVariables(classNode, methodNode, methodContext, context, generatedTarget)); } + methodContext.recordAudit(this, "Extract mixin to target %s", this.targetClass); // Remove original method if (this.remove) { + methodContext.recordAudit(this, "Remove original method"); context.postApply(() -> classNode.methods.removeAll(candidates.methods)); } return result.or(Patch.Result.APPLY); diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionPoint.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionPoint.java index 7ac2dd1..b0df55e 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionPoint.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionPoint.java @@ -1,26 +1,21 @@ package org.sinytra.adapter.patch.transformer.operation; -import com.mojang.logging.LogUtils; import com.mojang.serialization.Codec; import com.mojang.serialization.codecs.RecordCodecBuilder; import org.jetbrains.annotations.Nullable; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodNode; +import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; +import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; import org.sinytra.adapter.patch.api.MethodContext; import org.sinytra.adapter.patch.api.MethodTransform; import org.sinytra.adapter.patch.api.Patch; import org.sinytra.adapter.patch.api.PatchContext; import org.sinytra.adapter.patch.fixes.MethodUpgrader; -import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; -import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; -import org.slf4j.Logger; import java.util.Optional; -import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; - public record ModifyInjectionPoint(@Nullable String value, String target, boolean resetValues, boolean dontUpgrade) implements MethodTransform { - private static final Logger LOGGER = LogUtils.getLogger(); public static final Codec CODEC = RecordCodecBuilder.create(instance -> instance.group( Codec.STRING.optionalFieldOf("value").forGetter(i -> Optional.ofNullable(i.value())), Codec.STRING.fieldOf("target").forGetter(ModifyInjectionPoint::target), @@ -48,7 +43,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont AnnotationValueHandle handle = annotation.getValue("value").orElseThrow(() -> new IllegalArgumentException("Missing value handle")); handle.set(this.value); } - LOGGER.info(MIXINPATCH, "Changing mixin method target {}.{} to {}", classNode.name, methodNode.name, this.target); + methodContext.recordAudit(this, "Change injection point to %s", this.target); AnnotationValueHandle handle = annotation.getValue("target").orElse(null); if (handle != null) { String original = handle.get(); diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionTarget.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionTarget.java index ed8ec22..7bb6090 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionTarget.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyInjectionTarget.java @@ -1,23 +1,18 @@ package org.sinytra.adapter.patch.transformer.operation; -import com.mojang.logging.LogUtils; import com.mojang.serialization.Codec; import com.mojang.serialization.codecs.RecordCodecBuilder; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodNode; -import org.sinytra.adapter.patch.api.*; -import org.sinytra.adapter.patch.fixes.MethodUpgrader; import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.analysis.selector.AnnotationValueHandle; +import org.sinytra.adapter.patch.api.*; +import org.sinytra.adapter.patch.fixes.MethodUpgrader; import org.sinytra.adapter.patch.util.MethodQualifier; -import org.slf4j.Logger; import java.util.List; -import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; - public record ModifyInjectionTarget(List replacementMethods, Action action) implements MethodTransform { - private static final Logger LOGGER = LogUtils.getLogger(); public static final Codec CODEC = RecordCodecBuilder.create(instance -> instance.group( Codec.STRING.listOf().fieldOf("replacementMethods").forGetter(ModifyInjectionTarget::replacementMethods), Action.CODEC.optionalFieldOf("action", Action.OVERWRITE).forGetter(ModifyInjectionTarget::action) @@ -34,9 +29,9 @@ public Codec codec() { @Override public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchContext context) { - LOGGER.info(MIXINPATCH, "Redirecting mixin {}.{} to {}", classNode.name, methodNode.name, this.replacementMethods); AnnotationHandle annotation = methodContext.methodAnnotation(); + methodContext.recordAudit(this, "Change mixin target to %s", this.replacementMethods); if (annotation.matchesDesc(MixinConstants.OVERWRITE)) { if (this.replacementMethods.size() > 1) { throw new IllegalStateException("Cannot determine replacement @Overwrite method name, multiple specified: " + this.replacementMethods); @@ -47,7 +42,13 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont .ifPresent(str -> methodNode.name = str); } else { annotation.>getValue("method").ifPresentOrElse( - handle -> this.action.handler.apply(handle, methodContext.matchingTargets(), this.replacementMethods), + handle -> { + List original = handle.get(); + this.action.handler.apply(handle, methodContext.matchingTargets(), this.replacementMethods); + if (original.size() == 1 && handle.get().size() == 1) { + MethodUpgrader.upgradeMethod(methodNode, methodContext, original.getFirst(), handle.get().getFirst()); + } + }, () -> annotation.appendValue("method", this.replacementMethods) ); } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodAccess.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodAccess.java index 5aefe76..8ba4e6c 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodAccess.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodAccess.java @@ -1,25 +1,20 @@ package org.sinytra.adapter.patch.transformer.operation; -import com.mojang.logging.LogUtils; import com.mojang.serialization.Codec; import com.mojang.serialization.codecs.RecordCodecBuilder; import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; import org.objectweb.asm.tree.*; import org.sinytra.adapter.patch.api.*; -import org.slf4j.Logger; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; - public record ModifyMethodAccess(List changes) implements MethodTransform { public static final Codec CODEC = RecordCodecBuilder.create(instance -> instance.group( AccessChange.CODEC.listOf().fieldOf("changes").forGetter(ModifyMethodAccess::changes) ).apply(instance, ModifyMethodAccess::new)); - private static final Logger LOGGER = LogUtils.getLogger(); public record AccessChange(boolean add, int modifier) { public static final Codec CODEC = RecordCodecBuilder.create(instance -> instance.group( @@ -39,7 +34,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont for (AccessChange change : this.changes) { if (change.add) { if ((methodNode.access & change.modifier) == 0) { - LOGGER.info(MIXINPATCH, "Adding access modifier {} to method {}.{}{}", change.modifier, classNode.name, methodNode.name, methodNode.desc); + methodContext.recordAudit(this, "Adding access modifier %s", change.modifier); methodNode.access |= change.modifier; result = Patch.Result.APPLY; if (change.modifier == Opcodes.ACC_STATIC && methodContext.methodAnnotation().matchesDesc(MixinConstants.INJECT)) { @@ -49,7 +44,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont List newParams = new ArrayList<>(Arrays.asList(params)); newParams.addFirst(types.getFirst()); - methodContext.updateDescription(newParams); + methodContext.updateDescription(this, newParams); } else { throw new IllegalStateException("Cannot automatically determine target instance type for mixin " + classNode.name); } @@ -57,7 +52,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont } } else { if ((methodNode.access & change.modifier) != 0) { - LOGGER.info(MIXINPATCH, "Removing access modifier {} from method {}.{}{}", change.modifier, classNode.name, methodNode.name, methodNode.desc); + methodContext.recordAudit(this, "Removing access modifier %s", change.modifier); methodNode.access &= ~change.modifier; if (change.modifier == Opcodes.ACC_STATIC) { LocalVariableNode firstParam = methodNode.localVariables.stream().filter(lvn -> lvn.index == 0).findFirst().orElseThrow(); diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodParams.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodParams.java index 399f8e1..68e5391 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodParams.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMethodParams.java @@ -1,7 +1,6 @@ package org.sinytra.adapter.patch.transformer.operation; import com.mojang.datafixers.util.Pair; -import com.mojang.logging.LogUtils; import com.mojang.serialization.Codec; import com.mojang.serialization.codecs.RecordCodecBuilder; import org.jetbrains.annotations.Nullable; @@ -10,25 +9,23 @@ import org.objectweb.asm.commons.InstructionAdapter; import org.objectweb.asm.tree.*; import org.sinytra.adapter.patch.PatchInstance; +import org.sinytra.adapter.patch.analysis.LVTSnapshot; import org.sinytra.adapter.patch.analysis.LocalVariableLookup; import org.sinytra.adapter.patch.analysis.params.SimpleParamsDiffSnapshot; +import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; import org.sinytra.adapter.patch.api.*; import org.sinytra.adapter.patch.fixes.BytecodeFixerUpper; -import org.sinytra.adapter.patch.fixes.TypeAdapter; -import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; -import org.sinytra.adapter.patch.analysis.LVTSnapshot; import org.sinytra.adapter.patch.fixes.ModifyArgsOffsetTransformer; +import org.sinytra.adapter.patch.fixes.TypeAdapter; import org.sinytra.adapter.patch.transformer.operation.param.InjectParameterTransform; import org.sinytra.adapter.patch.transformer.operation.param.ParamTransformTarget; import org.sinytra.adapter.patch.transformer.operation.param.SwapParametersTransformer; import org.sinytra.adapter.patch.util.AdapterUtil; import org.sinytra.adapter.patch.util.SingleValueHandle; -import org.slf4j.Logger; import java.util.*; import java.util.function.Consumer; -import static org.sinytra.adapter.patch.PatchInstance.MIXINPATCH; import static org.sinytra.adapter.patch.transformer.operation.param.ParamTransformationUtil.calculateLVTIndex; import static org.sinytra.adapter.patch.transformer.operation.param.ParamTransformationUtil.findWrapOperationOriginalCall; @@ -39,8 +36,6 @@ public record ModifyMethodParams(SimpleParamsDiffSnapshot context, ParamTransfor SimpleParamsDiffSnapshot.CODEC.fieldOf("context").forGetter(ModifyMethodParams::context), ParamTransformTarget.CODEC.optionalFieldOf("targetInjectionPoint", ParamTransformTarget.ALL).forGetter(ModifyMethodParams::targetType) ).apply(instance, (context, targetInjectionPoint) -> new ModifyMethodParams(context, targetInjectionPoint, false, null))); - - private static final Logger LOGGER = LogUtils.getLogger(); public static ModifyMethodParams create(SimpleParamsDiffSnapshot diff, ParamTransformTarget targetType) { return new ModifyMethodParams(diff, targetType, false, null); @@ -185,7 +180,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont if (methodNode.parameters.size() > paramIndex) { int localIndex = calculateLVTIndex(newParameterTypes, isNonStatic, paramIndex); LVTSnapshot lvtSnapshot = LVTSnapshot.take(methodNode); - LOGGER.info("Substituting parameter {} for {} in {}.{}", paramIndex, substituteParamIndex, classNode.name, methodNode.name); + methodContext.recordAudit(this, "Substitute parameter %s for %s", paramIndex, substituteParamIndex); methodNode.parameters.remove(paramIndex); newParameterTypes.remove(paramIndex); int substituteIndex = calculateLVTIndex(newParameterTypes, isNonStatic, substituteParamIndex); @@ -216,7 +211,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont Type toType = newParameterTypes.get(to); newParameterTypes.set(from, toType); newParameterTypes.set(to, fromType); - LOGGER.info(MIXINPATCH, "Swapped parameters at positions {}({}) and {}({}) in {}.{}", from, fromNode.name, to, toNode.name, classNode.name, methodNode.name); + methodContext.recordAudit(this, "Swap parameters %s <%s> and %s <%s>", from, fromNode.name, to, toNode.name); int fromNewLVT = calculateLVTIndex(newParameterTypes, isNonStatic, from); int toNewLVT = calculateLVTIndex(newParameterTypes, isNonStatic, to); @@ -229,7 +224,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont } if (!this.context.removals().isEmpty()) { - LOGGER.info(MIXINPATCH, "Removing parameters {} from method {}.{}", this.context.removals(), classNode.name, methodNode.name); + methodContext.recordAudit(this, "Remove parameters %s", this.context.removals()); } this.context.removals().stream() .sorted(Comparator.comparingInt(i -> i).reversed()) @@ -237,7 +232,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont offsetMoves.forEach(move -> { int from = move.getFirst(); int to = move.getSecond(); - LOGGER.info(MIXINPATCH, "Moving parameter from index {} to {} in method {}.{}", from, to, classNode.name, methodNode.name); + methodContext.recordAudit(this, "Move parameter from index {} to {}", from, to); int tempIndex = -999; Pair<@Nullable ParameterNode, @Nullable LocalVariableNode> removed = removeLocalVariable(methodNode, from, offset, tempIndex, newParameterTypes); if (removed.getFirst() != null) { @@ -266,7 +261,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont .sorted(Comparator.>>comparingInt(Pair::getFirst).reversed()) .forEach(inline -> { int index = inline.getFirst(); - LOGGER.info(MIXINPATCH, "Inlining parameter {} of method {}.{}", index, classNode.name, methodNode.name); + methodContext.recordAudit(this, "Inlining parameter {}", index); int replaceIndex = -999 + index; removeLocalVariable(methodNode, index, offset, replaceIndex, newParameterTypes); for (AbstractInsnNode insn : methodNode.instructions) { @@ -278,7 +273,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont } }); - methodContext.updateDescription(newParameterTypes); + methodContext.updateDescription(this, newParameterTypes); return this.context.shouldComputeFrames() ? Patch.Result.COMPUTE_FRAMES : Patch.Result.APPLY; } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMixinType.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMixinType.java index 54e4f9b..d5ba874 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMixinType.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyMixinType.java @@ -25,6 +25,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont if (methodAnn == annotation.unwrap()) { methodNode.visibleAnnotations.set(i, replacement); annotation.refresh(replacement); + methodContext.recordAudit(this, "Modify type to %s", this.replacementDesc); break; } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyTargetClasses.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyTargetClasses.java index 7a9625d..6e6799e 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyTargetClasses.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/ModifyTargetClasses.java @@ -24,6 +24,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont List types = new ArrayList<>(valueHandle.get()); this.consumer.accept(types); valueHandle.set(types); + methodContext.recordAudit(this, "Modify target classes to %s", types); return Patch.Result.APPLY; } return Patch.Result.PASS; diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/param/TransformParameters.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/param/TransformParameters.java index 348ef8f..375a913 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/param/TransformParameters.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/transformer/operation/param/TransformParameters.java @@ -54,7 +54,7 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont result = result.or(transform.apply(classNode, methodNode, methodContext, context, newParameterTypes, offset)); } - methodContext.updateDescription(newParameterTypes); + methodContext.updateDescription(this, newParameterTypes); return result; } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/util/AdapterUtil.java b/definition/src/main/java/org/sinytra/adapter/patch/util/AdapterUtil.java index 7c5633e..3a40fa0 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/util/AdapterUtil.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/util/AdapterUtil.java @@ -10,7 +10,6 @@ import org.objectweb.asm.commons.InstructionAdapter; import org.objectweb.asm.tree.*; import org.objectweb.asm.util.Textifier; -import org.objectweb.asm.util.TraceFieldVisitor; import org.objectweb.asm.util.TraceMethodVisitor; import org.sinytra.adapter.patch.analysis.LocalVariableLookup; import org.sinytra.adapter.patch.analysis.selector.AnnotationHandle; 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 93ab390..b0de265 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 @@ -1,14 +1,17 @@ package org.sinytra.adapter.patch.test.mixin; +import com.mojang.logging.LogUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.objectweb.asm.tree.ClassNode; import org.sinytra.adapter.patch.api.Patch; import org.sinytra.adapter.patch.api.PatchEnvironment; import org.sinytra.adapter.patch.api.RefmapHolder; import org.sinytra.adapter.patch.fixes.FieldTypeUsageTransformer; -import org.sinytra.adapter.patch.transformer.SoftMethodParamsPatch; import org.sinytra.adapter.patch.transformer.dynfix.DynamicInjectionPointPatch; import org.sinytra.adapter.patch.util.provider.ClassLookup; +import org.slf4j.Logger; import org.spongepowered.asm.mixin.FabricUtil; import java.util.List; @@ -20,6 +23,35 @@ public class DynamicMixinPatchTest extends MinecraftMixinPatchTest { .transform(new FieldTypeUsageTransformer()) .build() ); + private static final Logger LOGGER = LogUtils.getLogger(); + + private static PatchEnvironment patchEnvironment; + + @BeforeAll + static void initialize() { + ClassLookup cleanLookup = createCleanLookup(); + ClassLookup dirtyLookup = createDirtyLookup(); + patchEnvironment = PatchEnvironment.create( + new RefmapHolder() { + @Override + public String remap(String cls, String reference) { + return reference; + } + + @Override + public void copyEntries(String from, String to) {} + }, + cleanLookup, + dirtyLookup, + new BytecodeFixerUpperTestFrontend(cleanLookup, dirtyLookup).unwrap(), + FabricUtil.COMPATIBILITY_LATEST + ); + } + + @AfterAll + static void postTest() { + LOGGER.info("Complete report:\n\n{}", patchEnvironment.auditTrail().getCompleteReport()); + } @Test void testUpdatedInjectionPointAtAssignment() throws Exception { @@ -230,25 +262,7 @@ void testDynamicParameterTypeAdapter() throws Exception { protected LoadResult load(String className, List allowedMethods) throws Exception { final ClassNode patched = loadClass(className); patched.methods.removeIf(m -> !allowedMethods.contains(m.name)); - ClassLookup cleanLookup = createCleanLookup(); - ClassLookup dirtyLookup = createDirtyLookup(); - final PatchEnvironment env = PatchEnvironment.create( - new RefmapHolder() { - @Override - public String remap(String cls, String reference) { - return reference; - } - - @Override - public void copyEntries(String from, String to) { - } - }, - createCleanLookup(), - createDirtyLookup(), - new BytecodeFixerUpperTestFrontend(cleanLookup, dirtyLookup).unwrap(), - FabricUtil.COMPATIBILITY_LATEST - ); - DYNAMIC_PATCHES.forEach(p -> p.apply(patched, env)); - return new LoadResult(env, patched, loadClass(className)); + DYNAMIC_PATCHES.forEach(p -> p.apply(patched, patchEnvironment)); + return new LoadResult(patchEnvironment, patched, loadClass(className)); } } diff --git a/test/src/test/java/org/sinytra/adapter/patch/test/mixin/MinecraftMixinPatchTest.java b/test/src/test/java/org/sinytra/adapter/patch/test/mixin/MinecraftMixinPatchTest.java index 624fb6c..9b26256 100644 --- a/test/src/test/java/org/sinytra/adapter/patch/test/mixin/MinecraftMixinPatchTest.java +++ b/test/src/test/java/org/sinytra/adapter/patch/test/mixin/MinecraftMixinPatchTest.java @@ -104,7 +104,7 @@ protected final void assertSameField( .stream().filter(m -> m.name.equals(testName + "Expected")) .findFirst().orElseThrow(); - LOGGER.info("Patched field node: \n{}", patched); + LOGGER.info("Patched field node: {} {}", patched.name, patched.desc); assertEquals(patched.desc, expected.desc, "Field types differ"); }