|
From: Avery P. <ape...@ni...> - 2003-09-29 23:03:21
|
Hi all, I have a fun problem with valgrind in the WvTask part of my WvStreams library (http://open.nit.ca/wiki?WvStreams). I can make a minimal test case for you if necessary, but the idea is this: setjmp(top); alloca(1024*1024); // reserve space on stack for toplevel setjmp(inside); longjmp(top); This allows me to allocate "extra stacks" within my own primary stack, rather than writing arch-specific stack changing code that uses malloc(). Unfortunately, next time I longjmp(inside) and try to use one of my new local variables, I get warnings like this: Conditional jump or move depends on uninitialised value(s) Use of uninitialised value of size 4 etc. My favourite one is on the following assertion: assert(magic_number == -WVTASK_MAGIC); (where WVTASK_MAGIC is a constant). This code is from my pre-valgrind days, and I used it to catch stack overflows. Entertainingly, the assertion passes (exactly the right 32-bit number is where it should be), but valgrind insists that I'm accessing uninitialized memory. I gather this is happening to me because of the following bit from the manual: * When the stack pointer register (%esp) moves up or down, A bits are set. The rule is that the area from %esp up to the base of the stack is marked as accessible, and below %esp is inaccessible. But in my case, I need the area below %esp to not be marked inaccessible. I really want only the memory below *the minimum historical value of %esp* to be inaccessible. I've tried using VALGRIND_MAKE_READABLE, but that appears to not work on the stack, or something. It doesn't seem to help, anyway. This is valgrind 20030725. Any suggestions? Thanks, Avery |
|
From: Nicholas N. <nj...@ca...> - 2003-11-01 12:33:53
|
On Fri, 26 Sep 2003, Avery Pennarun wrote: > I have a fun problem with valgrind in the WvTask part of my WvStreams > library (http://open.nit.ca/wiki?WvStreams). I can make a minimal test case > for you if necessary, but the idea is this: > > setjmp(top); > alloca(1024*1024); // reserve space on stack for toplevel > setjmp(inside); > longjmp(top); > > This allows me to allocate "extra stacks" within my own primary stack, > rather than writing arch-specific stack changing code that uses malloc(). > Unfortunately, next time I longjmp(inside) and try to use one of my new > local variables, I get warnings like this: > > Conditional jump or move depends on uninitialised value(s) > Use of uninitialised value of size 4 > > etc. > > My favourite one is on the following assertion: > > assert(magic_number == -WVTASK_MAGIC); > > (where WVTASK_MAGIC is a constant). This code is from my pre-valgrind days, > and I used it to catch stack overflows. Entertainingly, the assertion > passes (exactly the right 32-bit number is where it should be), but valgrind > insists that I'm accessing uninitialized memory. > > I gather this is happening to me because of the following bit from the > manual: > > * When the stack pointer register (%esp) moves up or down, A bits > are set. The rule is that the area from %esp up to the base of the > stack is marked as accessible, and below %esp is inaccessible. > > But in my case, I need the area below %esp to not be marked inaccessible. I > really want only the memory below *the minimum historical value of %esp* to > be inaccessible. > > I've tried using VALGRIND_MAKE_READABLE, but that appears to not work on the > stack, or something. It doesn't seem to help, anyway. Firstly, sorry for taking so long to reply. What you are doing is certainly quite unusual, and hence doesn't conform to Valgrind's expectations of normal stack behaviour. When the stack grows, the memory from old_esp..new_esp is marked as accessible, but uninitialised. When the stack shrinks, the memory from old_esp..new_esp is marked as inaccessible, and uninitialised. I think this is why you're getting the uninitialised errors. Where did you insert the VALGRIND_MAKE_READABLE call? I'm pretty sure that macro will work on the stack. If I've understood your technique, I think you want to do this: remember esp alloc big new chunk on the stack restore esp call VALGRIND_MAKE_READABLE on the new chunk The critical thing being that you call the macro after you restore %esp. But I'm not sure I understand entirely. Also, it's possible that Valgrind thinks your program is switching stacks, which might be causing the confusion. N |
|
From: Avery P. <ape...@ni...> - 2003-11-01 20:52:18
|
On Sat, Nov 01, 2003 at 12:33:51PM +0000, Nicholas Nethercote wrote: > Firstly, sorry for taking so long to reply. No problem, thanks for replying at all :) > When the stack grows, the memory from old_esp..new_esp is marked as > accessible, but uninitialised. When the stack shrinks, the memory from > old_esp..new_esp is marked as inaccessible, and uninitialised. I think > this is why you're getting the uninitialised errors. That's what I thought. > Where did you insert the VALGRIND_MAKE_READABLE call? I'm pretty sure > that macro will work on the stack. If I've understood your technique, I > think you want to do this: I don't remember now; I'll play with it some more, see if I can do it properly, and maybe send some sample code. > The critical thing being that you call the macro after you restore %esp. That may be what I was doing wrong. > Also, it's possible that Valgrind thinks your program is switching stacks, > which might be causing the confusion. I don't think so - it started warning me about switching stacks only after I enabled the stack-switch-detection feature, IIRC, and that didn't help. Thanks, Avery |
|
From: Avery P. <ape...@ni...> - 2003-11-02 20:20:41
|
On Sat, Nov 01, 2003 at 12:33:51PM +0000, Nicholas Nethercote wrote:
> Where did you insert the VALGRIND_MAKE_READABLE call? I'm pretty sure
> that macro will work on the stack. If I've understood your technique, I
> think you want to do this:
>
> remember esp
> alloc big new chunk on the stack
> restore esp
> call VALGRIND_MAKE_READABLE on the new chunk
>
> The critical thing being that you call the macro after you restore %esp.
Okay, so whatever I was doing before was definitely wrong, because I
followed this advice and stopped getting spurious valgrind errors. Yay!
Now, I think this disables a certain amount of valgrind checking. That is:
int x;
if (!setjmp)
longjmp(elsewhere)
// else someone longjmped back to me
VALGRIND_MAKE_READABLE(blah blah)
printf("x is now %d\n", x);
In this example, valgrind won't be able to discover that x was
uninitialized, since the memory was already marked readable to compensate
for it spuriously being marked *unreadable* earlier.
This actually doesn't bother me too much, because I expect to have rather
few errors with this problem: usually the function runs quite far before
doing the setjmp/longjmp, so 99% of the time it will have initialized the
variables by then anyway. Also, any functions I call from inside this one
will have valgrind do the right thing.
The inaccuracy, however, lies in throwing away data and then trying to
reconstruct it inaccurately.
Since I have total control over the setjmp/longjmp calls, would it be
possible to add a feature to valgrind like
if (!setjmp)
VALGRIND_STOP_INVALIDATING
longjmp(elsewhere)
// else someone longjmped back to me
VALGRIND_START_INVALIDATING
printf("x is now %d\n", x);
It would then need to be sure to invalidate only the areas between the old
and new stack pointer whenever %sp is changed (the normal way) rather than
blindly invalidating "everything under the current stack pointer."
This is pretty low priority, however. Thanks to your original message my
unit tests finally run under valgrind and I fixed two bugs last night thanks
to that.
Have fun,
Avery
P.S. Jeremy: I know the amount of stack usage by the kernel, signals, etc is
"undefined", but if you make the sub-stack big enough, all is mostly well.
sigaltstack() is probably a good idea though, you're right.
|
|
From: Peter \Firefly\ L. <fi...@di...> - 2003-11-03 11:50:43
|
On Sun, 2 Nov 2003, Avery Pennarun wrote:
> int x;
shouldn't x be volatile in any case?
(otherwise it might be stored in a register at setjmp/longjmp time -- and
they are not required to save/restore registers so when another task
longjmps back to your setjmp x will be trashed)
> if (!setjmp)
> longjmp(elsewhere)
> // else someone longjmped back to me
> VALGRIND_MAKE_READABLE(blah blah)
> printf("x is now %d\n", x);
What you would really like is to save/restore the validity bits for a
whole memory range together with your setjmp/longjmp. A stack of validity
stored bits won't be enough - you'll either need access to some id for
each range (and have valgrind manage the memory) or you'll need some sort
of escape hatch to copy these bits between valgrind and your memory space.
valgrind doesn't provide either at the moment, does it?
-Peter
|
|
From: Avery P. <ape...@ni...> - 2003-11-03 18:31:31
|
On Mon, Nov 03, 2003 at 12:50:40PM +0100, Peter Firefly Lund wrote:
> On Sun, 2 Nov 2003, Avery Pennarun wrote:
>
> > int x;
>
> shouldn't x be volatile in any case?
>
> (otherwise it might be stored in a register at setjmp/longjmp time -- and
> they are not required to save/restore registers so when another task
> longjmps back to your setjmp x will be trashed)
I'm willing to be proven wrong, but my understanding is that setjmp and
longjmp are allowed to trash any registers that any other function call is
allowed to trash - no more, no less. Therefore, the compiler should do the
right thing even if x isn't volatile.
> > if (!setjmp)
> > longjmp(elsewhere)
> > // else someone longjmped back to me
> > VALGRIND_MAKE_READABLE(blah blah)
> > printf("x is now %d\n", x);
>
> What you would really like is to save/restore the validity bits for a
> whole memory range together with your setjmp/longjmp. A stack of validity
> stored bits won't be enough - you'll either need access to some id for
> each range (and have valgrind manage the memory) or you'll need some sort
> of escape hatch to copy these bits between valgrind and your memory space.
Not really - imagine if I do
int x;
obj->set_xptr(&x);
if (!setjmp)
longjmp(someone who sets x);
// else someone longjmped back to me
printf("x is now %d\n", x);
I expect the flags for 'x' to not be wiped by longjmp *and* to be different
upon return than they were when I left. Suspending the auto-flag-changes
temporarily, then resuming them, would do this.
Revalidating the entire stack will also work, and that's what I'm doing
right now, but it loses some of valgrind's protection.
Have fun,
Avery
|
|
From: Nicholas N. <nj...@ca...> - 2003-11-09 17:11:11
|
On Sun, 2 Nov 2003, Avery Pennarun wrote:
> Since I have total control over the setjmp/longjmp calls, would it be
> possible to add a feature to valgrind like
>
> if (!setjmp)
> VALGRIND_STOP_INVALIDATING
> longjmp(elsewhere)
> // else someone longjmped back to me
> VALGRIND_START_INVALIDATING
> printf("x is now %d\n", x);
>
> It would then need to be sure to invalidate only the areas between the old
> and new stack pointer whenever %sp is changed (the normal way) rather than
> blindly invalidating "everything under the current stack pointer."
>
> This is pretty low priority, however.
Indeed. I don't think this feature is likely to be added, since it would
only ever be used in extremely obscure circumstances, which is exactly the
kind of feature we don't like adding. Sorry about that.
(I still don't quite understand why you need to use this unusual
setjmp/longjmp thing instead of switching stack in a more normal way,
although I'm sure you have a good reason.)
N
|
|
From: Avery P. <ape...@ni...> - 2003-11-09 22:57:34
|
On Sun, Nov 09, 2003 at 05:11:08PM +0000, Nicholas Nethercote wrote:
> On Sun, 2 Nov 2003, Avery Pennarun wrote:
>
> > if (!setjmp)
> > VALGRIND_STOP_INVALIDATING
> > longjmp(elsewhere)
> > // else someone longjmped back to me
> > VALGRIND_START_INVALIDATING
> > printf("x is now %d\n", x);
>
> I don't think this feature is likely to be added, since it would
> only ever be used in extremely obscure circumstances, which is exactly the
> kind of feature we don't like adding. Sorry about that.
Yeah, I understand and agree with that philosophy myself, so I'll try to
keep my disappointment to a minimum. Anyway, lacking this feature only
results in false negatives (valgrind doesn't find certain unlikely kinds of
coding errors) so I'm pretty safe.
> (I still don't quite understand why you need to use this unusual
> setjmp/longjmp thing instead of switching stack in a more normal way,
> although I'm sure you have a good reason.)
Now you're just being mean. From coregeind_core.html:
* Programs which switch stacks are not well handled. Valgrind does have
support for this, but I don't have great faith in it. It's difficult --
there's no cast-iron way to decide whether a large change in %esp is as a
result of the program switching stacks, or merely allocating a large object
temporarily on the current stack -- yet Valgrind needs to handle the two
situations differently.
*My* way of switching stacks, on the other hand, should at least behave 100%
consistently in valgrind and I don't have to depend on anything except
VALGRIND_MAKE_READABLE working - because I'm not actually switching stacks,
I'm just moving the stack pointer around *inside* the stack in a
well-understood way.
I originally did this for portability (WvTask works in Turbo C++ for DOS as
well as Windows, Unix, and probably any other ANSI C system). Little did I
realize before how useful this portability would be when I started using
valgrind :)
Have fun,
Avery
|
|
From: Jeremy F. <je...@go...> - 2003-11-01 21:33:03
|
On Fri, 2003-09-26 at 20:46, Avery Pennarun wrote: > I have a fun problem with valgrind in the WvTask part of my WvStreams > library (http://open.nit.ca/wiki?WvStreams). I can make a minimal test case > for you if necessary, but the idea is this: > > setjmp(top); > alloca(1024*1024); // reserve space on stack for toplevel > setjmp(inside); > longjmp(top); > > This allows me to allocate "extra stacks" within my own primary stack, > rather than writing arch-specific stack changing code that uses malloc(). Do you do something special with signals here (like block them all, or use a sigaltstack)? If not, then a signal could come in and trash whatever is below %esp at any time. On some systems/CPUs, it could be a quite lot of memory (>4k). > But in my case, I need the area below %esp to not be marked inaccessible. I > really want only the memory below *the minimum historical value of %esp* to > be inaccessible. In general, everything below the current %esp is considered to have undefined values because the kernel may trash it at any time. You'd probably be better off just allocating a chunk of memory and using that for stacks. It takes about as much time to extend the stack by 1MByte as do a malloc/brk/mmap to explicitly allocate the memory (it may even be faster to do a syscall than take the page fault trap). J |