Mixin icon indicating copy to clipboard operation
Mixin copied to clipboard

BeforeNew descriptor filtering doesn't work

Open Earthcomputer opened this issue 4 years ago • 1 comments

Example:

public class MultipleConstructors {
  public MultipleConstructors() {}
  public MultipleConstructors(int i) {}
}

public class TargetClass {
  public void foo() {
    new MultipleConstructors(); // new MultipleConstructors; invokespecial MultipleConstructors.<init>()V
    new MultipleConstructors(0); // new MultipleConstructors; iconst_0; invokespecial MultipleConstructors,<init>(I)V
    // return
  }
}

@Mixin(TargetClass.class)
public class MyMixin {
  // redirect no-arg version
  @Redirect(method = "foo", at = @At(value = "NEW", target = "LMultipleConstructors;<init>(I)V"))
  public MultipleConstructors myCtorRedirect(int value) {
    System.out.println("Constructing with " + value);
    return new MultipleConstructors();
  }
}

Expected bytecode:

new MultipleConstructors
invokespecial MultipleConstructors.<init>()V
aload 0
iconst_0
invokevirtual TargetClass.myCtorRedirect(I)LMultipleConstructors;
pop
return

Actual bytecode:

aload 0
invokevirtual TargetClass.myCtorRedirect(I)LMultipleConstructors;
pop
aload 0
iconst_0
invokevirtual TargetClass.myCtorRedirect(I)LMultipleConstructors;
pop
return

This then crashes with a VerifyError.

This is caused by Target.findInitNodeFor not checking the descriptor of the target, while BeforeNew.findCtor does.

There is also another issue if constructor calls happen to be nested (e.g. new String(new String(""))). A fix for this could be to keep a counter for the number of new instructions found, and decrement it when an <init> call is found, similar to how you would check balancing brackets.

Earthcomputer avatar Aug 18 '21 17:08 Earthcomputer

Good find, will fix.

Mumfrey avatar Aug 19 '21 11:08 Mumfrey