Modifiying <clinit> with ASM

Question

Hello I have a little problem with ASM. It produces a class with bytecode error:

Exception in thread "main" java.lang.VerifyError: Bad instruction
Exception Details:
  Location:
    me/test/Main.<clinit>()V @0: wide
  Reason:
    Error exists in the bytecode

    at java.lang.Class.getDeclaredMethods0(Native Method)
    at java.lang.Class.privateGetDeclaredMethods(Unknown Source)
    at java.lang.Class.privateGetMethodRecursive(Unknown Source)
    at java.lang.Class.getMethod0(Unknown Source)
    at java.lang.Class.getMethod(Unknown Source)
    at sun.launcher.LauncherHelper.validateMainClass(Unknown Source)
    at sun.launcher.LauncherHelper.checkAndLoadMain(Unknown Source)

When i use the following code to inject code into the <clinit> method, which fills an static byte array with the "key" (byte[]) variable:

   if (key.length > 128)
        this.visitVarInsn(SIPUSH, key.length);
    else
        this.visitVarInsn(BIPUSH, key.length);
    this.visitVarInsn(NEWARRAY, 8);
    for (int i = 0; i < key.length; i++) {
        this.visitInsn(DUP);
        if (i > 127) {
            this.visitVarInsn(SIPUSH, i + 32768);
        } else if (i < 6) {
            switch (i) {
                case 0:
                    this.visitInsn(ICONST_0);
                    break;
                case 1:
                    this.visitInsn(ICONST_1);
                    break;
                case 2:
                    this.visitInsn(ICONST_2);
                    break;
                case 3:
                    this.visitInsn(ICONST_3);
                    break;
                case 4:
                    this.visitInsn(ICONST_4);
                    break;
                case 5:
                    this.visitInsn(ICONST_5);
                    break;
                default:
                    System.out.println("Logic mistake!");
                    break;
            }
        }
        else {
            this.visitVarInsn(BIPUSH, i);
        }
        if (key[i] < 6 && key[i] >= 0) {
            switch (key[i]) {
                case 0:
                    this.visitInsn(ICONST_0);
                    break;
                case 1:
                    this.visitInsn(ICONST_1);
                    break;
                case 2:
                    this.visitInsn(ICONST_2);
                    break;
                case 3:
                    this.visitInsn(ICONST_3);
                    break;
                case 4:
                    this.visitInsn(ICONST_4);
                    break;
                case 5:
                    this.visitInsn(ICONST_5);
                    break;
                default:
                    System.out.println("Logic mistake!");
                    break;
            }
        } else {
            this.visitVarInsn(BIPUSH, key[i]);
        }
        this.visitInsn(BASTORE);

Answer 1

The main cause for your incorrect code is your multiple misuse of visitVarInsn.

visitVarInsn is designed for instructions whose argument is the index of a local variable. Since it depends on the particular instruction, how the argument is encoded into the instruction, this method produces incorrect code when used together with instructions, it is not designed for. So, instead of

this.visitVarInsn(NEWARRAY, 8);

you should use

this.visitIntInsn(NEWARRAY, Opcodes.T_BYTE);

The same applies to the incorrect invocations of this.visitVarInsn(SIPUSH, key.length); and this.visitVarInsn(BIPUSH, key.length);, but in these cases there are additional errors in your duplicated code, e.g. right your first occurrence,

if (key.length > 128)
    this.visitVarInsn(SIPUSH, key.length);
else
    this.visitVarInsn(BIPUSH, key.length);

will break, when the constant value is exactly 128. Further, this.visitVarInsn(SIPUSH, i + 32768); contains a spurious + 32768 additionally to the use of the wrong method.

Generally, you should not repeat this code for generating an optimal instruction. Do either, create a utility method whose correctness you have to prove only once, or use an existing method. E.g., if you inherit from InstructionAdapter rather than directly from MethodVisitor, you can use iconst(int) which will automatically generate ICONST_x, BIPUSH, SIPUSH or LDC. Or, when you inherit from GeneratorAdapter instead, you can use push(int) to achieve the same.

So using GeneratorAdapter, the code would be as simple as

push(key.length);
newArray(Type.BYTE_TYPE);
for(int i = 0; i < key.length; i++) {
    this.visitInsn(Opcodes.DUP);
    push(i);
    push(key[i]);
    visitInsn(Opcodes.BASTORE);
}