Stack changes and user-land thread packages

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Stack changes and user-land thread packages

rjwalsh (Bugzilla)
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 = 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.

--
Robert Walsh
Amalgamated Durables, Inc.  -  "We don't make the things you buy."
Email: [hidden email]

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

Julian Seward-2

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


-------------------------------------------------------
This SF.Net email is sponsored by Yahoo.
Introducing Yahoo! Search Developer Network - Create apps using Yahoo!
Search APIs Find out how you can build Yahoo! directly into your own
Applications - visit http://developer.yahoo.net/?fr=offad-ysdn-ostg-q22005
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

rjwalsh (Bugzilla)
> 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?

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.

--
Robert Walsh
Amalgamated Durables, Inc.  -  "We don't make the things you buy."
Email: [hidden email]

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

njn (Bugzilla)-2
In reply to this post by rjwalsh (Bugzilla)
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


-------------------------------------------------------
This SF.Net email is sponsored by Yahoo.
Introducing Yahoo! Search Developer Network - Create apps using Yahoo!
Search APIs Find out how you can build Yahoo! directly into your own
Applications - visit http://developer.yahoo.net/?fr=offad-ysdn-ostg-q22005
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

njn (Bugzilla)-2
In reply to this post by rjwalsh (Bugzilla)
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


-------------------------------------------------------
This SF.Net email is sponsored by Yahoo.
Introducing Yahoo! Search Developer Network - Create apps using Yahoo!
Search APIs Find out how you can build Yahoo! directly into your own
Applications - visit http://developer.yahoo.net/?fr=offad-ysdn-ostg-q22005
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

rjwalsh (Bugzilla)
In reply to this post by njn (Bugzilla)-2
On Mon, 2005-05-30 at 14:41 -0500, Nicholas Nethercote wrote:

> 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.
Yes.  I'll do that.

Regards,
 Robert.

--
Robert Walsh
Amalgamated Durables, Inc.  -  "We don't make the things you buy."
Email: [hidden email]

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

rjwalsh (Bugzilla)
In reply to this post by njn (Bugzilla)-2
> 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.

> 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.

Regards,
 Robert.

--
Robert Walsh
Amalgamated Durables, Inc.  -  "We don't make the things you buy."
Email: [hidden email]

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

njn (Bugzilla)-2
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


-------------------------------------------------------
This SF.Net email is sponsored by Yahoo.
Introducing Yahoo! Search Developer Network - Create apps using Yahoo!
Search APIs Find out how you can build Yahoo! directly into your own
Applications - visit http://developer.yahoo.net/?fr=offad-ysdn-ostg-q22005
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|

Re: Stack changes and user-land thread packages

rjwalsh (Bugzilla)
> Oh, yes, you'd have to expose VG_(handle_stack_new) to the client.
> Scratch that, then.

Fair enough.

Thanks for the feedback.

Regards,
 Robert.

--
Robert Walsh
Amalgamated Durables, Inc.  -  "We don't make the things you buy."
Email: [hidden email]

signature.asc (196 bytes) Download Attachment