Incorrect translation:Assembly to C

[adahsuzixin]

Hi
I tested retdec on the test case shown below:

├── test_driver_binary
│   └── test
├── test_driver_src
│   ├── Makefile
│   ├── test_driver.c
│   ├── tested_api.c
│   └── tested_api.S
└── test.py

source file

• test_driver.c

#include <stdio.h>
extern char getSecondByte(long long a);

int main(void)
{
    long long a;
    scanf("%llx", &a);
	char res = getSecondByte(a);
	printf("getSecondByte( %llx ) = %d\n",a,res);
	return res;
}

• tested_api.c

char getSecondByte(long long a) {
    return (a >> 8) & 0xff;
}

• Assemble tested_api.c into tested_api.S

	.file	"tested_api.c"
	.text
	.globl	getSecondByte
	.type	getSecondByte, @function
getSecondByte:
.LFB0:
	.cfi_startproc
	pushq	%rbp
	.cfi_def_cfa_offset 16
	.cfi_offset 6, -16
	movq	%rsp, %rbp
	.cfi_def_cfa_register 6
	movq	%rdi, -8(%rbp)
	movq	-8(%rbp), %rax
	sarq	$8, %rax
	popq	%rbp
	.cfi_def_cfa 7, 8
	ret
	.cfi_endproc
.LFE0:
	.size	getSecondByte, .-getSecondByte
	.ident	"GCC: (GNU) 8.3.1 20190507 (Red Hat 8.3.1-4)"
	.section	.note.GNU-stack,"",@progbits

• Makefile:Test the function in tested_api.S by test_driver.c

main:
	gcc -Wall -S tested_api.c -o tested_api.S 
	gcc test_driver.c tested_api.S -o test -g && mv test ../test_driver_binary/

• test.py

from regression_tests import *

class TestBase(Test):
    def test_produce_expected_output(self):
        self.assert_c_produces_output_when_run(
            input='0xa5a5a5a5a5a50020',
            expected_output='getSecondByte( a5a5a5a5a5a50020 ) = 0\n',
            expected_return_code=0
        )

class Test_shiftOrDivide(TestBase):
    shiftOrDivide = TestSettings(
        input=files_in_dir('test_driver_binary'),
        arch='x86-64'
    )

run the test

$ cd dir_of_retdec-regression-tests-framework
$ python3 runner.py dir_of_testcase_aboce

The output:

FAIL: test_produce_expected_output (......retdec_root.retdec-regression-tests.integration.mytest.shiftOrDivide.Test_shiftOrDivide)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/suzixin/retdec-regression-tests/integration/mytest/shiftOrDivide/test.py", line 8, in test_produce_expected_output
    expected_return_code=0
  File "/home/suzixin/retdec-regression-tests-framework/regression_tests/tools/decompiler_test.py", line 96, in assert_c_produces_output_when_run
    self.assertEqual(actual_return_code, expected_return_code, msg)
AssertionError: 1 != 0 : where the input was: 0xa5a5a5a5a5a50020

----------------------------------------------------------------------
Ran 1 test in 0.054s

FAILED (failures=1)


FAIL (failed 1 out of 1 test)

[adahsuzixin]

hello, I found the root cause of this issue:
In src/bin2llvmir/optimizations/idioms/idioms_analysis.cpp, we can see there is an idiom can perform the conversion,
If we comment this line, the issue solved.

// all arch
// all compilers
change_made |= analyse(bb, &IdiomsCommon::exchangeBitShiftSDiv2, 
                                                "IdiomsCommon::exchangeBitShiftSDiv2");

The problem is:
If the operand is negative, Division and shift operations are not equivalent.
for example:

0xA5A5A5A5A5A50020 / 256 = 0x‭FFA5A5A5A5A5A501
0xA5A5A5A5A5A50020 >> 8  = 0x‭FFA5A5A5A5A5A500

So why we perform this conversion for all arch / compilers?
It cause problems.

[xkubov]

Hi! Thank you very much for your investigation and input! As you wrote this is indeed unwanted behavior.

This might produce some noise in different situations but I think that we should definitely go in the correct way -> in this case shift instead of division. This behavior is only wanted in case of a logic shift (with unsigned int). I will provide a fix for this and I am going to add this good example into RetDec regression tests.