diff mbox

[review] Only clear the minsym array when necessary

Message ID gerrit.1572387394000.I7da994fe6747f67714e7efe9fdbb0dbc4d6ea532@gnutoolchain-gerrit.osci.io
State New, archived
Headers

Commit Message

Simon Marchi (Code Review) Oct. 29, 2019, 10:16 p.m. UTC 
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/442
......................................................................

Only clear the minsym array when necessary

The array starts out initialized to zero:
  minimal_symbol *msymbol_hash[MINIMAL_SYMBOL_HASH_SIZE] {};

So we only need to explicitly clear it if there were previous minsyms
added to it. This patch does that.

gdb/ChangeLog:

2019-10-29  Christian Biesinger  <cbiesinger@google.com>

	* minsyms.c (clear_minimal_symbol_hash_tables): New function.
	(build_minimal_symbol_hash_tables): Code to clear the table moved
	to clear_minimal_symbol_hash_tables.
	(minimal_symbol_reader::install): Call clear_minimal_symbol_hash_tables
	when needed.

Change-Id: I7da994fe6747f67714e7efe9fdbb0dbc4d6ea532
---
M gdb/minsyms.c
1 file changed, 14 insertions(+), 8 deletions(-)

Comments

Simon Marchi (Code Review) Oct. 30, 2019, 3:18 p.m. UTC | #1 
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/442
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

Thanks, this is ok with one nit fixed.
No need to re-review.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/442/1/gdb/minsyms.c 
File gdb/minsyms.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/442/1/gdb/minsyms.c@1351 
PS1, Line 1351: 
1283 | minimal_symbol_reader::install ()
     | ...
1342 |       mcount = compact_minimal_symbols (msymbols, mcount, m_objfile);
1343 |       msym_holder.reset (XRESIZEVEC (struct minimal_symbol,
1344 | 				     msym_holder.release (),
1345 | 				     mcount));
1346 | 
1347 |       /* Attach the minimal symbol table to the specified objfile.
1348 |          The strings themselves are also located in the storage_obstack
1349 |          of this objfile.  */
1350 | 
1351 |       if (m_objfile->per_bfd->minimal_symbol_count)

gdb style is to be explicit and write "!= 0" or "> 0".
Simon Marchi (Code Review) Oct. 30, 2019, 4:05 p.m. UTC | #2 
Christian Biesinger has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/442
......................................................................


Patch Set 1:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/442/1/gdb/minsyms.c 
File gdb/minsyms.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/442/1/gdb/minsyms.c@1351 
PS1, Line 1351: 
1283 | minimal_symbol_reader::install ()
     | ...
1342 |       mcount = compact_minimal_symbols (msymbols, mcount, m_objfile);
1343 |       msym_holder.reset (XRESIZEVEC (struct minimal_symbol,
1344 | 				     msym_holder.release (),
1345 | 				     mcount));
1346 | 
1347 |       /* Attach the minimal symbol table to the specified objfile.
1348 |          The strings themselves are also located in the storage_obstack
1349 |          of this objfile.  */
1350 | 
1351 |       if (m_objfile->per_bfd->minimal_symbol_count)

> gdb style is to be explicit and write "!= 0" or "> 0".

Thanks, fixed and pushed.
diff mbox

Patch

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index db3e546..10f02bb 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1238,6 +1238,16 @@ 
   return (mcount);
 }
 
+static void
+clear_minimal_symbol_hash_tables (struct objfile *objfile)
+{
+  for (size_t i = 0; i < MINIMAL_SYMBOL_HASH_SIZE; i++)
+    {
+      objfile->per_bfd->msymbol_hash[i] = 0;
+      objfile->per_bfd->msymbol_demangled_hash[i] = 0;
+    }
+}
+
 /* Build (or rebuild) the minimal symbol hash tables.  This is necessary
    after compacting or sorting the table since the entries move around
    thus causing the internal minimal_symbol pointers to become jumbled.  */
@@ -1248,14 +1258,7 @@ 
   int i;
   struct minimal_symbol *msym;
 
-  /* Clear the hash tables.  */
-  for (i = 0; i < MINIMAL_SYMBOL_HASH_SIZE; i++)
-    {
-      objfile->per_bfd->msymbol_hash[i] = 0;
-      objfile->per_bfd->msymbol_demangled_hash[i] = 0;
-    }
-
-  /* Now, (re)insert the actual entries.  */
+  /* (Re)insert the actual entries.  */
   for ((i = objfile->per_bfd->minimal_symbol_count,
 	msym = objfile->per_bfd->msymbols.get ());
        i > 0;
@@ -1345,6 +1348,9 @@ 
          The strings themselves are also located in the storage_obstack
          of this objfile.  */
 
+      if (m_objfile->per_bfd->minimal_symbol_count)
+	clear_minimal_symbol_hash_tables (m_objfile);
+
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);