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 decompilation2 of PowerPC:BE:64:VLEALT-32addr #1127

Closed arkup closed 4 years ago

arkup commented 5 years ago

Describe the bug

I've noticed that in PowerPC VLE function is not decompiled correctly.

ROM:00000000             sub_0:
ROM:00000000 01 30                       se_mr     r0, r3
ROM:00000002 18 03 A8 2B                 e_cmpi    cr0, r3, 0x2B # '+'
ROM:00000006 E4 03                       se_blt    loc_C
ROM:00000008 48 23                       se_li     r3, 2
ROM:0000000A 00 04                       se_blr
ROM:0000000C             # ---------------------------------------------------------------------------
ROM:0000000C
ROM:0000000C             loc_C:                                  # CODE XREF: sub_0+6↑j
ROM:0000000C 18 00 D0 00                 e_nop
ROM:00000010 48 13                       se_li     r3, 1
ROM:00000012 00 04                       se_blr
ROM:00000012             # End of function sub_0
-> Ghidra_9.1-BETA_DEV_20190923 

undefined8 FUN_00000000(int a1)

{
  ulonglong in_crall;

  if ((in_crall & 0x800000000000000) != 0) {
    return 1;
  }
  return 2;
}

To Reproduce

  1. Save as binary file: echo ATAYA6gr5ANIIwAEGADQAEgTAAQ= | base64 -d > decompile_if_else.bin
  2. Open the binary as PowerPC:BE:64:VLEALT-32addr (1.5)

Expected behavior

int __fastcall sub_0(int a1)
{
  int result; // r3

  if ( a1 < 43 )
    result = 1;
  else
    result = 2;
  return result;
}

Environment :

GhidorahRex commented 5 years ago

This looks like an issue with the e_cmpi instruction. Using the CRD32 field to offset the results of the compare into the CRALL register doesn't allow the compiler to figure out how to manage the compare.

If you update line 1072 of ppc_common.sinc to attach variables [ BI_CR_VLE BF_VLE ] and replace lines 558-559 of ppc_vle.sinc with:

tmpC:1 = (((tmpA s< tmpB) << 3) | ((tmpA s> tmpB) << 2) | ((tmpA == tmpB) << 1) | (xer_so & 1));
BF_VLE = tmpC;

the correct comparison comes out for the sample binary. Does that fix the issue that you're seeing elsewhere as well?

arkup commented 5 years ago

@GhidorahRex thanks it's good. I'll test more and let you know but seems you can close the issue.