Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: Allow injection point specifiers anywhere. #129

Merged
merged 1 commit into from
Mar 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,19 @@ public abstract class InjectionPoint {
}

/**
* Selector type for slice delmiters, ignored for normal injection points.
* <tt>Selectors</tt> can be supplied in {@link At} annotations by including
* a colon (<tt>:</tt>) character followed by the selector type
* (case-sensitive), eg:
* Additional specifier for injection points. <tt>Specifiers</tt> can be
* supplied in {@link At} annotations by including a colon (<tt>:</tt>)
* character followed by the specifier type (case-sensitive), eg:
*
* <blockquote><pre>&#064;At(value = "INVOKE:LAST", ... )</pre></blockquote>
*/
public enum Selector {

public enum Specifier {

/**
* Use all instructions from the query result.
*/
ALL,

/**
* Use the <em>first</em> instruction from the query result.
*/
Expand All @@ -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.
* <em>For internal use only. Currently {@link #FIRST}</em>
* 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;

}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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() {
Expand Down
14 changes: 7 additions & 7 deletions src/main/java/org/spongepowered/asm/mixin/injection/Slice.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 <tt>Slice</tt> identifies a section of a method to search for injection
Expand Down Expand Up @@ -162,10 +162,10 @@

/**
* Injection point which specifies the <em>start</em> 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:
*
* <blockquote><pre>&#064;At(value = "INVOKE:LAST", ... )</pre></blockquote>
*
Expand All @@ -182,9 +182,9 @@
/**
* Injection point which specifies the <em>end</em> 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:
*
* <blockquote><pre>&#064;At(value = "INVOKE:LAST", ... )</pre></blockquote>
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -294,7 +295,7 @@ private Collection<TargetNode> findTargetNodes(InjectorTarget injectorTarget, Li
IMixinContext mixin = this.info.getMixin();
MethodNode method = injectorTarget.getMethod();
Map<Integer, TargetNode> targetNodes = new TreeMap<Integer, TargetNode>();
Collection<AbstractInsnNode> nodes = new ArrayList<AbstractInsnNode>(32);
List<AbstractInsnNode> nodes = new ArrayList<AbstractInsnNode>(32);

for (InjectionPoint injectionPoint : injectionPoints) {
nodes.clear();
Expand All @@ -307,22 +308,37 @@ private Collection<TargetNode> 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);
}
}
}

return targetNodes.values();
}

protected void addTargetNode(MethodNode method, Map<Integer, TargetNode> 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<AbstractInsnNode> nodes) {
return injectionPoint.find(into.desc, injectorTarget.getSlice(injectionPoint), nodes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand All @@ -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<AbstractInsnNode> nodes = new LinkedList<AbstractInsnNode>();
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()
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public class BeforeConstant extends InjectionPoint {
private final boolean log;

public BeforeConstant(IMixinContext context, AnnotationNode node, String returnType) {
super(Annotations.<String>getValue(node, "slice", ""), Selector.DEFAULT, null);
super(Annotations.<String>getValue(node, "slice", ""), Specifier.DEFAULT, null);

Boolean empty = Annotations.<Boolean>getValue(node, "nullValue", (Boolean)null);
this.ordinal = Annotations.<Integer>getValue(node, "ordinal", Integer.valueOf(-1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -131,7 +131,7 @@ public InjectionPointData(IInjectionPointContext context, String at, List<String

Matcher matcher = InjectionPointData.AT_PATTERN.matcher(at);
this.type = InjectionPointData.parseType(matcher, at);
this.selector = InjectionPointData.parseSelector(matcher);
this.specifier = InjectionPointData.parseSpecifier(matcher);
}

private void parseArgs(List<String> args) {
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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())));
}

/**
Expand All @@ -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) {
Expand Down
Loading