This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: WIP fix for PR 12997


On Mon, Mar 19, 2012 at 11:58:11AM -0600, Tom Tromey wrote:
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
> 
> Tom> Now that you mention it, though, I realize my approach is wrong.
> Tom> I have to also store a per-module "read the TUs" flag somewhere.
> Tom> Oops.  I will fix this.
> 
> Here's another iteration.

This is looking good. One tiny remark/nitpick below.

> This time, it failed again, and I tracked it down to an apparent libdw
> bug.  I sent a patch to the elfutils list.
> 
> I suppose this means that you will need an elfutils release and a
> dependency bump in systemtap before applying this.

Unless the ABI changes, I think this is fine. Things didn't work
before when stap encountered debug_types. Now it works better, but
only if you have a newer elfutils. We can just add that hint to the
NEWS file. We could also add an elfutils version check somewhere and
add an explicit warn when we encounter debug_types with an old elfutils.

>    void setup_user(const std::vector<std::string>& modules, bool debuginfo_needed = true);
>  
>    module_cu_cache_t module_cu_cache;
> +  std::set<Dwarf*> module_tus_read;
>    mod_cu_function_cache_t cu_function_cache;
>    mod_function_cache_t mod_function_cache;

Could you do this as done with the other types used here?
That is, define a typedef in dwflpp.h for this with a quick
comment documenting its use? I know, nitpicking, but it really
helps to have all types used documented in one place. And yes,
the documentation that is there in dwflpp.h isn't that great,
but it is better than nothing.

Thanks,

Mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]