Re: [patch] Speed up find_pc_section

Joel Brobecker
2009-09-09T05:39:06Z

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