Hi Simon,
> > + [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 ;;
>
> ... 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.
For false positives, I would add a suppression file; for real bugs
I would add a workaround in gnulib.
> It can be difficult to assess wether a valgrind complaint is a false
> positive or not
Exactly. The source code of 'getdelim' in macOS has 120 lines of code,
with invocations of '__srefill' and 'sappend'. I don't have time to analyze
it in depth, because that would require to understand how these libc-
internal functions work.
I verified that by using the Gnulib code for 'getdelim' instead of the
system function, the valgrind error goes away. To me, that's a strong
indication that there is a bug in that macOS system function.
> 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.
I prefer 2) since I can't knowingly let Jim make a GNU sed release when
there is a high probability that this macOS system function has a bug
that can possibly be exploited like a CVE.
The "the entire gnulib replacement code on all systems" argument is
exaggerated, because
- It's not on all systems; it's on those systems where we have seen
suspicious valgrind errors.
- There are platforms on which a lot of Gnulib replacement code is
used. Such as Solaris 10 or mingw. Yes, it makes the binaries large.
Yes, it occasionally causes a symbol conflict here and there. But
that gets reported and fixed. It's not unmanageable.
Bruno
