Skip to content

Commit

Permalink
Provide more useful info when mixin classload is prohibited, closes S…
Browse files Browse the repository at this point in the history
  • Loading branch information
Mumfrey committed Jan 8, 2020
1 parent 8cdc32d commit bd40767
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
import org.spongepowered.asm.util.VersionNumber;

import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableList.Builder;
import com.google.gson.Gson;
import com.google.gson.annotations.SerializedName;

Expand Down Expand Up @@ -904,8 +906,17 @@ int getMixinCount() {
/**
* Get the list of mixin classes we will be applying
*/
@SuppressWarnings("unchecked")
public List<String> getClasses() {
return Collections.<String>unmodifiableList(this.mixinClasses);
Builder<String> list = ImmutableList.<String>builder();
for (List<String> classes : new List[] { this.mixinClasses, this.mixinClassesClient, this.mixinClassesServer} ) {
if (classes != null) {
for (String className : classes) {
list.add(this.mixinPackage + className);
}
}
}
return list.build();
}

/**
Expand Down Expand Up @@ -975,7 +986,7 @@ public Level getLoggingLevel() {
public boolean packageMatch(String className) {
return className.startsWith(this.mixinPackage);
}

/**
* Check whether this configuration bundle has a mixin for the specified
* class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,33 @@
*/
class MixinInfo implements Comparable<MixinInfo>, IMixinInfo {

/**
* Class variant, used to determine subtype
*/
enum Variant {

/**
* Standard mixin
*/
STANDARD,

/**
* Interface mixin
*/
INTERFACE,

/**
* Accessor mixin (interface mixin containing only accessors)
*/
ACCESSOR,

/**
* Type proxy
*/
PROXY

}

/**
* A MethodNode in a mixin
*/
Expand Down Expand Up @@ -649,22 +676,17 @@ MixinPreProcessorStandard createPreProcessor(MixinClassNode classNode) {
}

static SubType getTypeFor(MixinInfo mixin) {
if (!mixin.getClassInfo().isInterface()) {
return new SubType.Standard(mixin);
Variant variant = MixinInfo.getVariant(mixin.getClassInfo());
switch (variant) {
case STANDARD:
return new SubType.Standard(mixin);
case INTERFACE:
return new SubType.Interface(mixin);
case ACCESSOR:
return new SubType.Accessor(mixin);
default:
throw new IllegalStateException("Unsupported Mixin variant " + variant + " for " + mixin);
}

boolean containsNonAccessorMethod = false;
for (Method method : mixin.getClassInfo().getMethods()) {
containsNonAccessorMethod |= !method.isAccessor();
}

if (containsNonAccessorMethod) {
// If the mixin contains any other methods, treat it as a regular interface mixin
return new SubType.Interface(mixin);
}

// The mixin contains no non-accessor methods, so we can treat it as an accessor
return new SubType.Accessor(mixin);
}

}
Expand Down Expand Up @@ -853,7 +875,7 @@ static DeclaredTarget of(Object target, MixinInfo info) {
throw new InvalidMixinException(this, ex);
}
}

/**
* Parse the declared targets from the annotation into ClassInfo instances
* and perform initial validation of each target
Expand Down Expand Up @@ -1346,4 +1368,31 @@ public String toString() {
return String.format("%s:%s", this.parent.getName(), this.name);
}

static Variant getVariant(ClassNode classNode) {
return MixinInfo.getVariant(ClassInfo.fromClassNode(classNode));
}

static Variant getVariant(ClassInfo classInfo) {
// if (ProxyInfo.isProxy(classInfo)) {
// return Variant.PROXY;
// }

if (!classInfo.isInterface()) {
return Variant.STANDARD;
}

boolean containsNonAccessorMethod = false;
for (Method method : classInfo.getMethods()) {
containsNonAccessorMethod |= (!method.isAccessor() && !method.isSynthetic());
}

if (containsNonAccessorMethod) {
// If the mixin contains any other methods, treat it as a regular interface mixin
return Variant.INTERFACE;
}

// The mixin contains no non-accessor methods, so we can treat it as an accessor
return Variant.ACCESSOR;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.objectweb.asm.tree.AnnotationNode;
import org.objectweb.asm.tree.ClassNode;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.MixinEnvironment;
import org.spongepowered.asm.mixin.MixinEnvironment.Option;
import org.spongepowered.asm.mixin.MixinEnvironment.Phase;
Expand All @@ -45,17 +47,20 @@
import org.spongepowered.asm.mixin.throwables.MixinException;
import org.spongepowered.asm.mixin.throwables.MixinPrepareError;
import org.spongepowered.asm.mixin.transformer.MixinConfig.IListener;
import org.spongepowered.asm.mixin.transformer.MixinInfo.Variant;
import org.spongepowered.asm.mixin.transformer.ext.Extensions;
import org.spongepowered.asm.mixin.transformer.ext.IHotSwap;
import org.spongepowered.asm.mixin.transformer.ext.extensions.ExtensionCheckClass.ValidationFailedException;
import org.spongepowered.asm.mixin.transformer.ext.extensions.ExtensionClassExporter;
import org.spongepowered.asm.mixin.transformer.meta.MixinMerged;
import org.spongepowered.asm.mixin.transformer.throwables.IllegalClassLoadError;
import org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException;
import org.spongepowered.asm.mixin.transformer.throwables.MixinTransformerError;
import org.spongepowered.asm.mixin.transformer.throwables.ReEntrantTransformerError;
import org.spongepowered.asm.service.IMixinAuditTrail;
import org.spongepowered.asm.service.IMixinService;
import org.spongepowered.asm.service.MixinService;
import org.spongepowered.asm.util.Annotations;
import org.spongepowered.asm.util.PrettyPrinter;
import org.spongepowered.asm.util.ReEntranceLock;
import org.spongepowered.asm.util.perf.Profiler;
Expand Down Expand Up @@ -300,11 +305,14 @@ synchronized boolean applyMixins(MixinEnvironment environment, String name, Clas
}

SortedSet<MixinInfo> mixins = null;
boolean invalidRef = false;
MixinConfig packageOwnedByConfig = null;

for (MixinConfig config : this.configs) {
if (config.packageMatch(name)) {
invalidRef = true;
int packageLen = packageOwnedByConfig != null ? packageOwnedByConfig.getMixinPackage().length() : 0;
if (config.getMixinPackage().length() > packageLen) {
packageOwnedByConfig = config;
}
continue;
}

Expand All @@ -318,12 +326,11 @@ synchronized boolean applyMixins(MixinEnvironment environment, String name, Clas
}
}

if (invalidRef) {
throw new NoClassDefFoundError(String.format("%s is a mixin class and cannot be referenced directly", name));
if (packageOwnedByConfig != null) {
throw new IllegalClassLoadError(this.getInvalidClassError(name, targetClassNode, packageOwnedByConfig));
}

if (mixins != null) {

// Re-entrance is "safe" as long as we don't need to apply any mixins, if there are mixins then we need to panic now
if (locked) {
ReEntrantTransformerError error = new ReEntrantTransformerError("Re-entrance error.");
Expand Down Expand Up @@ -361,6 +368,30 @@ synchronized boolean applyMixins(MixinEnvironment environment, String name, Clas
}
return success;
}

private String getInvalidClassError(String name, ClassNode targetClassNode, MixinConfig ownedByConfig) {
if (ownedByConfig.getClasses().contains(name)) {
return String.format("Illegal classload request for %s. Mixin is defined in %s and cannot be referenced directly", name, ownedByConfig);
}

// AnnotationNode shadow = Annotations.getInvisible(targetClassNode, Shadow.class);
// if (shadow != null) {
// return String.format("Illegal classload request for UNRESOLVED @Shadow %s. "
// + "The proxy was referenced outside a mixin or the mixin processor encountered an internal error.", name);
// }

AnnotationNode mixin = Annotations.getInvisible(targetClassNode, Mixin.class);
if (mixin != null) {
Variant variant = MixinInfo.getVariant(targetClassNode);
if (variant == Variant.ACCESSOR) {
return String.format("Illegal classload request for accessor mixin %s. The mixin is missing from %s which owns "
+ "package %s* and the mixin has not been applied.", name, ownedByConfig, ownedByConfig.getMixinPackage());
}
}

return String.format("%s is in a defined mixin package %s* owned by %s and cannot be referenced directly",
name, ownedByConfig.getMixinPackage(), ownedByConfig);
}

/**
* Update a mixin class with new bytecode.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* This file is part of Mixin, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.asm.mixin.transformer.throwables;

/**
* Error thrown when a class defined in a mixin package is classloaded
*/
public class IllegalClassLoadError extends MixinTransformerError {

private static final long serialVersionUID = 1L;

public IllegalClassLoadError(String message) {
super(message);
}

public IllegalClassLoadError(Throwable cause) {
super(cause);
}

public IllegalClassLoadError(String message, Throwable cause) {
super(message, cause);
}

}

0 comments on commit bd40767

Please sign in to comment.