diff --git a/core/src/main/java/org/sinytra/adapter/analysis/InheritanceHandler.java b/core/src/main/java/org/sinytra/adapter/analysis/InheritanceHandler.java index 3d8c6bd..2adf4fe 100644 --- a/core/src/main/java/org/sinytra/adapter/analysis/InheritanceHandler.java +++ b/core/src/main/java/org/sinytra/adapter/analysis/InheritanceHandler.java @@ -3,11 +3,14 @@ import org.objectweb.asm.tree.ClassNode; import org.sinytra.adapter.util.provider.ClassLookup; +import java.util.concurrent.ConcurrentHashMap; import java.util.*; public class InheritanceHandler { private final ClassLookup classProvider; - private final Map> parentCache = new HashMap<>(); + // ConcurrentHashMap: one handler instance is shared by every mixin using the same class lookup, and the + // transformation is driven from a ForkJoinPool. + private final Map> parentCache = new ConcurrentHashMap<>(); public InheritanceHandler(ClassLookup classProvider) { this.classProvider = classProvider; @@ -24,11 +27,13 @@ public boolean isClassInherited(String child, String parent) { public Collection getClassParents(String name) { Collection parents = this.parentCache.get(name); - if (parents == null) { - parents = computeClassParents(name); - this.parentCache.put(name, parents); + if (parents != null) { + return parents; } - return parents; + // Not computeIfAbsent: computeClassParents recurses into getClassParents, which it rejects. + Collection computed = computeClassParents(name); + Collection prev = this.parentCache.putIfAbsent(name, computed); + return prev != null ? prev : computed; } private Collection computeClassParents(String name) { @@ -51,3 +56,4 @@ private Collection computeClassParents(String name) { return parents; } } + diff --git a/core/src/main/java/org/sinytra/adapter/analysis/locals/LocalVarAnalyzer.java b/core/src/main/java/org/sinytra/adapter/analysis/locals/LocalVarAnalyzer.java index 63cec50..b9b6801 100644 --- a/core/src/main/java/org/sinytra/adapter/analysis/locals/LocalVarAnalyzer.java +++ b/core/src/main/java/org/sinytra/adapter/analysis/locals/LocalVarAnalyzer.java @@ -9,6 +9,7 @@ import org.objectweb.asm.tree.*; import org.sinytra.adapter.analysis.params.EnhancedParamsDiff; import org.sinytra.adapter.analysis.params.ParamsDiffSnapshot; +import org.sinytra.adapter.env.ann.AtData; import org.sinytra.adapter.env.ctx.LocalVariable; import org.sinytra.adapter.env.ctx.MixinContext; import org.sinytra.adapter.env.ctx.TargetPair; @@ -26,11 +27,20 @@ public record CapturedLocalsInfo(AdapterUtil.CapturedLocals capturedLocals, Para @Nullable public static CapturedLocalsInfo getCapturedLocals(MixinContext context, TargetPair dirtyTarget) { + return getCapturedLocals(context, dirtyTarget, null); + } + + /** + * @param atData the effective {@code @At} data, or {@code null} to fall back to the annotation as written + * (adaptation may have moved the injection site) + */ + @Nullable + public static CapturedLocalsInfo getCapturedLocals(MixinContext context, TargetPair dirtyTarget, @Nullable AtData atData) { AdapterUtil.CapturedLocals capturedLocals = AdapterUtil.getCapturedLocals(context, dirtyTarget); if (capturedLocals == null) return null; // Get available local variables at the injection point in the target method - List available = context.methods().getTargetMethodLocals(capturedLocals.target()); + List available = context.methods().getTargetMethodLocals(capturedLocals.target(), atData); if (available == null) return null; List availableTypes = available.stream().map(LocalVariable::type).toList(); diff --git a/core/src/main/java/org/sinytra/adapter/env/MockMixinRuntime.java b/core/src/main/java/org/sinytra/adapter/env/MockMixinRuntime.java index b39b443..0e0457e 100644 --- a/core/src/main/java/org/sinytra/adapter/env/MockMixinRuntime.java +++ b/core/src/main/java/org/sinytra/adapter/env/MockMixinRuntime.java @@ -72,8 +72,8 @@ public static IMixinContext forClass(String className, String targetClass, Patch return new ClassMixinContext(className, targetClass, environment); } - public static ISliceContext forSlice(IMixinContext context, MethodNode methodNode) { - return new MethodSliceContext(context, methodNode); + public static ISliceContext forSlice(IMixinContext context, MethodNode methodNode, AnnotationNode annotation) { + return new MethodSliceContext(context, methodNode, annotation); } public static InjectionInfo forInjectionInfo(String className, String targetClass, PatchEnvironment environment) { @@ -102,7 +102,7 @@ public static InjectionInfo createModifyVariableInjectionInfo(IMixinContext mixi return info; } - private record MethodSliceContext(IMixinContext context, MethodNode methodNode) implements ISliceContext { + private record MethodSliceContext(IMixinContext context, MethodNode methodNode, AnnotationNode annotation) implements ISliceContext { @Override public IMixinContext getMixin() { return this.context; @@ -117,7 +117,9 @@ public String remap(String reference) { @Override public String getElementDescription() {return "dummy";} @Override public MethodSlice getSlice(String id) {throw new UnsupportedOperationException();} @Override public MethodNode getMethod() {return this.methodNode;} - @Override public AnnotationNode getAnnotationNode() {throw new UnsupportedOperationException();} + // NOTE: the remaining stubs below deliberately still throw. A loud failure is preferable to a + // silent wrong answer: if Mixin reaches another of these entry points we want to see it. + @Override public AnnotationNode getAnnotationNode() {return this.annotation;} @Override public ISelectorContext getParent() {throw new UnsupportedOperationException();} @Override public IAnnotationHandle getAnnotation() {throw new UnsupportedOperationException();} @Override public IAnnotationHandle getSelectorAnnotation() {throw new UnsupportedOperationException();} diff --git a/core/src/main/java/org/sinytra/adapter/env/ann/ClassTarget.java b/core/src/main/java/org/sinytra/adapter/env/ann/ClassTarget.java index edf6ba7..bdfbffc 100644 --- a/core/src/main/java/org/sinytra/adapter/env/ann/ClassTarget.java +++ b/core/src/main/java/org/sinytra/adapter/env/ann/ClassTarget.java @@ -44,6 +44,9 @@ public static ClassTarget parse(AnnotationHandle handle, RefMapper mapper) { .or(() -> handle.>getValue(MIXIN_TARGETS).map(v -> { List types = v.get().stream() .map(mapper::remap) + // "targets" holds dotted binary names while Type.getObjectType expects an internal name; + // only '.' becomes '/', the '$' of an inner class stays. + .map(name -> name.replace('.', '/')) .map(Type::getObjectType) .toList(); return new ClassTarget(types, Either.right(v)); diff --git a/core/src/main/java/org/sinytra/adapter/env/ann/ConstantData.java b/core/src/main/java/org/sinytra/adapter/env/ann/ConstantData.java index 753f850..2754ded 100644 --- a/core/src/main/java/org/sinytra/adapter/env/ann/ConstantData.java +++ b/core/src/main/java/org/sinytra/adapter/env/ann/ConstantData.java @@ -13,7 +13,7 @@ public class ConstantData { private static final PropertyContainerTemplate TEMPLATE = PropertyContainerTemplate.builder() - .requireOne(Keys.DOUBLE_VALUE, Keys.CLASS_VALUE) + .requireOne(Keys.DOUBLE_VALUE, Keys.CLASS_VALUE, Keys.INT_VALUE, Keys.FLOAT_VALUE, Keys.LONG_VALUE, Keys.BOOLEAN_VALUE) .build(); private final PropertyContainer properties; @@ -49,5 +49,11 @@ public static Optional parse(AnnotationHandle handle) { public static class Keys { public static final PropertyKey DOUBLE_VALUE = PropertyKey.create("doubleValue", Double.class); public static final PropertyKey CLASS_VALUE = PropertyKey.create("classValue", Type.class); + public static final PropertyKey INT_VALUE = PropertyKey.create("intValue", Integer.class); + public static final PropertyKey FLOAT_VALUE = PropertyKey.create("floatValue", Float.class); + public static final PropertyKey LONG_VALUE = PropertyKey.create("longValue", Long.class); + public static final PropertyKey BOOLEAN_VALUE = PropertyKey.create("booleanValue", Boolean.class); } } + + diff --git a/core/src/main/java/org/sinytra/adapter/env/ctx/MethodFinder.java b/core/src/main/java/org/sinytra/adapter/env/ctx/MethodFinder.java index caec2cc..bfc74f7 100644 --- a/core/src/main/java/org/sinytra/adapter/env/ctx/MethodFinder.java +++ b/core/src/main/java/org/sinytra/adapter/env/ctx/MethodFinder.java @@ -99,7 +99,31 @@ public Pair> findMethods(ClassLookup lookup, MethodQ } if (candidates.isEmpty() && targetClass.superName != null) { - return findMethods(lookup, new MethodQualifier(Type.getObjectType(targetClass.superName).getDescriptor(), qualifier.name(), qualifier.desc()), flags); + // Returns unconditionally: falling through would make the interface traversal below reachable, + // and interface declarations returned as owners break startup. + return findMethods(lookup, + new MethodQualifier(Type.getObjectType(targetClass.superName).getDescriptor(), qualifier.name(), qualifier.desc()), flags); + } + + if (candidates.isEmpty()) { + // superName reaches superclasses only; a target can equally be a default method on an interface, + // which is how NeoForge attaches helpers (BlockState#canStickTo is declared on + // IBlockStateExtension). Declines when more than one interface matches. + Pair> viaInterface = null; + for (String itf : targetClass.interfaces) { + Pair> found = findMethods(lookup, + new MethodQualifier(Type.getObjectType(itf).getDescriptor(), qualifier.name(), qualifier.desc()), flags); + if (found == null || found.getSecond().isEmpty()) { + continue; + } + if (viaInterface != null) { + return Pair.of(targetClass, List.of()); + } + viaInterface = found; + } + if (viaInterface != null) { + return viaInterface; + } } return Pair.of(targetClass, candidates); @@ -109,3 +133,5 @@ private static boolean hasFlag(int flags, int flag) { return (flags & flag) != 0; } } + + diff --git a/core/src/main/java/org/sinytra/adapter/env/ctx/MethodHelper.java b/core/src/main/java/org/sinytra/adapter/env/ctx/MethodHelper.java index 4bb6ddc..ec0011a 100644 --- a/core/src/main/java/org/sinytra/adapter/env/ctx/MethodHelper.java +++ b/core/src/main/java/org/sinytra/adapter/env/ctx/MethodHelper.java @@ -216,7 +216,7 @@ private InsnList getSlicedInsns(AnnotationHandle parentAnnotation, ClassNode cla }) .map(sliceAnn -> { IMixinContext mixinContext = MockMixinRuntime.forClass(classNode.name, targetClass.name, context.environment()); - ISliceContext sliceContext = MockMixinRuntime.forSlice(mixinContext, injectorMethod); + ISliceContext sliceContext = MockMixinRuntime.forSlice(mixinContext, injectorMethod, sliceAnn); return computeSlicedInsns(sliceContext, sliceAnn, mixinTarget); }) .orElse(targetMethod.instructions); @@ -244,7 +244,42 @@ public List getTargetMethodLocals(TargetPair target, int startPos @Nullable public List getTargetMethodLocals(TargetPair target, int startPos, int lvtCompatLevel) { - List targetInsns = findInjectionTargetInsns(target); + return getTargetMethodLocals(target, findInjectionTargetInsns(target), startPos, lvtCompatLevel); + } + + /** Like {@link #findInjectionTargetInsns(TargetPair)}, but from the given {@code @At} data. */ + public List findInjectionTargetInsns(@Nullable TargetPair target, @Nullable AtData atData) { + if (target == null) return List.of(); + if (atData == null) return findInjectionTargetInsns(target); + + return computeInjectionTargetInsns( + target, + this.context::injectionPointAnnotation, + (ctx, h) -> { + AnnotationNode atCopy = atData.toAnnotationNode(); + AnnotationHandle annCopy = this.context.methodAnnotation().copy(); + annCopy.setOrAppendNonNull(PROPERTY_AT, atCopy); + + return InjectionPoint.parse(ctx, this.context.methodNode(), annCopy.unwrap(), atCopy); + }, + true + ); + } + + /** Variant of {@link #getTargetMethodLocals(TargetPair)} using the effective {@code @At} data. */ + @Nullable + public List getTargetMethodLocals(TargetPair target, @Nullable AtData atData) { + Type[] targetParams = Type.getArgumentTypes(target.methodNode().desc); + boolean isStatic = isStatic(this.context.methodNode()); + int lvtOffset = isStatic ? 0 : 1; + // The starting LVT index is of the first var after all method parameters. Offset by 1 for instance methods to skip 'this' + int targetLocalPos = targetParams.length + lvtOffset; + return getTargetMethodLocals(target, findInjectionTargetInsns(target, atData), targetLocalPos, + this.context.environment().fabricLVTCompatibility()); + } + + @Nullable + private List getTargetMethodLocals(TargetPair target, List targetInsns, int startPos, int lvtCompatLevel) { if (targetInsns.isEmpty()) { LOGGER.debug("Skipping LVT patch, no target instructions found"); return null; diff --git a/core/src/main/java/org/sinytra/adapter/env/ctx/MixinContext.java b/core/src/main/java/org/sinytra/adapter/env/ctx/MixinContext.java index b9c7f35..327e56a 100644 --- a/core/src/main/java/org/sinytra/adapter/env/ctx/MixinContext.java +++ b/core/src/main/java/org/sinytra/adapter/env/ctx/MixinContext.java @@ -5,6 +5,7 @@ import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodNode; import org.sinytra.adapter.analysis.selector.AnnotationHandle; +import org.sinytra.adapter.env.ann.AtData; import org.sinytra.adapter.env.ann.ClassTarget; import org.sinytra.adapter.patch.mixin.MixinFlag; import org.sinytra.adapter.patch.mixin.MixinType; @@ -12,6 +13,7 @@ import org.sinytra.adapter.patch.resolver.Resolvers; import org.sinytra.adapter.types.TypeAdapter; import org.sinytra.adapter.util.AdapterUtil; +import org.sinytra.adapter.util.MethodQualifier; import org.sinytra.adapter.util.provider.ClassLookup; import java.util.ArrayList; @@ -36,6 +38,32 @@ public class MixinContext implements RefMapper, Auditor { private final Resolvers resolvers = new Resolvers(); private final Processors processors = new Processors(); + /** + * Where the injection point ended up after adaptation, published by PipelineMethodTransformer. + */ + @Nullable + private AtData effectiveAtData; + @Nullable + private MethodQualifier effectiveTargetMethod; + + public void setEffectiveInjectionPoint(@Nullable AtData atData, @Nullable MethodQualifier targetMethod) { + this.effectiveAtData = atData; + this.effectiveTargetMethod = targetMethod; + } + + /** + * The {@code @At} data that applies to this handler, or {@code null} when adaptation did not run. + */ + @Nullable + public AtData effectiveAtData() { + return this.effectiveAtData; + } + + @Nullable + public MethodQualifier effectiveTargetMethod() { + return this.effectiveTargetMethod; + } + public MixinContext(MixinType mixinType, PatchContext patchContext, ClassTarget classTarget, ClassNode classNode, MethodNode methodNode, AnnotationHandle methodAnnotation, AnnotationHandle injectionPointAnnotation, Set flags) { this.mixinType = mixinType; this.patchContext = patchContext; diff --git a/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironment.java b/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironment.java index e82698d..3623c20 100644 --- a/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironment.java +++ b/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironment.java @@ -38,4 +38,13 @@ static PatchEnvironment create(RefmapHolder refmapHolder, ClassLookup cleanClass Collection getKnownNamespaces(); boolean isKnownPackage(String pkg); + + /** + * When enabled, mixin handlers that could not be adapted are removed from their class rather than left in + * the jar, so the mod loads with those features disabled. Read from the + * {@code connector.stripFailingMixins} system property. + */ + default boolean stripFailingMixins() { + return Boolean.getBoolean("connector.stripFailingMixins"); + } } diff --git a/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironmentImpl.java b/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironmentImpl.java index f959a0e..d9d6569 100644 --- a/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironmentImpl.java +++ b/core/src/main/java/org/sinytra/adapter/env/ctx/PatchEnvironmentImpl.java @@ -8,8 +8,8 @@ import org.sinytra.adapter.util.provider.MixinClassLookup; import java.util.Collection; -import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; public final class PatchEnvironmentImpl implements PatchEnvironment { private final RefmapHolder refmapHolder; @@ -21,7 +21,8 @@ public final class PatchEnvironmentImpl implements PatchEnvironment { private final AuditTrail auditTrail; private final @Nullable Collection pkgNamespaces; - private final Map inheritanceHandlers = new HashMap<>(); + // Classes are processed in parallel, so the cache is shared across threads. + private final Map inheritanceHandlers = new ConcurrentHashMap<>(); public PatchEnvironmentImpl( RefmapHolder refmapHolder, @@ -77,7 +78,14 @@ public MixinClassGenerator classGenerator() { @Override public InheritanceHandler inheritanceHandler(ClassLookup lookup) { - return this.inheritanceHandlers.computeIfAbsent(lookup, InheritanceHandler::new); + // Not computeIfAbsent: the handler constructor can re-enter this method. + InheritanceHandler existing = this.inheritanceHandlers.get(lookup); + if (existing != null) { + return existing; + } + InheritanceHandler created = new InheritanceHandler(lookup); + InheritanceHandler prev = this.inheritanceHandlers.putIfAbsent(lookup, created); + return prev != null ? prev : created; } @Override diff --git a/core/src/main/java/org/sinytra/adapter/env/util/MixinAnnotations.java b/core/src/main/java/org/sinytra/adapter/env/util/MixinAnnotations.java index 0f10121..88804d2 100644 --- a/core/src/main/java/org/sinytra/adapter/env/util/MixinAnnotations.java +++ b/core/src/main/java/org/sinytra/adapter/env/util/MixinAnnotations.java @@ -6,12 +6,17 @@ public class MixinAnnotations { public static final String REDIRECT = "Lorg/spongepowered/asm/mixin/injection/Redirect;"; public static final String MODIFY_ARG = "Lorg/spongepowered/asm/mixin/injection/ModifyArg;"; public static final String MODIFY_ARGS = "Lorg/spongepowered/asm/mixin/injection/ModifyArgs;"; + public static final String MODIFY_ARGS_INTERNAL_NAME = "org/spongepowered/asm/mixin/injection/ModifyArgs"; public static final String MODIFY_VAR = "Lorg/spongepowered/asm/mixin/injection/ModifyVariable;"; public static final String MODIFY_CONST = "Lorg/spongepowered/asm/mixin/injection/ModifyConstant;"; + public static final String MODIFY_CONST_INTERNAL_NAME = "org/spongepowered/asm/mixin/injection/ModifyConstant"; public static final String OVERWRITE = "Lorg/spongepowered/asm/mixin/Overwrite;"; + public static final String OVERWRITE_INTERNAL_NAME = "org/spongepowered/asm/mixin/Overwrite"; // Interface mixins public static final String ACCESSOR = "Lorg/spongepowered/asm/mixin/gen/Accessor;"; + public static final String ACCESSOR_INTERNAL_NAME = "org/spongepowered/asm/mixin/gen/Accessor"; public static final String INVOKER = "Lorg/spongepowered/asm/mixin/gen/Invoker;"; + public static final String INVOKER_INTERNAL_NAME = "org/spongepowered/asm/mixin/gen/Invoker"; // Mixinextras annotations public static final String MODIFY_EXPR_VAL = "Lcom/llamalad7/mixinextras/injector/ModifyExpressionValue;"; public static final String MODIFY_EXPR_VAL_INTERNAL_NAME = "com/llamalad7/mixinextras/injector/ModifyExpressionValue"; diff --git a/core/src/main/java/org/sinytra/adapter/patch/DynamicPatches.java b/core/src/main/java/org/sinytra/adapter/patch/DynamicPatches.java index 343b08f..863f402 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/DynamicPatches.java +++ b/core/src/main/java/org/sinytra/adapter/patch/DynamicPatches.java @@ -23,9 +23,12 @@ public class DynamicPatches { public static Multimap methodTransformers(List patches) { return ImmutableMultimap.of( - TxPhase.EARLY, new LocalCaptureUpgradeTransformer(), TxPhase.LOADED, new FieldAccessorTypeTransformer(), - TxPhase.VALIDATED, new PipelineMethodTransformer(patches, false) + TxPhase.VALIDATED, new PipelineMethodTransformer(patches, false), + // Must run after the pipeline: captured-locals signatures are re-derived from the patched method + // at the adapted injection point. Safe only because Patcher#normalizeParameterAnnotations keeps the + // parameter annotation counts in sync when captured parameters are added here. + TxPhase.VALIDATED, new LocalCaptureUpgradeTransformer() ); } } diff --git a/core/src/main/java/org/sinytra/adapter/patch/MixinParser.java b/core/src/main/java/org/sinytra/adapter/patch/MixinParser.java index b07dc0d..56c175b 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/MixinParser.java +++ b/core/src/main/java/org/sinytra/adapter/patch/MixinParser.java @@ -65,6 +65,14 @@ public static MixinMethodHandle parseMixin(ClassTarget cls, MethodNode method, R properties.setProperty(ControlKeys.TARGET_CLASS, targetClass); properties.setProperty(ControlKeys.RETURN_TYPE, Type.getReturnType(method.desc)); properties.setProperty(SpecialKeys.STATIC, MethodHelper.isStatic(method)); + // @Overwrite / @Invoker name no "method"; set the qualifier derived from the mixin method so + // postProcessMixin below can complete the descriptor from the dirty target class. + if (!properties.hasProperty(MixinKeys.TARGET_METHOD)) { + MethodQualifier derived = mixinType.deriveTarget(properties, method); + if (derived != null) { + properties.setProperty(MixinKeys.TARGET_METHOD, derived); + } + } if (!postProcessMixin(properties, environment)) { continue; } @@ -102,6 +110,19 @@ private static boolean postProcessMixin(MutablePropertyContainer properties, Pat List targets = dirtyTarget.methods.stream() .filter(m -> target.matches(MethodQualifier.create(m))) .toList(); + + // Fall back to the superclass chain: an injector may name a method its target class only + // inherits (carve's @Redirect on Player, declared on LivingEntity), and Mixin resolves the target + // through the hierarchy too. + for (ClassNode cls = dirtyTarget; targets.isEmpty() && cls.superName != null; ) { + ClassNode parent = environment.dirtyClassLookup().getClass(cls.superName).orElse(null); + if (parent == null) break; + targets = parent.methods.stream() + .filter(m -> target.matches(MethodQualifier.create(m))) + .toList(); + cls = parent; + } + if (targets.size() == 1) { properties.setProperty(MixinKeys.TARGET_METHOD, MethodQualifier.create(targets.getFirst())); } diff --git a/core/src/main/java/org/sinytra/adapter/patch/Patcher.java b/core/src/main/java/org/sinytra/adapter/patch/Patcher.java index 8a1a9b1..01d4c79 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/Patcher.java +++ b/core/src/main/java/org/sinytra/adapter/patch/Patcher.java @@ -3,15 +3,21 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; import com.mojang.logging.LogUtils; +import org.objectweb.asm.Type; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.AnnotationNode; import org.objectweb.asm.tree.ClassNode; +import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; import org.sinytra.adapter.analysis.MixinGroups; import org.sinytra.adapter.analysis.selector.AnnotationHandle; +import org.sinytra.adapter.env.ann.AtData; import org.sinytra.adapter.env.ann.ClassTarget; import org.sinytra.adapter.env.ctx.*; import org.sinytra.adapter.env.util.MixinAnnotationConstants; import org.sinytra.adapter.patch.MixinParser.MixinMethodHandle; import org.sinytra.adapter.patch.config.key.MixinKeys; +import org.sinytra.adapter.patch.config.key.SpecialKeys; import org.sinytra.adapter.patch.config.MutableConfiguration; import org.sinytra.adapter.patch.config.PropertyContainerTemplate; import org.sinytra.adapter.patch.mixin.MixinFlag; @@ -24,6 +30,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.Set; import static org.sinytra.adapter.util.AdapterUtil.MIXINPATCH; @@ -60,19 +67,68 @@ public PatchResult process(ClassNode classNode) { if (mixinClass == null) return result; // Mixin-level transformations + List failedMixins = new ArrayList<>(); for (MixinParser.MixinMethodHandle mixin : mixinClass.mixins()) { - PatchResult subResult = processMixin(classNode, classTarget, context, mixin); + PatchResult subResult = processMixin(classNode, classTarget, context, mixin, failedMixins); result = result.or(subResult); } + if (!failedMixins.isEmpty() && this.environment.stripFailingMixins()) { + result = result.or(stripFailedMixins(classNode, failedMixins)); + } + context.run(); MixinGroups groups = MixinGroups.create(mixinClass.mixins().stream().map(MixinMethodHandle::methodNode).toList()); this.environment.auditTrail().processGroups(classNode, groups); + // Must run last: context.run() can still add parameter annotations, and ASM sizes their arrays from + // the descriptor as it stood when the array was created, so they can end up shorter. + if (normalizeParameterAnnotations(classNode)) { + result = result.or(PatchResult.COMPUTE_FRAMES); + } + return result; } - private PatchResult processMixin(ClassNode classNode, ClassTarget classTarget, PatchContext patchContext, MixinParser.MixinMethodHandle mixin) { + /** See PipelineMethodTransformer#skip: every give-up exit must record the mixin, or the jar is reported as compatible. */ + private static PatchResult skip(MixinContext context, org.sinytra.adapter.patch.config.Configuration config, String reason) { + context.environment().auditTrail().recordResult(context, config, org.sinytra.adapter.env.ctx.AuditTrail.Match.NONE); + // These early exits run outside any pushAudit/popAudit frame, so open one to report the reason. + context.pushAudit(SKIP_REASON_FRAME); + try { + context.recordCtxAudit("Skipped mixin: %s", reason); + } catch (RuntimeException ignored) { + // still best-effort: recordResult above is what makes the skip itself visible + } finally { + context.popAudit(); + } + return PatchResult.PASS; + } + + /** Audit frame marker for give-up exits; its {@code toString} is what the report shows. */ + private static final Object SKIP_REASON_FRAME = new Object() { + @Override + public String toString() { + return "skip"; + } + }; + + private PatchResult processMixin(ClassNode classNode, ClassTarget classTarget, PatchContext patchContext, MixinParser.MixinMethodHandle mixin, List failed) { + MixinType handlerType = mixin.mixinType(); + + // Types without a method target (@Accessor) carry no TARGET_METHOD; send them to their own validation. + if (!handlerType.targetsMethod()) { + MixinContext nonMethodContext = new MixinContext(handlerType, patchContext, classTarget, classNode, + mixin.methodNode(), mixin.methodAnnotation(), null, handlerType.getFlags()); + this.environment.auditTrail().prepareMethod(nonMethodContext); + + MutableConfiguration nonMethodConfig = MutableConfiguration.create(handlerType.getConfigurationTemplate()); + nonMethodConfig.mergeFrom(mixin.properties()); + + handlerType.validateNonMethodTarget(nonMethodContext, nonMethodConfig); + return PatchResult.PASS; + } + MethodQualifier target = mixin.properties().getProperty(MixinKeys.TARGET_METHOD).orElse(null); if (target == null) return PatchResult.PASS; @@ -104,7 +160,8 @@ private PatchResult processMixin(ClassNode classNode, ClassTarget classTarget, P TxResult preResult = mixinType.preProcess(mixinContext, configuration, mixinContext.getResolvers(), mixinContext.getProcessors()); if (preResult == TxResult.FAIL) { LOGGER.debug(MIXINPATCH, "Skipping mixin {} due to failed preProcess", mixinId); - return PatchResult.PASS; + failed.add(mixin); + return skip(mixinContext, configuration, "failed preProcess"); } // << RUN LOADED PHASE @@ -118,7 +175,8 @@ private PatchResult processMixin(ClassNode classNode, ClassTarget classTarget, P // Validate clean config if (!configuration.validate()) { LOGGER.debug(MIXINPATCH, "Skipping mixin {} due to invalid CLEAN config", mixinId); - return PatchResult.PASS; + failed.add(mixin); + return skip(mixinContext, configuration, "invalid CLEAN config"); } // Temporarily set this to a high number for frame analysis to work @@ -133,9 +191,102 @@ private PatchResult processMixin(ClassNode classNode, ClassTarget classTarget, P result = result.or(txResult); } + // A "locals = LocalCapture..." list is compiled against the injection site as it was when the mod was + // built, and Mixin aborts at runtime when it no longer lines up. A handler without the mark set by + // LocalCaptureUpgradeTransformer is failed via the failure list, as Match.or() would overwrite a NONE. + if (configuration.hasProperty(MixinKeys.LOCALS) + && !configuration.hasProperty(SpecialKeys.LOCALS_UPGRADED)) { + LOGGER.debug(MIXINPATCH, "Mixin {} captures locals that could not be verified; it would abort at runtime", mixinId); + failed.add(mixin); + } + + // A resolver failure records Match.NONE rather than throwing, so the audit trail is the only witness. + if (this.environment.auditTrail().getMatch(mixinContext) == AuditTrail.Match.NONE) { + failed.add(mixin); + } + return result; } + /** Removes handlers that could not be adapted so Mixin never registers them; only when nothing calls them. */ + private PatchResult stripFailedMixins(ClassNode classNode, List failed) { + boolean removedAny = false; + for (MixinParser.MixinMethodHandle mixin : failed) { + MethodNode method = mixin.methodNode(); + if (isReferenced(classNode, method)) { + LOGGER.debug(MIXINPATCH, "Not stripping {}#{}, it is referenced by other code in the class", + classNode.name, method.name); + continue; + } + if (classNode.methods.remove(method)) { + removedAny = true; + LOGGER.warn(MIXINPATCH, "Removed unadaptable mixin handler {}#{}{} - the mod loads with this feature disabled", + classNode.name, method.name, method.desc); + } + } + // Removing methods invalidates any previously computed frames. + return removedAny ? PatchResult.COMPUTE_FRAMES : PatchResult.PASS; + } + + /** + * Keeps a method's parameter-annotation arrays and counts in sync with its descriptor; a stale count + * aborts the whole mod's transform inside ASM's {@code MethodWriter}. + */ + private static boolean normalizeParameterAnnotations(ClassNode classNode) { + boolean changed = false; + for (MethodNode method : classNode.methods) { + int paramCount = Type.getArgumentTypes(method.desc).length; + + List[] visible = resizeParameterAnnotations(method.visibleParameterAnnotations, paramCount); + if (visible != method.visibleParameterAnnotations) { + method.visibleParameterAnnotations = visible; + changed = true; + } + List[] invisible = resizeParameterAnnotations(method.invisibleParameterAnnotations, paramCount); + if (invisible != method.invisibleParameterAnnotations) { + method.invisibleParameterAnnotations = invisible; + changed = true; + } + + int visibleCount = visible == null ? 0 : visible.length; + if (method.visibleAnnotableParameterCount != visibleCount) { + method.visibleAnnotableParameterCount = visibleCount; + changed = true; + } + int invisibleCount = invisible == null ? 0 : invisible.length; + if (method.invisibleAnnotableParameterCount != invisibleCount) { + method.invisibleAnnotableParameterCount = invisibleCount; + changed = true; + } + } + return changed; + } + + @SuppressWarnings("unchecked") + private static List[] resizeParameterAnnotations(List[] annotations, int paramCount) { + if (annotations == null || annotations.length == paramCount) { + return annotations; + } + List[] resized = (List[]) new List[paramCount]; + System.arraycopy(annotations, 0, resized, 0, Math.min(annotations.length, paramCount)); + return resized; + } + + private static boolean isReferenced(ClassNode classNode, MethodNode method) { + for (MethodNode other : classNode.methods) { + if (other == method) continue; + for (AbstractInsnNode insn : other.instructions) { + if (insn instanceof MethodInsnNode call + && call.name.equals(method.name) + && call.desc.equals(method.desc) + && call.owner.equals(classNode.name)) { + return true; + } + } + } + return false; + } + private Collection getTransformers(TxPhase phase) { return this.methodTransformers.get(phase); } @@ -178,3 +329,4 @@ public Patcher build() { } } } + diff --git a/core/src/main/java/org/sinytra/adapter/patch/config/key/MixinKeys.java b/core/src/main/java/org/sinytra/adapter/patch/config/key/MixinKeys.java index f4c37de..496c8db 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/config/key/MixinKeys.java +++ b/core/src/main/java/org/sinytra/adapter/patch/config/key/MixinKeys.java @@ -43,7 +43,11 @@ public class MixinKeys { .build(); public static final PropertyKey TARGET_CONSTANT = PropertyKey.builder("constant") .parser((value, mapper) -> { + // Same null guard as TARGET_AT above: "constant" is not always a single annotation, and the null + // would reach new AnnotationHandle(null) inside MixinParser and abort the whole transform. AnnotationNode node = ControlKeys.parseSingle(value, AnnotationNode.class); + if (node == null) return null; + AnnotationHandle handle = new AnnotationHandle(node); return ConstantData.parse(handle).orElse(null); }) @@ -51,11 +55,18 @@ public class MixinKeys { .build(); // Mixin data public static final PropertyKey CANCELLABLE = PropertyKey.create("cancellable", Boolean.class); + /** {@code @Accessor}/{@code @Invoker} field or method name. */ + public static final PropertyKey ACCESSOR_VALUE = PropertyKey.create("value", String.class); public static final PropertyKey INDEX = PropertyKey.create("index", Integer.class); public static final PropertyKey ORDINAL = PropertyKey.create("ordinal", Integer.class); public static final PropertyKey ARGS_ONLY = PropertyKey.create("argsOnly", Boolean.class); public static final PropertyKey SLICE = PropertyKey.builder("slice") - .parser((value, mapper) -> SliceData.parse(new AnnotationHandle((AnnotationNode) value), mapper)) + // A repeated annotation arrives as a List, so the bare cast to AnnotationNode throws; parseSingle + // unwraps the single element and yields null otherwise (the multi-element form is handled by SLICES). + .parser((value, mapper) -> { + AnnotationNode node = ControlKeys.parseSingle(value, AnnotationNode.class); + return node == null ? null : SliceData.parse(new AnnotationHandle(node), mapper); + }) .serializer(SliceData::toAnnotationNode) .build(); public static final PropertyKey> SLICES = PropertyKey.>builder("slice") @@ -68,3 +79,4 @@ public class MixinKeys { public static final PropertyKey LOCALS = PropertyKey.create("locals", LocalCapture.class); public static final PropertyKey REQUIRE = PropertyKey.create("require", Integer.class); } + diff --git a/core/src/main/java/org/sinytra/adapter/patch/config/key/SpecialKeys.java b/core/src/main/java/org/sinytra/adapter/patch/config/key/SpecialKeys.java index 9c7e1d5..0a75234 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/config/key/SpecialKeys.java +++ b/core/src/main/java/org/sinytra/adapter/patch/config/key/SpecialKeys.java @@ -15,6 +15,11 @@ public final class SpecialKeys { public static final PropertyKey EXTRACT_ORIGIN_PARAM = PropertyKey.create("_extract_origin_param"); public static final PropertyKey> REDIRECT_ADAPTER = PropertyKey.create("_redirect_adapter"); public static final PropertyKey STATIC = PropertyKey.create("_static"); + /** + * Set by {@code LocalCaptureUpgradeTransformer} once it has re-derived a capture list, so {@code Patcher} + * can tell an upgraded capture apart from one it could not verify. + */ + public static final PropertyKey LOCALS_UPGRADED = PropertyKey.create("_locals_upgraded"); private SpecialKeys() { } diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/AccessorMixin.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/AccessorMixin.java new file mode 100644 index 0000000..caf0f59 --- /dev/null +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/AccessorMixin.java @@ -0,0 +1,78 @@ +package org.sinytra.adapter.patch.mixin; + +import org.objectweb.asm.tree.ClassNode; +import org.sinytra.adapter.env.ctx.AuditTrail; +import org.sinytra.adapter.env.ctx.MixinContext; +import org.sinytra.adapter.patch.Recipe; +import org.sinytra.adapter.patch.TxResult; +import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.config.PropertyContainerTemplate; +import org.sinytra.adapter.patch.config.key.ControlKeys; +import org.sinytra.adapter.patch.config.key.MixinKeys; +import org.sinytra.adapter.patch.processor.Processors; +import org.sinytra.adapter.patch.resolver.Resolvers; + +/** + * Handles {@code @Accessor} mixins, the one handler that does not target a method at all. It opts out with + * {@link #targetsMethod()} because the method-first pipeline would mark every accessor as unadapted. + */ +public class AccessorMixin implements MixinType { + private static final PropertyContainerTemplate TEMPLATE = PropertyContainerTemplate.builder() + .require(ControlKeys.MIXIN_TYPE, ControlKeys.TARGET_CLASS, ControlKeys.RETURN_TYPE, ControlKeys.PARAMETERS) + .keys(MixinKeys.ACCESSOR_VALUE, MixinKeys.REQUIRE) + .build(); + + @Override + public PropertyContainerTemplate getConfigurationTemplate() { + return TEMPLATE; + } + + @Override + public boolean targetsMethod() { + return false; + } + + @Override + public TxResult preProcess(MixinContext context, MutableConfiguration clean, Resolvers resolvers, Processors processors) { + return TxResult.SUCCESS; + } + + @Override + public TxResult postProcess(MixinContext context, Configuration clean, MutableConfiguration dirty, Recipe recipe) { + return TxResult.SUCCESS; + } + + @Override + public void validateNonMethodTarget(MixinContext context, Configuration config) { + String declared = config.getProperty(MixinKeys.ACCESSOR_VALUE).orElse(null); + final String fieldName = (declared == null || declared.isEmpty()) + ? inferFieldName(context.methodNode().name) + : declared; + + String targetClass = config.getTargetClass(); + ClassNode target = targetClass == null ? null : context.dirtyLookup().getClass(targetClass).orElse(null); + + boolean found = target != null && fieldName != null + && target.fields.stream().anyMatch(f -> f.name.equals(fieldName)); + + context.environment().auditTrail().recordResult(context, config, + found ? AuditTrail.Match.FULL : AuditTrail.Match.NONE); + } + + /** Mixin infers the field from the method name when no value is given: {@code getFoo} accesses {@code foo}. */ + private static String inferFieldName(String methodName) { + String base; + if (methodName.startsWith("get") || methodName.startsWith("set")) { + base = methodName.substring(3); + } else if (methodName.startsWith("is")) { + base = methodName.substring(2); + } else { + return null; + } + if (base.isEmpty()) { + return null; + } + return Character.toLowerCase(base.charAt(0)) + base.substring(1); + } +} diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/InvokerMixin.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/InvokerMixin.java new file mode 100644 index 0000000..8232cd0 --- /dev/null +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/InvokerMixin.java @@ -0,0 +1,86 @@ +package org.sinytra.adapter.patch.mixin; + +import org.jetbrains.annotations.Nullable; +import org.objectweb.asm.tree.ClassNode; +import org.objectweb.asm.tree.MethodNode; +import org.sinytra.adapter.env.ctx.AuditTrail; +import org.sinytra.adapter.env.ctx.MixinContext; +import org.sinytra.adapter.patch.Recipe; +import org.sinytra.adapter.patch.TxResult; +import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.config.PropertyContainer; +import org.sinytra.adapter.patch.config.PropertyContainerTemplate; +import org.sinytra.adapter.patch.config.key.ControlKeys; +import org.sinytra.adapter.patch.config.key.MixinKeys; +import org.sinytra.adapter.patch.processor.Processors; +import org.sinytra.adapter.patch.resolver.Resolvers; +import org.sinytra.adapter.util.MethodQualifier; + +/** + * Handles {@code @Invoker} mixins. Like {@code @Accessor} an invoker is an interface mixin - Mixin generates a + * method that calls the target rather than injecting into a body - so the injection pipeline would always + * report a failure. It opts out with {@link #targetsMethod()} and looks the named method up instead. + */ +public class InvokerMixin implements MixinType { + private static final PropertyContainerTemplate TEMPLATE = PropertyContainerTemplate.builder() + .require(ControlKeys.MIXIN_TYPE, ControlKeys.TARGET_CLASS, ControlKeys.RETURN_TYPE, ControlKeys.PARAMETERS) + .keys(MixinKeys.ACCESSOR_VALUE, MixinKeys.REQUIRE) + .build(); + + @Override + public PropertyContainerTemplate getConfigurationTemplate() { + return TEMPLATE; + } + + @Override + public boolean targetsMethod() { + return false; + } + + @Override + public TxResult preProcess(MixinContext context, MutableConfiguration clean, Resolvers resolvers, Processors processors) { + return TxResult.SUCCESS; + } + + @Override + public TxResult postProcess(MixinContext context, Configuration clean, MutableConfiguration dirty, Recipe recipe) { + return TxResult.SUCCESS; + } + + @Override + public void validateNonMethodTarget(MixinContext context, Configuration config) { + MethodQualifier qualifier = deriveTarget(config, context.methodNode()); + String name = qualifier == null ? null : qualifier.name(); + + String targetClass = config.getTargetClass(); + ClassNode target = targetClass == null ? null : context.dirtyLookup().getClass(targetClass).orElse(null); + + boolean found = target != null && name != null + && target.methods.stream().anyMatch(m -> m.name.equals(name)); + + context.environment().auditTrail().recordResult(context, config, + found ? AuditTrail.Match.FULL : AuditTrail.Match.NONE); + } + + /** The invoker names its target with {@code value}; without one Mixin strips a call/invoke prefix. */ + @Nullable + @Override + public MethodQualifier deriveTarget(PropertyContainer properties, MethodNode method) { + String name = properties.getProperty(MixinKeys.ACCESSOR_VALUE).orElse(null); + if (name == null || name.isEmpty()) { + name = method.name; + for (String prefix : new String[]{"invoker", "invoke", "call"}) { + if (name.startsWith(prefix) && name.length() > prefix.length()) { + name = name.substring(prefix.length()); + break; + } + } + if (name.isEmpty()) { + return null; + } + name = Character.toLowerCase(name.charAt(0)) + name.substring(1); + } + return new MethodQualifier(name, null); + } +} diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinType.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinType.java index cd71d1b..e2fea30 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinType.java +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinType.java @@ -1,13 +1,17 @@ package org.sinytra.adapter.patch.mixin; +import org.jetbrains.annotations.Nullable; +import org.objectweb.asm.tree.MethodNode; import org.sinytra.adapter.env.ctx.MixinContext; import org.sinytra.adapter.patch.Recipe; import org.sinytra.adapter.patch.TxResult; import org.sinytra.adapter.patch.config.Configuration; import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.config.PropertyContainer; import org.sinytra.adapter.patch.config.PropertyContainerTemplate; import org.sinytra.adapter.patch.processor.Processors; import org.sinytra.adapter.patch.resolver.Resolvers; +import org.sinytra.adapter.util.MethodQualifier; import java.util.EnumSet; import java.util.Set; @@ -26,6 +30,27 @@ default boolean canInject(MixinContext context, Configuration config) { return true; } + /** + * Derives the target qualifier from the mixin method for types whose target is not named by a + * {@code method} property, e.g. {@code @Overwrite} and {@code @Invoker}. + */ + @Nullable + default MethodQualifier deriveTarget(PropertyContainer properties, MethodNode method) { + return null; + } + + /** + * Whether this handler targets a method and therefore belongs in the method-target pipeline. + * {@code @Accessor} returns false. + */ + default boolean targetsMethod() { + return true; + } + + /** Validation for types that opted out of the method pipeline; they must record their own audit result. */ + default void validateNonMethodTarget(MixinContext context, Configuration config) { + } + TxResult preProcess(MixinContext context, MutableConfiguration clean, Resolvers resolvers, Processors processors); TxResult postProcess(MixinContext context, Configuration clean, MutableConfiguration dirty, Recipe recipe); diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinTypes.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinTypes.java index 840f9fa..ec90ff8 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinTypes.java +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/MixinTypes.java @@ -14,25 +14,34 @@ public class MixinTypes { private static final Map MIXIN_TYPES = new HashMap<>(); // TODO: - // MixinConstants.MODIFY_ARGS, MixinConstants.MODIFY_CONST, MixinConstants.WRAP_WITH_CONDITION, - // MixinConstants.OVERWRITE (dont forget special modifyTarget handling) - // MixinConstants.ACCESSOR public static final MixinType INJECT = new InjectMixin(); public static final MixinType MODIFY_VAR = new ModifyVariableMixin(); public static final MixinType MODIFY_ARG = new ModifyArgMixin(); + public static final MixinType MODIFY_ARGS = new ModifyArgsMixin(); + public static final MixinType MODIFY_CONST = new ModifyConstantMixin(); public static final MixinType MODIFY_RET = new ModifyReturnValueMixin(); public static final MixinType REDIRECT = new RedirectMixin(); public static final MixinType WRAP_OP = new WrapOperationMixin(); public static final MixinType MODIFY_EXPR_VAL = new ModifyExpressionValueMixin(); + public static final MixinType OVERWRITE = new OverwriteMixin(); + public static final MixinType ACCESSOR = new AccessorMixin(); + public static final MixinType INVOKER = new InvokerMixin(); static { registerMixinType(Inject.class, INJECT); registerMixinType(ModifyVariable.class, MODIFY_VAR); registerMixinType(ModifyArg.class, MODIFY_ARG); + // MixinConstants.WRAP_WITH_CONDITION is deliberately left out; it appears in none of the corpus mods. + // The map is keyed by annotation internal names, so the "L...;" descriptor form would never match. + registerMixinType(MixinAnnotations.MODIFY_ARGS_INTERNAL_NAME, MODIFY_ARGS); + registerMixinType(MixinAnnotations.MODIFY_CONST_INTERNAL_NAME, MODIFY_CONST); registerMixinType(MixinAnnotations.MODIFY_RETURN_VAL_INTERNAL_NAME, MODIFY_RET); registerMixinType(Redirect.class, REDIRECT); registerMixinType(MixinAnnotations.WRAP_OPERATION_INTERNAL_NAME, WRAP_OP); registerMixinType(MixinAnnotations.MODIFY_EXPR_VAL_INTERNAL_NAME, MODIFY_EXPR_VAL); + registerMixinType(MixinAnnotations.OVERWRITE_INTERNAL_NAME, OVERWRITE); + registerMixinType(MixinAnnotations.ACCESSOR_INTERNAL_NAME, ACCESSOR); + registerMixinType(MixinAnnotations.INVOKER_INTERNAL_NAME, INVOKER); } @Nullable diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyArgsMixin.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyArgsMixin.java new file mode 100644 index 0000000..8f7c6c7 --- /dev/null +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyArgsMixin.java @@ -0,0 +1,55 @@ +package org.sinytra.adapter.patch.mixin; + +import org.sinytra.adapter.env.ctx.MixinContext; +import org.sinytra.adapter.env.param.MethodParameters; +import org.sinytra.adapter.patch.Recipe; +import org.sinytra.adapter.patch.TxResult; +import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.ConfigurationTemplates; +import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.config.PropertyContainerTemplate; +import org.sinytra.adapter.patch.config.key.MixinKeys; +import org.sinytra.adapter.patch.processor.Processors; +import org.sinytra.adapter.patch.resolver.Resolvers; +import org.sinytra.adapter.patch.resolver.injection.ArbitraryInjectionPointSubResolver; +import org.sinytra.adapter.patch.resolver.injection.InjectionPointResolver; + +import java.util.List; + +import static org.sinytra.adapter.env.param.MethodParameters.ParamGroup.SINGLE_ANY; + +/** + * Handles {@code @ModifyArgs} mixins. The handler signature is fixed - {@code void handler(Args args)} - + * whatever the injection point looks like, so there is no argument type to re-derive. + */ +public class ModifyArgsMixin implements MixinType { + private static final PropertyContainerTemplate TEMPLATE = ConfigurationTemplates.MIXIN_AT.extend() + .pluralKeys(MixinKeys.TARGET_METHOD) + .build(); + + @Override + public PropertyContainerTemplate getConfigurationTemplate() { + return TEMPLATE; + } + + @Override + public TxResult preProcess(MixinContext context, MutableConfiguration clean, Resolvers resolvers, Processors processors) { + resolvers.getOrThrow(InjectionPointResolver.class) + .addSubResolver(new ArbitraryInjectionPointSubResolver()); + + clean.setParameters(MethodParameters.create(context.methodNode(), List.of(SINGLE_ANY))); + + return TxResult.SUCCESS; + } + + @Override + public TxResult postProcess(MixinContext context, Configuration clean, MutableConfiguration dirty, Recipe recipe) { + dirty.inheritProperyIfAbsent(MixinKeys.ORDINAL); + + // The handler always takes the argument list and returns void. + dirty.inheritParameters(); + dirty.inheritReturnType(); + + return TxResult.SUCCESS; + } +} diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyConstantMixin.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyConstantMixin.java new file mode 100644 index 0000000..767c6b8 --- /dev/null +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyConstantMixin.java @@ -0,0 +1,88 @@ +package org.sinytra.adapter.patch.mixin; + +import org.objectweb.asm.Type; +import org.sinytra.adapter.env.ann.ConstantData; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.sinytra.adapter.env.ctx.MixinContext; +import org.sinytra.adapter.env.ctx.TargetPair; +import org.sinytra.adapter.env.param.MethodParameters; +import org.sinytra.adapter.patch.Recipe; +import org.sinytra.adapter.patch.TxResult; +import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.ConfigurationTemplates; +import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.config.PropertyContainerTemplate; +import org.sinytra.adapter.patch.config.key.MixinKeys; +import org.sinytra.adapter.patch.processor.Processors; +import org.sinytra.adapter.patch.resolver.Resolvers; +import org.spongepowered.asm.mixin.injection.points.BeforeConstant; + +import java.util.List; + +import static org.sinytra.adapter.env.param.MethodParameters.ParamGroup.SINGLE_ANY; + +/** + * Handles {@code @ModifyConstant} mixins, which carry no {@code @At}: the injection point comes from the + * {@code @Constant} annotation. Handlers whose constant no longer resolves fail in {@link #postProcess}, so + * they are dropped and reported rather than left in the jar to abort at runtime. + */ +public class ModifyConstantMixin implements MixinType { + private static final PropertyContainerTemplate TEMPLATE = ConfigurationTemplates.MIXIN_BASE.extend() + .keys(MixinKeys.TARGET_CONSTANT, MixinKeys.ORDINAL) + .pluralKeys(MixinKeys.SLICES) + .build(); + + @Override + public PropertyContainerTemplate getConfigurationTemplate() { + return TEMPLATE; + } + + @Override + public TxResult preProcess(MixinContext context, MutableConfiguration clean, Resolvers resolvers, Processors processors) { + clean.setParameters(MethodParameters.create(context.methodNode(), List.of(SINGLE_ANY))); + + return TxResult.SUCCESS; + } + + @Override + public TxResult postProcess(MixinContext context, Configuration clean, MutableConfiguration dirty, Recipe recipe) { + dirty.inheritProperyIfAbsent(MixinKeys.ORDINAL); + + // PropertyProcessor only forwards keys that are already set, so TARGET_CONSTANT has to be read off the + // annotation here; without it the dirty config fails validation and the mixin is dropped. + if (!dirty.hasProperty(MixinKeys.TARGET_CONSTANT)) { + context.methodAnnotation().getNested("constant") + .flatMap(ConstantData::parse) + .ifPresent(constant -> dirty.setProperty(MixinKeys.TARGET_CONSTANT, constant)); + } + + // MIXIN_BASE.require() mandates PARAMETERS and RETURN_TYPE, which every mixin type populates here. + dirty.setParameters(MethodParameters.create(context.methodNode(), List.of(SINGLE_ANY))); + dirty.setReturnType(Type.getReturnType(context.methodNode().desc)); + + if (dirty.getTargetMethod() == null) { + return TxResult.FAIL; + } + + TargetPair dirtyTarget = context.methods().findOwnMethodPair(context.dirtyLookup(), dirty.getTargetMethod()); + if (dirtyTarget == null) { + return TxResult.FAIL; + } + + // Use the pipeline's own machinery so the answer matches what Mixin does at runtime. + List constantInsns = context.methods().computeInjectionTargetInsns( + dirtyTarget, + () -> context.methodAnnotation().getNested("constant").orElse(null), + (ctx, handle) -> new BeforeConstant(ctx, handle.unwrap(), Type.getReturnType(context.methodNode().desc).getDescriptor()), + false + ); + + if (constantInsns.isEmpty()) { + return TxResult.FAIL; + } + + return TxResult.SUCCESS; + } +} + + diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyVariableMixin.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyVariableMixin.java index ebf41de..9d4dd5d 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyVariableMixin.java +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/ModifyVariableMixin.java @@ -63,7 +63,8 @@ public TxResult preProcess(MixinContext context, MutableConfiguration clean, Res @Override public TxResult postProcess(MixinContext context, Configuration clean, MutableConfiguration dirty, Recipe recipe) { - if (dirty.getTargetMethod() == null || dirty.getTargetMethod().desc() == null) { + if (dirty.getTargetMethod() == null || dirty.getTargetMethod().desc() == null + || clean.getTargetMethod() == null || clean.getTargetMethod().desc() == null) { return TxResult.FAIL; } diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/OverwriteMixin.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/OverwriteMixin.java new file mode 100644 index 0000000..03c77e9 --- /dev/null +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/OverwriteMixin.java @@ -0,0 +1,67 @@ +package org.sinytra.adapter.patch.mixin; + +import org.objectweb.asm.tree.MethodNode; +import org.sinytra.adapter.env.ctx.MixinContext; +import org.sinytra.adapter.env.ctx.TargetPair; +import org.sinytra.adapter.env.param.MethodParameters; +import org.sinytra.adapter.patch.Recipe; +import org.sinytra.adapter.patch.TxResult; +import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.config.PropertyContainer; +import org.sinytra.adapter.patch.config.PropertyContainerTemplate; +import org.sinytra.adapter.patch.config.key.ControlKeys; +import org.sinytra.adapter.patch.config.key.MixinKeys; +import org.sinytra.adapter.patch.processor.Processors; +import org.sinytra.adapter.patch.resolver.Resolvers; +import org.sinytra.adapter.util.MethodQualifier; + +import java.util.List; + +import static org.sinytra.adapter.env.param.MethodParameters.ParamGroup.METHOD_PARAMS; + +/** + * Handles {@code @Overwrite} mixins, the one handler that carries no target information at all: it replaces + * the target method wholesale and is identified by the handler's own name, which {@link MixinType#deriveTarget} + * supplies (name only) so the usual name resolution can complete it. + */ +public class OverwriteMixin implements MixinType { + /** TARGET_METHOD is not required here: {@code @Overwrite} derives it from the handler. */ + private static final PropertyContainerTemplate TEMPLATE = PropertyContainerTemplate.builder() + .require(ControlKeys.MIXIN_TYPE, ControlKeys.TARGET_CLASS, ControlKeys.PARAMETERS, ControlKeys.RETURN_TYPE) + .keys(MixinKeys.LOCALS, MixinKeys.REQUIRE) + .pluralKeys(MixinKeys.TARGET_METHOD) + .build(); + + @Override + public PropertyContainerTemplate getConfigurationTemplate() { + return TEMPLATE; + } + + @Override + public MethodQualifier deriveTarget(PropertyContainer properties, MethodNode method) { + return new MethodQualifier(method.name, null); + } + + @Override + public TxResult preProcess(MixinContext context, MutableConfiguration clean, Resolvers resolvers, Processors processors) { + // An overwrite takes over the whole method, so the handler mirrors the target's parameters. + clean.setParameters(MethodParameters.create(context.methodNode(), List.of(METHOD_PARAMS))); + + return TxResult.SUCCESS; + } + + @Override + public TxResult postProcess(MixinContext context, Configuration clean, MutableConfiguration dirty, Recipe recipe) { + if (dirty.getTargetMethod() == null || dirty.getTargetMethod().desc() == null) { + return TxResult.FAIL; + } + + TargetPair target = context.methods().findOwnMethodPair(context.dirtyLookup(), dirty.getTargetMethod()); + if (target == null) { + return TxResult.FAIL; + } + + return TxResult.SUCCESS; + } +} diff --git a/core/src/main/java/org/sinytra/adapter/patch/mixin/RedirectMixin.java b/core/src/main/java/org/sinytra/adapter/patch/mixin/RedirectMixin.java index f4784e1..6951bb8 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/mixin/RedirectMixin.java +++ b/core/src/main/java/org/sinytra/adapter/patch/mixin/RedirectMixin.java @@ -1,5 +1,8 @@ package org.sinytra.adapter.patch.mixin; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.Type; import org.sinytra.adapter.env.ctx.MethodHelper; import org.sinytra.adapter.env.ctx.MixinContext; @@ -23,6 +26,7 @@ import org.sinytra.adapter.util.MethodQualifier; import java.util.EnumSet; +import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -51,8 +55,16 @@ public TxResult preProcess(MixinContext context, MutableConfiguration clean, Res .addAfter(ParametersProcessor.class, new DivertRedirectProcessor()) .addAfter(ParametersProcessor.class, new ParameterUsageProcessor()); - if (clean.getAtData() == null || !MixinAnnotationConstants.AT_VAL_INVOKE.equals(clean.getAtData().getValue())) + if (clean.getAtData() == null) return TxResult.FAIL; + if (!MixinAnnotationConstants.AT_VAL_INVOKE.equals(clean.getAtData().getValue())) { + // @Redirect is also legal for FIELD targets. Failing unconditionally dropped such mixins even + // when the referenced member still exists, so model the handler and let the pipeline decide. + clean.setParameters(MethodParameters.create(context.methodNode(), + List.of(MethodParameters.ParamGroup.SINGLE_ANY))); + clean.setReturnType(Type.getReturnType(context.methodNode().desc)); + return TxResult.SUCCESS; + } MethodQualifier targetDesc = clean.getAtData().getTarget().flatMap(MethodQualifier::parse).orElse(null); if (targetDesc == null) @@ -86,18 +98,54 @@ public TxResult postProcess(MixinContext context, Configuration clean, MutableCo if (dirty.getTargetMethod() == null) return TxResult.FAIL; + // A non-INVOKE @Redirect targets a member, not a call: nothing to resolve and the ref stays valid. + if (!MixinAnnotationConstants.AT_VAL_INVOKE.equals(dirty.getAtData().getValue())) + return TxResult.SUCCESS; + MethodQualifier targetDesc = dirty.getAtData().getTarget().flatMap(MethodQualifier::parse).orElse(null); if (targetDesc == null || targetDesc.desc() == null) return TxResult.FAIL; + // Find the call inside the patched method instead of resolving the @At's owner through a class lookup: + // NeoForge declares such members on extension interfaces, which that lookup misses. + TargetPair injected = context.methods().findOwnMethodPair(context.dirtyLookup(), dirty.getTargetMethod()); + MethodInsnNode callInsn = null; + if (injected != null) { + List calls = new ArrayList<>(); + for (AbstractInsnNode node : context.methods().findInjectionTargetInsns(injected, dirty.getAtData())) { + if (node instanceof MethodInsnNode mi) { + calls.add(mi); + } + } + // Several call sites are normal; what must be unambiguous is the distinct owner+name+descriptor. + if (calls.stream().map(c -> c.owner + c.name + c.desc).distinct().limit(2).count() == 1) { + callInsn = calls.getFirst(); + } + } + TargetPair dirtyTarget = context.methods().findOwnMethodPair(context.dirtyLookup(), targetDesc); - if (dirtyTarget == null) + if (dirtyTarget == null) { + // Fall back to owner+name only, the same strategy ChangedMemberDescriptorSubResolver uses. + dirtyTarget = context.methods().findOwnMethodPair(context.dirtyLookup(), targetDesc.ignoreDesc()); + } + // Either the call or a resolvable target is enough. + if (dirtyTarget == null && callInsn == null) return TxResult.FAIL; List dirtyCaptured = context.methods().resolveCapturedMethodParams(recipe.clean(), recipe.dirty()); - List callTypes = Parameters.getParameterTypes(targetDesc.desc()); - if (!MethodHelper.isStatic(dirtyTarget.methodNode())) { - Type owner = Type.getObjectType(dirtyTarget.classNode().name); + + // Prefer the call: the @At may name a call whose descriptor differs from the (since changed) method + // declaration, and Mixin binds the handler to the call. + List callTypes = callInsn != null + ? Parameters.getParameterTypes(callInsn.desc) + : Parameters.getParameterTypes(targetDesc.desc()); + boolean staticCall = callInsn != null + ? callInsn.getOpcode() == Opcodes.INVOKESTATIC + : MethodHelper.isStatic(dirtyTarget.methodNode()); + if (!staticCall) { + Type owner = callInsn != null + ? Type.getObjectType(callInsn.owner) + : Type.getObjectType(dirtyTarget.classNode().name); callTypes.addFirst(owner); } @@ -121,3 +169,11 @@ private boolean isStaticRedirect(MixinContext context, MethodQualifier targetDes return methodParams.size() >= callTypes.size() && methodParams.subList(0, callTypes.size()).equals(callTypes); } } + + + + + + + + diff --git a/core/src/main/java/org/sinytra/adapter/patch/processor/TargetMethodProcessor.java b/core/src/main/java/org/sinytra/adapter/patch/processor/TargetMethodProcessor.java index 0391643..7a20ac4 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/processor/TargetMethodProcessor.java +++ b/core/src/main/java/org/sinytra/adapter/patch/processor/TargetMethodProcessor.java @@ -26,6 +26,8 @@ private static void upgradeCapturedLocals(MethodNode methodNode, MixinContext co AdapterUtil.CapturedLocals capturedLocals = AdapterUtil.getCapturedLocals(context, recipe); if (capturedLocals == null) return; + // NOTE: passing the adapted @At data here makes this processor rewrite the parameter list of far + // more mixins and broke unrelated mods; enable it only together with the parameter annotation fix. List availableLocals = context.methods().getTargetMethodLocals(capturedLocals.target()); // For now, only handle cases where all locals are part of the method's params, convenient when switching the target to a lambda if (availableLocals == null || !availableLocals.isEmpty()) return; diff --git a/core/src/main/java/org/sinytra/adapter/patch/processor/extract/ExtractMixinProcessor.java b/core/src/main/java/org/sinytra/adapter/patch/processor/extract/ExtractMixinProcessor.java index f8f831f..4b52ec1 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/processor/extract/ExtractMixinProcessor.java +++ b/core/src/main/java/org/sinytra/adapter/patch/processor/extract/ExtractMixinProcessor.java @@ -6,8 +6,10 @@ import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; import org.objectweb.asm.tree.*; +import org.sinytra.adapter.env.ctx.MethodHelper; import org.sinytra.adapter.env.ctx.MixinContext; import org.sinytra.adapter.env.ctx.TargetPair; +import org.sinytra.adapter.util.MethodQualifier; import org.sinytra.adapter.env.util.MixinAnnotations; import org.sinytra.adapter.patch.Recipe; import org.sinytra.adapter.patch.TxResult; @@ -60,6 +62,19 @@ public static PatchResult apply(String targetClassName, MixinContext context, Re Candidates candidates = findCandidates(classNode, methodNode); if (!candidates.canMove(classNode, isInherited)) return PatchResult.PASS; + // The member may have moved to a STATIC method on the new target, which Mixin rejects outright. + // StaticAccessProcessor runs before extraction and cannot catch it, so the modifier is synced here - + // decided before anything is moved, since a body reading local 0 cannot be made static. + MethodQualifier movedTarget = recipe.dirty().getTargetMethod(); + TargetPair movedPair = movedTarget == null ? null + : context.methods().findOwnMethodPair(context.dirtyLookup(), movedTarget); + boolean syncModifier = movedPair != null && MethodHelper.isStatic(movedPair.methodNode()) && !MethodHelper.isStatic(methodNode); + if (syncModifier && readsReceiverSlot(methodNode)) { + // A @Shadow access compiles to "aload 0 + getfield/invokevirtual", which Candidates#canMove lets + // through, but a static target has no receiver. Bail out so Patcher#stripFailedMixins drops it. + return PatchResult.PASS; + } + ClassNode targetClass = context.environment().dirtyClassLookup().getClass(targetClassName).orElse(null); // Get or generate new mixin class ClassNode generatedTarget = context.environment().classGenerator().getOrGenerateMixinClass(classNode, targetClassName, targetClass != null ? targetClass.superName : null); @@ -76,8 +91,30 @@ public static PatchResult apply(String targetClassName, MixinContext context, Re candidates.handleUpdates().forEach(c -> c.accept(generatedTarget)); + if (syncModifier) { + context.recordCtxAudit("Made extracted callback static to match static target %s", movedTarget.asDescriptor()); + methodNode.access |= Opcodes.ACC_STATIC; + + // The body reserved local 0 for the receiver, so its parameters shift down by one. + if (methodNode.localVariables != null) { + for (LocalVariableNode lvn : methodNode.localVariables) { + lvn.index--; + } + methodNode.localVariables.removeIf(lvn -> lvn.index < 0); + } + for (AbstractInsnNode insn : methodNode.instructions) { + if (insn instanceof VarInsnNode varInsn) { + varInsn.var--; + } else if (insn instanceof IincInsnNode iincInsn) { + iincInsn.var--; + } + } + methodNode.maxLocals = Math.max(0, methodNode.maxLocals - 1); + } + // Take care of captured locals - PatchResult result = PatchResult.PASS; + // Dropping the receiver invalidates the StackMapTable, so ask for a full recomputation. + PatchResult result = syncModifier ? PatchResult.COMPUTE_FRAMES : PatchResult.PASS; boolean capturesLocals = recipe.clean().hasProperty(MixinKeys.LOCALS); if (capturesLocals) { result = result.or(recreateLocalVariables(methodNode, context, recipe, generatedTarget)); @@ -187,6 +224,19 @@ private static int fixAccess(int access) { return access; } + /** Whether the body reads local 0, i.e. uses {@code this}. */ + private static boolean readsReceiverSlot(MethodNode methodNode) { + for (AbstractInsnNode insn : methodNode.instructions) { + if (insn instanceof VarInsnNode varInsn && varInsn.var == 0) { + return true; + } + if (insn instanceof IincInsnNode iincInsn && iincInsn.var == 0) { + return true; + } + } + return false; + } + /** * We can't bring captured locals along when extracting a mixin, so our best bet is to recreate them from scratch. *

@@ -376,3 +426,5 @@ private static PatchResult recreateLocalVariables(MethodNode methodNode, MixinCo return PatchResult.COMPUTE_FRAMES; } } + + diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/ArbitraryInjectionPointSubResolver.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/ArbitraryInjectionPointSubResolver.java index 762f95e..856e470 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/ArbitraryInjectionPointSubResolver.java +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/ArbitraryInjectionPointSubResolver.java @@ -93,11 +93,21 @@ private static AbstractInsnNode findCandidates(InstructionMatcher cleanMatcher, private static MethodInsnNode findReplacementInjectionPoint(AbstractInsnNode lastInsn, UnaryOperator flow, MixinContext context, @Nullable String injectionPointTarget) { // Require matching return types for ModifyExpressionValue mixins - if (context.hasFlag(MixinFlag.RETURN_TYPE_SENSITIVE) && injectionPointTarget != null) { + if (context.hasFlag(MixinFlag.RETURN_TYPE_SENSITIVE) && injectionPointTarget != null + && isCompleteMethodDescriptor(injectionPointTarget)) { Type desiredReturnType = Type.getReturnType(injectionPointTarget); return (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, flow, v -> v instanceof MethodInsnNode minsn && Type.getReturnType(minsn.desc).equals(desiredReturnType)); } return (MethodInsnNode) AdapterUtil.iterateInsns(lastInsn, flow, v -> v instanceof MethodInsnNode); } + + /** + * ASM's {@code Type.getReturnType} computes {@code desc.indexOf(')') + 1} and immediately reads that + * character, so a descriptor that ends right after ')' throws StringIndexOutOfBoundsException. + */ + private static boolean isCompleteMethodDescriptor(String desc) { + int paren = desc.indexOf(')'); + return paren >= 0 && paren + 1 < desc.length(); + } } diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/ChangedMemberDescriptorSubResolver.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/ChangedMemberDescriptorSubResolver.java new file mode 100644 index 0000000..b40efae --- /dev/null +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/ChangedMemberDescriptorSubResolver.java @@ -0,0 +1,70 @@ +package org.sinytra.adapter.patch.resolver.injection; + +import org.jetbrains.annotations.Nullable; +import org.objectweb.asm.tree.MethodInsnNode; +import org.sinytra.adapter.analysis.method.MethodCallAnalyzer; +import org.sinytra.adapter.env.ann.AtData; +import org.sinytra.adapter.env.ctx.MixinContext; +import org.sinytra.adapter.env.ctx.TargetPair; +import org.sinytra.adapter.patch.Recipe; +import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.resolver.SubResolver; +import org.sinytra.adapter.util.MethodQualifier; + +import java.util.List; + +import static org.sinytra.adapter.env.util.MixinAnnotationConstants.AT_VAL_INVOKE; +import static org.sinytra.adapter.env.util.MixinAnnotationConstants.AT_VAL_INVOKE_ASSIGN; + +/** + * Handles injection points whose target member survived but changed its descriptor, e.g. a method that gained + * a parameter; only the old {@code @At} descriptor is stale. + */ +public class ChangedMemberDescriptorSubResolver implements SubResolver { + @Nullable + @Override + public Configuration resolve(MixinContext context, Recipe recipe) { + AtData at = recipe.clean().getAtData(); + if (at == null) { + return null; + } + if (!AT_VAL_INVOKE.equals(at.getValue()) && !AT_VAL_INVOKE_ASSIGN.equals(at.getValue())) { + return null; + } + + String targetDesc = at.getTarget().orElse(null); + if (targetDesc == null) { + return null; + } + + MethodQualifier original = MethodQualifier.parse(targetDesc).orElse(null); + if (original == null || original.name() == null) { + return null; + } + + TargetPair dirtyPair = recipe.getDirtyTarget(); + if (dirtyPair == null) { + return null; + } + + // Match on owner + name only: the descriptor is precisely what changed. + List calls = MethodCallAnalyzer.getMethodCallMinsns(dirtyPair.methodNode(), original.ignoreDesc()); + if (calls.isEmpty()) { + return null; + } + + // Only adapt when the replacement is unambiguous; two different overloads would be a guess. + if (calls.stream().map(call -> call.name + call.desc).distinct().limit(2).count() > 1) { + return null; + } + + MethodInsnNode call = calls.getFirst(); + if (call.desc.equals(original.desc())) { + return null; + } + + return MutableConfiguration.create() + .setAtData(at.withTarget(call)); + } +} diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/InjectionPointResolver.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/InjectionPointResolver.java index d20851f..8080bfa 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/InjectionPointResolver.java +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/InjectionPointResolver.java @@ -2,6 +2,7 @@ import org.jetbrains.annotations.Nullable; import org.objectweb.asm.Type; +import org.spongepowered.asm.mixin.injection.points.BeforeConstant; import org.objectweb.asm.tree.AbstractInsnNode; import org.sinytra.adapter.env.ctx.MixinContext; import org.sinytra.adapter.patch.Recipe; @@ -16,10 +17,13 @@ public class InjectionPointResolver extends CompoundResolver { public InjectionPointResolver() { addSubResolver(new OverloadedInjectionPointSubResolver()); + addSubResolver(new ChangedMemberDescriptorSubResolver()); addSubResolver(new InheritedInjectionPointSubResolver()); addSubResolver(InjectionPointSubResolvers.REPLACED_TYPE); addSubResolver(InjectionPointSubResolvers.EXTRACTED_CALL); addSubResolver(InjectionPointSubResolvers.EXTRACTED_CODEPATH_CALL); + // Last on purpose: this is the most structural guess, so every narrower strategy declines first. + addSubResolver(new RelocatedMemberSubResolver()); } @Override @@ -43,6 +47,16 @@ protected Configuration tryReuse(MixinContext context, Recipe recipe) { // Try reusing the original List insns = context.methods().findInjectionTargetInsns(dirtyTarget); + if (insns.isEmpty() && context.methodAnnotation().getNested("constant").isPresent()) { + // @ModifyConstant has no @At, so the lookup above can never match it even after the target method + // was retargeted; ask the constant point instead. + insns = context.methods().computeInjectionTargetInsns( + dirtyTarget, + () -> context.methodAnnotation().getNested("constant").orElse(null), + (ctx, h) -> new BeforeConstant(ctx, h.unwrap(), Type.getReturnType(context.methodNode().desc).getDescriptor()), + false + ); + } if (!insns.isEmpty()) { return dirty.copyClean() .inheritAtData(); @@ -51,3 +65,4 @@ protected Configuration tryReuse(MixinContext context, Recipe recipe) { return null; } } + diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/RelocatedMemberSubResolver.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/RelocatedMemberSubResolver.java new file mode 100644 index 0000000..9320767 --- /dev/null +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/injection/RelocatedMemberSubResolver.java @@ -0,0 +1,124 @@ +package org.sinytra.adapter.patch.resolver.injection; + +import org.jetbrains.annotations.Nullable; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.MethodInsnNode; +import org.objectweb.asm.tree.MethodNode; +import org.sinytra.adapter.env.ann.AtData; +import org.sinytra.adapter.env.ctx.MixinContext; +import org.sinytra.adapter.env.ctx.TargetPair; +import org.sinytra.adapter.patch.Recipe; +import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.MutableConfiguration; +import org.sinytra.adapter.patch.resolver.SubResolver; +import org.sinytra.adapter.util.MethodQualifier; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.sinytra.adapter.env.util.MixinAnnotationConstants.AT_VAL_INVOKE; +import static org.sinytra.adapter.env.util.MixinAnnotationConstants.AT_VAL_INVOKE_ASSIGN; + +/** + * Handles injection points whose target member MOVED to a different owner: NeoForge lifts a private static + * helper onto the class of its first parameter, so owner and name change while the call keeps its shape. The + * new name is deliberately ignored. + *

+ * Matches the calls in the patched target method, not the receiving class: these helpers are usually + * {@code default} methods on extension interfaces. + */ +public class RelocatedMemberSubResolver implements SubResolver { + @Nullable + @Override + public Configuration resolve(MixinContext context, Recipe recipe) { + AtData at = recipe.clean().getAtData(); + if (at == null) { + return null; + } + if (!AT_VAL_INVOKE.equals(at.getValue()) && !AT_VAL_INVOKE_ASSIGN.equals(at.getValue())) { + return null; + } + + String targetDesc = at.getTarget().orElse(null); + if (targetDesc == null) { + return null; + } + + MethodQualifier original = MethodQualifier.parse(targetDesc).orElse(null); + if (original == null || original.owner() == null || original.name() == null || original.desc() == null) { + return null; + } + + TargetPair dirtyTarget = recipe.getDirtyTarget(); + if (dirtyTarget == null) { + return null; + } + + // ChangedMemberDescriptorSubResolver covers descriptor-only changes; this one needs the member gone. + if (context.methods().findOwnMethodPair(context.dirtyLookup(), original.ignoreDesc()) != null) { + return null; + } + + // "Lifted onto the first parameter's class" only applies to a static helper. + TargetPair cleanOrigin = context.methods().findOwnMethodPair(context.cleanLookup(), original.ignoreDesc()); + if (cleanOrigin == null || !isStatic(cleanOrigin.methodNode())) { + return null; + } + + Type[] oldArgs = Type.getArgumentTypes(original.desc()); + if (oldArgs.length < 1) { + return null; + } + Type receiver = oldArgs[0]; + if (receiver.getSort() != Type.OBJECT) { + return null; + } + Type newReturn = Type.getReturnType(original.desc()); + Type[] newArgs = Arrays.copyOfRange(oldArgs, 1, oldArgs.length); + + List matches = findShiftedCalls(dirtyTarget.methodNode(), receiver, newArgs, newReturn); + // Several call sites are fine; what must be unambiguous is the distinct owner+name+descriptor. + if (matches.stream().map(call -> call.owner + call.name + call.desc).distinct().limit(2).count() != 1) { + return null; + } + + MethodInsnNode call = matches.getFirst(); + context.recordCtxAudit("Adjusting injection target for relocated member: %s -> %s.%s%s", + original.asDescriptor(), call.owner, call.name, call.desc); + + return MutableConfiguration.create() + .setAtData(at.withTarget(call)); + } + + /** Finds the calls matching the shifted shape: the old call with its receiver lifted out of the arguments. */ + private static List findShiftedCalls(MethodNode target, Type receiver, Type[] newArgs, Type newReturn) { + List matches = new ArrayList<>(); + for (AbstractInsnNode insn = target.instructions.getFirst(); insn != null; insn = insn.getNext()) { + if (!(insn instanceof MethodInsnNode call)) { + continue; + } + int opcode = call.getOpcode(); + if (opcode != Opcodes.INVOKEVIRTUAL && opcode != Opcodes.INVOKEINTERFACE && opcode != Opcodes.INVOKESPECIAL) { + continue; + } + if (!call.owner.equals(receiver.getInternalName())) { + continue; + } + if (!Arrays.equals(Type.getArgumentTypes(call.desc), newArgs)) { + continue; + } + if (!Type.getReturnType(call.desc).equals(newReturn)) { + continue; + } + matches.add(call); + } + return matches; + } + + private static boolean isStatic(MethodNode methodNode) { + return (methodNode.access & Opcodes.ACC_STATIC) != 0; + } +} diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/special/SliceBoundaryResolver.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/special/SliceBoundaryResolver.java index 6e2a372..48b7a58 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/resolver/special/SliceBoundaryResolver.java +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/special/SliceBoundaryResolver.java @@ -88,7 +88,7 @@ private static Optional fixSlideInjectionPoint(AtData boundary, MethodNo private static boolean passesSliceCheck(SliceData slice, MixinContext context, TargetPair dirtyTarget) { Target mixinTarget = MockMixinRuntime.createMixinTarget(dirtyTarget); IMixinContext mixinContext = MockMixinRuntime.forClass(context.classNode().name, dirtyTarget.classNode().name, context.patchContext().environment()); - ISliceContext sliceContext = MockMixinRuntime.forSlice(mixinContext, context.methodNode()); + ISliceContext sliceContext = MockMixinRuntime.forSlice(mixinContext, context.methodNode(), slice.toAnnotationNode()); MethodSlice methodSlice = MethodSlice.parse(sliceContext, slice.toAnnotationNode()); InsnList insns = methodSlice.getSlice(mixinTarget); diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/target/SplitTargetMethodSubResolver.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/target/SplitTargetMethodSubResolver.java index 08aabc1..3f27ce7 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/resolver/target/SplitTargetMethodSubResolver.java +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/target/SplitTargetMethodSubResolver.java @@ -1,6 +1,7 @@ package org.sinytra.adapter.patch.resolver.target; import com.google.common.collect.Multimap; +import org.objectweb.asm.Type; import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodInsnNode; @@ -16,6 +17,7 @@ import org.sinytra.adapter.analysis.method.MethodInsnMatcher; import org.sinytra.adapter.env.ctx.TargetPair; import org.sinytra.adapter.util.MethodQualifier; +import org.spongepowered.asm.mixin.injection.points.BeforeConstant; import java.util.List; import java.util.Map; @@ -129,6 +131,18 @@ private static List findInsnsCalls(List methods, Mi return methods.stream() .map(pair -> { List insns = context.methods().findInjectionTargetInsns(pair); + if (insns.isEmpty() && context.methodAnnotation().getNested("constant").isPresent()) { + // A constant injector (@ModifyConstant) has no @At, so the lookup above can never match it; + // without asking the constant point, a method split away together with its constant is never + // considered a candidate (carpet's @ModifyConstant on Level#setBlock, moved by NeoForge into + // Level#markAndNotifyBlock). + insns = context.methods().computeInjectionTargetInsns( + pair, + () -> context.methodAnnotation().getNested("constant").orElse(null), + (ctx, h) -> new BeforeConstant(ctx, h.unwrap(), Type.getReturnType(context.methodNode().desc).getDescriptor()), + false + ); + } return !insns.isEmpty() ? new CandidateMethod(pair.methodNode(), insns) : null; }) .filter(Objects::nonNull) @@ -138,3 +152,5 @@ private static List findInsnsCalls(List methods, Mi private record CandidateMethod(MethodNode method, List insns) { } } + + diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodResolver.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodResolver.java index ac2560b..0fd2de2 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodResolver.java +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodResolver.java @@ -42,3 +42,8 @@ protected Configuration useFallback(MixinContext context, Recipe recipe) { return null; } } + + + + + diff --git a/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodSubResolvers.java b/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodSubResolvers.java index ab331ad..a1bb052 100644 --- a/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodSubResolvers.java +++ b/core/src/main/java/org/sinytra/adapter/patch/resolver/target/TargetMethodSubResolvers.java @@ -3,6 +3,8 @@ import com.mojang.datafixers.util.Pair; import org.jetbrains.annotations.Nullable; import org.objectweb.asm.Handle; +import org.objectweb.asm.Type; +import org.objectweb.asm.tree.AnnotationNode; import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.InvokeDynamicInsnNode; @@ -36,6 +38,7 @@ public class TargetMethodSubResolvers { ClassNode targetClass = context.dirtyLookup().getClass(recipe.clean().getTargetClass()).orElse(null); if (targetClass == null) return null; + List candidateMethods = targetClass.methods.stream() .filter(m -> MethodAnalyzer.isLambda(m) && m.desc.equals(cleanQualifier.desc())) .toList(); @@ -79,6 +82,11 @@ public class TargetMethodSubResolvers { * Handle cases where the target instructions have been moved into a lambda inside the target method */ public static final SubResolver MOVED_INTO_LAMBDA = (MixinContext context, Recipe recipe) -> { + // Decline relocation for handlers using mixinextras' @Local sugar: MixinExtras resolves the sugar + // against the local frame at the injection point, so re-pointing it at a lambda can fail validation at + // runtime, too late to strip the handler. @Local is the only sugar that depends on the injection point. + if (hasLocalSugar(context)) return null; + MethodQualifier cleanQualifier = recipe.clean().getTargetMethod(); TargetPair target = context.methods().findOwnMethodPair(context.dirtyLookup(), cleanQualifier); @@ -87,8 +95,10 @@ public class TargetMethodSubResolvers { for (AbstractInsnNode insn : target.methodNode().instructions) { // Find lambda invocations and search for target insns inside the lambda if (insn instanceof InvokeDynamicInsnNode indy && indy.bsmArgs.length > 1 && indy.bsmArgs[1] instanceof Handle handle) { + // Keep the handle's owner: an owner-less lookup starts at the mixin's target class. TargetPair lambda = MethodQualifier.parse(handle.getName()) - .map(q -> context.methods().findOwnMethodPair(context.dirtyLookup(), q)) + .map(q -> context.methods().findOwnMethodPair(context.dirtyLookup(), + new MethodQualifier(Type.getObjectType(handle.getOwner()).getDescriptor(), q.name(), q.desc()))) .orElse(null); if (lambda == null) return null; @@ -168,4 +178,36 @@ public static boolean isDirtyDeprecatedMethod(MixinContext context, MethodNode d TargetPair pair = context.methods().findOwnMethodPair(context.cleanLookup(), MethodQualifier.create(dirty)); return (pair == null || !AdapterUtil.isDeprecated(pair.methodNode())) && AdapterUtil.isDeprecated(dirty); } + + /** Whether any parameter of the handler is annotated with mixinextras' {@code @Local}. */ + private static boolean hasLocalSugar(MixinContext context) { + MethodNode method = context.methodNode(); + return hasLocalSugar(method.visibleParameterAnnotations) || hasLocalSugar(method.invisibleParameterAnnotations); + } + + private static boolean hasLocalSugar(java.util.List[] parameterAnnotations) { + if (parameterAnnotations == null) { + return false; + } + for (java.util.List annotations : parameterAnnotations) { + if (annotations == null) { + continue; + } + for (AnnotationNode annotation : annotations) { + if (isLocalSugar(annotation.desc)) { + return true; + } + } + } + return false; + } + + /** Matches by simple name: MixinExtras can be relocated into another package. */ + private static boolean isLocalSugar(String desc) { + return desc != null && desc.endsWith("/sugar/Local;"); + } } + + + + diff --git a/core/src/main/java/org/sinytra/adapter/transform/PipelineMethodTransformer.java b/core/src/main/java/org/sinytra/adapter/transform/PipelineMethodTransformer.java index 769dbae..827af42 100644 --- a/core/src/main/java/org/sinytra/adapter/transform/PipelineMethodTransformer.java +++ b/core/src/main/java/org/sinytra/adapter/transform/PipelineMethodTransformer.java @@ -47,11 +47,51 @@ public PipelineMethodTransformer(List methodPatches, boolean patche @Override public PatchResult apply(MixinContext context, Configuration config) { TargetPair cleanTarget = context.methods().findOwnMethodPair(context.cleanLookup(), config.getTargetMethod()); - if (cleanTarget == null) return PatchResult.PASS; + if (cleanTarget == null) { + // The target class can exist only in the patched view: supplied by the mod, by one of its + // libraries, or by the platform. Mod-supplied classes never need adaptation; platform classes pass + // only when the injection point already resolves in the patched class. + String targetClass = config.getProperty(ControlKeys.TARGET_CLASS).orElse(null); + TargetPair dirtyCandidate = context.methods().findOwnMethodPair(context.dirtyLookup(), config.getTargetMethod()); + + boolean modSupplied = !isPlatformClass(targetClass); + boolean platformButAlreadyResolvable = !modSupplied + && dirtyCandidate != null + && context.methods().hasInjectionTargetInsns(dirtyCandidate); + + if (targetClass != null + && context.dirtyLookup().getClass(targetClass).isPresent() + && context.cleanLookup().getClass(targetClass).isEmpty() + && (modSupplied || platformButAlreadyResolvable)) { + context.environment().auditTrail().recordResult(context, config, AuditTrail.Match.FULL); + context.setEffectiveInjectionPoint(config.getAtData(), config.getTargetMethod()); + try { + context.recordCtxAudit("No adaptation required: target class %s is not in the vanilla jar (%s)", + targetClass, modSupplied ? "supplied by the mod itself" : "injection point already resolves"); + } catch (RuntimeException ignored) { + // no audit frame at this exit + } + return PatchResult.PASS; + } + + // Publish the injection point: later stages need it, and leaving it unset disables capture re-derivation. + context.setEffectiveInjectionPoint(config.getAtData(), config.getTargetMethod()); + return skip(context, config, "target method not found in the clean (vanilla) class"); + } TargetPair dirtyTarget = context.methods().findOwnMethodPair(context.dirtyLookup(), config.getTargetMethod()); - if (!this.patchResolver.matches(config) && !failsDirtyInjectionCheck(context, config, dirtyTarget) && hasValidSlice(context, config, dirtyTarget)) + if (!this.patchResolver.matches(config) && !failsDirtyInjectionCheck(context, config, dirtyTarget) && hasValidSlice(context, config, dirtyTarget)) { + // Not a failure: the mixin already injects into the patched target as-is. + context.environment().auditTrail().recordResult(context, config, AuditTrail.Match.FULL); + // It resolves here, and captured-local transformers need the site even when nothing was rewritten. + context.setEffectiveInjectionPoint(config.getAtData(), config.getTargetMethod()); + try { + context.recordCtxAudit("No adaptation required: injection point already resolves in the patched target"); + } catch (RuntimeException ignored) { + // no audit frame at this exit + } return PatchResult.PASS; + } LOGGER.debug(MIXINPATCH, "Considering method {}", context.getMixinId()); @@ -67,6 +107,17 @@ public PatchResult apply(MixinContext context, Configuration config) { return PatchResult.PASS; } + /** Records a give-up exit so the mixin shows up in the audit trail instead of vanishing from the report. */ + private static PatchResult skip(MixinContext context, Configuration config, String reason, Object... args) { + context.environment().auditTrail().recordResult(context, config, AuditTrail.Match.NONE); + try { + context.recordCtxAudit("Skipped mixin: " + reason, args); + } catch (RuntimeException ignored) { + // Not every exit runs inside an audit frame; the reason is best-effort. + } + return PatchResult.PASS; + } + private PatchResult execute(MixinContext context, Configuration config) { String mixinId = context.getMixinId(); Resolvers resolvers = this.patchesOnly ? new Resolvers(false) : context.getResolvers(); @@ -102,7 +153,7 @@ private PatchResult execute(MixinContext context, Configuration config) { } } else if (res.type() == Resolver.ResultType.FAIL) { LOGGER.debug(MIXINPATCH, "Skipping mixin {} due to failed RESOLVER {}", mixinId, resolver.getClass().getSimpleName()); - return PatchResult.PASS; + return skip(context, config, "failed resolver %s", resolver.getClass().getSimpleName()); } } @@ -114,7 +165,7 @@ private PatchResult execute(MixinContext context, Configuration config) { TxResult postResult = lateMixinType.postProcess(context, cleanConfig, dirtyConfig, recipe); if (postResult == TxResult.FAIL) { LOGGER.debug(MIXINPATCH, "Skipping mixin {} due to failed postProcess", mixinId); - return PatchResult.PASS; + return skip(context, config, "failed postProcess"); } } } @@ -122,7 +173,7 @@ private PatchResult execute(MixinContext context, Configuration config) { // 3.1. Validate dirty config if (!dirtyConfig.validate()) { LOGGER.debug(MIXINPATCH, "Skipping mixin {} due to invalid DIRTY config", mixinId); - return PatchResult.PASS; + return skip(context, config, "invalid DIRTY config"); } // 4. Run Processors @@ -136,7 +187,7 @@ private PatchResult execute(MixinContext context, Configuration config) { } if (res == TxResult.FAIL) { LOGGER.debug(MIXINPATCH, "Skipping mixin {} due to failed PROCESSOR {}", mixinId, processor.getClass().getSimpleName()); - return PatchResult.PASS; + return skip(context, config, "failed processor %s", processor.getClass().getSimpleName()); } } @@ -147,6 +198,9 @@ private PatchResult execute(MixinContext context, Configuration config) { context.methodNode().parameters = null; } + // Publish where the injection point ended up; later transformers need the effective site. + context.setEffectiveInjectionPoint(dirtyConfig.getAtData(), dirtyConfig.getTargetMethod()); + return PatchResult.APPLY; } @@ -192,4 +246,21 @@ private boolean validateAtNode(MixinContext context, AtData at, TargetPair targe ); return !insns.isEmpty(); } + + /** Whether the name belongs to a platform namespace (game or mod loader) rather than to a mod. */ + private static boolean isPlatformClass(String internalName) { + return internalName.startsWith("net/minecraft/") + || internalName.startsWith("net/neoforged/") + || internalName.startsWith("com/mojang/") + || internalName.startsWith("net/fabricmc/") + || internalName.startsWith("org/spongepowered/"); + } } + + + + + + + + diff --git a/core/src/main/java/org/sinytra/adapter/transform/param/ReplaceParametersTransformer.java b/core/src/main/java/org/sinytra/adapter/transform/param/ReplaceParametersTransformer.java index 50845bf..0294b04 100644 --- a/core/src/main/java/org/sinytra/adapter/transform/param/ReplaceParametersTransformer.java +++ b/core/src/main/java/org/sinytra/adapter/transform/param/ReplaceParametersTransformer.java @@ -81,9 +81,11 @@ public PatchResult apply(ClassNode classNode, MethodNode methodNode, MixinContex } if (insn instanceof MethodInsnNode minsn && minsn.owner.equals(originalType.getInternalName())) { + // getMethodCallInsns is @Nullable: it returns null when the call source cannot be + // determined statically. Skip the rewrite instead of killing the whole jar transform. List insns = MethodCallAnalyzer.getMethodCallInsns(methodNode, minsn); // Find var load instruction - for (AbstractInsnNode callInsn : insns) { + for (AbstractInsnNode callInsn : insns == null ? List.of() : insns) { if (callInsn instanceof VarInsnNode varInsn && varInsn.var == localVar.index) { minsn.owner = this.type.getInternalName(); } diff --git a/core/src/main/java/org/sinytra/adapter/transform/preprocess/LocalCaptureUpgradeTransformer.java b/core/src/main/java/org/sinytra/adapter/transform/preprocess/LocalCaptureUpgradeTransformer.java index 0428493..4963a97 100644 --- a/core/src/main/java/org/sinytra/adapter/transform/preprocess/LocalCaptureUpgradeTransformer.java +++ b/core/src/main/java/org/sinytra/adapter/transform/preprocess/LocalCaptureUpgradeTransformer.java @@ -12,11 +12,14 @@ import org.sinytra.adapter.env.ctx.TargetPair; import org.sinytra.adapter.env.util.MixinAnnotations; import org.sinytra.adapter.patch.config.Configuration; +import org.sinytra.adapter.patch.config.MutableConfiguration; import org.sinytra.adapter.patch.config.key.MixinKeys; +import org.sinytra.adapter.patch.config.key.SpecialKeys; import org.sinytra.adapter.transform.MethodTransformer; import org.sinytra.adapter.analysis.locals.LocalVarAnalyzer; import org.sinytra.adapter.env.ctx.PatchResult; import org.sinytra.adapter.util.AdapterUtil; +import org.sinytra.adapter.env.util.TypeConstants; import java.util.*; @@ -40,8 +43,22 @@ public PatchResult apply(MixinContext context, Configuration config) { List> localAnnotations = AdapterUtil.getAnnotatedParameters(methodNode, paramTypes, MixinAnnotations.LOCAL, Pair::of); if (!localAnnotations.isEmpty()) return PatchResult.PASS; - LocalVarAnalyzer.CapturedLocalsInfo info = LocalVarAnalyzer.getCapturedLocals(context, dirtyTarget); - if (info == null || info.diff().isEmpty()) return PatchResult.PASS; + // Re-derive against the injection point the pipeline actually produced (MixinContext#effectiveAtData). + LocalVarAnalyzer.CapturedLocalsInfo info = LocalVarAnalyzer.getCapturedLocals(context, dirtyTarget, context.effectiveAtData()); + if (info == null) { + // effectiveAtData is null when the pipeline never published the injection point. A handler whose + // CallbackInfo is its last parameter captures nothing; anything else is left unmarked for Patcher. + if (capturesNoLocals(methodNode)) { + markUpgraded(config); + } + return PatchResult.PASS; + } + + if (info.diff().isEmpty()) { + // The compiled capture list still matches the patched method - nothing to re-derive. + markUpgraded(config); + return PatchResult.PASS; + } LocalVarAnalyzer.CapturedLocalsTransform transform = LocalVarAnalyzer.analyzeCapturedLocals(info.capturedLocals(), methodNode); @@ -61,8 +78,7 @@ public PatchResult apply(MixinContext context, Configuration config) { long matching = info.availableTypes().stream() .filter(expected::equals) .count(); - if (matching != 1) - return PatchResult.PASS; + if (matching != 1) return PatchResult.PASS; addOrdinal = false; } @@ -79,6 +95,7 @@ public PatchResult apply(MixinContext context, Configuration config) { } Type[] args = Type.getArgumentTypes(methodNode.desc); + ensureParameterAnnotationCapacity(methodNode, args.length); int start = info.capturedLocals().paramLocalStart(); for (int i = start; i < args.length; i++) { if (!usedOrdinals.contains(i)) continue; @@ -92,8 +109,53 @@ public PatchResult apply(MixinContext context, Configuration config) { PatchResult result = transform.remover().isEmpty() ? PatchResult.APPLY : transform.remover().apply(context); if (result == PatchResult.PASS) return PatchResult.PASS; + // Mark the capture as re-derived so Patcher knows this handler was handled. + markUpgraded(config); + context.recordCtxAudit("Upgrade captured locals"); context.environment().auditTrail().recordResult(context, config, AuditTrail.Match.FULL); return result; } + + /** Resizes the parameter-annotation arrays so they cover the current parameter count. */ + @SuppressWarnings("unchecked") + private static void ensureParameterAnnotationCapacity(MethodNode methodNode, int paramCount) { + methodNode.visibleParameterAnnotations = resizeParameterAnnotations(methodNode.visibleParameterAnnotations, paramCount); + methodNode.invisibleParameterAnnotations = resizeParameterAnnotations(methodNode.invisibleParameterAnnotations, paramCount); + } + + @SuppressWarnings("unchecked") + private static List[] resizeParameterAnnotations(List[] annotations, int paramCount) { + if (annotations == null || annotations.length == paramCount) { + return annotations; + } + List[] resized = (List[]) new List[paramCount]; + System.arraycopy(annotations, 0, resized, 0, Math.min(annotations.length, paramCount)); + return resized; + } + + /** Whether this injector declares no captured locals at all, i.e. its CallbackInfo is the last parameter. */ + private static boolean capturesNoLocals(MethodNode methodNode) { + Type[] params = Type.getArgumentTypes(methodNode.desc); + for (int i = 0; i < params.length; i++) { + boolean isCallback = params[i].equals(TypeConstants.CI_TYPE) || params[i].equals(TypeConstants.CIR_TYPE); + if (isCallback) { + return i + 1 >= params.length; + } + } + return false; + } + + /** Records that this handler's capture list is known to line up with the patched method. */ + private static void markUpgraded(Configuration config) { + if (config instanceof MutableConfiguration mutable) { + mutable.setProperty(SpecialKeys.LOCALS_UPGRADED, true); + } + } } + + + + + + diff --git a/core/src/main/java/org/sinytra/adapter/types/BytecodeFixerJarGenerator.java b/core/src/main/java/org/sinytra/adapter/types/BytecodeFixerJarGenerator.java index 5fb85fb..850b134 100644 --- a/core/src/main/java/org/sinytra/adapter/types/BytecodeFixerJarGenerator.java +++ b/core/src/main/java/org/sinytra/adapter/types/BytecodeFixerJarGenerator.java @@ -61,7 +61,18 @@ public boolean save(Path path, Attributes additionalAttributes) { jarEntry.setTime(ZIP_TIME); jos.putNextEntry(jarEntry); - ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES); + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES) { + @Override + protected String getCommonSuperClass(String type1, String type2) { + try { + return super.getCommonSuperClass(type1, type2); + } catch (Throwable t) { + // The default implementation resolves types through this class loader, which cannot + // see Minecraft classes. Degrade to Object so jar generation still succeeds. + return "java/lang/Object"; + } + } + }; entry.getValue().accept(cw); byte[] bytes = cw.toByteArray(); jos.write(bytes); diff --git a/test/mc-26.1.2/src/test/java/org/sinytra/adapter/patch/test/mc_26_1_2/mixin/DynamicMixinPatchTest.java b/test/mc-26.1.2/src/test/java/org/sinytra/adapter/patch/test/mc_26_1_2/mixin/DynamicMixinPatchTest.java index 421188e..68819b9 100644 --- a/test/mc-26.1.2/src/test/java/org/sinytra/adapter/patch/test/mc_26_1_2/mixin/DynamicMixinPatchTest.java +++ b/test/mc-26.1.2/src/test/java/org/sinytra/adapter/patch/test/mc_26_1_2/mixin/DynamicMixinPatchTest.java @@ -15,6 +15,8 @@ import org.slf4j.Logger; import org.spongepowered.asm.mixin.FabricUtil; +import static org.junit.jupiter.api.Assertions.assertNotNull; + import java.util.List; public class DynamicMixinPatchTest extends MinecraftMixinPatchTest { @@ -74,6 +76,18 @@ void testModifyArgChangedTargetDesc() throws Exception { ); } + @Test + void testRepeatedAnnotationsAreParsed() throws Exception { + // TARGET_CONSTANT used to dereference the null result of parseSingle for a repeated annotation and + // abort the transform for the whole mod. This test fails again if that null guard is removed. + LoadResult result = load( + "org/sinytra/adapter/test/mc_26_1_2/mixin/RepeatedAnnotationsMixin", + List.of("repeatedAnnotations") + ); + + assertNotNull(result.patched(), "the mixin should be parsed instead of aborting the transform"); + } + @Override protected LoadResult load(String className, List allowedMethods) throws Exception { ClassNode patched = loadClass(className); @@ -82,3 +96,4 @@ protected LoadResult load(String className, List allowedMethods) throws return new LoadResult(patchEnvironment, patched, loadClass(className)); } } + diff --git a/test/mc-26.1.2/src/testClasses/java/org/sinytra/adapter/test/mc_26_1_2/mixin/RepeatedAnnotationsMixin.java b/test/mc-26.1.2/src/testClasses/java/org/sinytra/adapter/test/mc_26_1_2/mixin/RepeatedAnnotationsMixin.java new file mode 100644 index 0000000..0b182ff --- /dev/null +++ b/test/mc-26.1.2/src/testClasses/java/org/sinytra/adapter/test/mc_26_1_2/mixin/RepeatedAnnotationsMixin.java @@ -0,0 +1,23 @@ +package org.sinytra.adapter.test.mc_26_1_2.mixin; + +import net.minecraft.world.entity.LivingEntity; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Constant; +import org.spongepowered.asm.mixin.injection.ModifyConstant; +import org.spongepowered.asm.mixin.injection.Slice; + +/** + * Fixture for the real-world shape of a repeated annotation, which ASM stores as a List. + */ +@Mixin(LivingEntity.class) +public class RepeatedAnnotationsMixin { + @ModifyConstant( + method = "getMaxAirSupply", + constant = {@Constant(intValue = 1), @Constant(intValue = 2)}, + slice = {@Slice(from = @At("HEAD")), @Slice(to = @At("RETURN"))} + ) + private static int repeatedAnnotations(int original) { + return original; + } +}