avast / avast/retdec

BUG: error in translating the "sbb" instruction

Open
#1,196 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
8.6k
Forks
1k
PR merge metrics
No merged PRs in 30d

Description

There is an error in lifting the x86 instruction "sbb" to IRs while calculating the AF flag.

Since %32 and 15 perform unsigned greater than calculations, %33 will only become true when %32 is greater than 15 or less than 0. Therefore, Retdec will only set AF to 1 when the least significant 4 bits of eax is 0xf. However, in a real CPU, if CF is 1 after this instruction is executed, AF will be set to 1. Otherwise, AF will be 0.

The original instruction is `sbb %eax, %eax`
The following is how IRs of "sbb" calculate the AF flag, where an error exists.
```
store volatile i64 4195574, i64* @_asm_program_counter
%19 = load i64, i64* @rax
%20 = trunc i64 %19 to i32 ; %eax
%21 = load i64, i64* @rax
%22 = trunc i64 %21 to i32 ; %eax
%23 = load i1, i1* @cf
%24 = zext i1 %23 to i32 ; @cf 32bits
%25 = add i32 %22, %24 ; %eax + @cf
%27 = and i32 %20, 15 ; (%eax)_low 4bits
%28 = and i32 %25, 15 ; (%eax + @cf)_low 4bits
%29 = sub i32 %27, %28 ; (%eax)_low4bits - (%eax + @cf)_low4bits
%30 = load i1, i1* @cf
%31 = zext i1 %30 to i32 ; @cf 32bits
%32 = add i32 %29, %31
%33 = icmp ugt i32 %32, 15
store i1 %33, i1* @az
```

The following are the completed IRs of "sbb".
```
store volatile i64 4195574, i64* @_asm_program_counter
%19 = load i64, i64* @rax
%20 = trunc i64 %19 to i32
%21 = load i64, i64* @rax
%22 = trunc i64 %21 to i32
%23 = load i1, i1* @cf
%24 = zext i1 %23 to i32
%25 = add i32 %22, %24
%26 = sub i32 %20, %25
%27 = and i32 %20, 15
%28 = and i32 %25, 15
%29 = sub i32 %27, %28
%30 = load i1, i1* @cf
%31 = zext i1 %30 to i32
%32 = add i32 %29, %31
%33 = icmp ugt i32 %32, 15
%34 = load i1, i1* @cf
%35 = zext i1 %34 to i32
%36 = sub i32 %26, %35
%37 = icmp ult i32 %20, %36
%38 = icmp ult i32 %25, -1
%39 = or i1 %37, %38
%40 = icmp ult i32 %20, %25
%41 = select i1 %34, i1 %39, i1 %40
%42 = load i1, i1* @cf
%43 = zext i1 %42 to i32
%44 = sub i32 %26, %43
%45 = xor i32 %20, %25
%46 = xor i32 %20, %44
%47 = and i32 %45, %46
%48 = icmp slt i32 %47, 0
store i1 %33, i1* @az
store i1 %41, i1* @cf
store i1 %48, i1* @of
%49 = icmp eq i32 %26, 0
store i1 %49, i1* @zf
%50 = icmp slt i32 %26, 0
store i1 %50, i1* @sf
%51 = trunc i32 %26 to i8
%52 = call i8 @llvm.ctpop.i8(i8 %51)
%53 = and i8 %52, 1
%54 = icmp eq i8 %53, 0
store i1 %54, i1* @pf
%55 = zext i32 %26 to i64
store i64 %55, i64* @rax
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by tracing RetDec's x86 lifting entry point for the `sbb` instruction and compare its generated IR with the examples in the issue. Verify that AF follows the CPU behavior when CF is set or clear, and confirm the completed IR still computes the other flags and result correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, reverse-engineering
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.