|
From: Robert W. <rj...@du...> - 2005-05-30 07:08:18
|
Hi all, I've put a new patch on my web site against the 2.4 tree: http://www.durables.org/~rjwalsh/software/valgrind The patch allows user-land thread packages (e.g. coroutines) to deal with stack changes gracefully. Currently, Valgrind notices when %esp changes and uses a heuristic to figure out if that possibly means a thread change, or just that something large was pushed onto the stack. It often gets it wrong, since the heuristic is basically: if delta %esp > some value, then assume a thread change The problem is, it's hard to know what a good "some value" is since it depends a lot of how your program is written. The change I've made allows you to register a memory range as a stack with Valgrind. Valgrind spots when %esp changes and checks if it's changed to a different registered stack. If it has, then it assumes a thread change. Usage: * id =3D VALGRIND_STACK_NEW(start, end) registers the memory between start and end as a new stack and returns an id you can use for the other client requests described below. This doesn't allocate memory or anything - it just records that a particular memory range is a stack. * VALGRIND_STACK_DELETE(id) removes the stack association for stack "id". This doesn't free memory or anything - it just tells Valgrind that the piece of memory register earlier is no longer a stack. * VALGRIND_STACK_CHANGE(id, start, end) changes the stack "id" to a new start-end range. Useful if you're implementing stack growth in user-land, too. See the stack_changes.c file in corecheck/tests for an example. Some points: * This is currently only against the 2.4 tree. I'll port it to 3.0 in the next day or so. * I haven't measured this for performance impact yet. * I haven't tested VALGRIND_STACK_DELETE at all. * I've only tested VALGRIND_STACK_CHANGE for the default process stack, not for anything registered by the user. * Right now, it falls back to the old algorithm if the new one doesn't indicate a stack change. Probably this should just go away. * It could probably be made a bit more efficient by using an interval skip-list to store the registered stack data instead of a simple linked-list. After I see the performance impact, I'll decide about this. If you have any thoughts, let me know. If there's going to be another 2.4 release, this might be useful to include. Regards, Robert. --=20 Robert Walsh Amalgamated Durables, Inc. - "We don't make the things you buy." Email: rj...@du... |
|
From: Julian S. <js...@ac...> - 2005-05-30 08:09:54
|
Robert Interesting patch. I was wondering if you can simply use the --max-stackframe flag in the 3 line to get the same result .. > if delta %esp > some value, then assume a thread change This allows you to set 'some value' on the command line. Furthermore if you don't do that it will tell you plausible values to try anyway. Could you try out --max-stackframe to see if it is good enough? J |
|
From: Robert W. <rj...@du...> - 2005-05-30 17:37:59
|
> I was wondering if you can simply use the --max-stackframe flag in > the 3 line to get the same result .. >=20 > > if delta %esp > some value, then assume a thread change >=20 > This allows you to set 'some value' on the command line. =20 > Furthermore if you don't do that it will tell you plausible > values to try anyway. >=20 > Could you try out --max-stackframe to see if it is good enough? I haven't tried this, but I'm fairly certain that it won't work in the case of the coroutine library we're using at work. We use lots of coroutines (thousands) each with a relatively small stack (1K, and maybe less when we get around to tuning it) that's allocated using malloc. They end up essentially being right next to each other in memory. The main CPU stack still needs to be able to handle big stack allocations, as it does all of the nasty GUI work, etc. when the coroutines are done. I like the idea of being able to have the client program explain where stacks are. For programs that don't do that, --max-stackframe is a good idea, but for programs that do, it means you don't have to have magic numbers on the command line or worry about different classes of stack (big v. small like we have.) Regards, Robert. --=20 Robert Walsh Amalgamated Durables, Inc. - "We don't make the things you buy." Email: rj...@du... |
|
From: Nicholas N. <nj...@cs...> - 2005-05-30 19:41:50
|
On Mon, 30 May 2005, Robert Walsh wrote: > * id = VALGRIND_STACK_NEW(start, end) registers the memory between > start and end as a new stack and returns an id you can use for > the other client requests described below. This doesn't > allocate memory or anything - it just records that a particular > memory range is a stack. > > * VALGRIND_STACK_DELETE(id) removes the stack association for > stack "id". This doesn't free memory or anything - it just > tells Valgrind that the piece of memory register earlier is no > longer a stack. Perhaps VALGRIND_STACK_REGISTER and VALGRIND_STACK_DEREGISTER would be better names? You'd avoid the confusion with memory allocation. N |
|
From: Robert W. <rj...@du...> - 2005-05-30 21:30:46
|
On Mon, 2005-05-30 at 14:41 -0500, Nicholas Nethercote wrote: > On Mon, 30 May 2005, Robert Walsh wrote: >=20 > > * id =3D VALGRIND_STACK_NEW(start, end) registers the memory betwe= en > > start and end as a new stack and returns an id you can use for > > the other client requests described below. This doesn't > > allocate memory or anything - it just records that a particular > > memory range is a stack. > > > > * VALGRIND_STACK_DELETE(id) removes the stack association for > > stack "id". This doesn't free memory or anything - it just > > tells Valgrind that the piece of memory register earlier is no > > longer a stack. >=20 > Perhaps VALGRIND_STACK_REGISTER and VALGRIND_STACK_DEREGISTER would be=20 > better names? You'd avoid the confusion with memory allocation. Yes. I'll do that. Regards, Robert. --=20 Robert Walsh Amalgamated Durables, Inc. - "We don't make the things you buy." Email: rj...@du... |
|
From: Nicholas N. <nj...@cs...> - 2005-05-30 19:46:42
|
On Mon, 30 May 2005, Robert Walsh wrote: > Hi all, > > I've put a new patch on my web site against the 2.4 tree: > > http://www.durables.org/~rjwalsh/software/valgrind Another comment: you've introduced new VG_USERREQ__* constants. These could be avoided by using the VG_USERREQ__CLIENT_CALL* constants to call VG_(handle_stack_new)() and the other functions. The functions would have to take a ThreadId as their first arg, but that's not difficult. N |
|
From: Robert W. <rj...@du...> - 2005-05-30 21:42:00
|
> Another comment: you've introduced new VG_USERREQ__* constants. These=20
> could be avoided by using the VG_USERREQ__CLIENT_CALL* constants to call=20
> VG_(handle_stack_new)() and the other functions.
That would be nice. How does one use these, exactly? I can't find
anything that currently uses them to use as an example. It looks like
it should be something like this:
#define VALGRIND_STACK_REGISTER(start, end) \
VALGRIND_NON_SIMD_CALL2(VG_(handle_stack_new), start, end)
But wouldn't that mean having to expose the VG_(...) stuff into user
land, or something? I'm a little confused by these, I guess.
> The functions would have=20
> to take a ThreadId as their first arg, but that's not difficult.
Really? I don't see that in the code anywhere:
case VG_USERREQ__CLIENT_CALL2: {
UWord (*f)(UWord, UWord) =3D (void*)arg[1];
if (f =3D=3D NULL)
VG_(message)(Vg_DebugMsg, "VG_USERREQ__CLIENT_CALL2: func=3D%p\=
n", f);
else
SET_CLCALL_RETVAL(tid, f ( arg[2], arg[3] ), (Addr)f );
break;
}
The function is invoked only with the args passed into the client
request.
Regards,
Robert.
--=20
Robert Walsh
Amalgamated Durables, Inc. - "We don't make the things you buy."
Email: rj...@du...
|
|
From: Nicholas N. <nj...@cs...> - 2005-05-30 22:10:55
|
On Mon, 30 May 2005, Robert Walsh wrote:
>> Another comment: you've introduced new VG_USERREQ__* constants. These
>> could be avoided by using the VG_USERREQ__CLIENT_CALL* constants to call
>> VG_(handle_stack_new)() and the other functions.
>
> That would be nice. How does one use these, exactly? I can't find
> anything that currently uses them to use as an example. It looks like
> it should be something like this:
>
> #define VALGRIND_STACK_REGISTER(start, end) \
> VALGRIND_NON_SIMD_CALL2(VG_(handle_stack_new), start, end)
>
> But wouldn't that mean having to expose the VG_(...) stuff into user
> land, or something? I'm a little confused by these, I guess.
Oh, yes, you'd have to expose VG_(handle_stack_new) to the client.
Scratch that, then.
>> The functions would have
>> to take a ThreadId as their first arg, but that's not difficult.
>
> Really? I don't see that in the code anywhere:
>
> case VG_USERREQ__CLIENT_CALL2: {
> UWord (*f)(UWord, UWord) = (void*)arg[1];
> if (f == NULL)
> VG_(message)(Vg_DebugMsg, "VG_USERREQ__CLIENT_CALL2: func=%p\n", f);
> else
> SET_CLCALL_RETVAL(tid, f ( arg[2], arg[3] ), (Addr)f );
> break;
> }
>
> The function is invoked only with the args passed into the client
> request.
It's a 2.4 --> 3.0 change.
Nick
|
|
From: Robert W. <rj...@du...> - 2005-05-30 22:11:32
|
> Oh, yes, you'd have to expose VG_(handle_stack_new) to the client.=20 > Scratch that, then. Fair enough. Thanks for the feedback. Regards, Robert. --=20 Robert Walsh Amalgamated Durables, Inc. - "We don't make the things you buy." Email: rj...@du... |