diff mbox

[RFA] Fix leak of symbol name in block_symbol_cache

Message ID 20191201191045.1256-1-philippe.waroquiers@skynet.be
State New
Headers show

Commit Message

Philippe Waroquiers Dec. 1, 2019, 7:10 p.m. UTC 
A symbol not found inserted in the cache has a xstrdup-ed name
that must be freed, but only the struct block_symbol_cache is freed.
Add a function destroy_block_symbol_cache that clears all slots
before releasing the cache.

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
	* symtab.c (symbol_cache_clear_slot):  Move close to cleared type.
	(destroy_block_symbol_cache): New function.
	(symbol_cache:~symbol_cache) Call destroy_block_symbol_cache.
	(resize_symbol_cache): Likewise.
---
 gdb/symtab.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

Comments

Tom Tromey Dec. 2, 2019, 10:24 p.m. UTC | #1 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> A symbol not found inserted in the cache has a xstrdup-ed name
Philippe> that must be freed, but only the struct block_symbol_cache is freed.
Philippe> Add a function destroy_block_symbol_cache that clears all slots
Philippe> before releasing the cache.

Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
Philippe> 	* symtab.c (symbol_cache_clear_slot):  Move close to cleared type.
Philippe> 	(destroy_block_symbol_cache): New function.
Philippe> 	(symbol_cache:~symbol_cache) Call destroy_block_symbol_cache.
Philippe> 	(resize_symbol_cache): Likewise.

Thanks.  This is ok.

It would be nicer if everything here was just using destructors, but the
trailing array in block_symbol_cache gets in the way of that.

Tom
diff mbox

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 894a323003..5c33fbf9ab 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -183,6 +183,16 @@  struct symbol_cache_slot
   } value;
 };
 
+/* Clear out SLOT.  */
+
+static void
+symbol_cache_clear_slot (struct symbol_cache_slot *slot)
+{
+  if (slot->state == SYMBOL_SLOT_NOT_FOUND)
+    xfree (slot->value.not_found.name);
+  slot->state = SYMBOL_SLOT_UNUSED;
+}
+
 /* Symbols don't specify global vs static block.
    So keep them in separate caches.  */
 
@@ -201,6 +211,19 @@  struct block_symbol_cache
   struct symbol_cache_slot symbols[1];
 };
 
+/* Clear all slots of BSC and free BSC.  */
+
+static void
+destroy_block_symbol_cache (struct block_symbol_cache *bsc)
+{
+  if (bsc != nullptr)
+    {
+      for (unsigned int i = 0; i < bsc->size; i++)
+	symbol_cache_clear_slot (&bsc->symbols[i]);
+      xfree (bsc);
+    }
+}
+
 /* The symbol cache.
 
    Searching for symbols in the static and global blocks over multiple objfiles
@@ -217,8 +240,8 @@  struct symbol_cache
 
   ~symbol_cache ()
   {
-    xfree (global_symbols);
-    xfree (static_symbols);
+    destroy_block_symbol_cache (global_symbols);
+    destroy_block_symbol_cache (static_symbols);
   }
 
   struct block_symbol_cache *global_symbols = nullptr;
@@ -1234,8 +1257,8 @@  resize_symbol_cache (struct symbol_cache *cache, unsigned int new_size)
 	  && new_size == 0))
     return;
 
-  xfree (cache->global_symbols);
-  xfree (cache->static_symbols);
+  destroy_block_symbol_cache (cache->global_symbols);
+  destroy_block_symbol_cache (cache->static_symbols);
 
   if (new_size == 0)
     {
@@ -1373,16 +1396,6 @@  symbol_cache_lookup (struct symbol_cache *cache,
   return {};
 }
 
-/* Clear out SLOT.  */
-
-static void
-symbol_cache_clear_slot (struct symbol_cache_slot *slot)
-{
-  if (slot->state == SYMBOL_SLOT_NOT_FOUND)
-    xfree (slot->value.not_found.name);
-  slot->state = SYMBOL_SLOT_UNUSED;
-}
-
 /* Mark SYMBOL as found in SLOT.
    OBJFILE_CONTEXT is the current objfile when the lookup was done, or NULL
    if it's not needed to distinguish lookups (STATIC_BLOCK).  It is *not*