SpongePowered / Mixin

Mixin is a trait/mixin and bytecode weaving framework for Java using ASM
MIT License
1.37k stars 185 forks source link

Constructor redirect incorrectly handles nested constructor invocations #659

Open Barteks2x opened 5 months ago

Barteks2x commented 5 months ago

This to the best of my knowledge doesn't occur anywhere in Minecraft.

Consider the following target:

public class Test {
    public Test(String v, Test t) {
    }

    public Test(Test test) {
    }

    public static void test() {
        Test t = new Test(new Test(null, null));
    }
}

Then, the bytecode of the test() method looks like this (package names removed in all of the following examples)

   L0
    LINENUMBER 11 L0
    NEW Test
    DUP
    NEW Test
    DUP
    ACONST_NULL
    ACONST_NULL
    INVOKESPECIAL Test.<init> (Ljava/lang/String;LTest;)V
    INVOKESPECIAL Test.<init> (LTest;)V
    ASTORE 0
   L1
    LINENUMBER 12 L1
    RETURN

In this case, Mixin code that attempts to match NEW with a corresponding INVOKESPECIAL fails, and mixin provides seemingly "impossible" error messages.

This redirect:

    @Redirect(method = "test", at = @At(value = "NEW", target = "(Ljava/lang/String;LTest;)LTest;"))
    private static Test modifiedTest(String s, Test test) {
        System.out.println("Constructing " + test);
        return null;
    }

Produces the following error:

Caused by: org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException: @Redirect factory method Test::modifiedTest from mixinconfig.mixins.json:common.TestMixin2 has an invalid signature. Found unexpected argument type java.lang.String at index 0, expected Test. Handler signature: (Ljava/lang/String;LTest;)LTest; Expected signature: (LTest;)LTest; [INJECT Applicator Phase -> mixinconfig.mixins.json:common.TestMixin2 -> Apply Injections ->  -> Inject -> cubicchunks.mixins.core.json:common.TestMixin2->@FactoryRedirectWrapper::modifiedTest(Ljava/lang/String;LTest;)LTest;]

So it seems like it's trying to use the one-argument constructor. But when I target the one argument constructor like this:

    @Redirect(method = "test", at = @At(value = "NEW", target = "(LTest;)LTest;"))
    private static Test modifiedTest(Test test) {
        System.out.println("Constructing " + test);
        return null;
    }

Results in this error:

Caused by: org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException: @Redirect factory method Test::modifiedTest from mixinconfig.mixins.json:common.TestMixin2 has an invalid signature. Found unexpected argument type Test at index 0, expected java.lang.String. Handler signature: (LTest;)LTest; Expected signature: (Ljava/lang/String;LTest;)LTest; [INJECT Applicator Phase -> mixinconfig.mixins.json:common.TestMixin2 -> Apply Injections ->  -> Inject -> mixinconfig.mixins.json:common.TestMixin2->@FactoryRedirectWrapper::modifiedTest(LTest;)LTest;]

So in this case, it's expecting the 2-argument constructor.

This is caused by the use of this method: https://github.com/SpongePowered/Mixin/blob/41a68854f6e63e8ec6d38e7d7612230d7f73a9bc/src/main/java/org/spongepowered/asm/mixin/injection/struct/Target.java#L641-L653 Which assumes that between a NEW and its corresponding INVOKESPECIAL, there cannot be another NEW and INVOKESPECIAL for the same class.