javassist icon indicating copy to clipboard operation
javassist copied to clipboard

NewExpr.replace Can Corrupt Stack

Open joakley-msft opened this issue 4 years ago • 0 comments

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.

joakley-msft avatar Jun 08 '20 14:06 joakley-msft