|
From: Jeremy F. <je...@go...> - 2005-03-28 00:22:16
|
Nicholas Nethercote wrote:
>>
>> I was hoping to drop all the magic symbol stuff altogether. I would
>> prefer it if each tool .so had only a single special symbol to an
>> init function; that init function would then call a core function
>> passing a structure full of pointers to functions for all its tool
>> functions. I don't really like all this implicit behaviour based on
>> what symbols happen to exist. I intended the dlsym() stuff to be a
>> backwards compatibility thing which would go away once all the tools
>> had been converted.
>
>
> I see. So in the same way tools register their 'tracking' functions, eg:
>
> VG_(init_new_mem_startup) ( & ac_new_mem_startup );
> VG_(init_new_mem_stack_signal) ( & ac_make_accessible );
>
> they could do the same things for all the other functions, eg:
>
> VG_(init_post_clo_init) ( & ac_post_clo_init );
> VG_(init_instrument) ( & ac_instrument );
> VG_(init_handle_client_request)( & ac_handle_client_request );
>
> (That's slightly different your suggestion of passing a struct of
> function pointers, but very similar.)
Yes, I think that's better because it removes the fragility of having a
structure as part of the interface.
[
As an aside, it's one of the things which makes OpenGL one of my
favorite APIs in terms of design. The API/ABI has no types more
complex than an array of scalars, which makes it very easy to have
multiple ABI-compatible implementations, and easy to maintain ABI
compatibility over many API revisions. OpenGL has lots of state,
but it is managed by lots of simple entrypoints which handle only
one aspect at a time (though some of those functions are generic
glGet(value_enum) form).
The tool interface is going to be a bit more complex, but the fewer
complex types the better if we actually want to achieve any kind of
long-term ABI stability.
]
> You're right about the defined_* ones, but the missing_* ones can be
> removed. They don't need to be weak either.
That's true if you've removed the USERREQ_MALLOC/FREE requests. The
missing_* weak-override stuff was used so that there is some
implementation for the MALLOC/FREE requests if the tool doesn't handle it.
>> The pthread_create code wants to allocate some memory in the client
>> which the client can free via VG_USERREQ_FREE. I had to make a
>> couple of small changes so this would work for both malloc-replacing
>> and non-malloc-replacing tools.
>
>
> Ah crap, I recently removed VG_USERREQ_{MALLOC,FREE} from the SVN repo
> because they weren't being used.
>
> But what is this memory that's being allocated by pthread_create()
> for? Will the client have to use VG_USERREQ_FREE for the
> thread-modelling to not leak memory? That would not be nice.
The basic problem is that the thread modelling code needs to know the
mapping between a libpthread thread identifier (pthread_t) and the
underlying Valgrind thread ID. The way I implement this is by
repointing the thread-start function to an intercept function in
vg_intercept.c which does a pthread_self() call; the pthread modelling
code wraps this pthread_self call and looks at the return value.
Because it knows what ThreadID is making the call, it can create the
mapping from pthread_t -> ThreadID.
This is implemented by allocating a small piece of memory (wrapper info,
known as vg_pthread_newthread_data) in the pthread_create "before"
wrapper function, which stores the original (start_func, arg) pair. It
then replaces the arguments to pthread_create()-proper with a
(start_func_wrapper, wrapper_info_ptr) pair. When the start_func_wrapper
gets called in the new thread, it calls pthread_self(), extracts the
original function and argument from the wrapper info, frees the wrapper
info and calls the original (*start_func)(arg).
By necessesity, the memory needs to be allocated within Valgrind, but
freed by the client code. We could add a special-case client callback
to free this specific piece of memory, but USERREQ_FREE seemed like the
right thing to use since it was there. Also, since this is allocated
memory being used by the VCPU, all its uses are subject to inspection by
the tool, so it has to all look OK to the tool (or suppressed around,
which definitely isn't nice).
J
|