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

NewExpr.replace Can Corrupt Stack #322

Open
joakley-msft opened this issue Jun 8, 2020 · 0 comments
Open

NewExpr.replace Can Corrupt Stack #322

joakley-msft opened this issue Jun 8, 2020 · 0 comments

Comments

@joakley-msft
Copy link

We previously used NewExpr.replace to change the construction of class A into construction of class B (with the same constructor parameters). If the bytecode sequence between the new opcode and invokespecial opcode is not one that NewExpr recognizes, however, the call to replace will still succeed but the stack may be corrupted, causing iteration of an ExprEditor to fail later in the method.

replace functions as expected on the following bytecode

         0: new           #9                  // class android/app/AlertDialog$Builder
         3: dup
         4: aload_1
         5: invokespecial #10                 // Method android/app/AlertDialog$Builder."<init>":(Landroid/content/Context;)V

but we have also encountered this bytecode which can eventually lead to "javassist.bytecode.BadBytecode: inconsistent stack height null" after replacement.

       5: new           #17                 // class android/app/AlertDialog$Builder
       8: astore_1
       9: aload_1
      10: aload_0
      11: invokespecial #20                 // Method android/app/AlertDialog$Builder."<init>":(Landroid/content/Context;)V

Examining the source for NewExpr, I see that there is a private canReplace method which recognizes a couple specific constructions. For any other constructions, it makes a blind assumption about what to do. Ideally, replace would support a broader range of constructions, but it would at least be nice for replace to throw a CannotCompileException immediately on an unsupported construction and for the Javadocs to indicate that such a failure could occur.

Due to this issue, we no longer use NewExpr.replace but instead use a CodeIterator to perform manipulations on the bytecode directly.

Thank you for making Javassist.

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