x86 and PUSH ESP

[MoebiuZ]

Preface

The correct behaviour for PUSH reg for all x86 processors is to decrement the SP/ESP/RSP value, then store the reg value in the address pointed by ESP.

If reg is ESP, then the value stored is the old ESP value (the value it had before decrementing it)

Recently we fixed a bug (https://github.com/radare/radare2/pull/11408) because the value of ESP being pushed was the new value, not the old.

Issue

After digging on intel reference manuals, I found an exception, and that's why I'm opening this issue:

• 8086 cpus (aka iAPX 86) should push the new sp value
• i286 cpus (aka iAPX 286) should push the old sp value (just like x86_32 and x86_64)

Reference: http://www.bitsavers.org/components/intel/80286/210498-001_iAPX_286_Programmers_Reference_1983.pdf

If I understand correctly, if someone wants to emulate 8086 or i286, they should e asm.arch = x86 and e asm.bits = 16, but with the current implementation, 8086 will not emulate correctly.

Both cpus are 16 bit, so we cannot just use if (rs == 4) { push new esp } else { push old esp } on anal_x86_cs.c to filter which behaviour push esp should take so, should we add/is worth adding an asm.cpu = 8086 for asm.arch = x86?

Bonus

Also I have a question. The current patch on PUSH works ok and uses a single esil expresion for all cases (ESP included). While it works, is not representing the real hardware operation, because is decrementing esp after storing the reg value, and not before (to do it close as real cpu, esil should be 0,%s,+,%d,%s,-=,%s,=[%d]).

As a rule of thumb, what should we do when implementing esil for emulation, simplify and make it just work or instead honor the real hardware behaviour as close as possible?

cc: @radare @ret2libc

[ret2libc]

Hi, I recently changed your previous fix because it was causing problems in analysis for some weird reasons (your changes seemed good indeed).

I think esil should just emulate the behavior at the instruction granularity and not beyond that. Meaning that the important thing is that given the same input state, once you execute the emulated instruction or the real one, th output states are the same. Those are my 2c at least. Cc @condret

[radare]

I think we can close this issue

[radare]

ping?

[radare]

please confirm behaviour and close. this issue has been fixed already. unless there's some missbehaviour or missing tests