This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch] Bug 8287: Skip uninteresting functions while debugging
- From: Tom Tromey <tromey at redhat dot com>
- To: Justin Lebar <justin dot lebar at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 20 Jul 2010 15:03:43 -0600
- Subject: Re: [Patch] Bug 8287: Skip uninteresting functions while debugging
- References: <AANLkTil4y3Kc2Q-Lq06_1N7VKDImGqolTGECsU2VqdwF@mail.gmail.com> <m3aaqi4vyy.fsf@fleche.redhat.com> <AANLkTin-9AzwBWuk8KwchWebJ8MHfWZe-Z-N2eBQQoHe@mail.gmail.com>
>>>>> "Justin" == Justin Lebar <justin.lebar@gmail.com> writes:
Tom> One concern of mine is that this code will not perform well with many
Tom> blacklists in place. It may make sense to keep a hash table indexed by
Tom> name (function or file), to make lookups faster. What do you think?
Justin> I imagine you'd have to have a pretty large blacklist to notice
Justin> a delay when using gdb interactively, even on modest hardware.
Justin> I don't know much about gdb scripting -- this might matter more
Justin> in that case. But would one single-step in a script?
Justin> FWIW, enabling/disabling/deleting breakpoints takes time linear
Justin> to the number of breakpoints.
Yeah, good points, let's just leave it as is.
Tom> Another random thought is whether we want to be able to blacklist based
Tom> on objfile name.
Justin> That seems like it might be nice. Regular expression matching
Justin> on file and function names has also been suggested.
All good ideas; it is fine to defer these if you'd rather.
Justin> While I'm thinking about it, one problem I've run into while
Justin> using this patch while debugging Firefox is that we somehow load
Justin> multiple copies of our smart pointer header, so I have to
Justin> blacklist both ../../nsCOMPtr.h and ../../../nsCOMPtr.h. I need
Justin> to investigate further exactly what about our build system is
Justin> causing this to see if it's something which should be fixed in
Justin> gdb or Mozilla's build, but my guess is that we may want to
Justin> support this in gdb somehow.
Allowing regular expressions is a solid gdb tradition.
Justin> This patch also fixes bug 11614: decode_variable() in linespec.c
Justin> does not obey its contract
Tom> It is somewhat preferred in gdb to submit things like this as separate
Tom> patches.
Justin> I've split them out in this new set. Because you need the
Justin> bug11614 patch in order for the blacklist patch to work (and
Justin> since bug11614 is a trivial fix), I've included both attachments
Justin> in this one e-mail.
This patch is ok. Please check it in.
Mention the PR in the ChangeLog entry, like:
2010-06-25 Justin Lebar <justin.lebar@gmail.com>
PR gdb/11614:
* linespec.c (decode_variable): Passing a non-null not_found_ptr
[...]
If you do this, and then use the ChangeLog entry as the commit message,
the commit will show up in bugzilla.
Justin> + if (b->pc != 0)
Justin> + ui_out_field_core_addr (uiout, "addr", b->gdbarch, b->pc); /* 4 */
Justin> + else
Justin> + ui_out_field_string (uiout, "addr", "n/a"); /* 4 */
Justin> + }
Tom> A small nit, but I'm not sure about the exact string "n/a" here.
Tom> If there is an analogous situation elsewhere it would be good to just do
Tom> whatever is done there.
Justin> We could just use the empty string if that would be clearer.
Yeah, I think so.
Justin> + /* First, check whether the file or function this entry is pending on has
Justin> + been loaded. It might be more sensible to do this on a solib load,
Justin> + but that doesn't seem to work for some reason. */
Tom> Let's figure this out.
Justin> I spent a while in #gdb with a few people (I don't recall who)
Justin> and we couldn't figure this out. I added a solib load observer,
Justin> but from within it, find_pc_partial_function always failed.
Justin> I'm not sure where to look to figure out what's going on.
Offhand I'm not sure either, I think all you can do is debug the
particular call.
Tom> Also, since the blacklist includes a PC value, I think you have to reset
Tom> it when re-running the inferior, and on some other state changes as well.
Justin> Hm. How do we handle this with breakpoints?
See breakpoint_re_set.
Tom> I think it would be more usual to make the commands "enable blacklist ..."
Tom> instead of "blacklist enable ...". Likewise for disable and delete.
Justin> I'm not sure I like "enable blacklist". The problem is that
Justin> "blacklist" is a noun referring to the list of all functions we
Justin> skip and also a verb meaning "add an entry to the blacklist".
Justin> "Enable blacklist" sounds to me like we're enabling an entire
Justin> list of blacklist entries.
Yeah, that makes sense.
Justin> Maybe we can name the thing which indicates that we shouldn't
Justin> step into a function something less confusing than a "blacklist
Justin> entry"?
Yes, that would be good. Any ideas for names?
Tom