[00/24] C++ification of struct so_list

  This series modernizes the struct so_list area by C++ifying it a little
bit and replacing the manual linked list implementation with
intrusive_list.  It also contains a few other cleanups written along the
way.

Simon Marchi (24):
  gdb: remove empty clear_solib functions
  gdb: add program_space parameter to target_so_ops::clear_solib
  gdb: make interps_notify work with references
  gdb: replace some so_list parameters to use references
  gdbsupport: use "reference" and "pointer" type aliases in
    intrusive_list
  gdbsupport: make intrusive_list's disposer accept a reference
  gdb: make get_cbfd_soname_build_id static
  gdb: allocate so_list with new, deallocate with delete
  gdb: rename lm_info_base to lm_info
  gdb: remove target_so_ops::free_so
  gdb: use gdb::checked_static_cast when casting lm_info
  gdb: make solib-svr4 not use so_list internally
  gdb: make solib-rocm not use so_list internally
  gdb: remove lm_info_vector typedef
  gdb: make so_list::lm_info a unique_ptr
  gdb: make clear_so a method of struct so_list
  gdb: remove target_section_table typedef
  gdb: make so_list::sections not a pointer
  gdb: make so_list::abfd a gdb_bfd_ref_ptr
  gdb: make so_list::{so_original_name,so_name} std::strings
  gdb: link so_list using intrusive_list
  gdb: don't call so_list::clear in free_so
  gdb: remove free_so function
  gdb: rename struct so_list to so

 gdb/bfd-target.c                         |   6 +-
 gdb/break-catch-load.c                   |   4 +-
 gdb/breakpoint.c                         |   6 +-
 gdb/bsd-uthread.c                        |  18 +-
 gdb/corelow.c                            |   7 +-
 gdb/exec.c                               |  24 +-
 gdb/exec.h                               |   6 +-
 gdb/hppa-tdep.c                          |   2 +-
 gdb/hppa-tdep.h                          |   4 +-
 gdb/inferior.c                           |  10 +-
 gdb/interps.c                            |  10 +-
 gdb/interps.h                            |  10 +-
 gdb/maint.c                              |   2 +-
 gdb/mi/mi-cmd-file.c                     |   6 +-
 gdb/mi/mi-interp.c                       |  26 +-
 gdb/mi/mi-interp.h                       |   6 +-
 gdb/nto-tdep.c                           |   6 +-
 gdb/nto-tdep.h                           |   3 +-
 gdb/observable.h                         |   7 +-
 gdb/progspace.c                          |   4 +-
 gdb/progspace.h                          |  26 +-
 gdb/record-full.c                        |   2 +-
 gdb/remote.c                             |   3 +-
 gdb/solib-aix.c                          |  69 +---
 gdb/solib-darwin.c                       |  84 ++---
 gdb/solib-dsbt.c                         |  73 ++--
 gdb/solib-frv.c                          |  58 ++--
 gdb/solib-rocm.c                         | 122 +++----
 gdb/solib-svr4.c                         | 421 ++++++++---------------
 gdb/solib-svr4.h                         |   4 +-
 gdb/solib-target.c                       | 121 +++----
 gdb/solib.c                              | 309 ++++++++---------
 gdb/solib.h                              |  13 +-
 gdb/solist.h                             |  77 ++---
 gdb/symfile.c                            |   2 +-
 gdb/symfile.h                            |   2 +-
 gdb/target-debug.h                       |   4 +-
 gdb/target-delegates.c                   |  14 +-
 gdb/target-section.h                     |   6 +-
 gdb/target.c                             |  10 +-
 gdb/target.h                             |   6 +-
 gdb/unittests/intrusive_list-selftests.c |   4 +-
 gdbsupport/intrusive_list.h              |  30 +-
 43 files changed, 658 insertions(+), 969 deletions(-)


base-commit: 635b2dd919b8c58e164b77c396041935fca1d66a
  

On 2023-10-10 21:39, Simon Marchi wrote:
> This series modernizes the struct so_list area by C++ifying it a little
> bit and replacing the manual linked list implementation with
> intrusive_list.  It also contains a few other cleanups written along the
> way.

Very nicely split.  Thanks.  I sent a few comments, but nothing too serious.
The only thing on my end that requires some churn is the pointer vs reference
thing in the disposer.  Otherwise, all LGTM.

Oh, I did notice that "struct so" ends up as an ungreppable/unsearchable
name, and wondered whether that is going to make our lives harder in practice.
Maybe we should consider renaming it to "shared_obj" or "shobj" or something easier
to grep.  Or maybe it doesn't matter.  But no real strong opinion, I think I'm OK
with "so" too.
  

On 10/17/23 15:20, Pedro Alves wrote:
> On 2023-10-10 21:39, Simon Marchi wrote:
>> This series modernizes the struct so_list area by C++ifying it a little
>> bit and replacing the manual linked list implementation with
>> intrusive_list.  It also contains a few other cleanups written along the
>> way.
> 
> Very nicely split.  Thanks.  I sent a few comments, but nothing too serious.
> The only thing on my end that requires some churn is the pointer vs reference
> thing in the disposer.  Otherwise, all LGTM.

I dropped the "use reference in disposer" patch.

> Oh, I did notice that "struct so" ends up as an ungreppable/unsearchable
> name, and wondered whether that is going to make our lives harder in practice.
> Maybe we should consider renaming it to "shared_obj" or "shobj" or something easier
> to grep.  Or maybe it doesn't matter.  But no real strong opinion, I think I'm OK
> with "so" too.

I wondered about that too.  If we are going to change the name, perhaps
we can find a more accurate one.  I'm currently working on trying to
make it possible for program spaces to have more than one solib
implementation providing so_lists / sos.  In the context of ROCm, this
will allow solib-svr4 to provide sos for the host, and solib-rocm to
provide sos for the GPU.  I will also try to see if the JIT mechanism
can then be changed to be one more solib provider, see if that
simplifies some things.  In the case of ROCm, we don't talk about shared
objects, but code objects that are loaded in the memory space.  In the
case of JIT... not sure what we call the generated code, but it's not
really a "shared object".  Perhaps there is a generic name we can find
for those pieces of code that can be loaded and unloaded at runtime.

In the mean time, I'll go with "so", since it just seems weird to have
intrusive_list<so_list>.  If we want to change it again it's not really
complicated, there aren't that many instances.

Simon
  

Hi Simon,

I went through the series again, sent a few extra small remarks, and
FWIW, it looks reasonable to me.

I do agree with Pedro's point that "struct so" becomes a bit harder to
search for, and "shobj" would have less false-positives, but I
nevertheless ok with using "so".  If it turns out to be a hurdle, we
can always change it later.

I have tested the AMDGPU part of the series (gdb.rocm/*.exp), both on
top of master and downstream ROCgdb.

So with the various nits fixed:
Reviewed-By: Lancelot Six <lancelot.six@amd.com>

Thanks for doing this!
Lancelot.
  

On 10/19/23 07:09, Lancelot SIX wrote:
> Hi Simon,
> 
> I went through the series again, sent a few extra small remarks, and
> FWIW, it looks reasonable to me.
> 
> I do agree with Pedro's point that "struct so" becomes a bit harder to
> search for, and "shobj" would have less false-positives, but I
> nevertheless ok with using "so".  If it turns out to be a hurdle, we
> can always change it later.

Since you both agree with each other, I'll go with shobj.

> 
> I have tested the AMDGPU part of the series (gdb.rocm/*.exp), both on
> top of master and downstream ROCgdb.
> 
> So with the various nits fixed:
> Reviewed-By: Lancelot Six <lancelot.six@amd.com>

Thanks for reviewing, I will push with all the small nits fixed.

Simon
  

On 2023-10-17 20:53, Simon Marchi wrote:
> On 10/17/23 15:20, Pedro Alves wrote:
>> On 2023-10-10 21:39, Simon Marchi wrote:
>>> This series modernizes the struct so_list area by C++ifying it a little
>>> bit and replacing the manual linked list implementation with
>>> intrusive_list.  It also contains a few other cleanups written along the
>>> way.
>>
>> Very nicely split.  Thanks.  I sent a few comments, but nothing too serious.
>> The only thing on my end that requires some churn is the pointer vs reference
>> thing in the disposer.  Otherwise, all LGTM.
> 
> I dropped the "use reference in disposer" patch.

Right, thanks.  I somehow thought that that would have some cascading effect
in the following patches.  That's what I meant by churn.