paulfloyd / freebsd_valgrind

Git repo used to Upstream the FreeBSD Port of Valgrind
GNU General Public License v2.0
15 stars 4 forks source link

memcheck/tests/varinfo[1-6] compiler optimization defeats test [clang] #70

Closed nbriggs closed 2 years ago

nbriggs commented 4 years ago

If any optimization is enabled for the varinfo[1..6] tests (which by default use -O) and the compilation is with clang, the tests fail.

Clang appears to optimize out the call to malloc(1) in croak()

This can be defeated with a volatile attribute as shown here for varinfo1.c and in corresponding places in varinfo[2..6], varinforestrict.c is compiled -O0 so doesn't need this modification to pass the test:

diff --git a/memcheck/tests/varinfo1.c b/memcheck/tests/varinfo1.c
index 6a739566f..8d8852189 100644
--- a/memcheck/tests/varinfo1.c
+++ b/memcheck/tests/varinfo1.c
@@ -21,13 +21,13 @@
 void croak ( void* aV )
 {
   char* a = (char*)aV;
-  char* undefp = malloc(1);
+  volatile char* undefp = malloc(1);
   char saved = *a;
   assert(undefp);
   *a = *undefp;
   (void) VALGRIND_CHECK_MEM_IS_DEFINED(a, 1);
   *a = saved;
-  free(undefp);
+  free((void *)undefp);
 }

 #include <stdio.h>

Disassembly of croak is attached with/without volatile.

varinfo1-croak-O.txt varinfo1-croak-volatile-O.txt

paulfloyd commented 4 years ago

That's really interesting. I'll take a look at Linux GCC and clang tonight.

Linux/clang doesn't look too rosy. This is just memcheck.

== 262 tests, 33 stderr failures, 12 stdout failures, 0 stderrB failures, 0 stdoutB failures, 0 post failures == memcheck/tests/amd64/insn_basic (stdout) memcheck/tests/amd64/insn_basic (stderr) memcheck/tests/amd64/insn_fpu (stdout) memcheck/tests/amd64/insn_fpu (stderr) memcheck/tests/amd64/insn_mmx (stdout) memcheck/tests/amd64/insn_mmx (stderr) memcheck/tests/amd64/insn_sse (stdout) memcheck/tests/amd64/insn_sse (stderr) memcheck/tests/amd64/insn_sse2 (stdout) memcheck/tests/amd64/insn_sse2 (stderr) memcheck/tests/amd64-freebsd/posix_fadvise (stderr) memcheck/tests/amd64-freebsd/posix_fallocate (stderr) memcheck/tests/amd64-linux/access_below_sp_1 (stderr) memcheck/tests/amd64-linux/access_below_sp_2 (stderr) memcheck/tests/clientperm (stderr) memcheck/tests/gone_abrt_xml (stderr) memcheck/tests/linux/sys-preadv2_pwritev2 (stderr) memcheck/tests/linux/with-space (stderr) memcheck/tests/memalign_test (stderr) memcheck/tests/origin4-many (stderr) memcheck/tests/origin5-bz2 (stderr) memcheck/tests/sem (stderr) memcheck/tests/varinfo1 (stderr) memcheck/tests/varinfo2 (stderr) memcheck/tests/varinfo3 (stderr) memcheck/tests/varinfo4 (stderr) memcheck/tests/varinfo5 (stderr) memcheck/tests/varinfo6 (stderr) memcheck/tests/vcpu_bz2 (stdout) memcheck/tests/vcpu_bz2 (stderr) memcheck/tests/wrap6 (stdout) memcheck/tests/x86/insn_basic (stdout) memcheck/tests/x86/insn_basic (stderr) memcheck/tests/x86/insn_cmov (stdout) memcheck/tests/x86/insn_cmov (stderr) memcheck/tests/x86/insn_fpu (stdout) memcheck/tests/x86/insn_fpu (stderr) memcheck/tests/x86/insn_mmx (stdout) memcheck/tests/x86/insn_mmx (stderr) memcheck/tests/x86/insn_mmxext (stdout) memcheck/tests/x86/insn_mmxext (stderr) memcheck/tests/x86/sh-mem-vec128-plo-no (stderr) memcheck/tests/x86/sh-mem-vec128-plo-yes (stderr) memcheck/tests/x86-freebsd/posix_fadvise (stderr) memcheck/tests/x86-freebsd/posix_fallocate (stderr)

paulfloyd commented 4 years ago

Ah this is a pain. With the installed clang and clang9 I don't get lineinfo for some of the variables. With clang 11 I do get it but there is a diff in the frame number. This could be a Valgrind issue, and it might warrant adding a filter for the frame number.

I've pushed most of the changes except to varinfo6, I think that this diff needs looking at

<    by 0x........: handle_compress (varinfo6.c:0)
---
>    by 0x........: handle_compress (varinfo6.c:4790)
nbriggs commented 4 years ago

This is the corresponding list of memcheck test failures from my (32-bit) x86 system, FreeBSD 12.1-RELEASE-p4: valgrind itself compiled with clang9 with -O2, tests compiled with clang9 and whatever optimization the tests select.

-- Finished tests in memcheck/tests ------------------------------------
== 215 tests, 21 stderr failures, 1 stdout failure, 0 stderrB failures, 0 stdoutB failures, 0 post failures ==
memcheck/tests/addressable               (stderr)
memcheck/tests/descr_belowsp             (stderr)
memcheck/tests/dw4                       (stderr)
memcheck/tests/fprw                      (stderr)
memcheck/tests/freebsd/pdfork_pdkill     (stderr)
memcheck/tests/gone_abrt_xml             (stderr)
memcheck/tests/leak-cases-full           (stderr)
memcheck/tests/leak-cases-summary        (stderr)
memcheck/tests/leak-cycle                (stderr)
memcheck/tests/leak-tree                 (stderr)
memcheck/tests/leak_cpp_interior         (stderr)
memcheck/tests/lks                       (stderr)
memcheck/tests/origin4-many              (stderr)
memcheck/tests/origin5-bz2               (stderr)
memcheck/tests/origin6-fp                (stderr)
memcheck/tests/sendmsg                   (stderr)
memcheck/tests/supp_unknown              (stderr)
memcheck/tests/test-plo-no               (stderr)
memcheck/tests/varinfo5                  (stderr)
memcheck/tests/varinfo6                  (stderr)
memcheck/tests/wrap6                     (stdout)
memcheck/tests/x86/pushfpopf             (stderr)
nbriggs commented 4 years ago

varinfo6 has the same line number and frame number issue on my system. These are the differences:

    by 0x........: BZ2_compressBlock (varinfo6.c:4072)
-   by 0x........: handle_compress (varinfo6.c:4790)
+   by 0x........: handle_compress (varinfo6.c:0)
    by 0x........: BZ2_bzCompress (varinfo6.c:4860)

However if you set optimization to -O0, the line number (4790) is correctly extracted.

- declared at varinfo6.c:3115, in frame #2 of thread 1
+ declared at varinfo6.c:3115, in frame #1 of thread 1
- Location 0x........ is 2 bytes inside local var "i"
- declared at varinfo6.c:1517, in frame #1 of thread 1
+ Address 0x........ is on thread 1's stack
+ in frame #0, created by BZ2_decompress (varinfo6.c:1510)

With optimization at -O0, valgrind generates the expected

==8538==  Location 0xfabfeb4a is 2 bytes inside local var "i"
==8538==  declared at varinfo6.c:1517, in frame #1 of thread 1
==8538== 
nbriggs commented 4 years ago

I just ran memcheck tests on Linux (Ubuntu 20.04) with clang (default, clang10) used to compile both valgrind and all the tests, and got the following results, which are pretty good --

== 233 tests, 14 stderr failures, 1 stdout failure, 0 stderrB failures, 0 stdoutB failures, 0 post failures ==
memcheck/tests/amd64/insn-pmovmskb       (stderr)
memcheck/tests/amd64-linux/access_below_sp_1 (stderr)
memcheck/tests/amd64-linux/access_below_sp_2 (stderr)
memcheck/tests/clientperm                (stderr)
memcheck/tests/linux/with-space          (stderr)
memcheck/tests/manuel1                   (stderr)
memcheck/tests/memalign_test             (stderr)
memcheck/tests/origin4-many              (stderr)
memcheck/tests/origin5-bz2               (stderr)
memcheck/tests/overlap                   (stderr)
memcheck/tests/varinfo1                  (stderr)
memcheck/tests/varinfo2                  (stderr)
memcheck/tests/varinfo5                  (stderr)
memcheck/tests/varinfo6                  (stderr)
memcheck/tests/wrap6                     (stdout)

The varinfo1 difference and 2 of 3 differences in varinfo2 are for frame numbers, the 3rd diff in varinfo2 is reporting stack allocated variable instead of local variable, basically the same difference as for varinfo6 above.

paulfloyd commented 4 years ago

I get better results with clang 11. Similar but not the same.

== 260 tests, 13 stderr failures, 1 stdout failure, 0 stderrB failures, 0 stdoutB failures, 0 post failures == memcheck/tests/amd64-linux/access_below_sp_1 (stderr) memcheck/tests/amd64-linux/access_below_sp_2 (stderr) memcheck/tests/clientperm (stderr) memcheck/tests/linux/sys-preadv2_pwritev2 (stderr) memcheck/tests/linux/with-space (stderr) memcheck/tests/memalign_test (stderr) memcheck/tests/origin5-bz2 (stderr) memcheck/tests/varinfo1 (stderr) memcheck/tests/varinfo2 (stderr) memcheck/tests/varinfo5 (stderr) memcheck/tests/varinfo6 (stderr) memcheck/tests/wrap6 (stdout) memcheck/tests/x86/sh-mem-vec128-plo-no (stderr) memcheck/tests/x86/sh-mem-vec128-plo-yes (stderr)

paulfloyd commented 2 years ago

I think that the debuginfo related tests are now as good as possible with clang. In the VG bugzilla there's a big patch for DWARF5 support, I don't think that it will help but you never know.

nbriggs commented 2 years ago

Looks pretty good on a real x86, clang 11.0.1, FreeBSD 13.0, valgrind 277f10d5:

== 693 tests, 20 stderr failures, 1 stdout failure, 2 stderrB failures, 0 stdoutB failures, 0 post failures ==
gdbserver_tests/mcblocklistsearch        (stderrB)
gdbserver_tests/mcinfcallWSRU            (stderrB)
memcheck/tests/dw4                       (stderr)
memcheck/tests/leak-cases-full           (stderr)
memcheck/tests/leak-cases-summary        (stderr)
memcheck/tests/leak-cycle                (stderr)
memcheck/tests/leak-tree                 (stderr)
memcheck/tests/leak_cpp_interior         (stderr)
memcheck/tests/lks                       (stderr)
memcheck/tests/manuel1                   (stderr)
memcheck/tests/origin5-bz2               (stderr)
memcheck/tests/varinfo2                  (stderr)
memcheck/tests/varinfo5                  (stderr)
memcheck/tests/varinfo6                  (stderr)
helgrind/tests/tls_threads               (stderr)
drd/tests/annotate_smart_pointer         (stderr)
drd/tests/annotate_trace_memory          (stderr)
drd/tests/annotate_trace_memory_xml      (stderr)
drd/tests/atomic_var                     (stderr)
drd/tests/pth_mutex_signal               (stderr)
drd/tests/std_thread2                    (stderr)
none/tests/manythreads                   (stdout)
none/tests/manythreads                   (stderr)
paulfloyd commented 2 years ago

I don't get quite the same list

== 694 tests, 20 stderr failures, 1 stdout failure, 1 stderrB failure, 0 stdoutB failures, 0 post failures ==
gdbserver_tests/mcblocklistsearch        (stderrB)
memcheck/tests/dw4                       (stderr)
memcheck/tests/leak-cases-full           (stderr)
memcheck/tests/leak-cases-summary        (stderr)
memcheck/tests/leak-cycle                (stderr)
memcheck/tests/leak-tree                 (stderr)
memcheck/tests/leak_cpp_interior         (stderr)
memcheck/tests/lks                       (stderr)
memcheck/tests/manuel1                   (stderr)
memcheck/tests/origin5-bz2               (stderr)
memcheck/tests/varinfo2                  (stderr)
memcheck/tests/varinfo5                  (stderr)
memcheck/tests/varinfo6                  (stderr)
helgrind/tests/tls_threads               (stderr)
drd/tests/annotate_smart_pointer         (stderr)
drd/tests/annotate_trace_memory          (stderr)
drd/tests/annotate_trace_memory_xml      (stderr)
drd/tests/atomic_var                     (stderr)
drd/tests/pth_mutex_signal               (stderr)
none/tests/fdleak_cmsg                   (stderr)
none/tests/manythreads                   (stdout)
none/tests/manythreads                   (stderr)

Most of the issues are related to getting source information. I do have one fix on the go which mainly affects shard libs and should fix varinfo5, but the other debuginfo problems are due to GCC (and Valgrind) using DWARF extensions that clang does not use.

I haven't looked at the gdbserver problems.

drd/tests/pth_mutex_signal fails just about everywhere

helgrind/tests/tls_threads looks very hard to fix

I'm hoping that all of the leak failures are down to the same root cause - some stray pointer to allocated memory that is left over in a register or on the stack. With GCC, different register allocation (or stack use maybe) means that the tests pass. Unfortunately for possible leaks memcheck doesn't say where it found the pointer.

fdleak_cmsg looks like some FreeBSD syscall strangeness. I'll be reimplementing the functions for FreeBSD 13.1 and later so I'm not planning on trying to fix it on older versions.

I also get failures with one of the two eventfd tests, it looks like the order of the output is non-deterministic.