Hi,

I added Julian to the CC who will probably correct any mistakes
I make. Or even might know a simple way to make valgrind detect
this idiom.

On Wed, Sep 07, 2016 at 04:21:29PM -0700, Paul Eggert wrote:
> On 09/07/2016 04:52 AM, Mark Wielaard wrote:
> > If valgrind believes the
> > memory isn't in valid memory then it will complain about an invalid access.
> > But if the memory is accessible but uninitialised then it will just track
> > the undefinedness complain later if such a value is used.
> 
> I think the former is what happened in Gnulib fts.c before Gnulib was fixed.

BTW. Do gnulib and glibc syncronize? I had no idea gnulib also contained
fts. I recently fixed some issue (adding LFS support) in glibc.

> > valgrind also has --partial-loads-ok (which in newer versions defaults
> > to =yes):
> > 
> >     Controls how Memcheck handles 32-, 64-, 128- and 256-bit naturally
> >     aligned loads from addresses for which some bytes are addressable
> >     and others are not.
> 
> Although this helps in some cases, it does not suffice in general since the
> problem can occur with 16-bit aligned loads. I think that is what happened
> with fts.c.
> 
> > Does anybody have an example program of the above issue compiled with
> > gcc that produces false positives with valgrind?
> > 
> 
> Sure, attached. On Fedora 24 x86-64 (GCC 6.1.1 20160621, valgrind 3.11.0),
> when I compile with "gcc -O2 flexouch.c" and run with "valgrind ./a.out",
> valgrind complains "Invalid read of size 2". This is because GCC compiles
> "p->d[0] == 2 && p->d[1] == 3" into "cmpw $770, 8(%rax); sete %al", which
> loads the uninitialized byte p->d[1] simultaneously with the initialized
> byte p->d[0].
> 
> As mentioned previously, although flexouch.c does not conform to C11, this
> is arguably a defect in C11.
> 

> #include <stddef.h>
> #include <stdlib.h>
> 
> struct s { struct s *next; char d[]; };
> 
> int
> main (void)
> {
>   struct s *p = malloc (offsetof (struct s, d) + 1);
>   p->d[0] = 1;
>   return p->d[0] == 2 && p->d[1] == 3;
> }

OK, I can replicate that with the same GCC when using -O2.

==25520== Invalid read of size 2
==25520==    at 0x400442: main (in /home/mark/src/tests/flexouch)
==25520==  Address 0x51fa048 is 8 bytes inside a block of size 9 alloc'd
==25520==    at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
==25520==    by 0x40043D: main (in /home/mark/src/tests/flexouch)

Without optimization the p->d[1] == 3 is never executed.

Dump of assembler code for function main:
=> 0x0000000000400430 <+0>:     sub    $0x8,%rsp
   0x0000000000400434 <+4>:     mov    $0x9,%edi
   0x0000000000400439 <+9>:     callq  0x400410 <malloc@plt>
   0x000000000040043e <+14>:    movb   $0x1,0x8(%rax)
   0x0000000000400442 <+18>:    cmpw   $0x302,0x8(%rax)
   0x0000000000400448 <+24>:    sete   %al
   0x000000000040044b <+27>:    add    $0x8,%rsp
   0x000000000040044f <+31>:    movzbl %al,%eax
   0x0000000000400452 <+34>:    retq   
End of assembler dump.

Note that valgrind will also get this "wrong" if you do
allocate enough space, but don't initialize d[1]:

==25906== Syscall param exit_group(status) contains uninitialised byte(s)
==25906==    at 0x4F00CC8: _Exit (in /usr/lib64/libc-2.23.so)
==25906==    by 0x4E7119A: __run_exit_handlers (in /usr/lib64/libc-2.23.so)
==25906==    by 0x4E71234: exit (in /usr/lib64/libc-2.23.so)
==25906==    by 0x4E58737: (below main) (in /usr/lib64/libc-2.23.so)
==25906==  Uninitialised value was created by a heap allocation
==25906==    at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
==25906==    by 0x40043D: main (in /home/mark/src/tests/flexouch)

I don't think there is anything valgrind can do to detect that
compw really only depends on d[0] if the result is false.

Cheers,

Mark

Reply via email to