This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Speed up find_pc_section
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Paul Pluzhnikov <ppluzhnikov at google dot com>
- Cc: Ulrich Weigand <uweigand at de dot ibm dot com>, Ulrich Weigand <Ulrich dot Weigand at de dot ibm dot com>, gdb-patches ml <gdb-patches at sourceware dot org>, Tom Tromey <tromey at redhat dot com>
- Date: Tue, 8 Sep 2009 22:38:48 -0700
- Subject: Re: [patch] Speed up find_pc_section
- References: <8ac60eac0908201340k6b759eb5o9bb73c8f473d8785@mail.gmail.com> <200908211130.n7LBUCJc011108@d12av02.megacenter.de.ibm.com> <8ac60eac0908231548x135edf2doa04fa59a49455bcd@mail.gmail.com>
> > (A cleaner fix might be to fix all callers to never pass in a NULL
> > section argument --most already don't-- and simply rely on it.)
>
> Attached patch does that ...
> Not tested (I still haven't figured out how to test overlays).
This is just an informal review of the patch, since it has not been
tested (I assume one would like to test it in an overlay environment
as well - maybe Ulrich could take care of that; otherwise we'll have
to do without).
I feel like a dummy, sometimes, but it took me a while to understand
why find_pc_section might return NULL if PC is inside an overlay
section. I guess I overlooked the fact that the section might not
be mapped...
> 2009-08-23 Paul Pluzhnikov <ppluzhnikov@google.com>
>
> * minsyms.c (lookup_minimal_symbol_by_pc_section_1): Assume
> non-NULL section.
The function description needs to be updated. I would also add
an assertion that section is not NULL.
> (lookup_minimal_symbol_by_pc_section): Check for NULL section.
> (lookup_minimal_symbol_by_pc): Adjust.
Looks like lookup_minimal_symbol_by_pc is now almost a duplicate
of lookup_minimal_symbol_by_pc_section. We could replace its
implementation by:
struct minimal_symbol *
lookup_minimal_symbol_by_pc (CORE_ADDR pc)
{
return lookup_minimal_symbol_by_pc_section (pc, NULL);
}
The comment from AndrewC needs to be preserved, but applies equally
to lookup_minimal_symbol_by_pc_section, I believe.
> /* We can not require the symbol found to be in pc_section, because
^^^^^^^^^^ section
> e.g. IRIX 6.5 mdebug relies on this code returning an absolute
Otherwise, the patch seems fine. I would love some input from Ulrich,
though.
--
Joel