wargio / r2dec-js

radare2 plugin - converts asm to pseudo-C code.
511 stars 48 forks source link

Wrong order of arguments of C function #271

Closed hehaoqian closed 2 years ago

hehaoqian commented 2 years ago

Version: radare2 5.6.6 and r2dec-js 5.6.6 OS: Ubuntu 20.04.3 LTS gcc: 9.3.0

The original function

int sub(int a, int b){
   return a - b;
}

Compiled with gcc test.c -c -o test.o

The disassembly:

endbr64                         
push rbp          
mov rbp, rsp          
mov dword [rbp - 4], edi       
mov dword [rbp - 8], esi       
mov eax, dword [rbp - 4]         
sub eax, dword [rbp - 8]         
pop rbp                          
ret

The disassembled code generated by pdd

/* r2dec pseudo code output */
/* ./test.o @ 0x8000040 */
#include <stdint.h>

int32_t sub (int64_t arg2, int64_t arg1) {
    int64_t var_8h;
    int64_t var_4h;
    rsi = arg2;
    rdi = arg1;
    /* [01] -r-x section size 22 named .text */
    *((rbp - 4)) = edi;
    *((rbp - 8)) = esi;
    eax = *((rbp - 4));
    eax -= *((rbp - 8));
    return eax;
}

I think the function prototype should be

int32_t sub (int64_t arg1, int64_t arg2) 

Component

Reproduce via JSON (pddi)

{"name":"issue_1648271483599","arch":"x86","archbits":64,"agj":[{"name":"sym.sub","offset":134217792,"ninstr":9,"nargs":2,"nlocals":2,"size":22,"stack":8,"type":"sym","blocks":[{"offset":134217792,"size":22,"ops":[{"offset":134217792,"esil":"","refptr":false,"fcn_addr":134217792,"fcn_last":134217810,"size":4,"opcode":"endbr64","disasm":"endbr64","bytes":"f30f1efa","family":"cpu","type":"null","reloc":false,"type_num":0,"type2_num":0,"flags":["section..text","sym..text","sym.sub","rip"],"comment":"WzAxXSAtci14IHNlY3Rpb24gc2l6ZSAyMiBuYW1lZCAudGV4dA=="},{"offset":134217796,"esil":"rbp,8,rsp,-,=[8],8,rsp,-=","refptr":false,"fcn_addr":134217792,"fcn_last":134217813,"size":1,"opcode":"push rbp","disasm":"push rbp","bytes":"55","family":"cpu","type":"rpush","reloc":false,"type_num":268435468,"type2_num":0},{"offset":134217797,"esil":"rsp,rbp,=","refptr":false,"fcn_addr":134217792,"fcn_last":134217811,"size":3,"opcode":"mov rbp, rsp","disasm":"mov rbp, rsp","bytes":"4889e5","family":"cpu","type":"mov","reloc":false,"type_num":9,"type2_num":0},{"offset":134217800,"esil":"edi,0x4,rbp,-,=[4]","refptr":true,"fcn_addr":134217792,"fcn_last":134217811,"size":3,"opcode":"mov dword [rbp - 4], edi","disasm":"mov dword [rbp - 4], edi","bytes":"897dfc","family":"cpu","type":"mov","reloc":false,"type_num":268435465,"type2_num":0},{"offset":134217803,"esil":"esi,0x8,rbp,-,=[4]","refptr":true,"fcn_addr":134217792,"fcn_last":134217811,"size":3,"opcode":"mov dword [rbp - 8], esi","disasm":"mov dword [rbp - 8], esi","bytes":"8975f8","family":"cpu","type":"mov","reloc":false,"type_num":268435465,"type2_num":0},{"offset":134217806,"esil":"0x4,rbp,-,[4],rax,=","refptr":true,"fcn_addr":134217792,"fcn_last":134217811,"size":3,"opcode":"mov eax, dword [rbp - 4]","disasm":"mov eax, dword [rbp - 4]","bytes":"8b45fc","family":"cpu","type":"mov","reloc":false,"type_num":9,"type2_num":0},{"offset":134217809,"esil":"0x8,rbp,-,[4],eax,-=,0x8,rbp,-,[4],0x80000000,-,!,31,$o,^,of,:=,31,$s,sf,:=,$z,zf,:=,$p,pf,:=,32,$b,cf,:=,3,$b,af,:=","refptr":true,"fcn_addr":134217792,"fcn_last":134217811,"size":3,"opcode":"sub eax, dword [rbp - 8]","disasm":"sub eax, dword [rbp - 8]","bytes":"2b45f8","family":"cpu","type":"sub","reloc":false,"type_num":18,"type2_num":0},{"offset":134217812,"esil":"rsp,[8],rbp,=,8,rsp,+=","refptr":false,"fcn_addr":134217792,"fcn_last":134217813,"size":1,"opcode":"pop rbp","disasm":"pop rbp","bytes":"5d","family":"cpu","type":"pop","reloc":false,"type_num":14,"type2_num":0},{"offset":134217813,"esil":"rsp,[8],rip,=,8,rsp,+=","refptr":false,"fcn_addr":134217792,"fcn_last":134217813,"size":1,"opcode":"ret","disasm":"ret","bytes":"c3","family":"cpu","type":"ret","reloc":false,"type_num":5,"type2_num":0}]}]}],"isj":[{"name":"test.c","flagname":"sym.test.c","realname":"test.c","ordinal":1,"bind":"LOCAL","size":0,"type":"FILE","vaddr":134217728,"paddr":0,"is_imported":false},{"name":".text","flagname":"sym..text","realname":".text","ordinal":2,"bind":"LOCAL","size":0,"type":"SECT","vaddr":134217792,"paddr":64,"is_imported":false},{"name":".data","flagname":"sym..data","realname":".data","ordinal":3,"bind":"LOCAL","size":0,"type":"SECT","vaddr":134217814,"paddr":86,"is_imported":false},{"name":".bss","flagname":"sym..bss","realname":".bss","ordinal":4,"bind":"LOCAL","size":0,"type":"SECT","vaddr":134217814,"paddr":86,"is_imported":false},{"name":".note.GNU-stack","flagname":"sym..note.GNU_stack","realname":".note.GNU-stack","ordinal":5,"bind":"LOCAL","size":0,"type":"SECT","vaddr":134217857,"paddr":129,"is_imported":false},{"name":".note.gnu.property","flagname":"sym..note.gnu.property","realname":".note.gnu.property","ordinal":6,"bind":"LOCAL","size":0,"type":"SECT","vaddr":134217864,"paddr":136,"is_imported":false},{"name":".eh_frame","flagname":"sym..eh_frame","realname":".eh_frame","ordinal":7,"bind":"LOCAL","size":0,"type":"SECT","vaddr":134217896,"paddr":168,"is_imported":false},{"name":".comment","flagname":"sym..comment","realname":".comment","ordinal":8,"bind":"LOCAL","size":0,"type":"SECT","vaddr":134217814,"paddr":86,"is_imported":false},{"name":"sub","flagname":"sym.sub","realname":"sub","ordinal":9,"bind":"GLOBAL","size":22,"type":"FUNC","vaddr":134217792,"paddr":64,"is_imported":false}],"Csj":[],"icj":[],"afvj":{"sp":[],"bp":[{"name":"var_8h","kind":"var","type":"int64_t","ref":{"base":"rbp","offset":-8}},{"name":"var_4h","kind":"var","type":"int64_t","ref":{"base":"rbp","offset":-4}}],"reg":[{"name":"arg2","kind":"reg","type":"int64_t","ref":"rsi"},{"name":"arg1","kind":"reg","type":"int64_t","ref":"rdi"}]},"afcfj":[],"aflj":[{"offset":134217792,"name":"sym.sub","size":22,"is-pure":"true","realsz":22,"noreturn":false,"stackframe":8,"calltype":"amd64","cost":11,"cc":1,"bits":64,"type":"sym","nbbs":1,"is-lineal":true,"ninstrs":9,"edges":0,"ebbs":1,"signature":"sym.sub (int64_t arg1, int64_t arg2);","minbound":134217792,"maxbound":134217814,"indegree":0,"outdegree":0,"nlocals":2,"nargs":2,"bpvars":[{"name":"var_8h","kind":"var","type":"int64_t","ref":{"base":"rbp","offset":-8}},{"name":"var_4h","kind":"var","type":"int64_t","ref":{"base":"rbp","offset":-4}}],"spvars":[],"regvars":[{"name":"arg2","kind":"reg","type":"int64_t","ref":"rsi"},{"name":"arg1","kind":"reg","type":"int64_t","ref":"rdi"}],"difftype":"new"}]}
wargio commented 2 years ago

can you check the output from r2? because those args (arg1, arg2) comes from r2

hehaoqian commented 2 years ago

Output of "afv"

var int64_t var_8h @ rbp-0x8
var int64_t var_4h @ rbp-0x4
arg int64_t arg2 @ rsi
arg int64_t arg1 @ rdi
wargio commented 2 years ago

i can confirm that the problem is that those 2 args are inverted by r2 in the json structure.

wargio commented 2 years ago

fixed in https://github.com/wargio/r2dec-js/commit/d986d4535f69af0f24045adb8d04b6b8f7941af7

trufae commented 2 years ago

I don't think this is a correct fix. the arguments must be sorted depending on the calling convention associated with the function. not sorted by name or by register index. Right now the json output doesn't order them in any specific way because it's assume that's part of the frontend who understand the calling convention. but I can fix that in the r2 side so we can ensure the listing honours the calling convention which is probably the expected output.