From 56902db8d0c1199ce2c6b4efa8e7b5735b0dd051 Mon Sep 17 00:00:00 2001 From: LlamaLad7 Date: Fri, 30 Aug 2024 12:33:15 +0100 Subject: [PATCH] Fix: Fix interface injector visibility checks. `InjectionInfo.parse` can be very expensive and in the case of some MixinExtras features is not pure. The correct approach would be to use `InjectionInfo.getInjectorAnnotation`, but the override is no longer needed anyway since we only support Java 8+ so Mixin will already make injector methods private and synthetic. Except, Mixin does it based on the current `COMPATIBILITY_LEVEL`, which is wrong, because we could be on JAVA_17 and yet a class compiled with JAVA_8 had no choice but to use public methods. The original implementation was also incomplete because it forgot about default methods. --- .../asm/mixin/MixinEnvironment.java | 12 +++++++++++- .../transformer/MixinApplicatorInterface.java | 16 ---------------- .../asm/mixin/transformer/MixinInfo.java | 4 ++++ .../transformer/MixinPreProcessorInterface.java | 7 ++++--- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/spongepowered/asm/mixin/MixinEnvironment.java b/src/main/java/org/spongepowered/asm/mixin/MixinEnvironment.java index 3c963397..17c1151e 100644 --- a/src/main/java/org/spongepowered/asm/mixin/MixinEnvironment.java +++ b/src/main/java/org/spongepowered/asm/mixin/MixinEnvironment.java @@ -1080,7 +1080,17 @@ static String getSupportedVersions() { } return sb.toString(); } - + + public static CompatibilityLevel forClassVersion(int version) { + CompatibilityLevel latest = null; + for (CompatibilityLevel level : values()) { + if (level.getClassVersion() >= version) { + return level; + } + latest = level; + } + return latest; + } } /** 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 99aec8db..830736ff 100644 --- a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java +++ b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java @@ -24,12 +24,10 @@ */ 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.Feature; import org.spongepowered.asm.mixin.injection.struct.InjectionInfo; import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException; @@ -37,7 +35,6 @@ import org.spongepowered.asm.mixin.transformer.throwables.InvalidInterfaceMixinException; import org.spongepowered.asm.util.Annotations; import org.spongepowered.asm.util.Constants; -import org.spongepowered.asm.util.LanguageFeatures; /** * Applicator for interface mixins, mainly just disables things which aren't @@ -166,17 +163,4 @@ protected void applyInjections(MixinTargetContext mixin, int injectorOrder) { return; } } - - @Override - 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(LanguageFeatures.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/MixinInfo.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinInfo.java index 980bcd26..41701876 100644 --- a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinInfo.java +++ b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinInfo.java @@ -1268,6 +1268,10 @@ List getTargets() { Set getInterfaces() { return this.getState().getInterfaces(); } + + int getClassVersion() { + return this.getState().getClassNode().version; + } /** * Get transformer extensions 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 a01aa57f..ae17ed22 100644 --- a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java +++ b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java @@ -108,10 +108,11 @@ protected void prepareMethod(MixinMethodNode mixinMethod, Method method) { method, this.mixin)); } - // Make injectors private synthetic if the current runtime supports it + CompatibilityLevel classLevel = CompatibilityLevel.forClassVersion(mixin.getClassVersion()); + // Make injectors private synthetic if the current class version supports it if (isPublic - && !currentLevel.supports(LanguageFeatures.PRIVATE_METHODS_IN_INTERFACES) - && currentLevel.supports(LanguageFeatures.PRIVATE_SYNTHETIC_METHODS_IN_INTERFACES)) { + && !classLevel.supports(LanguageFeatures.PRIVATE_METHODS_IN_INTERFACES) + && classLevel.supports(LanguageFeatures.PRIVATE_SYNTHETIC_METHODS_IN_INTERFACES)) { Bytecode.setVisibility(mixinMethod, Bytecode.Visibility.PRIVATE); mixinMethod.access |= Opcodes.ACC_SYNTHETIC; }