Re: diffutils-3.7.41-7475 exercises undefined behaviour

[Jim Meyering]

On Sat, Jul 24, 2021 at 8:36 PM Bruno Haible <bruno@clisp.org> wrote:
> Using uninitialized storage is undefined behavior in C. [1] But 'cmp' does 
> this.
>
> How to reproduce:
> 0. Install valgrind and the attached 'valgrind-wrap' script.
> 1. Build diffutils:
>      ./configure ...
>      make
> 2. valgrind-wrap src/cmp src/diff src/diff3 src/sdiff
> 3. make check
>
> The result is that this valgrind 'error' appears a couple of times:
>
>   Conditional jump or move depends on uninitialised value(s)
>      at 0x4020F0: block_compare (cmp.c:620)
>      by 0x4020F0: cmp (cmp.c:463)
>      by 0x4020F0: main (cmp.c:361)
>
> Would it be possible to run block_compare on an initialized buffer,
> instead of on a partially uninitialized one?

Hi Bruno,
Thanks a lot for testing like that!

I've done as you suggested, but so far, I have been unable to evoke a
used-uninitialized error on a Fedora 34 system using its libc-2.33,
valgrind-3.17.0 and this compiler:
  gcc version 11.1.1 20210531 (Red Hat 11.1.1-3) (GCC)

Would you please describe your environment and share the names of the
test(s) that evoked such failures?

In spite of that, I think I can see what's happening: at cmp:460, we
put a single-byte sentinel at the end of each buffer, yet the
following block_compare reads full words, so depending on alignment, a
final word-read may read the sentinel byte and possibly a few
(uninitialized ones) beyond it, but that shouldn't matter (no risk of
buffer overrun), because the sentinel is guaranteed to stop further
processing. We could easily avoid this by using a full sentinel word,
rather than just a byte.