Re: [PATCH] Add proper handling for non-local references in nested subprograms

[Pedro Alves <palves@redhat.com>]

Hi Pierre-Marie,

I would have helped this reader to include an example of "non-local references
in nested subprograms" in the mail body :-)  Given the reference to
"subprograms" in the subject, I assumed this was an Ada-specific
patch.  I happened to skim the patch anyway, until I saw at the end
that this also handles C nested functions.  Nice!  :-)

On 03/02/2015 02:36 PM, Pierre-Marie de Rodat wrote:
> GDB's current behavior when dealing with non-local references in the
> context of nested subprograms is approximative:
> 
>    - code using valops.c:value_of_variable read the first available stack
>      frame that holds the corresponding variable (whereas there can be
>      multiple candidates for this);
> 
>    - code directly relying on read_var_value will instead read non-local
>      variables in frames where they are not even defined.
> 
> This change adds necessary information to symbols (the block they belong
> to) and to blocks (the static link property, if any) so that GDB can
> make the proper decisions when dealing with non-local variables.
> 
> Regtested on x86_64-linux with both GCC 4.9.2 and a patched GCC (see 
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53927>).
> 
> PS: I'm aware that this patches increases the size of two critical data 
> structures (namely struct block and struct symbol) and I'm completely 
> open to suggestions. :-)

Right, that's a problem.  We've been moving in the opposite direction...
The block knows which symbols it has inside.  When we look up symbols,
we always know which block we're searching the symbols in...  If
you need to know which block a symbol look up found a symbol in,
there's the "block_found" for that.  That's obviously an ugly hack, but
it's there and so you can use it.  If someone is motivated to clean this
up, it'd be better to make the symbol lookup functions return a
structure that included both symbol and block (and maybe more), in
the spirit of struct bound_minimal_symbol:

/* Several lookup functions return both a minimal symbol and the
   objfile in which it is found.  This structure is used in these
   cases.  */

struct bound_minimal_symbol
{
  /* The minimal symbol that was found, or NULL if no minimal symbol
     was found.  */

  struct minimal_symbol *minsym;

  /* If MINSYM is not NULL, then this is the objfile in which the
     symbol is defined.  */

  struct objfile *objfile;
};

For the block->static_link case, maybe put the static link chain
in a separate hash indexed by block?

> 
>      Since I'm not sure of how this issue should be solved, I'm 
> nevertheless posting this patch here so this matter can be discussed. In 
> the context of this feature, I think we need a backlink from all symbols 
> to the corresponding embedding block but on the other hand only a few 
> blocks have static link: maybe we could turn this static_link field into 
> a objfile-based hashtable lookup. Thoughts?

Ah, yeah, something like that.  :-)

Thanks,
Pedro Alves