gdb/dwarf: store dwo_file_up in dwo_file_set

Heap-allocated dwo_file objects, stored in dwarf2_per_bfd::dwo_files,
are never freed.  They are created in one of the
create_dwo_unit_in_dwp_* or lookup_dwo_cutu functions.  I confirmed this
by running:

  $ make check TESTS="gdb.cp/anon-ns.exp" RUNTESTFLAGS="--target_board=fission-dwp"
  $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.cp/anon-ns/anon-ns -ex "p main" -ex "file" -batch

... and checking the ASan leak report.  I also debugged this invocation
of GDB, placed a breakpoint on ~dwo_file, and didn't see any hit.

Change the dwo_file set to hold dwo_file_up objects.  When the
dwarf2_per_bfd object gets destroyed, dwo_file objects will
automatically get destroyed.  With this change, I see the related leaks
disappear in the ASan leak report, and my ~dwo_file breakpoint gets hit
when debugging GDB.

Change-Id: Icb38539c3f9e553f3625c625a00fc63dd6e9f3c5
Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f87d0d900afa04c523206e8690afccd86cc5dfac..916381820920df56cec421a12d054349bcb87bd2 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -416,8 +416,6 @@ struct dwo_file
   dwo_unit_set tus;
 };
 
-using dwo_file_up = std::unique_ptr<dwo_file>;
-
 /* See dwarf2/read.h.  */
 
 std::size_t
@@ -434,7 +432,7 @@ dwo_file_hash::operator() (const dwo_file_search &search) const noexcept
 /* See dwarf2/read.h.  */
 
 std::size_t
-dwo_file_hash::operator() (const dwo_file *file) const noexcept
+dwo_file_hash::operator() (const dwo_file_up &file) const noexcept
 {
   return (*this) ({ file->dwo_name.c_str (), file->comp_dir });
 }
@@ -443,7 +441,7 @@ dwo_file_hash::operator() (const dwo_file *file) const noexcept
 
 bool
 dwo_file_eq::operator() (const dwo_file_search &search,
-                        const dwo_file *dwo_file) const noexcept
+                        const dwo_file_up &dwo_file) const noexcept
 {
   if (search.dwo_name != dwo_file->dwo_name)
     return false;
@@ -457,7 +455,8 @@ dwo_file_eq::operator() (const dwo_file_search &search,
 /* See dwarf2/read.h.  */
 
 bool
-dwo_file_eq::operator() (const dwo_file *a, const dwo_file *b) const noexcept
+dwo_file_eq::operator() (const dwo_file_up &a,
+                        const dwo_file_up &b) const noexcept
 { return (*this) ({ a->dwo_name.c_str (), a->comp_dir }, b); }
 
 /* These sections are what may appear in a DWP file.  */
@@ -3985,7 +3984,7 @@ process_skeletonless_type_units (dwarf2_per_objfile *per_objfile,
 {
   /* Skeletonless TUs in DWP files without .gdb_index is not supported yet.  */
   if (get_dwp_file (per_objfile) == nullptr)
-    for (dwo_file *file : per_objfile->per_bfd->dwo_files)
+    for (const dwo_file_up &file : per_objfile->per_bfd->dwo_files)
       for (dwo_unit *unit : file->tus)
        process_skeletonless_type_unit (unit, per_objfile, storage);
 }
@@ -6813,7 +6812,7 @@ lookup_dwo_file (dwarf2_per_bfd *per_bfd, const char *dwo_name,
 {
   auto it = per_bfd->dwo_files.find (dwo_file_search {dwo_name, comp_dir});
 
-  return it != per_bfd->dwo_files.end () ? *it : nullptr;
+  return it != per_bfd->dwo_files.end () ? it->get() : nullptr;
 }
 
 /* die_reader_func for create_dwo_cu.  */
@@ -7484,18 +7483,18 @@ create_dwo_unit_in_dwp_v1 (dwarf2_per_bfd *per_bfd,
       dwarf_read_debug_printf ("Creating virtual DWO: %s",
                               virtual_dwo_name.c_str ());
 
-      dwo_file = new struct dwo_file;
-      dwo_file->dwo_name = std::move (virtual_dwo_name);
-      dwo_file->comp_dir = comp_dir;
-      dwo_file->sections.abbrev = sections.abbrev;
-      dwo_file->sections.line = sections.line;
-      dwo_file->sections.loc = sections.loc;
-      dwo_file->sections.macinfo = sections.macinfo;
-      dwo_file->sections.macro = sections.macro;
-      dwo_file->sections.str_offsets = sections.str_offsets;
+      dwo_file_up new_dwo_file = std::make_unique<struct dwo_file> ();
+      new_dwo_file->dwo_name = std::move (virtual_dwo_name);
+      new_dwo_file->comp_dir = comp_dir;
+      new_dwo_file->sections.abbrev = sections.abbrev;
+      new_dwo_file->sections.line = sections.line;
+      new_dwo_file->sections.loc = sections.loc;
+      new_dwo_file->sections.macinfo = sections.macinfo;
+      new_dwo_file->sections.macro = sections.macro;
+      new_dwo_file->sections.str_offsets = sections.str_offsets;
 
       /* The "str" section is global to the entire DWP file.  */
-      dwo_file->sections.str = dwp_file->sections.str;
+      new_dwo_file->sections.str = dwp_file->sections.str;
 
       /* The info or types section is assigned below to dwo_unit,
         there's no need to record it in dwo_file.
@@ -7504,8 +7503,10 @@ create_dwo_unit_in_dwp_v1 (dwarf2_per_bfd *per_bfd,
         types we'll grow the vector and eventually have to reallocate space
         for it, invalidating all copies of pointers into the previous
         contents.  */
-      auto [it, inserted] = per_bfd->dwo_files.emplace (dwo_file);
+      auto [it, inserted]
+       = per_bfd->dwo_files.emplace (std::move (new_dwo_file));
       gdb_assert (inserted);
+      dwo_file = it->get ();
     }
   else
     dwarf_read_debug_printf ("Using existing virtual DWO: %s",
@@ -7670,36 +7671,35 @@ create_dwo_unit_in_dwp_v2 (dwarf2_per_bfd *per_bfd,
       dwarf_read_debug_printf ("Creating virtual DWO: %s",
                               virtual_dwo_name.c_str ());
 
-      dwo_file = new struct dwo_file;
-      dwo_file->dwo_name = std::move (virtual_dwo_name);
-      dwo_file->comp_dir = comp_dir;
-      dwo_file->sections.abbrev =
-       create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.abbrev,
-                                    sections.abbrev_offset,
-                                    sections.abbrev_size);
-      dwo_file->sections.line =
-       create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.line,
-                                    sections.line_offset,
-                                    sections.line_size);
-      dwo_file->sections.loc =
-       create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.loc,
-                                    sections.loc_offset, sections.loc_size);
-      dwo_file->sections.macinfo =
-       create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.macinfo,
-                                    sections.macinfo_offset,
-                                    sections.macinfo_size);
-      dwo_file->sections.macro =
-       create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.macro,
-                                    sections.macro_offset,
-                                    sections.macro_size);
-      dwo_file->sections.str_offsets =
-       create_dwp_v2_or_v5_section (per_bfd,
-                                    &dwp_file->sections.str_offsets,
-                                    sections.str_offsets_offset,
-                                    sections.str_offsets_size);
+      dwo_file_up new_dwo_file = std::make_unique<struct dwo_file> ();
+      new_dwo_file->dwo_name = std::move (virtual_dwo_name);
+      new_dwo_file->comp_dir = comp_dir;
+      new_dwo_file->sections.abbrev
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.abbrev,
+                                      sections.abbrev_offset,
+                                      sections.abbrev_size);
+      new_dwo_file->sections.line
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.line,
+                                      sections.line_offset,
+                                      sections.line_size);
+      new_dwo_file->sections.loc
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.loc,
+                                      sections.loc_offset, sections.loc_size);
+      new_dwo_file->sections.macinfo
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.macinfo,
+                                      sections.macinfo_offset,
+                                      sections.macinfo_size);
+      new_dwo_file->sections.macro
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.macro,
+                                      sections.macro_offset,
+                                      sections.macro_size);
+      new_dwo_file->sections.str_offsets
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.str_offsets,
+                                      sections.str_offsets_offset,
+                                      sections.str_offsets_size);
 
       /* The "str" section is global to the entire DWP file.  */
-      dwo_file->sections.str = dwp_file->sections.str;
+      new_dwo_file->sections.str = dwp_file->sections.str;
 
       /* The info or types section is assigned below to dwo_unit,
         there's no need to record it in dwo_file.
@@ -7708,8 +7708,10 @@ create_dwo_unit_in_dwp_v2 (dwarf2_per_bfd *per_bfd,
         types we'll grow the vector and eventually have to reallocate space
         for it, invalidating all copies of pointers into the previous
         contents.  */
-      auto [it, inserted] = per_bfd->dwo_files.emplace (dwo_file);
+      auto [it, inserted]
+       = per_bfd->dwo_files.emplace (std::move (new_dwo_file));
       gdb_assert (inserted);
+      dwo_file = it->get ();
     }
   else
     dwarf_read_debug_printf ("Using existing virtual DWO: %s",
@@ -7838,41 +7840,36 @@ create_dwo_unit_in_dwp_v5 (dwarf2_per_bfd *per_bfd,
       dwarf_read_debug_printf ("Creating virtual DWO: %s",
                               virtual_dwo_name.c_str ());
 
-      dwo_file = new struct dwo_file;
-      dwo_file->dwo_name = std::move (virtual_dwo_name);
-      dwo_file->comp_dir = comp_dir;
-      dwo_file->sections.abbrev =
-       create_dwp_v2_or_v5_section (per_bfd,
-                                    &dwp_file->sections.abbrev,
-                                    sections.abbrev_offset,
-                                    sections.abbrev_size);
-      dwo_file->sections.line =
-       create_dwp_v2_or_v5_section (per_bfd,
-                                    &dwp_file->sections.line,
-                                    sections.line_offset, sections.line_size);
-      dwo_file->sections.macro =
-       create_dwp_v2_or_v5_section (per_bfd,
-                                    &dwp_file->sections.macro,
-                                    sections.macro_offset,
-                                    sections.macro_size);
-      dwo_file->sections.loclists =
-       create_dwp_v2_or_v5_section (per_bfd,
-                                    &dwp_file->sections.loclists,
-                                    sections.loclists_offset,
-                                    sections.loclists_size);
-      dwo_file->sections.rnglists =
-       create_dwp_v2_or_v5_section (per_bfd,
-                                    &dwp_file->sections.rnglists,
-                                    sections.rnglists_offset,
-                                    sections.rnglists_size);
-      dwo_file->sections.str_offsets =
-       create_dwp_v2_or_v5_section (per_bfd,
-                                    &dwp_file->sections.str_offsets,
-                                    sections.str_offsets_offset,
-                                    sections.str_offsets_size);
+      dwo_file_up new_dwo_file = std::make_unique<struct dwo_file> ();
+      new_dwo_file->dwo_name = std::move (virtual_dwo_name);
+      new_dwo_file->comp_dir = comp_dir;
+      new_dwo_file->sections.abbrev
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.abbrev,
+                                      sections.abbrev_offset,
+                                      sections.abbrev_size);
+      new_dwo_file->sections.line
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.line,
+                                      sections.line_offset,
+                                      sections.line_size);
+      new_dwo_file->sections.macro
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.macro,
+                                      sections.macro_offset,
+                                      sections.macro_size);
+      new_dwo_file->sections.loclists
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.loclists,
+                                      sections.loclists_offset,
+                                      sections.loclists_size);
+      new_dwo_file->sections.rnglists
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.rnglists,
+                                      sections.rnglists_offset,
+                                      sections.rnglists_size);
+      new_dwo_file->sections.str_offsets
+       = create_dwp_v2_or_v5_section (per_bfd, &dwp_file->sections.str_offsets,
+                                      sections.str_offsets_offset,
+                                      sections.str_offsets_size);
 
       /* The "str" section is global to the entire DWP file.  */
-      dwo_file->sections.str = dwp_file->sections.str;
+      new_dwo_file->sections.str = dwp_file->sections.str;
 
       /* The info or types section is assigned below to dwo_unit,
         there's no need to record it in dwo_file.
@@ -7881,8 +7878,10 @@ create_dwo_unit_in_dwp_v5 (dwarf2_per_bfd *per_bfd,
         types we'll grow the vector and eventually have to reallocate space
         for it, invalidating all copies of pointers into the previous
         contents.  */
-      auto [it, inserted] = per_bfd->dwo_files.emplace (dwo_file);
+      auto [it, inserted]
+       = per_bfd->dwo_files.emplace (std::move (new_dwo_file));
       gdb_assert (inserted);
+      dwo_file = it->get ();
     }
   else
     dwarf_read_debug_printf ("Using existing virtual DWO: %s",
@@ -8134,7 +8133,7 @@ dwarf2_locate_dwo_sections (struct objfile *objfile, bfd *abfd,
    by PER_CU.  This is for the non-DWP case.
    The result is NULL if DWO_NAME can't be found.  */
 
-static struct dwo_file *
+static dwo_file_up
 open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
                        const char *comp_dir)
 {
@@ -8149,7 +8148,7 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
       return NULL;
     }
 
-  dwo_file_up dwo_file (new struct dwo_file);
+  dwo_file_up dwo_file = std::make_unique<struct dwo_file> ();
   dwo_file->dwo_name = dwo_name;
   dwo_file->comp_dir = comp_dir;
   dwo_file->dbfd = std::move (dbfd);
@@ -8171,7 +8170,7 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
 
   bfd_cache_close (dwo_file->dbfd.get ());
 
-  return dwo_file.release ();
+  return dwo_file;
 }
 
 /* This function is mapped across the sections and remembers the offset and
@@ -8507,12 +8506,15 @@ lookup_dwo_cutu (dwarf2_cu *cu, const char *dwo_name, const char *comp_dir,
          /* Read in the file and build a table of the CUs/TUs it contains.
 
             NOTE: This will be nullptr if unable to open the file.  */
-         dwo_file = open_and_init_dwo_file (cu, dwo_name, comp_dir);
+         dwo_file_up new_dwo_file
+           = open_and_init_dwo_file (cu, dwo_name, comp_dir);
 
-         if (dwo_file != nullptr)
+         if (new_dwo_file != nullptr)
            {
-             auto [it, inserted] = per_bfd->dwo_files.emplace (dwo_file);
+             auto [it, inserted]
+               = per_bfd->dwo_files.emplace (std::move (new_dwo_file));
              gdb_assert (inserted);
+             dwo_file = (*it).get ();
            }
        }
 

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 164f9ae114097bef34002d0a0635aca79ed9a769..44747bf7ccdd15b104e2355a380db4a343df400f 100644 (file)
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -445,6 +445,8 @@ using signatured_type_set
 
 struct dwo_file;
 
+using dwo_file_up = std::unique_ptr<dwo_file>;
+
 /* This is used when looking up entries in a dwo_file_set.  */
 
 struct dwo_file_search
@@ -464,7 +466,7 @@ struct dwo_file_hash
   using is_transparent = void;
 
   std::size_t operator() (const dwo_file_search &search) const noexcept;
-  std::size_t operator() (const dwo_file *file) const noexcept;
+  std::size_t operator() (const dwo_file_up &file) const noexcept;
 };
 
 /* Equal function for dwo_file objects, using their dwo_name and comp_dir as
@@ -475,13 +477,14 @@ struct dwo_file_eq
   using is_transparent = void;
 
   bool operator() (const dwo_file_search &search,
-                  const dwo_file *dwo_file) const noexcept;
-  bool operator() (const dwo_file *a, const dwo_file *b) const noexcept;
+                  const dwo_file_up &dwo_file) const noexcept;
+  bool operator() (const dwo_file_up &a, const dwo_file_up &b) const noexcept;
 };
 
 /* Set of dwo_file objects, using their dwo_name and comp_dir as identity.  */
 
-using dwo_file_set = gdb::unordered_set<dwo_file *, dwo_file_hash, dwo_file_eq>;
+using dwo_file_up_set
+  = gdb::unordered_set<dwo_file_up, dwo_file_hash, dwo_file_eq>;
 
 struct dwp_file;
 
@@ -635,7 +638,7 @@ public:
   struct tu_stats tu_stats;
 
   /* Set of dwo_file objects.  */
-  dwo_file_set dwo_files;
+  dwo_file_up_set dwo_files;
 
   /* True if we've checked for whether there is a DWP file.  */
   bool dwp_checked = false;