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

CtMethod.insertBefore messes up stack map when inserting unconditional "return null;" #352

Open
kriegaex opened this issue Dec 16, 2020 · 0 comments

Comments

@kriegaex
Copy link

kriegaex commented Dec 16, 2020

I think this issue is similar to fixed issue #328. I originally found the problem description on StackOverflow, please also read the related discussion.

Here is an MCVE reproducing the problem with both 3.27.0-GA and latest master 1c4e31b:
https://github.com/kriegaex/SO_Javassist_VerifyErrorReplacingMethodReturnValue_64561436

The problem is inserting return null; into a method by insertBefore(). This fails in some cases, causing a VerifyError. A workaround is to insert if (true) return null; instead, then the error goes away. See here:

  private void replaceReturnValue(CtClass targetClass) throws CannotCompileException {
    // Fails in Javassist without ASM stack map repair
    String injectedCode = "return null;";
    // Works in Javassist without ASM stack map repair
    // String injectedCode = "if (true) return null;";

    for (CtMethod ctMethod : targetClass.getDeclaredMethods()) {
      if (LOG_RETURN_VALUE_REPLACER) {
        log("Replacing return value for method " + ctMethod.getLongName());
        log("Injected code: " + injectedCode);
      }
      ctMethod.insertBefore(injectedCode);
    }
  }

In some method bodies it works, e.g.

  public String greet_JavassistWorks1(String recipient) {
    return "Hello " + recipient + "!";
  }

  public String greet_JavassistWorks2(String recipient) {
    String prefix = "Hello ";
    String postfix = "!";
    return prefix + recipient + postfix;
  }

  public String greet_JavassistWorks3(String recipient) {
    if (new Random().nextBoolean()) {
      String prefix = "Hello ";
      String postfix = "!";
      return prefix + recipient + postfix;
    }
    else {
      return "Lazy to greet today...";
    }
  }

This one fails, though:

  public String greet_JavassistFails(String recipient) {
    if (new Random().nextBoolean()) {
      String prefix = "Hello ";
      String postfix = "!";
      return prefix + recipient + postfix;
    }
    else {
      String greeting = "Lazy to greet today...";
      return greeting;
    }
  }

See class TargetClass.

If I repair the broken class Javassist produces via ASM, setting REPAIR_STACK_MAP_USING_ASM = true; in class JavassistMethodReturnValueReplacer, it works.

If you also set DUMP_CLASS_FILES = true, the sample program will output both the Javassist-generated, unrepaired class file and the ASM-repaired version. You can compare them using a byte code viewer and will notice differences in stack map frame generation, similar to #328.

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

No branches or pull requests

1 participant