NationalSecurityAgency / ghidra

Ghidra is a software reverse engineering (SRE) framework
https://www.nsa.gov/ghidra
Apache License 2.0
51.6k stars 5.87k forks source link

Recognize optimized bitwise testing patterns on ARMv7 #4150

Open CreepNT opened 2 years ago

CreepNT commented 2 years ago

Is your feature request related to a problem? Please describe. In optimized code for ARMv7 in Thumb mode, bitwise test conditionals (e.g. if (a & 0x10)) can be turned into a sequence of instructions Ghidra fails to interpret properly, generating pseudo-code like if ((int)(a << 0x1B) < 0). This "abuses" the fact that most significant bit being set results in the N bit being set in the APSR.

For example, the following C snippet

struct InStruct {
    uint32_t attr; //bitfield - we'll assume 0x10 is tested here
    uint32_t field4;
};

void func(struct InStruct* pInStruct, uint32_t* pOut) {
    if (pInStruct->attr & 0x10) {
        *pOut = pInStruct->field4;
    }
}

would be compiled "naively" to something like:

; AAPCS -> r0 holds pInStruct, r1 holds pOut
; Machine code in comment is little endian
ldr r2, [r0]    ; 02 68
tst r2, #0x10   ; 12 F0 10 0F
itt ne          ; 1C BF
ldr.ne r2, [r0, #4] ; 42 68
str.ne r2, [r1] ; 0A 60

which Ghidra decompiles to a readable snippet:

if ((pInStruct->attr & 0x10) != 0) {
    *pOut = pInStruct->field4;
}

However, this sequence can be optimized to save 2 bytes by doing the following instead:

ldr r2, [r0] ; 02 68
lsls r3, r2, #0x1B ; D3 06 - 2 bytes saved
itt mi ; 44 BF
ldrmi r2, [r0, #4] ; 42 68
strmi r2, [r1] ; 0A 60

My understanding of how it works is because lsls sets the flags in APSR, and Negative flag is set whenever MSB is set (thanks to 2's complement). Ghidra decompiles this assembly to the following much less readable pseudo-C code:

if ((int)(pInStruct->attr << 0x1B) < 0) {
    *pOut = pInStruct->field4;
}

Describe the solution you'd like Decompiler should be able to recognize such patterns and emit the proper bitwise AND check instead of convoluted shift + <0 check.

Describe alternatives you've considered At the moment, I use a lookup table to match the shift level to the proper bit (e.g. << 0x1B is & 0x10), but this is inconvenient and can be error-prone (reading & 0x20 instead of & 0x10 can lead to confusion later on if & 0x20 is used for completely different purposes).

xerpi commented 10 months ago

Another example:

foo:
    ldr      rX, [addr]
    lsls     rY, rX, #0x1f
    bmi      foo

Gets decompiled into (Ghidra 11.0):

    do {
    } while (*addr << 0x1f < 0);

Where it could be simplified to:

    do {
    } while (*addr & 1);