Re: [PATCH] Make blockvector a little more self-contained

[Simon Marchi <simark@simark.ca>]

On 2025-11-18 22:30, Tom Tromey wrote:
> This patch changes blockvector to be a little more self-contained.
> 
> The idea here is that code outside of blockvector shouldn't really
> know how it operates.  After this patch, this still doesn't fully
> happen -- a couple spots check the result of map() and make decisions
> based on that -- but this is a step toward making that happen.  The
> longer term idea here is that this is needed to enable lazier CU
> expansion.
> 
> Meanwhile, this patch seems like a simple cleanup.  Relocation is now
> handled by the blockvector itself and the non-const map() method can
> be removed.

Overall, LGTM.

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

See minor comments below.

> 
> There wasn't a great spot to move the section_offsets typedef.  I
> chose defs.h.  I've also updated the comment there as it has been out
> of date for a long time.  I've also removed an obsolete comment from
> the symbol-relocation code.

I honestly think that this typedef could be removed, that it would be
clearer to see "std::vector<CORE_ADDR>" in the code.

> diff --git a/gdb/block.c b/gdb/block.c
> index 9fb04635975..e959adc968c 100644
> --- a/gdb/block.c
> +++ b/gdb/block.c
> @@ -366,6 +366,28 @@ block::static_link () const
>    return (struct dynamic_prop *) objfile_lookup_static_link (objfile, this);
>  }
>  
> +/* See block.h.  */
> +
> +void
> +block::relocate (struct objfile *objfile, const section_offsets &offsets)
> +{
> +  int block_line_section = SECT_OFF_TEXT (objfile);
> +
> +  set_start (start () + offsets[block_line_section]);
> +  set_end (end () + offsets[block_line_section]);
> +
> +  for (blockrange &r : ranges ())
> +    {
> +      r.set_start (r.start () + offsets[block_line_section]);
> +      r.set_end (r.end () + offsets[block_line_section]);
> +    }
> +
> +  /* We only want to iterate over the local symbols, not any
> +     symbols in included symtabs.  */
> +  for (struct symbol *sym : multidict_symbols ())
> +    sym->relocate (objfile, offsets);

Do you know what the comment above refers to?  What are we doing special
to avoid iterating on the symbols in included symtabs?

> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 845c49e057d..4f2b88859a5 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -1458,6 +1458,11 @@ struct symbol : public general_symbol_info, public allocate_on_obstack<symbol>
>  
>    void set_symtab (struct symtab *symtab);
>  
> +  /* Relocate this symbol.  OBJFILE is the objfile holding this
> +     blockvector, and OFFSETS is the relocation offsets to use.  */
> +
> +  void relocate (struct objfile *objfile, const section_offsets &offsets);

The comment says "this blockvector", looks like a copy-pasto.

Also, a struct symbol already knows its objfile (it has an objfile()
method) if it is objfile-owned, which should be the case here.  So it
seems superfluous to pass the objfile here.  Actually, the objfile
parameter is not even used in symbol::relocate.

Simon