NationalSecurityAgency / ghidra

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

8051: Wrong decompilation #1276

Open beketata opened 4 years ago

beketata commented 4 years ago

Describe the bug Decompilation result for a simple 8051 code is wrong.

To Reproduce C source code sample compiled in Keil:

#include <REG51.H>

typedef unsigned char byte;
typedef unsigned short word;

byte Send_SBUF( byte bt )
{
  word wr = 0;

  TI = 0;
  SBUF = bt;

  do
  {
    if( TI )
      break;
  }
  while( wr++ < 0x37CD );

  if( TI )
    return 1;

  return 0;
}

Decompilation result is:

/* WARNING: Removing unreachable block (CODE,0x001d) */

bool Send_SBUF(byte bt)
{
  bool bVar1;
  byte bVar2;

  write_volatile_1(SBUF,bt);
  bVar2 = 0;
  do {
    bVar1 = bVar2 < 0xcd;
    bVar2 = bVar2 + 1;
  } while ((BANK0_R4 < 0x37U - ((bVar1 << 7) >> 7)) << 7 < '\0');

  TI = 0;
  return false;
}
                             *************************************************************
                             *                           FUNCTION                         
                             *************************************************************
                             bool  __stdcall  Send_SBUF (byte  bt )
             bool              R7:1           <RETURN>
             byte              R7:1           bt
                             Send_SBUF
       CODE:0000 e4              CLR        A
       CODE:0001 fd              MOV        R5 ,A
       CODE:0002 fc              MOV        R4 ,A
       CODE:0003 c2  99           CLR        TI                                               = ??
       CODE:0005 8f  99           MOV        SBUF ,bt                                         = ??
                             LAB_CODE_0007                                   XREF[1]:     CODE:0018 (j)   
       CODE:0007 20  99  10       JB         TI ,LAB_CODE_001a                                = ??
       CODE:000a 0d              INC        R5
       CODE:000b ed              MOV        A,R5
       CODE:000c ae  04           MOV        R6 ,BANK0_R4                                     = ??
       CODE:000e 70  01           JNZ        LAB_CODE_0011
       CODE:0010 0c              INC        R4
                             LAB_CODE_0011                                   XREF[1]:     CODE:000e (j)   
       CODE:0011 14              DEC        A
       CODE:0012 c3              CLR        CY
       CODE:0013 94  cd           SUBB       A,#0xcd
       CODE:0015 ee              MOV        A,R6
       CODE:0016 94  37           SUBB       A,#0x37
       CODE:0018 40  ed           JC         LAB_CODE_0007
                             LAB_CODE_001a                                   XREF[1]:     CODE:0007 (j)   
       CODE:001a 30  99  03       JNB        TI ,LAB_CODE_0020                                = ??
       CODE:001d 7f  01           MOV        bt ,#0x1
       CODE:001f 22              RET
                             LAB_CODE_0020                                   XREF[1]:     CODE:001a (j)   
       CODE:0020 7f  00           MOV        bt ,#0x0
       CODE:0022 22              RET

Actually there are two issues in the decompiled code:

  1. SCON.TI bit is not checked in the timeout cycle. Not only SBUF is volatile, but TI also is. So, there is no unreachable block in code.

  2. while ((BANK0_R4 < 0x37U - ((bVar1 << 7) >> 7)) << 7 < '\0'); instead of while( wr++ < 0x37CD ); is awful.

Expected behavior As original C source code.

Attachments Raw binary zipped file

Environment:

fenugrec commented 2 years ago

I was having a vaguely similar issue (many unreachable code blocks). Workaround was to edit the memory areas to set "SFR-BITS" to volatile . This should be fixed at the .pspec level if possible ?

This probably applies to #3061 as well.