Re: getdelim: Work around buggy implementation on macOS 10.13

[Simon Josefsson]

Bruno Haible <bruno@clisp.org> writes:

> While testing a GNU sed snapshot on macOS 10.13, I see this test failure:
...
> ==85029== Invalid read of size 16
...
> An out-of-bounds read. Oh oh. When I reconfigure and recompile with the
> environment variable
>   gl_cv_func_working_getdelim=no
> this test succeeds. Assuming that the GNU sed code is correct (it's not a
> particularly complex code), this proves that the out-of-bounds read comes
> from the macOS getdelim() function.
...
> +      [case "$host_os" in
> +         darwin*)
> +           dnl On macOS 10.13, valgrind detected an out-of-bounds read during
> +           dnl the GNU sed test suite:
> +           dnl   Invalid read of size 16
> +           dnl      at 0x100EE6A05: _platform_memchr$VARIANT$Base (in 
> /usr/lib/system/libsystem_platform.dylib)
> +           dnl      by 0x100B7B0BD: getdelim (in 
> /usr/lib/system/libsystem_c.dylib)
> +           dnl      by 0x10000B0BE: ck_getdelim (utils.c:254)
> +           gl_cv_func_working_getdelim=no ;;

I don't care strongly about this issue, but this brings up a design
perspective for gnulib wrt valgrind: we have several valgrind
suppression files (see lib/*.valgrind) to silence valgrind complaints
already.  Your solution here choses a different path.

It can be difficult to assess wether a valgrind complaint is a false
positive or not, especially for system functions (and we've had valgrind
complaints for libc issues that looked problematic in theory but not in
practice).  Unless we can trigger a real bug in the code and test for
that, we have a choice how to handle valgrind complaints: 1) add a
valgrind suppressions file for the complaint, or 2) unconditionally
bring in a gnulib replacement code without testing for that behaviour.

I prefer 1) since 2) will over time leads to us bringing in the entire
gnulib replacement code on all systems, which is really bloated and
leads to other problems.

What do you think about adding a valgrind suppressions file for the
output you found instead?  And possibly improve documentation about how
gnulib intends these suppression files to be used by people who run the
test-suite under valgrind, if anything is missing in that area today.

/Simon

signature.asc
Description: PGP signature