From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Andi Kleen <andi@firstfloor.org>
Cc: Vegard Nossum <vegard.nossum@gmail.com>,
Bart Van Assche <bart.vanassche@gmail.com>,
John Reiser <jreiser@BitWagon.com>,
Pekka Enberg <penberg@cs.helsinki.fi>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@elte.hu>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Christoph Lameter <clameter@sgi.com>,
Daniel Walker <dwalker@mvista.com>,
Randy Dunlap <randy.dunlap@oracle.com>,
Josh Aune <luken@omner.org>, Pekka Paalanen <pq@iki.fi>
Subject: Re: [ANNOUNCE] kmemcheck v7
Date: Sat, 10 May 2008 21:45:19 +0100 [thread overview]
Message-ID: <4826095F.3070501@goop.org> (raw)
In-Reply-To: <20080510174841.GC31954@one.firstfloor.org>
[-- Attachment #1: Type: text/plain, Size: 4891 bytes --]
Andi Kleen wrote:
>> It tracks changes to the stack pointer, and any memory below it is
>> considered uninitialized. But, yes, if you mean that if you use the
>>
>
> But it does not invalidate anything below the stack pointer as soon
> as it changes right ?
>
Yeah, as soon as the stack pointer changes, everything below it is
invalidated (except if the stack-pointer change was actually determined
to be a stack switch).
>> variable (or slot) once in a function, then again later, it will still
>> be considered initialized. But that's no different from any other memory.
>>
>
> What I meant is e.g.
>
> f1();
> f2();
>
> both f1 and f2 use the same stack memory, but f2 uses it uninitialized,
> then I think valgrind would still think it is initialized in f2 from the
> execution of f1. It would only detect such things in f1 (assuming there
> were no other users of the stack before that)
>
No, it won't. If the stack pointer goes up then down between f1 and f2,
then f2 will get fresh values.
The big thing Valgrind hasn't traditionally helped with is overruns of
on-stack arrays. You may be thinking of that.
> In theory it could throw away all stack related uninitizedness on each
> SP change, but that would be likely prohibitively expensive and also
> it might be hard to know the exact boundaries of the stack.
>
No, its not all that expensive compared the overall cost of valgrind and
the amount of diagnostic power it provides. Determining stack
boundaries has always been a bit fraught. Typically a stack switch has
been determined heuristically by looking for a "large" change in stack
pointer, but there's a callback to specifically mark a range of memory
as a stack, so that movements into and out of a stack can be determined
as a switch (added specifically to deal with small densely packed stacks
in uml).
> BTW on running a test program here it doesn't seem to detect any uninitialized
> stack frames here with 3.2.3. Test program is http://halobates.de/t10.c
> (should be compiled without optimization)
>
Hm, I'd expect it to. Oh, your test program doesn't use the value.
Valgrind doesn't complain about uninitialized values unless they
actually affect execution (ie, a conditional depends on one, you use it
as an address for a dereference, or pass it to a syscall).
The attached version emits errors as I'd expect:
$ valgrind t10
==30474== Memcheck, a memory error detector.
==30474== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==30474== Using LibVEX rev 1804, a library for dynamic binary translation.
==30474== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==30474== Using valgrind-3.3.0, a dynamic binary instrumentation framework.
==30474== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==30474== For more details, rerun with: -v
==30474==
f1 set y to 1
==30474== Conditional jump or move depends on uninitialised value(s)
==30474== at 0x8048420: test (t10.c:22)
==30474== by 0x8048451: main (t10.c:29)
==30474==
==30474== Use of uninitialised value of size 4
==30474== at 0xB5C5B6: _itoa_word (in /lib/libc-2.8.so)
==30474== by 0xB5FF90: vfprintf (in /lib/libc-2.8.so)
==30474== by 0xB6769F: printf (in /lib/libc-2.8.so)
==30474== by 0x8048436: test (t10.c:23)
==30474== by 0x8048451: main (t10.c:29)
==30474==
==30474== Conditional jump or move depends on uninitialised value(s)
==30474== at 0xB5C5BE: _itoa_word (in /lib/libc-2.8.so)
==30474== by 0xB5FF90: vfprintf (in /lib/libc-2.8.so)
==30474== by 0xB6769F: printf (in /lib/libc-2.8.so)
==30474== by 0x8048436: test (t10.c:23)
==30474== by 0x8048451: main (t10.c:29)
==30474==
==30474== Conditional jump or move depends on uninitialised value(s)
==30474== at 0xB5EADE: vfprintf (in /lib/libc-2.8.so)
==30474== by 0xB6769F: printf (in /lib/libc-2.8.so)
==30474== by 0x8048436: test (t10.c:23)
==30474== by 0x8048451: main (t10.c:29)
==30474==
==30474== Conditional jump or move depends on uninitialised value(s)
==30474== at 0xB60828: vfprintf (in /lib/libc-2.8.so)
==30474== by 0xB6769F: printf (in /lib/libc-2.8.so)
==30474== by 0x8048436: test (t10.c:23)
==30474== by 0x8048451: main (t10.c:29)
==30474==
==30474== Conditional jump or move depends on uninitialised value(s)
==30474== at 0xB5EB88: vfprintf (in /lib/libc-2.8.so)
==30474== by 0xB6769F: printf (in /lib/libc-2.8.so)
==30474== by 0x8048436: test (t10.c:23)
==30474== by 0x8048451: main (t10.c:29)
f2 set y to 13123572
==30474==
==30474== ERROR SUMMARY: 20 errors from 6 contexts (suppressed: 13 from 1)
==30474== malloc/free: in use at exit: 0 bytes in 0 blocks.
==30474== malloc/free: 0 allocs, 0 frees, 0 bytes allocated.
==30474== For counts of detected errors, rerun with: -v
==30474== All heap blocks were freed -- no leaks are possible.
J
[-- Attachment #2: t10.c --]
[-- Type: text/x-csrc, Size: 258 bytes --]
#include <stdio.h>
int y;
void f1(void)
{
int x = 1;
y = x;
}
void f2(void)
{
int x;
y = x;
}
void test()
{
f1();
if (y)
printf("f1 set y to %d\n", y);
f2();
if (y)
printf("f2 set y to %d\n", y);
}
main()
{
char buf[16 * 1024];
test();
}
next prev parent reply other threads:[~2008-05-10 20:45 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-04-04 13:44 [ANNOUNCE] kmemcheck v7 Vegard Nossum
2008-04-04 13:45 ` [PATCH 1/3] kmemcheck: add the kmemcheck core Vegard Nossum
2008-04-04 13:46 ` [PATCH 2/3] x86: add hooks for kmemcheck Vegard Nossum
2008-04-04 13:47 ` [PATCH 3/3] slub: " Vegard Nossum
2008-05-10 9:07 ` [ANNOUNCE] kmemcheck v7 Bart Van Assche
2008-05-10 9:06 ` Pekka Enberg
2008-05-10 11:04 ` Bart Van Assche
2008-05-10 12:02 ` Vegard Nossum
2008-05-10 12:37 ` Andi Kleen
2008-05-10 13:22 ` Bart Van Assche
2008-05-10 17:17 ` Jeremy Fitzhardinge
2008-05-10 17:48 ` Andi Kleen
2008-05-10 20:45 ` Jeremy Fitzhardinge [this message]
2008-05-10 21:29 ` John Reiser
2008-05-10 23:05 ` Jeremy Fitzhardinge
2008-05-10 21:31 ` Andi Kleen
2008-05-10 22:59 ` Jeremy Fitzhardinge
2008-05-10 13:29 ` Bart Van Assche
2008-05-10 17:17 ` Jeremy Fitzhardinge
2008-05-10 20:35 ` Jeff Dike
2008-05-11 11:23 ` John Reiser
2008-05-11 12:08 ` John Reiser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4826095F.3070501@goop.org \
--to=jeremy@goop.org \
--cc=a.p.zijlstra@chello.nl \
--cc=andi@firstfloor.org \
--cc=bart.vanassche@gmail.com \
--cc=clameter@sgi.com \
--cc=dwalker@mvista.com \
--cc=jreiser@BitWagon.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luken@omner.org \
--cc=mingo@elte.hu \
--cc=paulmck@linux.vnet.ibm.com \
--cc=penberg@cs.helsinki.fi \
--cc=pq@iki.fi \
--cc=randy.dunlap@oracle.com \
--cc=vegard.nossum@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).