NationalSecurityAgency / ghidra

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

Incorrect decompilation of PowerPC:BE:64:VLEALT-32addr #1123

Closed arkup closed 5 years ago

arkup commented 5 years ago

Describe the bug

I've noticed that in PowerPC VLE mem copy function is not decompiled correctly showing infinity loop

-> Ghidra_9.1-BETA_DEV_20190923 

void FUN_00000004(uchar *dest,uchar *src,ulonglong counter)
{
  int i;

  if (0 < (longlong)counter) {
    do {
      counter = counter & 0xffff; // <-- counter is not decreased
      i = (int)counter;
      dest[i] = src[i];
    } while (i != 0);  //<--infinity loop 
  }
  return;
}

To Reproduce

  1. Save as binary file
    echo GADQABgA0AB8oCt54QotBgRgAOB8pACufKMBrioA5fkABBgA0AAYANAA | base64 -d > decompile_ppc_vle.bin
  2. Open the binary as PowerPC:BE:64:VLEALT-32addr (1.5)
  3. Decompile
    • On a side note when creating a function at offset 0x0 there seems to be an issue with renaming variables in this decompiled code

Expected behavior

void copy(uint8 *pDest, const uint8 *pSrc, unsigned __int32 Count)
{
  int i; // r0

  LOWORD(i) = Count;
  if ( (int)Count > 0 )
  {
    do
    {
      i = (unsigned __int16)(i - 1);
      pDest[i] = pSrc[i];
    }
    while ( i > 0 );
  }
}

Screenshots N/A

Attachments N/A

Environment

Additional context N/A

mumbel commented 5 years ago

When Eliminate unreachable code is unchecked, it does get counter = counter + lVar1 & 0xffff with lVar1=0xffffffffffffffff

caheckman commented 5 years ago

Looks like a problem with the 'se_bmaski' instruction. You can try the following patch on your Ghidra installation as an immediate fix.

diff --git a/Ghidra/Processors/PowerPC/data/languages/ppc_vle.sinc b/Ghidra/Processors/PowerPC/data/languages/ppc_vle.sinc
index 587a4e4e..f3d4a42f 100644
--- a/Ghidra/Processors/PowerPC/data/languages/ppc_vle.sinc
+++ b/Ghidra/Processors/PowerPC/data/languages/ppc_vle.sinc
@@ -720,9 +720,8 @@ IMM16B: val                                         is IMM_0_10_VLE & IMM_16_20_VLE [ val = (IMM_16_20_VLE << 11) |

 :se_bmaski RX_VLE,OIM5_VLE             is $(ISVLE) & OP6_VLE=11 & BIT9_VLE=0 & RX_VLE & OIM5_VLE {
        RX_VLE = ~0;
-       tmp:1 = OIM5_VLE;
-       if (tmp == 0) goto inst_next;
-       RX_VLE = RX_VLE >> ($(REGISTER_SIZE) - tmp);
+       sa:4 = (8 * $(REGISTER_SIZE) - OIM5_VLE) * zext( OIM5_VLE != 0:1 );
+       RX_VLE = RX_VLE >> sa;
 }

 :se_bseti RX_VLE,OIM5_VLE              is $(ISVLE) & OP6_VLE=25 & BIT9_VLE=0 & RX_VLE & OIM5_VLE {