Skip to content

Commit

Permalink
Improve BeforeInvoke permissive search mode, verbosity++, closes Spon…
Browse files Browse the repository at this point in the history
  • Loading branch information
Mumfrey committed Jun 27, 2018
1 parent c08fb6b commit 09b2a04
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,15 @@ public static enum Option {
*/
REFMAP_REMAP_SOURCE_ENV(Option.ENVIRONMENT, Inherit.INDEPENDENT, "refMapRemappingEnv", "searge"),

/**
* When <tt>mixin.env.remapRefMap</tt> is enabled and a refmap is
* available for a mixin config, certain injection points are allowed to
* fail over to a "permissive" match which ignores the member descriptor
* in the refmap. To disable this behaviour, set this property to
* <tt>false</tt>.
*/
REFMAP_REMAP_ALLOW_PERMISSIVE(Option.ENVIRONMENT, Inherit.INDEPENDENT, "allowPermissiveMatch", true, "true"),

/**
* Globally ignore the "required" attribute of all configurations
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.spongepowered.asm.mixin.injection.InjectionPoint.AtCode;
import org.spongepowered.asm.mixin.injection.struct.InjectionPointData;
import org.spongepowered.asm.mixin.injection.struct.MemberInfo;
import org.spongepowered.asm.mixin.refmap.IMixinContext;

/**
* <p>This injection point searches for INVOKEVIRTUAL, INVOKESTATIC and
Expand Down Expand Up @@ -67,8 +68,26 @@
*/
@AtCode("INVOKE")
public class BeforeInvoke extends InjectionPoint {

/**
* Member search type, the <tt>PERMISSIVE</tt> search is only used when
* refmap remapping is enabled.
*/
public enum SearchType {

STRICT,
PERMISSIVE

}

protected final MemberInfo target, permissiveTarget;
protected final MemberInfo target;

/**
* This option enables a fallback "permissive" search to occur if initial
* search fails <b>if and only if the {@link Option#REFMAP_REMAP} option is
* enabled and the context mixin's parent config has a valid refmap</b>.
*/
protected final boolean allowPermissive;

/**
* This strategy can be used to identify a particular invocation if the same
Expand All @@ -81,22 +100,32 @@ public class BeforeInvoke extends InjectionPoint {
* Class name (description) for debug logging
*/
protected final String className;

/**
*
*/
protected final IMixinContext context;

/**
* Logger reference
*/
protected final Logger logger = LogManager.getLogger("mixin");

/**
* True to turn on strategy debugging to the console
*/
private boolean log = false;

private final Logger logger = LogManager.getLogger("mixin");

public BeforeInvoke(InjectionPointData data) {
super(data);

this.target = data.getTarget();
this.ordinal = data.getOrdinal();
this.log = data.get("log", false);
this.className = this.getClassName();
this.permissiveTarget = data.getContext().getOption(Option.REFMAP_REMAP) ? this.target.transform(null) : null;
this.context = data.getContext();
this.allowPermissive = this.context.getOption(Option.REFMAP_REMAP) && this.context.getOption(Option.REFMAP_REMAP_ALLOW_PERMISSIVE)
&& !this.context.getReferenceMapper().isDefault();
}

private String getClassName() {
Expand Down Expand Up @@ -124,19 +153,23 @@ public BeforeInvoke setLogging(boolean logging) {
public boolean find(String desc, InsnList insns, Collection<AbstractInsnNode> nodes) {
this.log("{} is searching for an injection point in method with descriptor {}", this.className, desc);

if (!this.find(desc, insns, nodes, this.target)) {
return this.find(desc, insns, nodes, this.permissiveTarget);
if (!this.find(desc, insns, nodes, this.target, SearchType.STRICT) && this.target.desc != null && this.allowPermissive) {
this.logger.warn("STRICT match for {} using \"{}\" in {} returned 0 results, attempting permissive search. "
+ "To inhibit permissive search set mixin.env.allowPermissiveMatch=false", this.className, this.target, this.context);
return this.find(desc, insns, nodes, this.target, SearchType.PERMISSIVE);
}
return true;
}

protected boolean find(String desc, InsnList insns, Collection<AbstractInsnNode> nodes, MemberInfo target) {
if (target == null) {
protected boolean find(String desc, InsnList insns, Collection<AbstractInsnNode> nodes, MemberInfo member, SearchType searchType) {
if (member == null) {
return false;
}

MemberInfo target = searchType == SearchType.PERMISSIVE ? member.transform(null) : member;

int ordinal = 0;
boolean found = false;
int found = 0;

ListIterator<AbstractInsnNode> iter = insns.iterator();
while (iter.hasNext()) {
Expand All @@ -152,7 +185,9 @@ protected boolean find(String desc, InsnList insns, Collection<AbstractInsnNode>
if (this.matchesInsn(nodeInfo, ordinal)) {
this.log("{} > > > found a matching insn at ordinal {}", this.className, ordinal);

found |= this.addInsn(insns, nodes, insn);
if (this.addInsn(insns, nodes, insn)) {
found++;
}

if (this.ordinal == ordinal) {
break;
Expand All @@ -165,8 +200,13 @@ protected boolean find(String desc, InsnList insns, Collection<AbstractInsnNode>

this.inspectInsn(desc, insns, insn);
}

if (searchType == SearchType.PERMISSIVE && found > 1) {
this.logger.warn("A permissive match for {} using \"{}\" in {} matched {} instructions, this may cause unexpected behaviour. "
+ "To inhibit permissive search set mixin.env.allowPermissiveMatch=false", this.className, member, this.context, found);
}

return found;
return found > 0;
}

protected boolean addInsn(InsnList insns, Collection<AbstractInsnNode> nodes, AbstractInsnNode insn) {
Expand All @@ -189,7 +229,7 @@ protected boolean matchesInsn(MemberInfo nodeInfo, int ordinal) {

protected void log(String message, Object... params) {
if (this.log) {
this.logger.info(message, params);
this.logger.info(message, params);
}
}

Expand Down

0 comments on commit 09b2a04

Please sign in to comment.