Re: [PING^2] [PATCH] Cache the text section offset of shared libraries

[Jon Turney <jon dot turney at dronecode dot org dot uk>]

On 12/01/2020 00:03, Hannes Domani via gdb-patches wrote:
> Ping.

This seems ok for what is it is, but it would be easier to evaluate if 
you had some commentary on why it's useful (e.g. why is 
pe_text_section_offset() slow?  why is windows_xfer_shared_library() 
called multiple times for the same library?)

> Am Samstag, 4. Januar 2020, 18:55:08 MEZ hat Hannes Domani via gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:
>
> > Ping.
> >
> > Am Samstag, 21. Dezember 2019, 17:33:49 MEZ hat Hannes Domani via gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:
> >
> > > This improves the startup time of an application with
> > > > 300 dynamically loaded plugins from 2m10s to 10s.
> > > And the shutdown time improves from 2m to 2s.
> > >
> > > gdb/ChangeLog:
> > >
> > > 2019-12-21  Hannes Domani  <ssbssa@yahoo.de>
> > >
> > >       * i386-cygwin-tdep.c (core_process_module_section): Update.
> > >       * windows-nat.c (struct lm_info_windows): Add text_offset.
> > >       (windows_xfer_shared_libraries): Update.
> > >       * windows-tdep.c (windows_xfer_shared_library):
> > >       Add text_offset_cached argument.
> > >       * windows-tdep.h (windows_xfer_shared_library): Update.
> > > ---
> > > gdb/i386-cygwin-tdep.c |  2 +-
> > > gdb/windows-nat.c      |  2 ++
> > > gdb/windows-tdep.c    | 20 ++++++++++++++------
> > > gdb/windows-tdep.h    |  1 +
> > > 4 files changed, 18 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/gdb/i386-cygwin-tdep.c b/gdb/i386-cygwin-tdep.c
> > > index 25c3cfcbb9..6bf6166f73 100644
> > > --- a/gdb/i386-cygwin-tdep.c
> > > +++ b/gdb/i386-cygwin-tdep.c
> > > @@ -137,7 +137,7 @@ core_process_module_section (bfd *abfd, asection *sect, void *obj)
> > >     /* The first module is the .exe itself.  */
> > >     if (data->module_count != 0)
> > >       windows_xfer_shared_library (module_name, base_addr,
> > > -                data->gdbarch, data->obstack);
> > > +                NULL, data->gdbarch, data->obstack);
> > >     data->module_count++;
> > >
> > > out:
> > > diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
> > > index 10d5c95d09..6216af8175 100644
> > > --- a/gdb/windows-nat.c
> > > +++ b/gdb/windows-nat.c
> > > @@ -679,6 +679,7 @@ windows_nat_target::store_registers (struct regcache *regcache, int r)
> > > struct lm_info_windows : public lm_info_base
> > > {
> > >     LPVOID load_addr = 0;
> > > +  CORE_ADDR text_offset = 0;
> > > };
> > >
> > > static struct so_list solib_start, *solib_end;
> > > @@ -2949,6 +2950,7 @@ windows_xfer_shared_libraries (struct target_ops *ops,
> > >
> > >         windows_xfer_shared_library (so->so_name, (CORE_ADDR)
> > >                     (uintptr_t) li->load_addr,
> > > +                  &li->text_offset,
> > >                     target_gdbarch (), &obstack);
> > >       }
> > >     obstack_grow_str0 (&obstack, "</library-list>\n");
> > > diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> > > index bb69a79996..6fb119361b 100644
> > > --- a/gdb/windows-tdep.c
> > > +++ b/gdb/windows-tdep.c
> > > @@ -373,19 +373,27 @@ display_tib (const char * args, int from_tty)
> > >
> > > void
> > > windows_xfer_shared_library (const char* so_name, CORE_ADDR load_addr,
> > > +                CORE_ADDR *text_offset_cached,
> > >                   struct gdbarch *gdbarch, struct obstack *obstack)

It's unfortunate that this can't take a lm_info_windows* instead, but 
that seems unworkable given this needs to work on the executable module 
as well...

> > > {
> > > -  CORE_ADDR text_offset;
> > > +  CORE_ADDR text_offset = text_offset_cached ? *text_offset_cached : 0;
> > >
> > >     obstack_grow_str (obstack, "<library name=\"");
> > >     std::string p = xml_escape_text (so_name);
> > >     obstack_grow_str (obstack, p.c_str ());
> > >     obstack_grow_str (obstack, "\"><segment address=\"");
> > > -  gdb_bfd_ref_ptr dll (gdb_bfd_open (so_name, gnutarget, -1));
> > > -  /* The following calls are OK even if dll is NULL.
> > > -    The default value 0x1000 is returned by pe_text_section_offset
> > > -    in that case.  */
> > > -  text_offset = pe_text_section_offset (dll.get ());
> > > +
> > > +  if (!text_offset)
> > > +    {
> > > +      gdb_bfd_ref_ptr dll (gdb_bfd_open (so_name, gnutarget, -1));
> > > +      /* The following calls are OK even if dll is NULL.
> > > +    The default value 0x1000 is returned by pe_text_section_offset
> > > +    in that case.  */
> > > +      text_offset = pe_text_section_offset (dll.get ());
> > > +      if (text_offset_cached)
> > > +    *text_offset_cached = text_offset;
> > > +    }
> > > +
> > >     obstack_grow_str (obstack, paddress (gdbarch, load_addr + text_offset));
> > >     obstack_grow_str (obstack, "\"/></library>");
> > > }
> > > diff --git a/gdb/windows-tdep.h b/gdb/windows-tdep.h
> > > index 594c675d97..969afce816 100644
> > > --- a/gdb/windows-tdep.h
> > > +++ b/gdb/windows-tdep.h
> > > @@ -27,6 +27,7 @@ extern void init_w32_command_list (void);
> > >
> > > extern void windows_xfer_shared_library (const char* so_name,
> > >                       CORE_ADDR load_addr,
> > > +                    CORE_ADDR *text_offset_cached,
> > >                       struct gdbarch *gdbarch,
> > >                       struct obstack *obstack);
> > >
> > > --
> > > 2.24.1