Re: [PATCH 2/2] Free multidicts from blockvector

[Simon Marchi <simark@simark.ca>]

On 10/23/25 1:17 PM, Tom Tromey wrote:
> Currently, nothing in the tree ever calls mdict_free.  However, code
> does heap-allocate some multidicts.  A simple way to see this is to
> use valgrind, run "gdb -readnow" on the executable created by
> gdb.dwarf2/struct-with-sig.exp, and then use "file" to clear the
> objfile list.  This yields:
> 
> ==1522843== 144 (16 direct, 128 indirect) bytes in 1 blocks are definitely lost in loss record 905 of 3,005
> ==1522843==    at 0x4843866: malloc (vg_replace_malloc.c:446)
> ==1522843==    by 0x48E397: xmalloc (alloc.c:52)
> ==1522843==    by 0x59DE66: multidictionary* xnew<multidictionary>() (poison.h:102)
> ==1522843==    by 0x59CFF4: mdict_create_hashed_expandable(language) (dictionary.c:965)
> ==1522843==    by 0x50A269: buildsym_compunit::finish_block_internal(symbol*, pending**, pending_block*, dynamic_prop const*, unsigned long, unsigned long, int, int) (buildsym.c:221)
> ==1522843==    by 0x50AE04: buildsym_compunit::end_compunit_symtab_get_static_block(unsigned long, int, int) (buildsym.c:818)
> ==1522843==    by 0x50C4CF: buildsym_compunit::end_expandable_symtab(unsigned long) (buildsym.c:1037)
> ==1522843==    by 0x61DBC6: process_full_type_unit (read.c:4970)
> 
> This patch fixes the leaks by calling mdict_free when a blockvector is
> destroyed.
> ---
>  gdb/block.c | 6 ++++++
>  gdb/block.h | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/gdb/block.c b/gdb/block.c
> index 02e6d0e4be2bab2c1b3750a2f396e4bc97b7bc5d..2d8d40ec4eb94a1c891a3f8b48972eff3050e66d 100644
> --- a/gdb/block.c
> +++ b/gdb/block.c
> @@ -842,6 +842,12 @@ blockvector::append_block (struct block *block)
>    m_blocks.push_back (block);
>  }
>  
> +blockvector::~blockvector ()
> +{
> +  for (struct block *bl : m_blocks)
> +    mdict_free (bl->multidict ());
> +}

Ideally it would be struct block's just to call that.  You could add a
destructor to struct block and call it explicitly here.  It would be the
same result, but would be more correct encapsulation-wise.  But I guess
this is fine too for the time being.

> +
>  /* Implement 'maint info blocks' command.  If passed an argument then
>     print a list of all blocks at the given address.  With no arguments
>     then list all blocks at the current address of the current inferior.  */
> diff --git a/gdb/block.h b/gdb/block.h
> index 61070619ca7daa7b3310a9c5cd8d6ca86c044ada..98ce5a3839dbb8419c9e3a918d27b3b58485fea4 100644
> --- a/gdb/block.h
> +++ b/gdb/block.h
> @@ -424,6 +424,8 @@ struct blockvector
>      : m_blocks (nblocks, nullptr)
>    {}
>  
> +  ~blockvector ();

Since there is a user-defined destructor, I would add a
DISABLE_COPY_AND_ASSIGN.  Otherwise, LGTM.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon