gdb: fix owner passed to remove_target_sections in clear_solib

Commit 8971d2788e7 ("gdb: link so_list using intrusive_list") introduced
a bug in clear_solib.  Instead of passing an `so_list *` to
remove_target_sections, it passed an `so_list **`.  This was not caught
by the compiler, because remove_target_sections takes a `void *` as the
"owner", so you can pass it any pointer and it won't complain.

This happened because I previously had a patch to change the type of the
disposer parameter to be a reference rather than a pointer, so had to
change `so` to `&so`.  When dropping that patch, I forgot to revert this
bit and / or it got re-introduced when handling subsequent merge
conflicts.  And I didn't properly retest.

Fix that, but try to make things less error prone.  Add a union to
represent the possible owner kinds for a target_section.  Trying to pass
a pointer to another type than those will not compile.

Change-Id: I600cab5ea0408ccc5638467b760768161ca3036c

diff --git a/gdb/exec.c b/gdb/exec.c
index 0f9f9d076c684f76510a51ca2053394dad4a8a3d..5956012338fa6d0289396aa59808f9805e9f39d2 100644 (file)
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -492,8 +492,8 @@ exec_file_attach (const char *filename, int from_tty)
       /* Add the executable's sections to the current address spaces'
         list of sections.  This possibly pushes the exec_ops
         target.  */
-      current_program_space->add_target_sections (&current_program_space->ebfd,
-                                                 sections);
+      current_program_space->add_target_sections
+       (current_program_space->ebfd.get (), sections);
 
       /* Tell display code (if any) about the changed file name.  */
       if (deprecated_exec_file_display_hook)
@@ -599,8 +599,8 @@ build_section_table (struct bfd *some_bfd)
    current set of target sections.  */
 
 void
-program_space::add_target_sections (const void *owner,
-                                   const std::vector<target_section> &sections)
+program_space::add_target_sections
+  (target_section_owner owner, const std::vector<target_section> &sections)
 {
   if (!sections.empty ())
     {
@@ -643,7 +643,7 @@ program_space::add_target_sections (struct objfile *objfile)
        continue;
 
       m_target_sections.emplace_back (osect->addr (), osect->endaddr (),
-                                     osect->the_bfd_section, (void *) objfile);
+                                     osect->the_bfd_section, objfile);
     }
 }
 
@@ -651,15 +651,15 @@ program_space::add_target_sections (struct objfile *objfile)
    OWNER must be the same value passed to add_target_sections.  */
 
 void
-program_space::remove_target_sections (const void *owner)
+program_space::remove_target_sections (target_section_owner owner)
 {
-  gdb_assert (owner != NULL);
+  gdb_assert (owner.v () != nullptr);
 
   auto it = std::remove_if (m_target_sections.begin (),
                            m_target_sections.end (),
                            [&] (target_section &sect)
                            {
-                             return sect.owner == owner;
+                             return sect.owner.v () == owner.v ();
                            });
   m_target_sections.erase (it, m_target_sections.end ());
 

diff --git a/gdb/maint.c b/gdb/maint.c
index e0dc5bc0c7aacff272bde80c2c5903cffbe2afa3..0635af3dfc4ecf47d76b856a9e24daec14e111a7 100644 (file)
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -509,7 +509,7 @@ maintenance_info_target_sections (const char *arg, int from_tty)
                  (8 + digits), "",
                  hex_string_custom (sec.addr, addr_size),
                  hex_string_custom (sec.endaddr, addr_size),
-                 sec.owner);
+                 sec.owner.v ());
     }
 }
 

diff --git a/gdb/progspace.c b/gdb/progspace.c
index cca651fdb6f8675a5d452fc352d84cbaeee7495c..839707e9d71f6e8c29ecf520240c2f04f72a8320 100644 (file)
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -207,10 +207,11 @@ program_space::exec_close ()
     {
       /* Removing target sections may close the exec_ops target.
         Clear ebfd before doing so to prevent recursion.  */
+      bfd *saved_ebfd = ebfd.get ();
       ebfd.reset (nullptr);
       ebfd_mtime = 0;
 
-      remove_target_sections (&ebfd);
+      remove_target_sections (saved_ebfd);
 
       exec_filename.reset (nullptr);
     }

diff --git a/gdb/progspace.h b/gdb/progspace.h
index a480f560a77a67371f6704ef1035eb0a1d9dd333..a22e427400e929765ef1424f3483ac867e5a4abd 100644 (file)
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -284,11 +284,11 @@ struct program_space
   bool empty ();
 
   /* Remove all target sections owned by OWNER.  */
-  void remove_target_sections (const void *owner);
+  void remove_target_sections (target_section_owner owner);
 
   /* Add the sections array defined by SECTIONS to the
      current set of target sections.  */
-  void add_target_sections (const void *owner,
+  void add_target_sections (target_section_owner owner,
                            const std::vector<target_section> &sections);
 
   /* Add the sections of OBJFILE to the current set of target

diff --git a/gdb/solib.c b/gdb/solib.c
index 71970636dc58278ed9e025bd579ef3064327859f..b9fb911a81014dfeb60fc6f8d67d0fba5e2eaf87 100644 (file)
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1195,7 +1195,7 @@ clear_solib (void)
   current_program_space->so_list.clear_and_dispose ([] (shobj *so)
     {
       notify_solib_unloaded (current_program_space, *so);
-      current_program_space->remove_target_sections (&so);
+      current_program_space->remove_target_sections (so);
       delete so;
     });
 

diff --git a/gdb/target-section.h b/gdb/target-section.h
index 1c902baa7778a10c471023d24648bdb55bc778e7..62149d382dd637c78629fd726546fbeafa87d607 100644 (file)
--- a/gdb/target-section.h
+++ b/gdb/target-section.h
@@ -20,6 +20,38 @@
 #ifndef GDB_TARGET_SECTION_H
 #define GDB_TARGET_SECTION_H
 
+struct bfd;
+struct objfile;
+struct shobj;
+
+/* A union representing the possible owner types of a target_section.  */
+
+union target_section_owner
+{
+  target_section_owner () : m_v (nullptr) {}
+  target_section_owner (const bfd *bfd) : bfd (bfd) {}
+  target_section_owner (const objfile *objfile) : objfile (objfile) {}
+  target_section_owner (const shobj *shobj) : shobj (shobj) {}
+
+  /* Use this to access the type-erased version of the owner, for
+     comparisons, printing, etc.  We don't access the M_V member
+     directly, because pedantically it is not valid to access a
+     non-active union member.  */
+  const void *v () const
+  {
+    void *tmp;
+    memcpy (&tmp, this, sizeof (*this));
+    return tmp;
+  }
+
+  const struct bfd *bfd;
+  const struct objfile *objfile;
+  const struct shobj *shobj;
+
+private:
+  const void *m_v;
+};
+
 /* Struct target_section maps address ranges to file sections.  It is
    mostly used with BFD files, but can be used without (e.g. for handling
    raw disks, or files not in formats handled by BFD).  */
@@ -27,7 +59,7 @@
 struct target_section
 {
   target_section (CORE_ADDR addr_, CORE_ADDR end_, struct bfd_section *sect_,
-                 void *owner_ = nullptr)
+                 target_section_owner owner_ = {})
     : addr (addr_),
       endaddr (end_),
       the_bfd_section (sect_),
@@ -44,11 +76,11 @@ struct target_section
   struct bfd_section *the_bfd_section;
 
   /* The "owner" of the section.
-     It can be any unique value.  It is set by add_target_sections
-     and used by remove_target_sections.
+
+     It is set by add_target_sections and used by remove_target_sections.
      For example, for executables it is a pointer to exec_bfd and
-     for shlibs it is the so_list pointer.  */
-  const void *owner;
+     for shlibs it is the shobj pointer.  */
+  target_section_owner owner;
 };
 
 #endif /* GDB_TARGET_SECTION_H */