From e70693a79a68a524a88b9286c87615f345c851e4 Mon Sep 17 00:00:00 2001 From: LlamaLad7 Date: Sun, 3 Mar 2024 16:34:02 +0000 Subject: [PATCH] New: Allow injection point specifiers anywhere. (#129) Takes the relevant changes from https://github.com/SpongePowered/Mixin/commit/e02f4cae3fe51d6bd83de5e020956587ead8087c. --- .../asm/mixin/injection/InjectionPoint.java | 42 +++++++++------- .../asm/mixin/injection/Slice.java | 14 +++--- .../asm/mixin/injection/code/Injector.java | 34 +++++++++---- .../asm/mixin/injection/code/MethodSlice.java | 50 ++++++++++++++----- .../mixin/injection/code/MethodSlices.java | 10 ++++ .../injection/points/BeforeConstant.java | 2 +- .../mixin/injection/struct/InjectionInfo.java | 2 + .../injection/struct/InjectionPointData.java | 18 +++---- 8 files changed, 115 insertions(+), 57 deletions(-) diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/InjectionPoint.java b/src/main/java/org/spongepowered/asm/mixin/injection/InjectionPoint.java index 00d5abb23..fb3bd5439 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/InjectionPoint.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/InjectionPoint.java @@ -162,15 +162,19 @@ public abstract class InjectionPoint { } /** - * Selector type for slice delmiters, ignored for normal injection points. - * Selectors can be supplied in {@link At} annotations by including - * a colon (:) character followed by the selector type - * (case-sensitive), eg: + * Additional specifier for injection points. Specifiers can be + * supplied in {@link At} annotations by including a colon (:) + * character followed by the specifier type (case-sensitive), eg: * * 
@At(value = "INVOKE:LAST", ... )
*/ - public enum Selector { - + public enum Specifier { + + /** + * Use all instructions from the query result. + */ + ALL, + /** * Use the first instruction from the query result. */ @@ -186,13 +190,13 @@ public enum Selector { * more than one instruction this should be considered a fail-fast error * state and a runtime exception will be thrown. */ - ONE; + ONE, /** - * Default selector type used if no selector is explicitly specified. - * For internal use only. Currently {@link #FIRST} + * Use the default setting as defined by the consumer. For slices this + * is {@link #FIRST}, for all other consumers this is {@link #ALL} */ - public static final Selector DEFAULT = Selector.FIRST; + DEFAULT; } @@ -276,26 +280,26 @@ enum ShiftByViolationBehaviour { } private final String slice; - private final Selector selector; + private final Specifier specifier; private final String id; private final IMessageSink messageSink; protected InjectionPoint() { - this("", Selector.DEFAULT, null); + this("", Specifier.DEFAULT, null); } protected InjectionPoint(InjectionPointData data) { - this(data.getSlice(), data.getSelector(), data.getId(), data.getMessageSink()); + this(data.getSlice(), data.getSpecifier(), data.getId(), data.getMessageSink()); } - public InjectionPoint(String slice, Selector selector, String id) { - this(slice, selector, id, null); + public InjectionPoint(String slice, Specifier specifier, String id) { + this(slice, specifier, id, null); } - public InjectionPoint(String slice, Selector selector, String id, IMessageSink messageSink) { + public InjectionPoint(String slice, Specifier specifier, String id, IMessageSink messageSink) { this.slice = slice; - this.selector = selector; + this.specifier = specifier; this.id = id; this.messageSink = messageSink; } @@ -304,8 +308,8 @@ public String getSlice() { return this.slice; } - public Selector getSelector() { - return this.selector; + public Specifier getSpecifier(Specifier defaultSpecifier) { + return this.specifier == Specifier.DEFAULT ? defaultSpecifier : this.specifier; } public String getId() { diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/Slice.java b/src/main/java/org/spongepowered/asm/mixin/injection/Slice.java index 857432dbf..68689107d 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/Slice.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/Slice.java @@ -28,7 +28,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import org.spongepowered.asm.mixin.injection.InjectionPoint.Selector; +import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier; /** * A Slice identifies a section of a method to search for injection @@ -162,10 +162,10 @@ /** * Injection point which specifies the start of the slice region. - * {@link At}s supplied here should generally specify a {@link Selector} + * {@link At}s supplied here should generally use a {@link Specifier} * in order to identify which instruction should be used for queries which - * return multiple results. The selector is specified by appending the - * selector type to the injection point type as follows: + * return multiple results. The specifier is supplied by appending the + * specifier type to the injection point type as follows: * * 
@At(value = "INVOKE:LAST", ... )
* @@ -182,9 +182,9 @@ /** * Injection point which specifies the end of the slice region. * Like {@link #from}, {@link At}s supplied here should generally specify a - * {@link Selector} in order to identify which instruction should be used - * for queries which return multiple results. The selector is specified by - * appending the selector type to the injection point type as follows: + * {@link Specifier} in order to identify which instruction should be used + * for queries which return multiple results. The specifier is supplied by + * appending the specifier type to the injection point type as follows: * * 
@At(value = "INVOKE:LAST", ... )
* diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/code/Injector.java b/src/main/java/org/spongepowered/asm/mixin/injection/code/Injector.java index c37a3d563..b7b55ff3e 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/code/Injector.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/code/Injector.java @@ -42,6 +42,7 @@ import org.spongepowered.asm.mixin.injection.Coerce; import org.spongepowered.asm.mixin.injection.InjectionPoint; import org.spongepowered.asm.mixin.injection.InjectionPoint.RestrictTargetLevel; +import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier; import org.spongepowered.asm.mixin.injection.invoke.RedirectInjector; import org.spongepowered.asm.mixin.injection.struct.InjectionInfo; import org.spongepowered.asm.mixin.injection.struct.InjectionNodes.InjectionNode; @@ -294,7 +295,7 @@ private Collection findTargetNodes(InjectorTarget injectorTarget, Li IMixinContext mixin = this.info.getMixin(); MethodNode method = injectorTarget.getMethod(); Map targetNodes = new TreeMap(); - Collection nodes = new ArrayList(32); + List nodes = new ArrayList(32); for (InjectionPoint injectionPoint : injectionPoints) { nodes.clear(); @@ -307,15 +308,20 @@ private Collection findTargetNodes(InjectorTarget injectorTarget, Li injectorTarget, injectorTarget.getMergedBy(), injectorTarget.getMergedPriority())); } - if (this.findTargetNodes(method, injectionPoint, injectorTarget, nodes)) { + if (!this.findTargetNodes(method, injectionPoint, injectorTarget, nodes)) { + continue; + } + + Specifier specifier = injectionPoint.getSpecifier(Specifier.ALL); + if (specifier == Specifier.ONE && nodes.size() != 1) { + throw new InvalidInjectionException(this.info, String.format("%s on %s has specifier :ONE but matched %d instructions", + injectionPoint, this, nodes.size())); + } else if (specifier != Specifier.ALL && nodes.size() > 1) { + AbstractInsnNode specified = nodes.get(specifier == Specifier.FIRST ? 0 : nodes.size() - 1); + this.addTargetNode(method, targetNodes, injectionPoint, specified); + } else { for (AbstractInsnNode insn : nodes) { - Integer key = method.instructions.indexOf(insn); - TargetNode targetNode = targetNodes.get(key); - if (targetNode == null) { - targetNode = new TargetNode(insn); - targetNodes.put(key, targetNode); - } - targetNode.nominators.add(injectionPoint); + this.addTargetNode(method, targetNodes, injectionPoint, insn); } } } @@ -323,6 +329,16 @@ private Collection findTargetNodes(InjectorTarget injectorTarget, Li return targetNodes.values(); } + protected void addTargetNode(MethodNode method, Map targetNodes, InjectionPoint injectionPoint, AbstractInsnNode insn) { + Integer key = method.instructions.indexOf(insn); + TargetNode targetNode = targetNodes.get(key); + if (targetNode == null) { + targetNode = new TargetNode(insn); + targetNodes.put(key, targetNode); + } + targetNode.nominators.add(injectionPoint); + } + protected boolean findTargetNodes(MethodNode into, InjectionPoint injectionPoint, InjectorTarget injectorTarget, Collection nodes) { return injectionPoint.find(into.desc, injectorTarget.getSlice(injectionPoint), nodes); diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlice.java b/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlice.java index 79891e95f..58201c05b 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlice.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlice.java @@ -37,7 +37,7 @@ import org.spongepowered.asm.mixin.MixinEnvironment.Option; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.InjectionPoint; -import org.spongepowered.asm.mixin.injection.InjectionPoint.Selector; +import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier; import org.spongepowered.asm.mixin.injection.Slice; import org.spongepowered.asm.mixin.injection.struct.InjectionPointAnnotationContext; import org.spongepowered.asm.mixin.injection.throwables.InjectionError; @@ -325,6 +325,11 @@ public int realIndexOf(AbstractInsnNode insn) { * Descriptive name of the slice, used in exceptions */ private final String name; + + /** + * Success counts for from and to injection points + */ + private int successCountFrom, successCountTo; /** * ctor @@ -361,8 +366,8 @@ public String getId() { */ public InsnListReadOnly getSlice(MethodNode method) { int max = method.instructions.size() - 1; - int start = this.find(method, this.from, 0, 0, this.name + "(from)"); - int end = this.find(method, this.to, max, start, this.name + "(to)"); + int start = this.find(method, this.from, 0, 0, "from"); + int end = this.find(method, this.to, max, start, "to"); if (start > end) { throw new InvalidSliceException(this.owner, String.format("%s is negative size. Range(%d -> %d)", this.describe(), start, end)); @@ -389,32 +394,53 @@ public InsnListReadOnly getSlice(MethodNode method) { * @param defaultValue Value to return if injection point is null (open * ended) * @param failValue Value to use if query fails - * @param description Description for error message + * @param argument The name of the argument ("from" or "to") for debug msgs * @return matching insn index */ - private int find(MethodNode method, InjectionPoint injectionPoint, int defaultValue, int failValue, String description) { + private int find(MethodNode method, InjectionPoint injectionPoint, int defaultValue, int failValue, String argument) { if (injectionPoint == null) { return defaultValue; } + String description = String.format("%s(%s)", this.name, argument); Deque nodes = new LinkedList(); InsnListReadOnly insns = new InsnListReadOnly(method.instructions); boolean result = injectionPoint.find(method.desc, insns, nodes); - Selector select = injectionPoint.getSelector(); - if (nodes.size() != 1 && select == Selector.ONE) { + Specifier specifier = injectionPoint.getSpecifier(Specifier.FIRST); + if (specifier == Specifier.ALL) { + throw new InvalidSliceException(this.owner, String.format("ALL is not a valid specifier for slice %s", this.describe(description))); + } + if (nodes.size() != 1 && specifier == Specifier.ONE) { throw new InvalidSliceException(this.owner, String.format("%s requires 1 result but found %d", this.describe(description), nodes.size())); } if (!result) { - if (this.owner.getMixin().getOption(Option.DEBUG_VERBOSE)) { - MethodSlice.logger.warn("{} did not match any instructions", this.describe(description)); - } return failValue; } - return method.instructions.indexOf(select == Selector.FIRST ? nodes.getFirst() : nodes.getLast()); + if ("from".equals(argument)) { + this.successCountFrom++; + } else { + this.successCountTo++; + } + + return method.instructions.indexOf(specifier == Specifier.FIRST ? nodes.getFirst() : nodes.getLast()); } - + + /** + * Perform post-injection debugging and validation tasks + */ + public void postInject() { + if (this.owner.getMixin().getOption(Option.DEBUG_VERBOSE)) { + if (this.from != null && this.successCountFrom == 0) { + MethodSlice.logger.warn("{} did not match any instructions", this.describe(this.name + "(from)")); + } + if (this.to != null && this.successCountTo == 0) { + MethodSlice.logger.warn("{} did not match any instructions", this.describe(this.name + "(to)")); + } + } + } + /* (non-Javadoc) * @see java.lang.Object#toString() */ diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlices.java b/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlices.java index fb9979aa1..0789b7620 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlices.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/code/MethodSlices.java @@ -82,6 +82,16 @@ public MethodSlice get(String id) { return this.slices.get(id); } + /** + * Called to do post-injection validation/debug logging for slices + */ + public void postInject() { + for (MethodSlice slice : this.slices.values()) { + slice.postInject(); + } + } + + /* (non-Javadoc) * @see java.lang.Object#toString() */ diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/points/BeforeConstant.java b/src/main/java/org/spongepowered/asm/mixin/injection/points/BeforeConstant.java index e8030562e..bbad13edc 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/points/BeforeConstant.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/points/BeforeConstant.java @@ -147,7 +147,7 @@ public class BeforeConstant extends InjectionPoint { private final boolean log; public BeforeConstant(IMixinContext context, AnnotationNode node, String returnType) { - super(Annotations.getValue(node, "slice", ""), Selector.DEFAULT, null); + super(Annotations.getValue(node, "slice", ""), Specifier.DEFAULT, null); Boolean empty = Annotations.getValue(node, "nullValue", (Boolean)null); this.ordinal = Annotations.getValue(node, "ordinal", Integer.valueOf(-1)); diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionInfo.java b/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionInfo.java index bb686fb55..bf6be2c55 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionInfo.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionInfo.java @@ -473,6 +473,8 @@ public void postInject() { String.format("Critical injection failure: %s %s%s in %s failed injection check, %d succeeded of %d allowed.%s", description, this.methodName, this.method.desc, this.mixin, this.injectedCallbackCount, this.maxCallbackCount, extraInfo)); } + + this.slices.postInject(); } /** diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionPointData.java b/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionPointData.java index cb4290a94..b41cdd7ce 100644 --- a/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionPointData.java +++ b/src/main/java/org/spongepowered/asm/mixin/injection/struct/InjectionPointData.java @@ -36,7 +36,7 @@ import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.IInjectionPointContext; import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.InjectionPoint.Selector; +import org.spongepowered.asm.mixin.injection.InjectionPoint.Specifier; import org.spongepowered.asm.mixin.injection.modify.LocalVariableDiscriminator; import org.spongepowered.asm.mixin.injection.selectors.ITargetSelector; import org.spongepowered.asm.mixin.injection.selectors.InvalidSelectorException; @@ -86,7 +86,7 @@ public class InjectionPointData { /** * Selector parsed from the at argument, only used by slices */ - private final Selector selector; + private final Specifier specifier; /** * Target @@ -131,7 +131,7 @@ public InjectionPointData(IInjectionPointContext context, String at, List args) { @@ -172,10 +172,10 @@ public String getType() { } /** - * Get the selector value parsed from the injector + * Get the specifier value parsed from the injector */ - public Selector getSelector() { - return this.selector; + public Specifier getSpecifier() { + return this.specifier; } /** @@ -362,7 +362,7 @@ public String toString() { } private static Pattern createPattern() { - return Pattern.compile(String.format("^(.+?)(:(%s))?$", Joiner.on('|').join(Selector.values()))); + return Pattern.compile(String.format("^(.+?)(:(%s))?$", Joiner.on('|').join(Specifier.values()))); } /** @@ -380,8 +380,8 @@ private static String parseType(Matcher matcher, String at) { return matcher.matches() ? matcher.group(1) : at; } - private static Selector parseSelector(Matcher matcher) { - return matcher.matches() && matcher.group(3) != null ? Selector.valueOf(matcher.group(3)) : Selector.DEFAULT; + private static Specifier parseSpecifier(Matcher matcher) { + return matcher.matches() && matcher.group(3) != null ? Specifier.valueOf(matcher.group(3)) : Specifier.DEFAULT; } private static int parseInt(String string, int defaultValue) {