[RFA,05/42] Move pending_macros to buildsym_compunit

Message ID 20180523045851.11660-6-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey May 23, 2018, 4:58 a.m. UTC
  This moves the pending_macros global into buildsym_compunit.

gdb/ChangeLog
2018-05-22  Tom Tromey  <tom@tromey.com>

	* buildsym.c (~buildsym_compunit): Free the macro table.
	(struct buildsym_compunit) <get_macro_table, release_macros>: New
	methods.
	<m_pending_macros>: New member.
	(pending_macros): Remove.
	(~scoped_free_pendings, get_macro_table, prepare_for_building)
	(reset_symtab_globals, end_symtab_get_static_block)
	(end_symtab_with_blockvector, augment_type_symtab)
	(buildsym_init): Update.
---
 gdb/ChangeLog  | 12 ++++++++++++
 gdb/buildsym.c | 55 +++++++++++++++++++++++++++----------------------------
 2 files changed, 39 insertions(+), 28 deletions(-)
  

Comments

Simon Marchi July 7, 2018, 3:41 p.m. UTC | #1
On 2018-05-23 12:58 AM, Tom Tromey wrote:
> This moves the pending_macros global into buildsym_compunit.

LGTM.  As in the other patch, I would like it if the new fields
were private though.

The only access of m_pending_macros outside the class is to check if
the macro table is instantiated.  So it could be replaced with this
method:

  bool has_macro_table () const
  { return m_pending_macros != nullptr; }

Simon
  
Tom Tromey July 8, 2018, 4:35 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2018-05-23 12:58 AM, Tom Tromey wrote:
>> This moves the pending_macros global into buildsym_compunit.

Simon> LGTM.  As in the other patch, I would like it if the new fields
Simon> were private though.

Simon> The only access of m_pending_macros outside the class is to check if
Simon> the macro table is instantiated.  So it could be replaced with this
Simon> method:

Simon>   bool has_macro_table () const
Simon>   { return m_pending_macros != nullptr; }

In the end the members are all private, and nothing outside the class
refers to this member directly.  I didn't look to see where this
reference occurred at this point in the series, but maybe
augment_type_symtab, which ends up as a method of buildsym_compunit?
Anyway I think the final result should be to your liking.

Tom
  

Patch

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 5dd6f7e343..c3961254da 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -118,6 +118,9 @@  struct buildsym_compunit
   {
     struct subfile *subfile, *nextsub;
 
+    if (m_pending_macros != nullptr)
+      free_macro_table (m_pending_macros);
+
     for (subfile = subfiles;
 	 subfile != NULL;
 	 subfile = nextsub)
@@ -135,6 +138,22 @@  struct buildsym_compunit
     m_last_source_file.reset (new_name);
   }
 
+  struct macro_table *get_macro_table ()
+  {
+    if (m_pending_macros == nullptr)
+      m_pending_macros = new_macro_table (&objfile->per_bfd->storage_obstack,
+					  objfile->per_bfd->macro_cache,
+					  compunit_symtab);
+    return m_pending_macros;
+  }
+
+  struct macro_table *release_macros ()
+  {
+    struct macro_table *result = m_pending_macros;
+    m_pending_macros = nullptr;
+    return result;
+  }
+
   /* The objfile we're reading debug info from.  */
   struct objfile *objfile;
 
@@ -169,6 +188,10 @@  struct buildsym_compunit
 
   /* Language of this compunit_symtab.  */
   enum language language;
+
+  /* The macro table for the compilation unit whose symbols we're
+     currently reading.  */
+  struct macro_table *m_pending_macros = nullptr;
 };
 
 /* The work-in-progress of the compunit we are building.
@@ -229,10 +252,6 @@  struct subfile_stack
 
 static struct subfile_stack *subfile_stack;
 
-/* The macro table for the compilation unit whose symbols we're
-   currently reading.  */
-static struct macro_table *pending_macros;
-
 static void free_buildsym_compunit (void);
 
 static int compare_line_numbers (const void *ln1p, const void *ln2p);
@@ -342,10 +361,6 @@  scoped_free_pendings::~scoped_free_pendings ()
     }
   global_symbols = NULL;
 
-  if (pending_macros)
-    free_macro_table (pending_macros);
-  pending_macros = NULL;
-
   if (pending_addrmap)
     obstack_free (&pending_addrmap_obstack, NULL);
   pending_addrmap = NULL;
@@ -999,17 +1014,7 @@  get_macro_table (void)
   struct objfile *objfile;
 
   gdb_assert (buildsym_compunit != NULL);
-
-  objfile = buildsym_compunit->objfile;
-
-  if (! pending_macros)
-    {
-      pending_macros = new_macro_table (&objfile->per_bfd->storage_obstack,
-					objfile->per_bfd->macro_cache,
-					buildsym_compunit->compunit_symtab);
-    }
-
-  return pending_macros;
+  return buildsym_compunit->get_macro_table ();
 }
 
 /* Init state to prepare for building a symtab.
@@ -1034,7 +1039,6 @@  prepare_for_building (CORE_ADDR start_addr)
   gdb_assert (file_symbols == NULL);
   gdb_assert (global_symbols == NULL);
   gdb_assert (global_using_directives == NULL);
-  gdb_assert (pending_macros == NULL);
   gdb_assert (pending_addrmap == NULL);
   gdb_assert (current_subfile == NULL);
   gdb_assert (buildsym_compunit == nullptr);
@@ -1191,10 +1195,6 @@  reset_symtab_globals (void)
   global_symbols = NULL;
   global_using_directives = NULL;
 
-  /* We don't free pending_macros here because if the symtab was successfully
-     built then ownership was transferred to the symtab.  */
-  pending_macros = NULL;
-
   if (pending_addrmap)
     obstack_free (&pending_addrmap_obstack, NULL);
   pending_addrmap = NULL;
@@ -1289,7 +1289,7 @@  end_symtab_get_static_block (CORE_ADDR end_addr, int expandable, int required)
       && file_symbols == NULL
       && global_symbols == NULL
       && have_line_numbers == 0
-      && pending_macros == NULL
+      && buildsym_compunit->m_pending_macros == NULL
       && global_using_directives == NULL)
     {
       /* Ignore symtabs that have no functions with real debugging info.  */
@@ -1442,7 +1442,7 @@  end_symtab_with_blockvector (struct block *static_block,
 
   COMPUNIT_BLOCK_LINE_SECTION (cu) = section;
 
-  COMPUNIT_MACRO_TABLE (cu) = pending_macros;
+  COMPUNIT_MACRO_TABLE (cu) = buildsym_compunit->release_macros ();
 
   /* Default any symbols without a specified symtab to the primary symtab.  */
   {
@@ -1593,7 +1593,7 @@  augment_type_symtab (void)
     }
   if (pending_blocks != NULL)
     complaint (&symfile_complaints, _("Blocks in a type symtab"));
-  if (pending_macros != NULL)
+  if (buildsym_compunit->m_pending_macros != NULL)
     complaint (&symfile_complaints, _("Macro in a type symtab"));
   if (have_line_numbers)
     complaint (&symfile_complaints,
@@ -1765,7 +1765,6 @@  buildsym_init (void)
   gdb_assert (file_symbols == NULL);
   gdb_assert (global_symbols == NULL);
   gdb_assert (global_using_directives == NULL);
-  gdb_assert (pending_macros == NULL);
   gdb_assert (pending_addrmap == NULL);
   gdb_assert (buildsym_compunit == NULL);
 }