|
From: Guilhem B. <gu...@my...> - 2005-11-07 13:10:16
|
Hello,
I'm sorry if this has already been reported. Here is a simple tescase
(Linux FC4 x86_64, Intel Xeon EM64T, Valgrind 3.0.1):
#include <assert.h>
#include <stdlib.h>
main()
{
char *from, *ptr;
int length=3;
assert(from= ptr= malloc(10));
from[0]= from[1]= from[2]= 0;
#if 1
// valgrind error
for (; !*ptr && length; ptr++, length--);
#else
// no valgrind error
for (; length && !*ptr; ptr++, length--);
#endif
free(from);
}
Gives:
[DEL 14:59 /m/tmp $] gcc -g testcase.c; valgrind ./a.out
==26420== Memcheck, a memory error detector.
==26420== Copyright (C) 2002-2005, and GNU GPL'd, by Julian Seward et al.
==26420== Using LibVEX rev 1367, a library for dynamic binary translation.
==26420== Copyright (C) 2004-2005, and GNU GPL'd, by OpenWorks LLP.
==26420== Using valgrind-3.0.1, a dynamic binary instrumentation framework.
==26420== Copyright (C) 2000-2005, and GNU GPL'd, by Julian Seward et al.
==26420== For more details, rerun with: -v
==26420==
==26420== Conditional jump or move depends on uninitialised value(s)
==26420== at 0x4005C5: main (testcase.c:11)
==26420==
==26420== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 9 from 4)
==26420== malloc/free: in use at exit: 0 bytes in 0 blocks.
==26420== malloc/free: 1 allocs, 1 frees, 10 bytes allocated.
==26420== For counts of detected errors, rerun with: -v
==26420== No malloc'd blocks -- no leaks are possible.
It goes like this:
Start of first iteration (before the test):
ptr points at the first of the 3 bytes (0), length is 3.
Start of 2nd (before the test):
ptr points at the second of the 3 bytes (0), length is 2.
Start of 3rd (before the test):
ptr points at the third of the 3 bytes (0), length is 1.
Start of 4th (before the test):
ptr points to uninitialized memory, length is 0;
as ptr is uninitialized, Valgrind will cough on *ptr, but the
"&& length" guarantees that in fact *ptr does not count in the
condition (as length is 0, the "&& length" makes the condition false
for sure).
In another unrelated thread, Julian Seward had told me "the message is
emitted when V sees a use of undefined CPU condition codes for a
conditional branch or move". But here, because of the "&& length", the
condition code is not undefined, it's 0.
I'll fix the MySQL code which triggers this warning, but just wanted
to know if this is an expected "problem".
Note that if malloc(10) is replaced by malloc(3) the error becomes:
[DEL 14:52 /m/tmp $] gcc -g testcase.c; valgrind ./a.out
==26358== Memcheck, a memory error detector.
==26358== Copyright (C) 2002-2005, and GNU GPL'd, by Julian Seward et al.
==26358== Using LibVEX rev 1367, a library for dynamic binary translation.
==26358== Copyright (C) 2004-2005, and GNU GPL'd, by OpenWorks LLP.
==26358== Using valgrind-3.0.1, a dynamic binary instrumentation framework.
==26358== Copyright (C) 2000-2005, and GNU GPL'd, by Julian Seward et al.
==26358== For more details, rerun with: -v
==26358==
==26358== Invalid read of size 1
==26358== at 0x4005C0: main (testcase.c:11)
==26358== Address 0x11F77033 is 0 bytes after a block of size 3 alloc'd
==26358== at 0x11B1FE96: malloc (vg_replace_malloc.c:149)
==26358== by 0x400560: main (testcase.c:8)
==26358==
==26358== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 9 from 4)
==26358== malloc/free: in use at exit: 0 bytes in 0 blocks.
==26358== malloc/free: 1 allocs, 1 frees, 3 bytes allocated.
==26358== For counts of detected errors, rerun with: -v
==26358== No malloc'd blocks -- no leaks are possible.
Thank you for your kind help.
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Mr. Guilhem Bichot <gu...@my...>
/ /|_/ / // /\ \/ /_/ / /__ MySQL AB, Full-Time Software Developer
/_/ /_/\_, /___/\___\_\___/ Bordeaux, France
<___/ www.mysql.com
|
|
From: Guilhem B. <gu...@my...> - 2005-11-07 13:14:00
|
On Mon, Nov 07, 2005 at 02:09:18PM +0100, Guilhem Bichot wrote: > Hello, > > I'm sorry if this has already been reported. Here is a simple tescase > (Linux FC4 x86_64, Intel Xeon EM64T, Valgrind 3.0.1): Just to bring more platform information: Reproduced on the above platform with gcc 4.0.0; and also on SuSE 9.1 on Athlon CPU, Valgrind 2.4.0, gcc 3.4.4. |
|
From: Olly B. <ol...@su...> - 2005-11-07 13:27:41
|
On 2005-11-07, Guilhem Bichot <gu...@my...> wrote:
> ptr points to uninitialized memory, length is 0;
> as ptr is uninitialized, Valgrind will cough on *ptr, but the
> "&& length" guarantees that in fact *ptr does not count in the
> condition (as length is 0, the "&& length" makes the condition false
> for sure).
Accessing uninitialised values invokes undefined behaviour in C, so
anything could happen.
Cheers,
Olly
|
|
From: Tom H. <to...@co...> - 2005-11-07 14:10:42
|
In message <200...@my...>
Guilhem Bichot <gu...@my...> wrote:
> In another unrelated thread, Julian Seward had told me "the message is
> emitted when V sees a use of undefined CPU condition codes for a
> conditional branch or move". But here, because of the "&& length", the
> condition code is not undefined, it's 0.
But because the length check is second and && is a sequence point
the compiled code is required to look at *ptr before considering
the length so your code is broken - it is looking at an uninitialised
value and taking a decision (whether or not to check the length) based
on it.
> I'll fix the MySQL code which triggers this warning, but just wanted
> to know if this is an expected "problem".
It isn't a problem at all. Your code is broken and valgrind is telling
you about it as intended.
Tom
--
Tom Hughes (to...@co...)
http://www.compton.nu/
|
|
From: Nicholas N. <nj...@cs...> - 2005-11-07 14:53:39
|
On Mon, 7 Nov 2005, Tom Hughes wrote: >> I'll fix the MySQL code which triggers this warning, but just wanted >> to know if this is an expected "problem". > > It isn't a problem at all. Your code is broken and valgrind is telling > you about it as intended. One could argue that the code isn't broken... at least from Memcheck's point of view it doesn't know that the code is C. Having said that, as Dennis Lubert said it would be hard for Memcheck to avoid this warning since it considers the two parts of the conjunction separately, and since the warning is arguably useful, changing the code seems the best thing to do. Nick |
|
From: Serge S. <se...@lx...> - 2005-11-07 14:16:40
|
Guilhem Bichot wrote:
> I'm sorry if this has already been reported. Here is a simple tescase
> (Linux FC4 x86_64, Intel Xeon EM64T, Valgrind 3.0.1):
>
> #include <assert.h>
> #include <stdlib.h>
>
> main()
> {
> char *from, *ptr;
> int length=3;
> assert(from= ptr= malloc(10));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
That's unrelated to valgrind, but this code is incorrect (compiling
with -DNDEBUG will produce invalid executable). Maybe I'm a bit too
pedantic and it is just a small test program, but one should be aware
of such issues and avoid them automatically in order not to add
something similar into production code :)
> from[0]= from[1]= from[2]= 0;
> #if 1
> // valgrind error
> for (; !*ptr && length; ptr++, length--);
> #else
> // no valgrind error
> for (; length && !*ptr; ptr++, length--);
> #endif
> free(from);
> }
Uninitialized variables are better to be avoided even if they do not
harm in some specific code. By changing this code in the future, you
risk getting some nasty surprize if you forget that you need to take
special care about that uninitialized variable.
|
|
From: Dennis L. <pla...@pr...> - 2005-11-07 14:19:05
|
Am Montag, den 07.11.2005, 14:09 +0100 schrieb Guilhem Bichot:
> Start of 4th (before the test):
> ptr points to uninitialized memory, length is 0;
> as ptr is uninitialized, Valgrind will cough on *ptr, but the
> "&& length" guarantees that in fact *ptr does not count in the
> condition (as length is 0, the "&& length" makes the condition false
> for sure).
>
> In another unrelated thread, Julian Seward had told me "the message is
> emitted when V sees a use of undefined CPU condition codes for a
> conditional branch or move". But here, because of the "&& length", the
> condition code is not undefined, it's 0.
According to ISO9899 (C standard) its undefined. And the test to *ptr is
undefined too. Valgrind does not care about the complete logic of
C-Code, in fact it does not know anything. All it sees is similar to:
if( *ptr )
{
if( length)
{
goto cond_false;
}
else
{
goto cond_true;
}
}
else
{
goto cond_false;
}
and so it sees the check for *ptr and sees that it is in fact accessing
undefined memory and is totally correct in saying that the conditional
there is depending on unitialized memory.
In fact some compilers without optimization really emit assembler code
that is the same in the operator&& case and the illustrated one.
As soon as valgrind sees the rely on undefined memory, it emits a
warning. It cannot and should never try to look forward and analyze the
logic if relying on the value does matter.
The C Standard does leave this to the so-elusive implementation defined
domain, which can even include allowing the program to segfault (even if
this would not make much sense)
greets
Dennis
|