Mixin icon indicating copy to clipboard operation
Mixin copied to clipboard

References to @Shadow fields declared in super-mixins do not get remapped in dev environment

Open PhiProven opened this issue 4 years ago • 1 comments

Related to #431; see that issue for a more extensive discussion of the relevant code paths.

If an obfuscated mod dependency defines a @Shadow field in a super-mixin that gets unmapped in dev environment, then accesses to it from a child-mixin won't get remapped and will still reference the obfuscated name. The shadow itself will however apply correctly in the deobfuscated environment, causing NoSuchFieldErrors once some method tries to access it with the original obfuscated name.

A similar issue occurs if the field declared in the super-mixin gets renamed because of a conflicting @Unique. In the following I will describe this issue instead, as it is easier to set up since it does not require an obfuscated dependency. The relevant code paths are however the same for both issues.

Consider the following two mixins:

@Mixin(Screen.class)
public class SuperMixin
{
    @Unique
    protected Text title = new LiteralText("Reference got remapped correctly");
}

@Mixin(TitleScreen.class)
public class ChildMixin extends SuperMixin
{
	@Inject(at = @At("HEAD"), method = "init()V")
	private void init(CallbackInfo info) {
		System.out.println(this.title.getString());
	}
}

The title field is already present in Screenand will hence get conformed to a unique name due to the @Unique annotation. However, running these mixins produces the output [STDOUT]: Title Screen, showing that the child-mixin still references the original title field and does not get remapped to the conformed field.

The issue lies within MixinPreProcessorStandard.transformField(...)

protected void transformField(FieldInsnNode fieldNode) {
        Activity activity = this.activities.begin("%s::%s:%s", fieldNode.owner, fieldNode.name, fieldNode.desc);
        Section metaTimer = this.profiler.begin("meta");
        ClassInfo owner = ClassInfo.forDescriptor(fieldNode.owner, TypeLookup.DECLARED_TYPE);
        if (owner == null) {
            throw new ClassMetadataNotFoundException(fieldNode.owner.replace('/', '.'));
        }
        
        Field field = owner.findField(fieldNode, ClassInfo.INCLUDE_PRIVATE);
        metaTimer.end();
        
        if (field != null && field.isRenamed()) {
            fieldNode.name = field.getName();
        }
        activity.end();
    }

Note that owner.findField(fieldNode, ClassInfo.INCLUDE_PRIVATE); will only try to lookup the field in the class itself, i.e., ChildMixin, instead of traversing the class hierarchy, in contrast to MixinPreProcessorStandard.transformMethod(...) which uses owner.findMethodInHierarchy(methodNode, SearchType.ALL_CLASSES, ClassInfo.INCLUDE_PRIVATE);. Hence this will not find the metadata for the title field declared in the super-mixin and thus not get remapped.

Using findFieldInHierarchy(...) instead of findField(...) should fix the issue.

Best, PhiPro

PhiProven avatar Aug 19 '20 15:08 PhiProven

Thanks for the detailed writeup again, will look into this.

Mumfrey avatar Aug 20 '20 10:08 Mumfrey