Menu
▾
▴
[Valgrind-developers] [valgrind] Bug 492210 - False positive on x86/amd64 with ZF taken directly from addition
|
From: Paul F. <pa...@so...> - 2024-09-15 19:43:27
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=7e78a9a990c990631adb3f7c7edfd8cda418547f commit 7e78a9a990c990631adb3f7c7edfd8cda418547f Author: Paul Floyd <pj...@wa...> Date: Sun Sep 15 09:52:56 2024 +0200 Bug 492210 - False positive on x86/amd64 with ZF taken directly from addition Also adds similar checks for short and char equivalents to the original int reproducer. Initial fix provided by Alexander Monakov <amo...@gm...> Two versions of the testcase, one with default options and one with -expensive-definedness-checks=yes because the byte operations subb and addb need the flag turned on explicitly. Diff: --- .gitignore | 1 + NEWS | 1 + VEX/priv/guest_amd64_helpers.c | 37 +++++++++- memcheck/tests/amd64/Makefile.am | 3 + memcheck/tests/amd64/bug492210.c | 106 ++++++++++++++++++++++++++++ memcheck/tests/amd64/bug492210_1.stderr.exp | 12 ++++ memcheck/tests/amd64/bug492210_1.vgtest | 4 ++ memcheck/tests/amd64/bug492210_2.stderr.exp | 0 memcheck/tests/amd64/bug492210_2.vgtest | 4 ++ 9 files changed, 167 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index bf477a4fa6..c15cf93e00 100644 --- a/.gitignore +++ b/.gitignore @@ -1070,6 +1070,7 @@ /memcheck/tests/amd64/bt_everything /memcheck/tests/amd64/bug132146 /memcheck/tests/amd64/bug279698 +/memcheck/tests/amd64/bug492210 /memcheck/tests/amd64/defcfaexpr /memcheck/tests/amd64/fxsave-amd64 /memcheck/tests/amd64/int3-amd64 diff --git a/NEWS b/NEWS index 3906beedf9..b9cb1d5dc2 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 490651 Stop using -flto-partition=one 491394 (vgModuleLocal_addDiCfSI): Assertion 'di->fsm.have_rx_map && di->fsm.rw_map_count' failed +492210 False positive on x86/amd64 with ZF taken directly from addition 492214 statx(fd, NULL, AT_EMPTY_PATH) is supported since Linux 6.11 but not supported in valgrind 492663 Valgrind ignores debug info for some binaries diff --git a/VEX/priv/guest_amd64_helpers.c b/VEX/priv/guest_amd64_helpers.c index da1cabc3cb..9997b42669 100644 --- a/VEX/priv/guest_amd64_helpers.c +++ b/VEX/priv/guest_amd64_helpers.c @@ -1062,6 +1062,7 @@ IRExpr* guest_amd64_spechelper ( const HChar* function_name, # define binop(_op,_a1,_a2) IRExpr_Binop((_op),(_a1),(_a2)) # define mkU64(_n) IRExpr_Const(IRConst_U64(_n)) # define mkU32(_n) IRExpr_Const(IRConst_U32(_n)) +# define mkU16(_n) IRExpr_Const(IRConst_U16(_n)) # define mkU8(_n) IRExpr_Const(IRConst_U8(_n)) Int i, arity = 0; @@ -1140,6 +1141,15 @@ IRExpr* guest_amd64_spechelper ( const HChar* function_name, } + /* 4, */ + if (isU64(cc_op, AMD64G_CC_OP_ADDL) && isU64(cond, AMD64CondZ)) { + /* long long add, then Z --> test ((int)(dst+src) == 0) */ + return unop(Iop_1Uto64, + binop(Iop_CmpEQ32, + unop(Iop_64to32, binop(Iop_Add64, cc_dep1, cc_dep2)), + mkU32(0))); + } + /* 8, 9 */ if (isU64(cc_op, AMD64G_CC_OP_ADDL) && isU64(cond, AMD64CondS)) { /* long add, then S (negative) @@ -1166,6 +1176,29 @@ IRExpr* guest_amd64_spechelper ( const HChar* function_name, mkU64(1)); } + /*---------------- ADDW ----------------*/ + + /* 4, */ + if (isU64(cc_op, AMD64G_CC_OP_ADDW) && isU64(cond, AMD64CondZ)) { + + /* long long add, then Z --> test ((short)(dst+src) == 0) */ + return unop(Iop_1Uto64, + binop(Iop_CmpEQ16, + unop(Iop_64to16, binop(Iop_Add64, cc_dep1, cc_dep2)), + mkU16(0))); + } + + /*---------------- ADDB ----------------*/ + + /* 4, */ + if (isU64(cc_op, AMD64G_CC_OP_ADDB) && isU64(cond, AMD64CondZ)) { + /* long long add, then Z --> test ((char)(dst+src) == 0) */ + return unop(Iop_1Uto64, + binop(Iop_CmpEQ8, + unop(Iop_64to8, binop(Iop_Add64, cc_dep1, cc_dep2)), + mkU8(0))); + } + /*---------------- SUBQ ----------------*/ /* 0, */ @@ -1307,6 +1340,8 @@ IRExpr* guest_amd64_spechelper ( const HChar* function_name, mkU8(31)), mkU64(1)); } + + /* 1, */ if (isU64(cc_op, AMD64G_CC_OP_SUBL) && isU64(cond, AMD64CondNO)) { /* No action. Never yet found a test case. */ } @@ -1578,7 +1613,7 @@ IRExpr* guest_amd64_spechelper ( const HChar* function_name, if (isU64(cc_op, AMD64G_CC_OP_SUBB) && isU64(cond, AMD64CondZ)) { /* byte sub/cmp, then Z --> test dst==src */ return unop(Iop_1Uto64, - binop(Iop_CmpEQ8, + binop(Iop_CmpEQ8, unop(Iop_64to8,cc_dep1), unop(Iop_64to8,cc_dep2))); } diff --git a/memcheck/tests/amd64/Makefile.am b/memcheck/tests/amd64/Makefile.am index 906b8f393b..c0139bdb20 100644 --- a/memcheck/tests/amd64/Makefile.am +++ b/memcheck/tests/amd64/Makefile.am @@ -13,6 +13,8 @@ EXTRA_DIST = \ bt_everything.vgtest \ bug132146.vgtest bug132146.stderr.exp bug132146.stdout.exp \ bug279698.vgtest bug279698.stderr.exp bug279698.stdout.exp \ + bug492210_1.vgtest bug492210_1.stderr.exp \ + bug492210_2.vgtest bug492210_2.stderr.exp \ fxsave-amd64.vgtest fxsave-amd64.stdout.exp fxsave-amd64.stderr.exp \ insn-bsfl.vgtest insn-bsfl.stdout.exp insn-bsfl.stderr.exp \ insn-pcmpistri.vgtest insn-pcmpistri.stdout.exp insn-pcmpistri.stderr.exp \ @@ -46,6 +48,7 @@ check_PROGRAMS = \ bt_everything \ bug132146 \ bug279698 \ + bug492210 \ fxsave-amd64 \ insn-bsfl \ insn-pmovmskb \ diff --git a/memcheck/tests/amd64/bug492210.c b/memcheck/tests/amd64/bug492210.c new file mode 100644 index 0000000000..27e1b17a7c --- /dev/null +++ b/memcheck/tests/amd64/bug492210.c @@ -0,0 +1,106 @@ +/* + * Bug 492210 False positive on x86/amd64 with ZF taken directly from addition + * + * The problem is that the Z flag wasn't being calculated for addl instructions. + * + * The same problem exists for addw and addb. + */ + + +unsigned char b; +unsigned short w; +unsigned long l; +unsigned long long q; + +extern void test(void); + +asm("\n" +".text\n" +"test:\n" + +"\tmovb b, %al\n" +"\tmovb $1, %ah\n" +"\taddb %al, %ah\n" +"\tje label1\n" +"\tlabel1:\n" + +"\taddb %al, %ah\n" +"\tjne label2\n" +"\tlabel2:\n" + +"\tsubb %al, %ah\n" +"\tje label3\n" +"\tlabel3:\n" + +"\tsubb %al, %ah\n" +"\tjne label4\n" +"\tlabel4:\n" + +"\tmov w, %ax\n" +"\tmovw $1, %bx\n" +"\taddw %ax, %bx\n" +"\tje label5\n" +"\tlabel5:\n" + +"\taddw %ax, %bx\n" +"\tjne label6\n" +"\tlabel6:\n" + +"\tsubw %ax, %bx\n" +"\tje label7\n" +"\tlabel7:\n" + +"\tsubw %ax, %bx\n" +"\tjne label8\n" +"\tlabel8:\n" + +"\tmov l, %eax\n" +"\tmov $1, %ebx\n" +"\tadd %eax, %ebx\n" +"\tje label9\n" +"\tlabel9:\n" + +"\tadd %eax, %ebx\n" +"\tjne label10\n" +"\tlabel10:\n" + +"\tsub %eax, %ebx\n" +"\tje label11\n" +"\tlabel11:\n" + +"\tsub %eax, %ebx\n" +"\tjne label12\n" +"\tlabel12:\n" + +"\tmovq q, %rax\n" +"\tmovq $1, %rbx\n" +"\taddq %rax, %rbx\n" +"\tje label13\n" +"\tlabel13:\n" + +"\taddq %rax, %rbx\n" +"\tjne label14\n" +"\tlabel14:\n" + +"\tsubq %rax, %rbx\n" +"\tje label15\n" +"\tlabel15:\n" + +"\tsubq %rax, %rbx\n" +"\tjne label16\n" +"\tlabel16:\n" + +"\tret\n" +".previous\n" +); + +int main() +{ + unsigned long long uninit; + uninit &= 0xfffffffffffffffe; + b = uninit; + w = uninit; + l = uninit; + q = uninit; + test(); +} diff --git a/memcheck/tests/amd64/bug492210_1.stderr.exp b/memcheck/tests/amd64/bug492210_1.stderr.exp new file mode 100644 index 0000000000..afaaad650e --- /dev/null +++ b/memcheck/tests/amd64/bug492210_1.stderr.exp @@ -0,0 +1,12 @@ +Conditional jump or move depends on uninitialised value(s) + ... + +Conditional jump or move depends on uninitialised value(s) + ... + +Conditional jump or move depends on uninitialised value(s) + ... + +Conditional jump or move depends on uninitialised value(s) + ... + diff --git a/memcheck/tests/amd64/bug492210_1.vgtest b/memcheck/tests/amd64/bug492210_1.vgtest new file mode 100644 index 0000000000..e80be84ec8 --- /dev/null +++ b/memcheck/tests/amd64/bug492210_1.vgtest @@ -0,0 +1,4 @@ +prog: bug492210 +vgopts: -q + + diff --git a/memcheck/tests/amd64/bug492210_2.stderr.exp b/memcheck/tests/amd64/bug492210_2.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/amd64/bug492210_2.vgtest b/memcheck/tests/amd64/bug492210_2.vgtest new file mode 100644 index 0000000000..1c4e3614f8 --- /dev/null +++ b/memcheck/tests/amd64/bug492210_2.vgtest @@ -0,0 +1,4 @@ +prog: bug492210 +vgopts: -q --expensive-definedness-checks=yes + + |
