From 38825f7bd98f5edec6eb4aee1a4398be90f42b28 Mon Sep 17 00:00:00 2001 From: Chocohead Date: Mon, 28 Jun 2021 17:54:19 +0100 Subject: [PATCH] Allow interface Mixins (#51) * Allow mixining interfaces Reapplies #13 Fixes #34 Includes (a form of) SpongePowered/Mixin#413 and SpongePowered/Mixin#415 * Fix mods using static vagueness Whilst SpongePowered/Mixin#415 is generating things more correctly, there are mods (such as Better End) which rely on the current behaviour * Fix lambdas in interface Mixins Fixes #43 --- .../AnnotatedMixinElementHandlerInjector.java | 8 ----- .../asm/mixin/gen/AccessorGenerator.java | 14 +++++---- .../gen/AccessorGeneratorFieldSetter.java | 7 +++++ .../gen/AccessorGeneratorMethodProxy.java | 5 ++-- .../asm/mixin/injection/code/Injector.java | 5 ++-- .../asm/mixin/struct/MemberRef.java | 10 ++++--- .../transformer/MixinApplicatorInterface.java | 29 ++++++++++++------- .../mixin/transformer/MixinPostProcessor.java | 2 +- .../MixinPreProcessorInterface.java | 9 ++++-- 9 files changed, 54 insertions(+), 35 deletions(-) diff --git a/src/ap/java/org/spongepowered/tools/obfuscation/AnnotatedMixinElementHandlerInjector.java b/src/ap/java/org/spongepowered/tools/obfuscation/AnnotatedMixinElementHandlerInjector.java index afd76ceb7..382a797ad 100644 --- a/src/ap/java/org/spongepowered/tools/obfuscation/AnnotatedMixinElementHandlerInjector.java +++ b/src/ap/java/org/spongepowered/tools/obfuscation/AnnotatedMixinElementHandlerInjector.java @@ -152,10 +152,6 @@ public void notifyRemapped() { } public void registerInjector(AnnotatedElementInjector elem) { - if (this.mixin.isInterface()) { - this.ap.printMessage(Kind.ERROR, "Injector in interface is unsupported", elem.getElement()); - } - for (String reference : elem.getAnnotation().getList("method")) { ITargetSelector targetSelector = TargetSelector.parse(reference); @@ -260,10 +256,6 @@ private boolean registerInjector(AnnotatedElementInjector elem, String reference * and process the references */ public void registerInjectionPoint(AnnotatedElementInjectionPoint elem, String format) { - if (this.mixin.isInterface()) { - this.ap.printMessage(Kind.ERROR, "Injector in interface is unsupported", elem.getElement()); - } - if (!elem.shouldRemap()) { return; } diff --git a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGenerator.java b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGenerator.java index e11bded50..d6b9aa293 100644 --- a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGenerator.java +++ b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGenerator.java @@ -26,11 +26,11 @@ import java.util.ArrayList; +import org.apache.logging.log4j.LogManager; import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.AnnotationNode; import org.objectweb.asm.tree.MethodNode; -import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException; -import org.spongepowered.asm.mixin.refmap.IMixinContext; +import org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException; import org.spongepowered.asm.util.asm.ASM; /** @@ -54,10 +54,12 @@ public AccessorGenerator(AccessorInfo info, boolean isStatic) { } protected void checkModifiers() { - if (this.info.isStatic() && !this.targetIsStatic) { - IMixinContext context = this.info.getContext(); - throw new InvalidInjectionException(context, String.format("%s is invalid. Accessor method is%s static but the target is not.", - this.info, this.info.isStatic() ? "" : " not")); + if (this.info.isStatic() != this.targetIsStatic) { + if (!this.targetIsStatic) { + throw new InvalidAccessorException(this.info, String.format("%s is invalid. Accessor method is static but the target is not.", this.info)); + } else { + LogManager.getLogger("mixin").info("{} should be static as its target is", this.info); + } } } diff --git a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorFieldSetter.java b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorFieldSetter.java index cee3d330b..f10c65ff0 100644 --- a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorFieldSetter.java +++ b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorFieldSetter.java @@ -31,6 +31,7 @@ import org.objectweb.asm.tree.MethodNode; import org.objectweb.asm.tree.VarInsnNode; import org.spongepowered.asm.mixin.MixinEnvironment.Option; +import org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException; import org.spongepowered.asm.mixin.Mutable; import org.spongepowered.asm.mixin.transformer.ClassInfo.Method; import org.spongepowered.asm.mixin.transformer.MixinTargetContext; @@ -52,6 +53,12 @@ public AccessorGeneratorFieldSetter(AccessorInfo info) { @Override public void validate() { + if (Bytecode.hasFlag(this.info.getClassNode(), Opcodes.ACC_INTERFACE)) { + //This will result in a ClassFormatError when the class is verified + throw new InvalidAccessorException(info, String.format("%s tried to change interface field %s::%s", + this.info, this.info.getClassNode().name, this.targetField.name)); + } + super.validate(); Method method = this.info.getClassInfo().findMethod(this.info.getMethod()); diff --git a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorMethodProxy.java b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorMethodProxy.java index c8504c761..34dd81965 100644 --- a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorMethodProxy.java +++ b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorMethodProxy.java @@ -75,9 +75,10 @@ public MethodNode generate() { method.instructions.add(new VarInsnNode(Opcodes.ALOAD, 0)); } Bytecode.loadArgs(this.argTypes, method.instructions, this.targetIsStatic ? 0 : 1); + boolean isInterface = Bytecode.hasFlag(this.info.getClassNode(), Opcodes.ACC_INTERFACE); boolean isPrivate = Bytecode.hasFlag(this.targetMethod, Opcodes.ACC_PRIVATE); - int opcode = this.targetIsStatic ? Opcodes.INVOKESTATIC : (isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL); - method.instructions.add(new MethodInsnNode(opcode, this.info.getClassNode().name, this.targetMethod.name, this.targetMethod.desc, false)); + int opcode = this.targetIsStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : (isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL); + method.instructions.add(new MethodInsnNode(opcode, this.info.getClassNode().name, this.targetMethod.name, this.targetMethod.desc, isInterface)); method.instructions.add(new InsnNode(this.returnType.getOpcode(Opcodes.IRETURN))); return method; } 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 197189a6e..d985037a5 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 @@ -405,9 +405,10 @@ protected AbstractInsnNode invokeHandler(InsnList insns) { * @return injected insn node */ protected AbstractInsnNode invokeHandler(InsnList insns, MethodNode handler) { + boolean isInterface = Bytecode.hasFlag(classNode, Opcodes.ACC_INTERFACE); boolean isPrivate = (handler.access & Opcodes.ACC_PRIVATE) != 0; - int invokeOpcode = this.isStatic ? Opcodes.INVOKESTATIC : isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL; - MethodInsnNode insn = new MethodInsnNode(invokeOpcode, this.classNode.name, handler.name, handler.desc, false); + int invokeOpcode = this.isStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL; + MethodInsnNode insn = new MethodInsnNode(invokeOpcode, this.classNode.name, handler.name, handler.desc, isInterface); insns.add(insn); this.info.addCallbackInvocation(handler); return insn; diff --git a/src/main/java/org/spongepowered/asm/mixin/struct/MemberRef.java b/src/main/java/org/spongepowered/asm/mixin/struct/MemberRef.java index b8baec329..5d3a6344a 100644 --- a/src/main/java/org/spongepowered/asm/mixin/struct/MemberRef.java +++ b/src/main/java/org/spongepowered/asm/mixin/struct/MemberRef.java @@ -27,6 +27,7 @@ import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.FieldInsnNode; import org.objectweb.asm.tree.MethodInsnNode; +import org.spongepowered.asm.mixin.transformer.ClassInfo; import org.spongepowered.asm.mixin.transformer.throwables.MixinTransformerError; import org.spongepowered.asm.util.Bytecode; @@ -241,7 +242,7 @@ public void setOpcode(int opcode) { if (tag == 0) { throw new MixinTransformerError("Invalid opcode " + Bytecode.getOpcodeName(opcode) + " for method handle " + this.handle + "."); } - boolean itf = tag == Opcodes.H_INVOKEINTERFACE; + boolean itf = this.handle.isInterface(); this.handle = new org.objectweb.asm.Handle(tag, this.handle.getOwner(), this.handle.getName(), this.handle.getDesc(), itf); } @@ -252,7 +253,8 @@ public String getOwner() { @Override public void setOwner(String owner) { - boolean itf = this.handle.getTag() == Opcodes.H_INVOKEINTERFACE; + boolean itf = this.handle.isInterface(); + assert ClassInfo.forName(owner).isInterface() == itf; this.handle = new org.objectweb.asm.Handle(this.handle.getTag(), owner, this.handle.getName(), this.handle.getDesc(), itf); } @@ -263,7 +265,7 @@ public String getName() { @Override public void setName(String name) { - boolean itf = this.handle.getTag() == Opcodes.H_INVOKEINTERFACE; + boolean itf = this.handle.isInterface(); this.handle = new org.objectweb.asm.Handle(this.handle.getTag(), this.handle.getOwner(), name, this.handle.getDesc(), itf); } @@ -274,7 +276,7 @@ public String getDesc() { @Override public void setDesc(String desc) { - boolean itf = this.handle.getTag() == Opcodes.H_INVOKEINTERFACE; + boolean itf = this.handle.isInterface(); this.handle = new org.objectweb.asm.Handle(this.handle.getTag(), this.handle.getOwner(), this.handle.getName(), desc, itf); } } diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java index 0d7987a68..fe73a5eb9 100644 --- a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java +++ b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java @@ -24,10 +24,13 @@ */ package org.spongepowered.asm.mixin.transformer; +import java.lang.reflect.Modifier; import java.util.Map.Entry; import org.objectweb.asm.tree.FieldNode; import org.objectweb.asm.tree.MethodNode; +import org.spongepowered.asm.mixin.MixinEnvironment; +import org.spongepowered.asm.mixin.MixinEnvironment.CompatibilityLevel.LanguageFeature; import org.spongepowered.asm.mixin.injection.struct.InjectionInfo; import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException; import org.spongepowered.asm.mixin.transformer.ClassInfo.Field; @@ -96,28 +99,34 @@ protected void applyInitialisers(MixinTargetContext mixin) { */ @Override protected void prepareInjections(MixinTargetContext mixin) { - // disabled for interface mixins for (MethodNode method : this.targetClass.methods) { try { InjectionInfo injectInfo = InjectionInfo.parse(mixin, method); if (injectInfo != null) { - throw new InvalidInterfaceMixinException(mixin, injectInfo + " is not supported on interface mixin method " + method.name); + //Make sure we're running on a Java version which supports interfaces having method bodies + if (!MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.METHODS_IN_INTERFACES)) { + throw new InvalidInterfaceMixinException(mixin, injectInfo + " is not supported on interface mixin method " + method.name); + } } } catch (InvalidInjectionException ex) { String description = ex.getInjectionInfo() != null ? ex.getInjectionInfo().toString() : "Injection"; throw new InvalidInterfaceMixinException(mixin, description + " is not supported in interface mixin"); } } + + super.prepareInjections(mixin); } - /* (non-Javadoc) - * @see org.spongepowered.asm.mixin.transformer.MixinApplicator - * #applyInjections( - * org.spongepowered.asm.mixin.transformer.MixinTargetContext) - */ @Override - protected void applyInjections(MixinTargetContext mixin) { - // Do nothing - } + protected void checkMethodVisibility(MixinTargetContext mixin, MethodNode mixinMethod) { + //Allow injecting into static interface methods where it isn't possible to control the access of the injection method + if (Modifier.isStatic(mixinMethod.access) && !MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.PRIVATE_METHODS_IN_INTERFACES)) { + InjectionInfo injectInfo = InjectionInfo.parse(mixin, mixinMethod); + if (injectInfo != null) { + return; + } + } + super.checkMethodVisibility(mixin, mixinMethod); + } } diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPostProcessor.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPostProcessor.java index a0931ecf6..4e9ff44bb 100644 --- a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPostProcessor.java +++ b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPostProcessor.java @@ -213,7 +213,7 @@ private static void createProxy(MethodNode methodNode, ClassInfo targetClass, Me Type[] args = Type.getArgumentTypes(methodNode.desc); Type returnType = Type.getReturnType(methodNode.desc); Bytecode.loadArgs(args, methodNode.instructions, 0); - methodNode.instructions.add(new MethodInsnNode(Opcodes.INVOKESTATIC, targetClass.getName(), method.getName(), methodNode.desc, false)); + methodNode.instructions.add(new MethodInsnNode(Opcodes.INVOKESTATIC, targetClass.getName(), method.getName(), methodNode.desc, targetClass.isInterface())); methodNode.instructions.add(new InsnNode(returnType.getOpcode(Opcodes.IRETURN))); methodNode.maxStack = Bytecode.getFirstNonArgLocalIndex(args, false); methodNode.maxLocals = 0; diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java index 2f54a5d81..5294e884f 100644 --- a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java +++ b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java @@ -27,6 +27,8 @@ import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.AnnotationNode; import org.objectweb.asm.tree.FieldNode; +import org.spongepowered.asm.mixin.MixinEnvironment; +import org.spongepowered.asm.mixin.MixinEnvironment.CompatibilityLevel.LanguageFeature; import org.spongepowered.asm.mixin.transformer.ClassInfo.Method; import org.spongepowered.asm.mixin.transformer.MixinInfo.MixinClassNode; import org.spongepowered.asm.mixin.transformer.MixinInfo.MixinMethodNode; @@ -58,8 +60,11 @@ class MixinPreProcessorInterface extends MixinPreProcessorStandard { protected void prepareMethod(MixinMethodNode mixinMethod, Method method) { // Userland interfaces should not have non-public methods except for lambda bodies if (!Bytecode.hasFlag(mixinMethod, Opcodes.ACC_PUBLIC) && !Bytecode.hasFlag(mixinMethod, Opcodes.ACC_SYNTHETIC)) { - throw new InvalidInterfaceMixinException(this.mixin, "Interface mixin contains a non-public method! Found " + method + " in " - + this.mixin); + //On versions that support it private methods are also allowed + if (!Bytecode.hasFlag(mixinMethod, Opcodes.ACC_PRIVATE) || !MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.PRIVATE_METHODS_IN_INTERFACES)) { + throw new InvalidInterfaceMixinException(this.mixin, "Interface mixin contains a non-public method! Found " + method + " in " + + this.mixin); + } } super.prepareMethod(mixinMethod, method);