Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BeforeNew descriptor filtering doesn't work #515

Closed
Earthcomputer opened this issue Aug 18, 2021 · 1 comment
Closed

BeforeNew descriptor filtering doesn't work #515

Earthcomputer opened this issue Aug 18, 2021 · 1 comment
Labels

Comments

@Earthcomputer
Copy link

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.

@Mumfrey
Copy link
Member

Mumfrey commented Aug 19, 2021

Good find, will fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants