JonathanSalwan / Triton

Triton is a dynamic binary analysis library. Build your own program analysis tools, automate your reverse engineering, perform software verification or just emulate code.
https://triton-library.github.io
Apache License 2.0
3.5k stars 533 forks source link

ADC/SBB semantics for flags #1124

Closed SweetVishnya closed 2 years ago

SweetVishnya commented 2 years ago

Flags semantics for adc/sbb are the same as for add/sub. Does this magic work? Do we consider a case when overflow occurs due to carry flag cf=1 and simple add does not cause an overflow? (a + b does not overflow when a + b + 1 overflows)

https://github.com/JonathanSalwan/Triton/blob/master/src/libtriton/arch/x86/x86Semantics.cpp#L3028

JonathanSalwan commented 2 years ago

I remember running the qemu-ir test suits via the Pin tracer with success on this but maybe we could double check and add unit tests with unicorn.

JonathanSalwan commented 2 years ago

I've did some tests and can not managed to reproduce a result that set the overflow flag... I've also tried directly with my CPU, for example compiling this following sample and then using GDB to check eflags.

// gcc -masm=intel test.c
int main() {
  asm ("stc");
  asm ("mov ebx, 0xfffffffe");
  asm ("mov eax, 0x00000001");
  asm ("adc eax, ebx");

  asm ("stc");
  asm ("mov ebx, 0xfffffffe");
  asm ("mov eax, 0x00000002");
  asm ("adc eax, ebx");

  asm ("clc");
  asm ("mov rbx, -1");
  asm ("mov rax, 2");

  asm ("clc");
  asm ("mov rbx, -1");
  asm ("mov rax, 1");

  asm ("stc");
  asm ("mov rbx, -1");
  asm ("mov rax, 1");
}
JonathanSalwan commented 2 years ago

Mmmh, according to the intel manual:

The ADC instruction does not distinguish between signed or unsigned operands. Instead, the processor evaluates the result for both data types and sets the OF and CF flags to indicate a carry in the signed or unsigned result, respectively.

Still according to the manual, you pointed something that may be wrong in our semantics. The behavior of this->cfAdd_s maybe wrong in this case and this->ofAdd_s is clearly not what are expected.

JonathanSalwan commented 2 years ago

Mmmmh, everything looks good in fact. OF and CF are correctly defined. Can you confirm or give me a counter example?

SweetVishnya commented 2 years ago

I still haven't found time to check it. But let's believe that everything is ok.