[binutils-gdb] gdb: allocate subfile with new

[binutils-gdb] gdb: allocate subfile with new

[Simon Marchi]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=71bc95ed2032567e2a7aebc3efe1c55a77abb7b2

commit 71bc95ed2032567e2a7aebc3efe1c55a77abb7b2
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Thu Apr 7 08:06:50 2022 -0400

    gdb: allocate subfile with new
    
    Allocate struct subfile with new, initialize its fields instead of
    memset-ing it to 0.  Use a unique_ptr for the window after a subfile has
    been allocated but before it is linked in the buildsym_compunit's list
    of subfile (and therefore owned by the buildsym_compunit.
    
    I can't test the change in xcoffread.c, it's best-effort.  I couldn't
    find where subfiles are freed in that file, I assume they were
    intentionally (or not) leaked.
    
    Change-Id: Ib3b6877de31b7e65bc466682f08dbf5840225f24

Diff:
---
 gdb/buildsym.c  | 37 ++++++++++++++++---------------------
 gdb/buildsym.h  | 22 ++++++++++++++++------
 gdb/xcoffread.c | 10 ++--------
 3 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 0df355151f4..aedc3c67b9a 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -97,7 +97,7 @@ buildsym_compunit::~buildsym_compunit ()
       nextsub = subfile->next;
       xfree (subfile->name);
       xfree (subfile->line_vector);
-      xfree (subfile);
+      delete subfile;
     }
 
   struct pending *next, *next1;
@@ -504,13 +504,12 @@ void
 buildsym_compunit::start_subfile (const char *name)
 {
   const char *subfile_dirname;
-  struct subfile *subfile;
 
   subfile_dirname = m_comp_dir.get ();
 
   /* See if this subfile is already registered.  */
 
-  for (subfile = m_subfiles; subfile; subfile = subfile->next)
+  for (subfile *subfile = m_subfiles; subfile; subfile = subfile->next)
     {
       char *subfile_name;
 
@@ -537,13 +536,9 @@ buildsym_compunit::start_subfile (const char *name)
 
   /* This subfile is not known.  Add an entry for it.  */
 
-  subfile = XNEW (struct subfile);
-  memset (subfile, 0, sizeof (struct subfile));
-
-  subfile->next = m_subfiles;
-  m_subfiles = subfile;
+  subfile_up subfile (new struct subfile);
 
-  m_current_subfile = subfile;
+  m_current_subfile = subfile.get ();
 
   subfile->name = xstrdup (name);
 
@@ -562,11 +557,8 @@ buildsym_compunit::start_subfile (const char *name)
      source file.  */
 
   subfile->language = deduce_language_from_filename (subfile->name);
-  if (subfile->language == language_unknown
-      && subfile->next != NULL)
-    {
-      subfile->language = subfile->next->language;
-    }
+  if (subfile->language == language_unknown && m_subfiles != nullptr)
+    subfile->language = m_subfiles->language;
 
   /* If the filename of this subfile ends in .C, then change the
      language of any pending subfiles from C to C++.  We also accept
@@ -586,12 +578,14 @@ buildsym_compunit::start_subfile (const char *name)
 
   /* And patch up this file if necessary.  */
   if (subfile->language == language_c
-      && subfile->next != NULL
-      && (subfile->next->language == language_cplus
-	  || subfile->next->language == language_fortran))
-    {
-      subfile->language = subfile->next->language;
-    }
+      && m_subfiles != nullptr
+      && (m_subfiles->language == language_cplus
+	  || m_subfiles->language == language_fortran))
+    subfile->language = m_subfiles->language;
+
+  /* Link this subfile at the front of the subfile list.  */
+  subfile->next = m_subfiles;
+  m_subfiles = subfile.release ();
 }
 
 /* For stabs readers, the first N_SO symbol is assumed to be the
@@ -791,7 +785,8 @@ buildsym_compunit::watch_main_source_file_lossage ()
 	  else
 	    prev_mainsub_alias->next = mainsub_alias->next;
 	  xfree (mainsub_alias->name);
-	  xfree (mainsub_alias);
+
+	  delete mainsub_alias;
 	}
     }
 }
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 5f0e0230fd9..50ecd33d544 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -45,16 +45,26 @@ struct dynamic_prop;
 
 struct subfile
 {
-  struct subfile *next;
+  subfile () = default;
+
+  /* There's nothing wrong with copying a subfile, but we don't need to, so use
+     this to avoid copying one by mistake.  */
+  DISABLE_COPY_AND_ASSIGN (subfile);
+
+  struct subfile *next = nullptr;
+
   /* Space for this is malloc'd.  */
-  char *name;
+  char *name = nullptr;
+
   /* Space for this is malloc'd.  */
-  struct linetable *line_vector;
-  int line_vector_length;
-  enum language language;
-  struct symtab *symtab;
+  struct linetable *line_vector = nullptr;
+  int line_vector_length = 0;
+  enum language language = language_unknown;
+  struct symtab *symtab = nullptr;
 };
 
+using subfile_up = std::unique_ptr<subfile>;
+
 /* Record the symbols defined for each context in a list.  We don't
    create a struct block for the context until we know how long to
    make it.  */
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index c5d2d0a492f..296e71356dd 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -632,8 +632,6 @@ process_linenos (CORE_ADDR start, CORE_ADDR end)
   if (offset == 0)
     goto return_after_cleanup;
 
-  memset (&main_subfile, '\0', sizeof (main_subfile));
-
   if (inclIndx == 0)
     /* All source lines were in the main source file.  None in include
        files.  */
@@ -651,8 +649,6 @@ process_linenos (CORE_ADDR start, CORE_ADDR end)
 
       for (ii = 0; ii < inclIndx; ++ii)
 	{
-	  struct subfile *tmpSubfile;
-
 	  /* If there is main file source before include file, enter it.  */
 	  if (offset < inclTable[ii].begin)
 	    {
@@ -675,14 +671,12 @@ process_linenos (CORE_ADDR start, CORE_ADDR end)
 	  else
 	    {
 	      /* Have a new subfile for the include file.  */
+	      inclTable[ii].subfile = new subfile;
 
-	      tmpSubfile = inclTable[ii].subfile = XNEW (struct subfile);
-
-	      memset (tmpSubfile, '\0', sizeof (struct subfile));
 	      firstLine = &(inclTable[ii].funStartLine);
 
 	      /* Enter include file's lines now.  */
-	      enter_line_range (tmpSubfile, inclTable[ii].begin,
+	      enter_line_range (inclTable[ii].subfile, inclTable[ii].begin,
 				inclTable[ii].end, start, 0, firstLine);
 	    }