[Spice-devel] [PATCH spice-server] ci: Workaround bug in Valgrind detecting memcpy instead of memmove

[Frediano Ziglio]

> 
> On Mon, Apr 16, 2018 at 05:02:33PM +0100, Frediano Ziglio wrote:
> > Due to a bug in current packaged Valgrind check-valgrind is failing
> > with:
> > 
> > ==17986== Source and destination overlap in memcpy_chk(0x72c060, 0x72c068,
> > 33)
> > ==17986==    at 0x4C344F0: __memcpy_chk (vg_replace_strmem.c:1581)
> > ==17986==    by 0x40E7E9: check_vmc_error_message
> > (test-stream-device.c:166)
> > ==17986==    by 0x40EFD4: test_stream_device_format_after_data
> > (test-stream-device.c:349)
> > ==17986==    by 0x7A012E9: test_case_run (gtestutils.c:2157)
> > ==17986==    by 0x7A012E9: g_test_run_suite_internal (gtestutils.c:2241)
> > ==17986==    by 0x7A0121A: g_test_run_suite_internal (gtestutils.c:2253)
> > ==17986==    by 0x7A014C1: g_test_run_suite (gtestutils.c:2329)
> > ==17986==    by 0x7A014E0: g_test_run (gtestutils.c:1594)
> > ==17986==    by 0x40951A: main (test-stream-device.c:410)
> > ==17986==
> > 
> > Note that source code calls memmove instead of memcpy (memmove
> > supports overlapping).
> > Disable call to __memmove_chk to avoid the issue.
> 
> __memcpy_chk? Not clear how this is achieved from the log. By disabling
> _FORTIFY_SOURCE I assume?
> 
> Christophe
> 

The bug is this:

Spice use fortify option by default, in config.h you get:

/* Enable compile-time and run-time bounds-checking, and some warnings. */
#if !defined _FORTIFY_SOURCE &&  defined __OPTIMIZE__ && __OPTIMIZE__
# define _FORTIFY_SOURCE 2
#endif

when you call memove in your code you have this included
(/usr/include/bits/string_fortified.h):

__fortify_function void *
__NTH (memmove (void *__dest, const void *__src, size_t __len))
{
  return __builtin___memmove_chk (__dest, __src, __len, __bos0 (__dest));
}

which basically results in a call to __memmove_chk

$ objdump -t server/tests/test-stream-device.o | grep memmove
0000000000000000         *UND*  0000000000000000 __memmove_chk

(the command is not enough to prove but with disassembly and other you
can check that the memmove from source turns into a call or jump to
__memmove_chk)

The call became a call to __memmove_chk@@GLIBC_<xxx> in the executable.
The bug (in Valgrind) is that this __memmove_chk at GLIBC_<xxx> call is
turned into a Valgrind function which handle a __memcpy_chk checking
that destination and source pointers does not overlap (which is expected
for memmove).

Adding -D_FORTIFY_SOURCE=0 the memmove call is not turned into __memmove_chk
and Valgrind handle memmove correctly.
Not actually a big problem, the replacement Valgrind use should do
enough checks.

Frediano

> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> > This should be temporary till Valgrind is fixed in Fedora.
> > Was tested by me and Snir.
> > ---
> >  .gitlab-ci.yml | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 535d220d..655232c2 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -34,7 +34,9 @@ check-valgrind:
> >      dnf install valgrind
> >      gstreamer1-libav gstreamer1-plugins-ugly gstreamer1-plugins-good
> >      gstreamer1-plugins-bad-free
> >      -y
> > -  - ./autogen.sh --enable-valgrind --enable-extra-checks
> > +  - >
> > +    CFLAGS='-O2 -pipe -g -D_FORTIFY_SOURCE=0'
> > +    ./autogen.sh --enable-valgrind --enable-extra-checks
> >    - make
> >    - make check-valgrind || (cat server/tests/test-suite-memcheck.log &&
> >    exit 1)
> >