This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add proper handling for non-local references in nested functions
- From: Doug Evans <xdje42 at gmail dot com>
- To: Pierre-Marie de Rodat <derodat at adacore dot com>
- Cc: Kevin Buettner <kevinb at redhat dot com>, gdb-patches at sourceware dot org
- Date: Sun, 26 Jul 2015 13:34:12 -0700
- Subject: Re: [PATCH] Add proper handling for non-local references in nested functions
- Authentication-results: sourceware.org; auth=none
- References: <54F47563 dot 4050103 at adacore dot com> <54FF0D05 dot 70907 at redhat dot com> <550C1170 dot 9070208 at adacore dot com> <55685B60 dot 3000004 at redhat dot com> <55775EB0 dot 4080701 at adacore dot com> <55AF5F7E dot 5000600 at adacore dot com> <20150722173957 dot 7ed51f18 at pinnacle dot lan> <55B0C583 dot 6050601 at adacore dot com>
Pierre-Marie de Rodat <derodat@adacore.com> writes:
> Kevin,
>
> On 07/23/2015 02:39 AM, Kevin Buettner wrote:
>> I had to make the following change in order to obtain a clean build
>> using your patches:
>>
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index e20aead..d6e3d55 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -11457,7 +11457,7 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
>>
>> newobj = pop_context ();
>> /* Make a block for the local symbols within. */
>> - block = finish_block (newobj->name, &newobj->static_link,
>> + block = finish_block (newobj->name, newobj->static_link,
>> &local_symbols, newobj->old_blocks,
>> lowpc, highpc);
>
> Huh, I actually have a warning I did not notice but not an error, so I
> guess you built GDB as C++. Anyway, this is a big mistake that should
> have broken the feature I intend to add: quite bad! Thank you very
> much for pointing me at it.
>
> I had a look at why testsuite was clean even with this error, and I
> realized that all my testing was with the wrong compiler[1]: this part
> of the code was actually not exercized... So I picked the proper
> compiler and this time I saw this was not working! This time I fixed
> the feature, built in C++ mode just to be sure. Here's the updated
> patch:
>
> - I fixed yacc parsers in all languages so that block information
> from symbol lookups are properly transmitted to returned expressions.
>
> - I fixed the usage of hash table to save static link (I do mappings
> in objfiles.c, not sets so my usage of htab was invalid).
>
> So this time, tested both with a GCC release and a patched GCC: no
> regression on x86_64-linux. I kept the "static link" terminology
> because I'm waiting for a consensus before changing everything. ;-)
>
>
> [1] In nested functions, an unpatched GCC creates local variables that
> are references to the non-local ones, so the feature works at the user
> level, but not using the static link machinery. The aim of my change
> is to make GDB work even without these "fake" local reference
> variables.
>
> --
> Pierre-Marie de Rodat
>
> From 2e65051fd50bb99985bbc2c8ed13c67514e9cc08 Mon Sep 17 00:00:00 2001
> From: Pierre-Marie de Rodat <derodat@adacore.com>
> Date: Thu, 5 Feb 2015 17:00:06 +0100
> Subject: [PATCH] DWARF: handle non-local references in nested functions
>
> GDB's current behavior when dealing with non-local references in the
> context of nested fuctions 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 the necessary context to symbol reads (to get 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 varibale
> references.
>
> gdb/ChangeLog:
>
> * ada-lang.c (ada_read_var_value): Add a VAR_BLOCK argument and pass
> it to default_read_var_value.
> * block.c (block_static_link): New accessor.
> * block.h (block_static_link): Declare it.
> * buildsym.c (finish_block_internal): Add a static_link
> argument. If there is a static link, associate it to the new
> block.
> (finish_block): Add a static link argument and pass it to
> finish_block_internal.
> (end_symtab_get_static_block): Update calls to finish_block and
> to finish_block_internal.
> (end_symtab_with_blockvector): Update call to
> finish_block_internal.
> * buildsym.h: Forward-declare struct dynamic_prop.
> (struct context_stack): Add a static_link field.
> (finish_block): Add a static link argument.
> * c-exp.y: Remove an obsolete
> comment (evaluation of variables already start from the selected
> frame, and now they climb *up* the call stack) and propagate the
> block information to the produced expression.
> * d-exp.y: Likewise.
> * f-exp.y: Likewise.
> * go-exp.y: Likewise.
> * jv-exp.y: Likewise.
> * m2-exp.y: Likewise.
> * p-exp.y: Likewise.
> * coffread.c (coff_symtab_read): Update calls to finish_block.
> * dbxread.c (process_one_symbol): Likewise.
> * xcoffread.c (read_xcoff_symtab): Likewise.
> * compile/compile-c-symbols.c (convert_one_symbol): Add a
> VAR_BLOCK parameter and pass it to calls to read_var_value.
> (convert_symbol_sym): Pass the block corresponding to SYM to the
> call to convert_one_symbol.
> * compile/compile-loc2c.c (do_compile_dwarf_expr_to_c): Update
> call to read_var_value.
> * dwarf2loc.c (block_op_get_frame_base): New.
> (dwarf2_block_frame_base_locexpr_funcs): Implement the
> get_frame_base method.
> (dwarf2_block_frame_base_loclist_funcs): Likewise.
> (dwarf2locexpr_baton_eval): Add a frame argument and use it
> instead of the selected frame in order to evaluate the
> expression.
> (dwarf2_evaluate_property): Add a frame argument. Update call
> to dwarf2_locexpr_baton_eval to provide a frame in available and
> to handle the absence of address stack.
> * dwarf2loc.h (dwarf2_evaluate_property): Add a frame argument.
> * dwarf2read.c (attr_to_dynamic_prop): Add a forward
> declaration.
> (read_func_scope): Record any available static link description.
> Update call to finish_block.
> (read_lexical_block_scope): Update call to finish_block.
> * findvar.c (follow_static_link): New.
> (get_hosting_frame): New.
> (default_read_var_value): Add a VAR_BLOCK argument. Use
> get_hosting_frame to handle non-local references.
> (read_var_value): Add a VAR_BLOCK argument and pass it to the
> LA_READ_VAR_VALUE method.
> * gdbtypes.c (resolve_dynamic_range): Update calls to
> dwarf2_evaluate_property.
> (resolve_dynamic_type_internal): Likewise.
> * infcmd.c (finish_command_continuation): Update call to
> read_var_value, passing it the block coming from symbol lookup.
> * infrun.c (insert_exception_resume_breakpoint): Likewise.
> * language.h (struct language_defn): Add a VAR_BLOCK argument to
> the LA_READ_VAR_VALUE method.
> * objfiles.c (objfile_register_static_link): New.
> (objfile_lookup_static_link): New.
> (free_objfile): Free the STATIC_LINKS hashed map if needed.
> * objfiles.h: Include hashtab.h.
> (struct objfile): Add a STATIC_LINKS field.
> (objfile_register_static_link): New.
> (objfile_lookup_static_link): New.
> * printcmd.c (print_variable_and_value): Update call to
> read_var_value.
> * python/py-finishbreakpoint.c (bpfinishpy_init): Likewise.
> * python/py-frame.c (frapy_read_var): Update call to
> read_var_value, passing it the block coming from symbol lookup.
> * python/py-framefilter.c (extract_sym): Add a SYM_BLOCK
> parameter and set the pointed value to NULL (TODO).
> (enumerate_args): Update call to extract_sym.
> (enumerate_locals): Update calls to extract_sym and to
> read_var_value.
> * python/py-symbol.c (sympy_value): Update call to
> read_var_value (TODO).
> * stack.c (read_frame_local): Update call to read_var_value.
> (read_frame_arg): Likewise.
> (return_command): Likewise.
> * symtab.h (struct symbol_block_ops): Add a get_frame_base
> method.
> (struct symbol): Add a block field.
> (SYMBOL_BLOCK): New accessor.
> * valops.c (value_of_variable): Remove frame/block handling and
> pass the block argument to read_var_value, which does this job
> now.
> (value_struct_elt_for_reference): Update calls to
> read_var_value.
> (value_of_this): Pass the block found to read_var_value.
> * value.h (read_var_value): Add a VAR_BLOCK argument.
> (default_read_var_value): Likewise.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/nested-subp1.exp: New file.
> * gdb.base/nested-subp1.c: New file.
> * gdb.base/nested-subp2.exp: New file.
> * gdb.base/nested-subp2.c: New file.
> * gdb.base/nested-subp3.exp: New file.
> * gdb.base/nested-subp3.c: New file.
>...
Hi.
Several nits and questions inline. grep for ====.
One thing I still want to do is take this patch and run it through
the perf testsuite.
Also, I still need to look at follow_static_link, get_hosting_frame closer.
Cool stuff though, there's clearly missing functionality we need here.
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index ded195f..5c43a7a 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -13750,7 +13750,8 @@ ada_get_symbol_name_cmp (const char *lookup_name)
> /* Implement the "la_read_var_value" language_defn method for Ada. */
>
> static struct value *
> -ada_read_var_value (struct symbol *var, struct frame_info *frame)
> +ada_read_var_value (struct symbol *var, const struct block *var_block,
> + struct frame_info *frame)
> {
> const struct block *frame_block = NULL;
> struct symbol *renaming_sym = NULL;
> @@ -13766,7 +13767,7 @@ ada_read_var_value (struct symbol *var, struct frame_info *frame)
>
> /* This is a typical case where we expect the default_read_var_value
> function to work. */
> - return default_read_var_value (var, frame);
> + return default_read_var_value (var, var_block, frame);
> }
>
> const struct language_defn ada_language_defn = {
> diff --git a/gdb/block.c b/gdb/block.c
> index f7621aa..f4b8e4f 100644
> --- a/gdb/block.c
> +++ b/gdb/block.c
> @@ -428,6 +428,21 @@ set_block_compunit_symtab (struct block *block, struct compunit_symtab *cu)
> gb->compunit_symtab = cu;
> }
>
> +/* See block.h. */
> +
> +struct dynamic_prop *
> +block_static_link (const struct block *block)
> +{
> + struct objfile *objfile = block_objfile (block);
> +
> + /* Only objfile-owned blocks that materialize top function scopes can have
> + static links. */
> + if (objfile == NULL || BLOCK_FUNCTION (block) == NULL)
> + return NULL;
> +
> + return (struct dynamic_prop *) objfile_lookup_static_link (objfile, block);
> +}
> +
> /* Return the compunit of the global block. */
>
> static struct compunit_symtab *
> diff --git a/gdb/block.h b/gdb/block.h
> index d8ad343..6e7d247 100644
> --- a/gdb/block.h
> +++ b/gdb/block.h
> @@ -190,6 +190,12 @@ extern struct block *allocate_global_block (struct obstack *obstack);
> extern void set_block_compunit_symtab (struct block *,
> struct compunit_symtab *);
>
> +/* Return a property to evaluate the static link associated to BLOCK. Note
> + that only objfile-owned and function-level blocks can have a static link.
> + Return NULL if there is no such property. */
> +
====
Add a comment here stating that the term "static_link" is derived from
DW_AT_static_link.
> +extern struct dynamic_prop *block_static_link (const struct block *block);
> +
> /* A block iterator. This structure should be treated as though it
> were opaque; it is only defined here because we want to support
> stack allocation of iterators. */
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 2a24a25..a2be2cb 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -331,7 +331,8 @@ free_pending_blocks (void)
> file). Put the block on the list of pending blocks. */
>
> static struct block *
> -finish_block_internal (struct symbol *symbol, struct pending **listhead,
> +finish_block_internal (struct symbol *symbol, struct dynamic_prop *static_link,
> + struct pending **listhead,
> struct pending_block *old_blocks,
====
Move the static_link property here.
[Arguments to functions aren't in completely random order,
and here static_link is among the collection of random things
about the block like start,end. So it reads better to me if
static_link appears with start,end]
> CORE_ADDR start, CORE_ADDR end,
> int is_global, int expandable)
> @@ -422,6 +423,9 @@ finish_block_internal (struct symbol *symbol, struct pending **listhead,
> BLOCK_FUNCTION (block) = NULL;
> }
>
> + if (static_link != NULL)
> + objfile_register_static_link (objfile, block, static_link);
> +
> /* Now "free" the links of the list, and empty the list. */
>
> for (next = *listhead; next; next = next1)
> @@ -512,11 +516,12 @@ finish_block_internal (struct symbol *symbol, struct pending **listhead,
> }
>
> struct block *
> -finish_block (struct symbol *symbol, struct pending **listhead,
> +finish_block (struct symbol *symbol, struct dynamic_prop *static_link,
> + struct pending **listhead,
> struct pending_block *old_blocks,
> CORE_ADDR start, CORE_ADDR end)
> {
> - return finish_block_internal (symbol, listhead, old_blocks,
> + return finish_block_internal (symbol, static_link, listhead, old_blocks,
> start, end, 0, 0);
> }
>
> @@ -1218,7 +1223,7 @@ end_symtab_get_static_block (CORE_ADDR end_addr, int expandable, int required)
> struct context_stack *cstk = pop_context ();
>
> /* Make a block for the local symbols within. */
> - finish_block (cstk->name, &local_symbols, cstk->old_blocks,
> + finish_block (cstk->name, NULL, &local_symbols, cstk->old_blocks,
> cstk->start_addr, end_addr);
>
> if (context_stack_depth > 0)
> @@ -1289,7 +1294,7 @@ end_symtab_get_static_block (CORE_ADDR end_addr, int expandable, int required)
> else
> {
> /* Define the STATIC_BLOCK. */
> - return finish_block_internal (NULL, &file_symbols, NULL,
> + return finish_block_internal (NULL, NULL, &file_symbols, NULL,
> last_source_start_addr, end_addr,
> 0, expandable);
> }
> @@ -1317,7 +1322,7 @@ end_symtab_with_blockvector (struct block *static_block,
> end_addr = BLOCK_END (static_block);
>
> /* Create the GLOBAL_BLOCK and build the blockvector. */
> - finish_block_internal (NULL, &global_symbols, NULL,
> + finish_block_internal (NULL, NULL, &global_symbols, NULL,
> last_source_start_addr, end_addr,
> 1, expandable);
> blockvector = make_blockvector ();
> diff --git a/gdb/buildsym.h b/gdb/buildsym.h
> index f98203e..8828ad2 100644
> --- a/gdb/buildsym.h
> +++ b/gdb/buildsym.h
> @@ -39,6 +39,8 @@ struct compunit_symtab;
> struct block;
> struct pending_block;
>
> +struct dynamic_prop;
> +
> #ifndef EXTERN
> #define EXTERN extern
> #endif
> @@ -141,6 +143,11 @@ struct context_stack
>
> struct symbol *name;
>
> + /* Expression that computes the frame base of the lexically enclosing
> + function, if any. NULL otherwise. */
> +
> + struct dynamic_prop *static_link;
> +
> /* PC where this context starts */
>
> CORE_ADDR start_addr;
> @@ -192,9 +199,10 @@ extern struct symbol *find_symbol_in_list (struct pending *list,
> char *name, int length);
>
> extern struct block *finish_block (struct symbol *symbol,
> - struct pending **listhead,
> - struct pending_block *old_blocks,
> - CORE_ADDR start, CORE_ADDR end);
> + struct dynamic_prop *static_link,
> + struct pending **listhead, struct
> + pending_block *old_blocks, CORE_ADDR start,
> + CORE_ADDR end);
>
> extern void record_block_range (struct block *,
> CORE_ADDR start, CORE_ADDR end_inclusive);
> diff --git a/gdb/c-exp.y b/gdb/c-exp.y
> index b408215..3b7e572 100644
> --- a/gdb/c-exp.y
> +++ b/gdb/c-exp.y
> @@ -1070,10 +1070,7 @@ variable: name_not_typename
> }
>
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> - /* We want to use the selected frame, not
> - another more inner frame which happens to
> - be in the same block. */
> - write_exp_elt_block (pstate, NULL);
> + write_exp_elt_block (pstate, sym.block);
> write_exp_elt_sym (pstate, sym.symbol);
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> }
> diff --git a/gdb/coffread.c b/gdb/coffread.c
> index 7722cdb..e41e9a5 100644
> --- a/gdb/coffread.c
> +++ b/gdb/coffread.c
> @@ -1144,7 +1144,7 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
> enter_linenos (fcn_line_ptr, fcn_first_line,
> fcn_last_line, objfile);
>
> - finish_block (newobj->name, &local_symbols,
> + finish_block (newobj->name, NULL, &local_symbols,
> newobj->old_blocks, newobj->start_addr,
> fcn_cs_saved.c_value
> + fcn_aux_saved.x_sym.x_misc.x_fsize
> @@ -1188,7 +1188,7 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
> cs->c_value + ANOFFSET (objfile->section_offsets,
> SECT_OFF_TEXT (objfile));
> /* Make a block for the local symbols within. */
> - finish_block (0, &local_symbols, newobj->old_blocks,
> + finish_block (0, NULL, &local_symbols, newobj->old_blocks,
> newobj->start_addr, tmpaddr);
> }
> /* Now pop locals of block just finished. */
> diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
> index 5f27583..f1dfccf 100644
> --- a/gdb/compile/compile-c-symbols.c
> +++ b/gdb/compile/compile-c-symbols.c
> @@ -134,16 +134,16 @@ symbol_substitution_name (struct symbol *sym)
> return concat ("__", SYMBOL_NATURAL_NAME (sym), "_ptr", (char *) NULL);
> }
>
> -/* Convert a given symbol, SYM, to the compiler's representation.
> - CONTEXT is the compiler instance. IS_GLOBAL is true if the
> - symbol came from the global scope. IS_LOCAL is true if the symbol
> - came from a local scope. (Note that the two are not strictly
> - inverses because the symbol might have come from the static
> - scope.) */
> +/* Convert a given symbol, SYM (located in VAR_BLOCK, if any), to the
> + compiler's representation. CONTEXT is the compiler instance. IS_GLOBAL is
> + true if the symbol came from the global scope. IS_LOCAL is true if the
> + symbol came from a local scope. (Note that the two are not strictly
> + inverses because the symbol might have come from the static scope.) */
>
> static void
> convert_one_symbol (struct compile_c_instance *context,
> struct symbol *sym,
> + const struct block *var_block,
> int is_global,
> int is_local)
> {
> @@ -247,7 +247,7 @@ convert_one_symbol (struct compile_c_instance *context,
> SYMBOL_PRINT_NAME (sym));
> }
>
> - val = read_var_value (sym, frame);
> + val = read_var_value (sym, var_block, frame);
> if (VALUE_LVAL (val) != lval_memory)
> error (_("Symbol \"%s\" cannot be used for compilation "
> "evaluation as its address has not been found."),
> @@ -339,7 +339,8 @@ convert_symbol_sym (struct compile_c_instance *context, const char *identifier,
> fprintf_unfiltered (gdb_stdlog,
> "gcc_convert_symbol \"%s\": global symbol\n",
> identifier);
> - convert_one_symbol (context, global_sym.symbol, 1, 0);
> + convert_one_symbol (context, global_sym.symbol, global_sym.block, 1,
> + 0);
> }
> }
>
> @@ -347,7 +348,7 @@ convert_symbol_sym (struct compile_c_instance *context, const char *identifier,
> fprintf_unfiltered (gdb_stdlog,
> "gcc_convert_symbol \"%s\": local symbol\n",
> identifier);
> - convert_one_symbol (context, sym, 0, is_local_symbol);
> + convert_one_symbol (context, sym, block, 0, is_local_symbol);
> }
>
> /* Convert a minimal symbol to its gcc form. CONTEXT is the compiler
> diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
> index 6f53814..18ca4ae 100644
> --- a/gdb/compile/compile-loc2c.c
> +++ b/gdb/compile/compile-loc2c.c
> @@ -636,7 +636,7 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
> "there is no selected frame"),
> SYMBOL_PRINT_NAME (sym));
>
> - val = read_var_value (sym, frame);
> + val = read_var_value (sym, NULL, frame);
> if (VALUE_LVAL (val) != lval_memory)
> error (_("Symbol \"%s\" cannot be used for compilation evaluation "
> "as its address has not been found."),
> diff --git a/gdb/d-exp.y b/gdb/d-exp.y
> index c95222b..336b671 100644
> --- a/gdb/d-exp.y
> +++ b/gdb/d-exp.y
> @@ -1066,9 +1066,7 @@ push_variable (struct parser_state *ps, struct stoken name)
> }
>
> write_exp_elt_opcode (ps, OP_VAR_VALUE);
> - /* We want to use the selected frame, not another more inner frame
> - which happens to be in the same block. */
> - write_exp_elt_block (ps, NULL);
> + write_exp_elt_block (ps, sym.block);
> write_exp_elt_sym (ps, sym.symbol);
> write_exp_elt_opcode (ps, OP_VAR_VALUE);
> return 1;
> diff --git a/gdb/dbxread.c b/gdb/dbxread.c
> index 6098b35..32893f6 100644
> --- a/gdb/dbxread.c
> +++ b/gdb/dbxread.c
> @@ -2766,8 +2766,8 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, char *name,
> newobj = pop_context ();
>
> /* Make a block for the local symbols within. */
> - block = finish_block (newobj->name, &local_symbols,
> - newobj->old_blocks,
> + block = finish_block (newobj->name, NULL,
> + &local_symbols, newobj->old_blocks,
> newobj->start_addr, newobj->start_addr + valu);
>
> /* For C++, set the block's scope. */
> @@ -2868,7 +2868,7 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, char *name,
> newobj->start_addr = valu;
> }
> /* Make a block for the local symbols within. */
> - finish_block (0, &local_symbols, newobj->old_blocks,
> + finish_block (0, NULL, &local_symbols, newobj->old_blocks,
> newobj->start_addr, valu);
> }
> }
> @@ -3165,7 +3165,7 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, char *name,
>
> newobj = pop_context ();
> /* Make a block for the local symbols within. */
> - block = finish_block (newobj->name, &local_symbols,
> + block = finish_block (newobj->name, NULL, &local_symbols,
> newobj->old_blocks, newobj->start_addr,
> valu);
>
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index c75767e..edfa220 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -381,12 +381,42 @@ locexpr_find_frame_base_location (struct symbol *framefunc, CORE_ADDR pc,
> *start = symbaton->data;
> }
>
> +/* Implement the struct symbol_block_ops::get_frame_base method. */
> +
> +static CORE_ADDR
> +block_op_get_frame_base (struct symbol *framefunc, struct frame_info *frame)
> +{
> + if (SYMBOL_BLOCK_OPS (framefunc)->find_frame_base_location != NULL)
> + {
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> + struct type *type = builtin_type (gdbarch)->builtin_data_ptr;
> + struct dwarf2_locexpr_baton *dlbaton = SYMBOL_LOCATION_BATON (framefunc);
> + const gdb_byte *start;
> + size_t length;
> + struct value *result;
> +
> + SYMBOL_BLOCK_OPS (framefunc)->find_frame_base_location
> + (framefunc, get_frame_pc (frame), &start, &length);
> + result = dwarf2_evaluate_loc_desc (type, frame, start, length,
> + dlbaton->per_cu);
> +
> + /* The DW_AT_frame_base attribute contains a location description which
> + computes the base address itself. However, the call to
> + dwarf2_evaluate_loc_desc returns a value representing a variable at
> + that address. The frame base address is thus this variable's
> + address. */
> + return value_address (result);
> + }
> + return 0;
> +}
====
If this is implemented on top of symbol_block_ops::find_frame_base_location,
do we need another method? Or can we just have a wrapper that calls
find_frame_base_location?
For one, I see block_op_get_frame_base being used for both
dwarf2_block_frame_base_locexpr_funcs and
dwarf2_block_frame_base_loclist_funcs.
> +
> /* Vector for inferior functions as represented by LOC_BLOCK, if the inferior
> function uses DWARF expression for its DW_AT_frame_base. */
>
> const struct symbol_block_ops dwarf2_block_frame_base_locexpr_funcs =
> {
> - locexpr_find_frame_base_location
> + locexpr_find_frame_base_location,
> + block_op_get_frame_base
> };
>
> /* Implement find_frame_base_location method for LOC_BLOCK functions using
> @@ -406,7 +436,8 @@ loclist_find_frame_base_location (struct symbol *framefunc, CORE_ADDR pc,
>
> const struct symbol_block_ops dwarf2_block_frame_base_loclist_funcs =
> {
> - loclist_find_frame_base_location
> + loclist_find_frame_base_location,
> + block_op_get_frame_base
> };
>
> /* See dwarf2loc.h. */
> @@ -2396,13 +2427,14 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
> }
>
> /* Evaluates a dwarf expression and stores the result in VAL, expecting
> - that the dwarf expression only produces a single CORE_ADDR. ADDR is a
> - context (location of a variable) and might be needed to evaluate the
> - location expression.
> + that the dwarf expression only produces a single CORE_ADDR. FRAME is the
> + frame in which the expression is evaluated. ADDR is a context (location of
> + a variable) and might be needed to evaluate the location expression.
> Returns 1 on success, 0 otherwise. */
>
> static int
> dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
> + struct frame_info *frame,
> CORE_ADDR addr,
> CORE_ADDR *valp)
> {
> @@ -2417,7 +2449,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
> ctx = new_dwarf_expr_context ();
> cleanup = make_cleanup_free_dwarf_expr_context (ctx);
>
> - baton.frame = get_selected_frame (NULL);
> + baton.frame = frame;
> baton.per_cu = dlbaton->per_cu;
> baton.obj_address = addr;
>
> @@ -2461,19 +2493,24 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
>
> int
> dwarf2_evaluate_property (const struct dynamic_prop *prop,
> + struct frame_info *frame,
> struct property_addr_info *addr_stack,
> CORE_ADDR *value)
> {
> if (prop == NULL)
> return 0;
>
> + if (frame == NULL && has_stack_frames ())
> + frame = get_selected_frame (NULL);
> +
> switch (prop->kind)
> {
> case PROP_LOCEXPR:
> {
> const struct dwarf2_property_baton *baton = prop->data.baton;
>
> - if (dwarf2_locexpr_baton_eval (&baton->locexpr, addr_stack->addr,
> + if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
> + addr_stack ? addr_stack->addr : 0,
> value))
> {
> if (baton->referenced_type)
> @@ -2490,7 +2527,6 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
> case PROP_LOCLIST:
> {
> struct dwarf2_property_baton *baton = prop->data.baton;
> - struct frame_info *frame = get_selected_frame (NULL);
> CORE_ADDR pc = get_frame_address_in_block (frame);
> const gdb_byte *data;
> struct value *val;
> diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
> index f3630ac..2415656 100644
> --- a/gdb/dwarf2loc.h
> +++ b/gdb/dwarf2loc.h
> @@ -122,12 +122,19 @@ struct property_addr_info
> struct property_addr_info *next;
> };
>
> -/* Converts a dynamic property into a static one. ADDR_STACK is the stack
> - of addresses that might be needed to evaluate the property.
> +/* Converts a dynamic property into a static one. FRAME is the frame in which
> + the property is evaluated; if NULL, the selected frame (if any) is used
> + instead.
> +
> + ADDR_STACK is the stack of addresses that might be needed to evaluate the
> + property. When evaluating a property that is not related to a type, it can
> + be NULL.
> +
> Returns 1 if PROP could be converted and the static value is passed back
> into VALUE, otherwise returns 0. */
>
> int dwarf2_evaluate_property (const struct dynamic_prop *prop,
> + struct frame_info *frame,
> struct property_addr_info *addr_stack,
> CORE_ADDR *value);
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index f440956..d6e3d55 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1736,6 +1736,10 @@ static void load_full_type_unit (struct dwarf2_per_cu_data *per_cu);
>
> static void read_signatured_type (struct signatured_type *);
>
> +static int attr_to_dynamic_prop (const struct attribute *attr,
> + struct die_info *die, struct dwarf2_cu *cu,
> + struct dynamic_prop *prop);
> +
> /* memory allocation interface */
>
> static struct dwarf_block *dwarf_alloc_block (struct dwarf2_cu *);
> @@ -11393,6 +11397,16 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
> if (attr)
> dwarf2_symbol_mark_computed (attr, newobj->name, cu, 1);
>
> + /* If there is a location for the static link, record it. */
> + newobj->static_link = NULL;
> + attr = dwarf2_attr (die, DW_AT_static_link, cu);
> + if (attr)
> + {
> + newobj->static_link = obstack_alloc (&objfile->objfile_obstack,
> + sizeof (*newobj->static_link));
> + attr_to_dynamic_prop (attr, die, cu, newobj->static_link);
> + }
> +
> cu->list_in_scope = &local_symbols;
>
> if (die->child != NULL)
> @@ -11443,7 +11457,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
>
> newobj = pop_context ();
> /* Make a block for the local symbols within. */
> - block = finish_block (newobj->name, &local_symbols, newobj->old_blocks,
> + block = finish_block (newobj->name, newobj->static_link,
> + &local_symbols, newobj->old_blocks,
> lowpc, highpc);
>
> /* For C++, set the block's scope. */
> @@ -11529,7 +11544,7 @@ read_lexical_block_scope (struct die_info *die, struct dwarf2_cu *cu)
> if (local_symbols != NULL || using_directives != NULL)
> {
> struct block *block
> - = finish_block (0, &local_symbols, newobj->old_blocks,
> + = finish_block (0, NULL, &local_symbols, newobj->old_blocks,
> newobj->start_addr, highpc);
>
> /* Note that recording ranges after traversing children, as we
> diff --git a/gdb/f-exp.y b/gdb/f-exp.y
> index c57f919..07892e0 100644
> --- a/gdb/f-exp.y
> +++ b/gdb/f-exp.y
> @@ -521,10 +521,7 @@ variable: name_not_typename
> innermost_block = sym.block;
> }
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> - /* We want to use the selected frame, not
> - another more inner frame which happens to
> - be in the same block. */
> - write_exp_elt_block (pstate, NULL);
> + write_exp_elt_block (pstate, sym.block);
> write_exp_elt_sym (pstate, sym.symbol);
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> break;
> diff --git a/gdb/findvar.c b/gdb/findvar.c
> index 2079b4b..38b9515 100644
> --- a/gdb/findvar.c
> +++ b/gdb/findvar.c
> @@ -32,6 +32,7 @@
> #include "block.h"
> #include "objfiles.h"
> #include "language.h"
> +#include "dwarf2loc.h"
>
> /* Basic byte-swapping routines. All 'extract' functions return a
> host-format integer from a target-format integer at ADDR which is
> @@ -409,11 +410,166 @@ minsym_lookup_iterator_cb (struct objfile *objfile, void *cb_data)
> return (data->result.minsym != NULL);
> }
>
> +/* Given static link expression and the frame it lives in, look for the frame
> + the static links points to and return it. Return NULL if we could not find
> + such a frame. */
> +
> +static struct frame_info *
> +follow_static_link (struct frame_info *frame,
> + const struct dynamic_prop *static_link)
> +{
> + CORE_ADDR upper_frame_base;
> +
> + if (!dwarf2_evaluate_property (static_link, frame, NULL, &upper_frame_base))
====
It gets harder and harder to reason about correctness the more
we blur lines between foo-independent and foo-dependent parts of gdb.
[In debug case "foo" == "debug-format".]
I guess to begin with, why are we calling a dwarf-specific function here
and what guarantees are in place (and easily discernable from reading
the code!) that the right thing will happen for non-dwarf targets?
> + return NULL;
> +
> + /* Now climb up the stack frame until we reach the frame we are interested
> + in. */
> + for (; frame != NULL; frame = get_prev_frame (frame))
> + {
> + struct symbol *framefunc = get_frame_function (frame);
> +
> + /* Protect ourselves against bad things such as circular call stacks. */
====
Here's a good question.
gdb has other mechanisms to catch corrupt (e.g., circular) stacks
(e.g., UNWIND_INNER_ID). Is this QUIT here for protection or in case
of really large stacks (e.g., due to infinite recursion)?
> + QUIT;
> +
> + /* If we don't know how to compute FRAME's base address, don't give up:
> + maybe the frame we are looking for is upper in the stace frame. */
> + if (framefunc != NULL
> + && SYMBOL_BLOCK_OPS (framefunc)->get_frame_base
====
!= NULL
> + && (SYMBOL_BLOCK_OPS (framefunc)->get_frame_base (framefunc, frame)
> + == upper_frame_base))
> + break;
> + }
> +
> + return frame;
> +}
> +
> +/* Assuming VAR is a symbol that can be reached from FRAME thanks to lexical
> + rules, look for the frame that is actually hosting VAR and return it. If,
> + for some reason, we found no such frame, return NULL.
> +
> + This kind of computation is necessary to correctly handle lexically nested
> + functions.
> +
> + Note that in some cases, we know what scope VAR comes from but we cannot
> + reach the specific frame that hosts the instance of VAR we are looking for.
> + For backward compatibility purposes (with old compilers), we then look for
> + the first frame that can host it. */
> +
> +static struct frame_info *
> +get_hosting_frame (struct symbol *var, const struct block *var_block,
> + struct frame_info *frame)
> +{
> + const struct block *frame_block = NULL;
> +
> + if (!symbol_read_needs_frame (var))
> + return NULL;
> +
> + /* Some symbols for local variables have no block: this happens when they are
> + not produced by a debug information reader, for instance when GDB creates
> + synthetic symbols. Without block information, we must assume they are
> + local to FRAME. In this case, there is nothing to do. */
> + else if (var_block == NULL)
> + return frame;
> +
> + /* We currently assume that all symbols with a location list need a frame.
> + This is true in practice because selecting the location description
> + requires to compute the CFA, hence requires a frame. However we have
> + tests that embed global/static symbols with null location lists.
> + We want to get <optimized out> instead of <frame required> when evaluating
> + them so return a frame instead of raising an error. */
> + else if (var_block == block_global_block (var_block)
> + || var_block == block_static_block (var_block))
> + return frame;
> +
> + /* We have to handle the "my_func::my_local_var" notation. This requires us
> + to look for upper frames when we find no block for the current frame: here
> + and below, handle when frame_block == NULL. */
> + if (frame != NULL)
> + frame_block = get_frame_block (frame, NULL);
> +
> + /* Climb up the call stack until reaching the frame we are looking for. */
> + while (frame != NULL && frame_block != var_block)
> + {
> + /* Protect ourselves against bad things such as circular call stacks. */
> + QUIT;
> +
> + if (frame_block == NULL)
> + {
> + frame = get_prev_frame (frame);
> + if (frame == NULL)
> + break;
> + frame_block = get_frame_block (frame, NULL);
> + }
> +
> + /* If we failed to find the proper frame, fallback to the heuristic
> + method below. */
> + else if (frame_block == block_global_block (frame_block))
> + {
> + frame = NULL;
> + break;
> + }
> +
> + /* Assuming we have a block for this frame: if we are at the function
> + level, the immediate upper lexical block is in an outer function:
> + follow the static link. */
> + else if (BLOCK_FUNCTION (frame_block))
> + {
> + const struct dynamic_prop *static_link
> + = block_static_link (frame_block);
> + int could_climb_up = 0;
> +
> + if (static_link != NULL)
> + {
> + frame = follow_static_link (frame, static_link);
> + if (frame != NULL)
> + {
> + frame_block = get_frame_block (frame, NULL);
> + could_climb_up = frame_block != NULL;
> + }
> + }
> + if (!could_climb_up)
> + {
> + frame = NULL;
> + break;
> + }
> + }
> +
> + else
> + /* We must be in some function nested lexical block. Just get the
> + outer block: both must share the same frame. */
> + frame_block = BLOCK_SUPERBLOCK (frame_block);
> + }
> +
> + /* Old compilers may not provide a static link, or they may provide an
> + invalid one. For such cases, fallback on the old way to evaluate
> + non-local references: just climb up the call stack and pick the first
> + frame that contains the variable we are looking for. */
> + if (frame == NULL)
> + {
> + frame = block_innermost_frame (var_block);
> + if (!frame)
> + {
> + if (BLOCK_FUNCTION (var_block)
> + && !block_inlined_p (var_block)
> + && SYMBOL_PRINT_NAME (BLOCK_FUNCTION (var_block)))
> + error (_("No frame is currently executing in block %s."),
> + SYMBOL_PRINT_NAME (BLOCK_FUNCTION (var_block)));
> + else
> + error (_("No frame is currently executing in specified"
> + " block"));
> + }
> + }
> +
> + return frame;
> +}
> +
> /* A default implementation for the "la_read_var_value" hook in
> the language vector which should work in most situations. */
>
> struct value *
> -default_read_var_value (struct symbol *var, struct frame_info *frame)
> +default_read_var_value (struct symbol *var, const struct block *var_block,
> + struct frame_info *frame)
> {
> struct value *v;
> struct type *type = SYMBOL_TYPE (var);
> @@ -427,7 +583,10 @@ default_read_var_value (struct symbol *var, struct frame_info *frame)
> check_typedef (type);
>
> if (symbol_read_needs_frame (var))
> - gdb_assert (frame);
> + gdb_assert (frame != NULL);
> +
> + if (frame != NULL)
> + frame = get_hosting_frame (var, var_block, frame);
>
> if (SYMBOL_COMPUTED_OPS (var) != NULL)
> return SYMBOL_COMPUTED_OPS (var)->read_variable (var, frame);
> @@ -610,14 +769,15 @@ default_read_var_value (struct symbol *var, struct frame_info *frame)
> /* Calls VAR's language la_read_var_value hook with the given arguments. */
>
> struct value *
> -read_var_value (struct symbol *var, struct frame_info *frame)
> +read_var_value (struct symbol *var, const struct block *var_block,
> + struct frame_info *frame)
> {
> const struct language_defn *lang = language_def (SYMBOL_LANGUAGE (var));
>
> gdb_assert (lang != NULL);
> gdb_assert (lang->la_read_var_value != NULL);
>
> - return lang->la_read_var_value (var, frame);
> + return lang->la_read_var_value (var, var_block, frame);
> }
>
> /* Install default attributes for register values. */
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index be761e6..fe6af7c 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -1874,7 +1874,7 @@ resolve_dynamic_range (struct type *dyn_range_type,
> gdb_assert (TYPE_CODE (dyn_range_type) == TYPE_CODE_RANGE);
>
> prop = &TYPE_RANGE_DATA (dyn_range_type)->low;
> - if (dwarf2_evaluate_property (prop, addr_stack, &value))
> + if (dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
> {
> low_bound.kind = PROP_CONST;
> low_bound.data.const_val = value;
> @@ -1886,7 +1886,7 @@ resolve_dynamic_range (struct type *dyn_range_type,
> }
>
> prop = &TYPE_RANGE_DATA (dyn_range_type)->high;
> - if (dwarf2_evaluate_property (prop, addr_stack, &value))
> + if (dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
> {
> high_bound.kind = PROP_CONST;
> high_bound.data.const_val = value;
> @@ -2128,7 +2128,8 @@ resolve_dynamic_type_internal (struct type *type,
>
> /* Resolve data_location attribute. */
> prop = TYPE_DATA_LOCATION (resolved_type);
> - if (prop != NULL && dwarf2_evaluate_property (prop, addr_stack, &value))
> + if (prop != NULL
> + && dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
> {
> TYPE_DYN_PROP_ADDR (prop) = value;
> TYPE_DYN_PROP_KIND (prop) = PROP_CONST;
> diff --git a/gdb/go-exp.y b/gdb/go-exp.y
> index 1f43306..9fa1bbd 100644
> --- a/gdb/go-exp.y
> +++ b/gdb/go-exp.y
> @@ -611,10 +611,7 @@ variable: name_not_typename
> }
>
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> - /* We want to use the selected frame, not
> - another more inner frame which happens to
> - be in the same block. */
> - write_exp_elt_block (pstate, NULL);
> + write_exp_elt_block (pstate, sym.block);
> write_exp_elt_sym (pstate, sym.symbol);
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> }
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 4948d27..2872292 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1660,7 +1660,7 @@ finish_command_continuation (void *arg, int err)
> {
> struct value *func;
>
> - func = read_var_value (a->function, get_current_frame ());
> + func = read_var_value (a->function, NULL, get_current_frame ());
> TRY
> {
> /* print_return_value can throw an exception in some
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 9a46242..0844823 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -6082,14 +6082,13 @@ insert_exception_resume_breakpoint (struct thread_info *tp,
> {
> TRY
> {
> - struct symbol *vsym;
> + struct symbol_in_block vsym;
> struct value *value;
> CORE_ADDR handler;
> struct breakpoint *bp;
>
> - vsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN,
> - NULL).symbol;
> - value = read_var_value (vsym, frame);
> + vsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL);
> + value = read_var_value (vsym.symbol, vsym.block, frame);
> /* If the value was optimized out, revert to the old behavior. */
> if (! value_optimized_out (value))
> {
> diff --git a/gdb/jv-exp.y b/gdb/jv-exp.y
> index 2e6de6f..bbdb330 100644
> --- a/gdb/jv-exp.y
> +++ b/gdb/jv-exp.y
> @@ -1284,9 +1284,7 @@ push_variable (struct parser_state *par_state, struct stoken name)
> }
>
> write_exp_elt_opcode (par_state, OP_VAR_VALUE);
> - /* We want to use the selected frame, not another more inner frame
> - which happens to be in the same block. */
> - write_exp_elt_block (par_state, NULL);
> + write_exp_elt_block (par_state, sym.block);
> write_exp_elt_sym (par_state, sym.symbol);
> write_exp_elt_opcode (par_state, OP_VAR_VALUE);
> return 1;
> diff --git a/gdb/language.h b/gdb/language.h
> index 2675b82..ea8442f 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -241,13 +241,19 @@ struct language_defn
> void (*la_value_print) (struct value *, struct ui_file *,
> const struct value_print_options *);
>
> - /* Given a symbol VAR, and a stack frame id FRAME, read the value
> - of the variable an return (pointer to a) struct value containing
> - the value.
> + /* Given a symbol VAR, the corresponding block VAR_BLOCK (if any) and a
> + stack frame id FRAME, read the value of the variable and return (pointer
> + to a) struct value containing the value.
> +
> + VAR_BLOCK is needed if there's a possibility for VAR to be outside
> + FRAME. This is what happens if FRAME correspond to a nested function
> + and VAR is defined in the outer function. If callers know that VAR is
> + located in FRAME, NULL can be passed as VAR_BLOCK.
====
"If callers know that VAR is located in FRAME or is global, ..." ?
>
> Throw an error if the variable cannot be found. */
>
> struct value *(*la_read_var_value) (struct symbol *var,
> + const struct block *var_block,
> struct frame_info *frame);
>
> /* PC is possibly an unknown languages trampoline.
> diff --git a/gdb/m2-exp.y b/gdb/m2-exp.y
> index a203218..5c5652e 100644
> --- a/gdb/m2-exp.y
> +++ b/gdb/m2-exp.y
> @@ -636,10 +636,7 @@ variable: NAME
> }
>
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> - /* We want to use the selected frame, not
> - another more inner frame which happens to
> - be in the same block. */
> - write_exp_elt_block (pstate, NULL);
> + write_exp_elt_block (pstate, sym.block);
> write_exp_elt_sym (pstate, sym.symbol);
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> }
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index c6f9f00..95b4a97 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -199,6 +199,77 @@ set_objfile_main_name (struct objfile *objfile,
> objfile->per_bfd->language_of_main = lang;
> }
>
> +/* Helper structure to map blocks to static link properties in hash tables. */
> +
> +struct static_link_htab_entry
> +{
> + const struct block *block;
> + struct dynamic_prop *static_link;
====
It's too bad this isn't const struct dynamic_prop *static_link;
I'm guessing it can't be that without fixing some laxness elsewhere
(which I wouldn't impose on this patch), but can you check?
> +};
> +
> +/* Return whether P1 (a struct static_link_htab_entry *) is a mapping from P2
> + (a struct block *). */
> +
> +static int
> +static_link_htab_entry_eq (const void *p1, const void *p2)
> +{
> + const struct static_link_htab_entry *entry
> + = (const struct static_link_htab_entry *) p1;
> + const struct block *block = (const struct block *) p2;
====
blank line here
Also, this is a non-standard implementation of an htab eq function.
Generally both p1 and p2 point to an element in the hash table.
I see hashtab.h has this:
/* Compare a table entry with a possible entry. The entry already in
the table always comes first, so the second element can be of a
different type (but in this case htab_find and htab_find_slot
cannot be used; instead the variants that accept a hash value
must be used). */
typedef int (*htab_eq) (const void *, const void *);
so this eq function is possibly ok, except I also see calls to
htab_find,htab_find_slot below. AIUC, one of these needs to change.
> + return block == entry->block;
> +}
> +
> +/* Register STATIC_LINK as the static link for BLOCK, which is part of OBJFILE.
> + Must not be called more than once for each BLOCK. */
> +
> +void
> +objfile_register_static_link (struct objfile *objfile,
> + const struct block *block,
> + struct dynamic_prop *static_link)
> +{
> + void **slot;
> + struct static_link_htab_entry *entry;
> +
> + if (objfile->static_links == NULL)
> + objfile->static_links = htab_create_alloc
> + (1, htab_hash_pointer, static_link_htab_entry_eq, NULL,
> + xcalloc, xfree);
> +
> + /* Create a slot for the mapping, make sure it's the first mapping for this
> + block and then create the mapping itself. */
> + slot = htab_find_slot (objfile->static_links, block, INSERT);
> + gdb_assert (*slot == NULL);
> +
> + entry = (struct static_link_htab_entry *) obstack_alloc
> + (&objfile->objfile_obstack, sizeof (*entry));
> + entry->block = block;
> + entry->static_link = static_link;
> + *slot = (void *) entry;
> +}
> +
> +/* Look for a static link for BLOCK, which is part of OBJFILE. Return NULL if
> + none was found. */
> +
> +struct dynamic_prop *
> +objfile_lookup_static_link (struct objfile *objfile,
> + const struct block *block)
> +{
> + struct static_link_htab_entry *entry;
> +
> + if (objfile->static_links == NULL)
> + return NULL;
> + entry
> + = (struct static_link_htab_entry *) htab_find (objfile->static_links,
> + block);
> + if (entry == NULL)
> + return NULL;
> + else
====
I don't know how others feel, but "else" clauses in particular situations
like this are just noise. How about removing it?
> + {
> + gdb_assert (entry->block == block);
> + return entry->static_link;
> + }
> +}
> +
>
>
> /* Called via bfd_map_over_sections to build up the section table that
> @@ -653,6 +724,11 @@ free_objfile (struct objfile *objfile)
> /* Rebuild section map next time we need it. */
> get_objfile_pspace_data (objfile->pspace)->section_map_dirty = 1;
>
> + /* Free the map for static links. There's no need to free static link
> + themselves since they were allocated on the objstack. */
> + if (objfile->static_links != NULL)
> + htab_delete (objfile->static_links);
> +
> /* The last thing we do is free the objfile struct itself. */
> xfree (objfile);
> }
> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index a0dc69b..aa2e966 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -20,6 +20,7 @@
> #if !defined (OBJFILES_H)
> #define OBJFILES_H
>
> +#include "hashtab.h"
> #include "gdb_obstack.h" /* For obstack internals. */
> #include "symfile.h" /* For struct psymbol_allocation_list. */
> #include "progspace.h"
> @@ -412,6 +413,16 @@ struct objfile
> table, so we have to keep them here to relocate them
> properly. */
> struct symbol *template_symbols;
> +
> + /* Associate a static link (struct dynamic_prop *) to all blocks (struct
> + block *) that have one. For nested functions, the static link is the
> + expression that computes the frame base of the lexically enclosing
> + function.
> +
> + Very few blocks have a static link, so it's more memory efficient to
> + store these here. Static links must be allocated on the objfile's
> + obstack. */
> + struct htab *static_links;
====
s/struct htab */htab_t /
> };
>
> /* Defines for the objfile flag word. */
> @@ -719,4 +730,11 @@ extern const char *objfile_debug_name (const struct objfile *objfile);
> extern void set_objfile_main_name (struct objfile *objfile,
> const char *name, enum language lang);
>
> +extern void objfile_register_static_link (struct objfile *objfile,
> + const struct block *block,
> + struct dynamic_prop *static_link);
> +
> +extern struct dynamic_prop *objfile_lookup_static_link
> + (struct objfile *objfile, const struct block *block);
> +
> #endif /* !defined (OBJFILES_H) */
> diff --git a/gdb/p-exp.y b/gdb/p-exp.y
> index a2f86d6..173d9fb 100644
> --- a/gdb/p-exp.y
> +++ b/gdb/p-exp.y
> @@ -769,10 +769,7 @@ variable: name_not_typename
> }
>
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> - /* We want to use the selected frame, not
> - another more inner frame which happens to
> - be in the same block. */
> - write_exp_elt_block (pstate, NULL);
> + write_exp_elt_block (pstate, sym.block);
> write_exp_elt_sym (pstate, sym.symbol);
> write_exp_elt_opcode (pstate, OP_VAR_VALUE);
> current_type = sym.symbol->type; }
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index f51e25c..553cc71 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -1988,7 +1988,11 @@ print_variable_and_value (const char *name, struct symbol *var,
> struct value *val;
> struct value_print_options opts;
>
> - val = read_var_value (var, frame);
> + /* READ_VAR_VALUE needs a block in order to deal with non-local
> + references (i.e. to handle nested functions). In this context, we
> + print variables that are local to this frame, so we can avoid passing
> + a block to it. */
> + val = read_var_value (var, NULL, frame);
> get_user_print_options (&opts);
> opts.deref_ref = 1;
> common_val_print (val, stream, indent, &opts, current_language);
> diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
> index e3d4867..345642e 100644
> --- a/gdb/python/py-finishbreakpoint.c
> +++ b/gdb/python/py-finishbreakpoint.c
> @@ -265,7 +265,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
> /* Ignore Python errors at this stage. */
> self_bpfinish->return_type = type_to_type_object (ret_type);
> PyErr_Clear ();
> - func_value = read_var_value (function, frame);
> + func_value = read_var_value (function, NULL, frame);
> self_bpfinish->function_value =
> value_to_value_object (func_value);
> PyErr_Clear ();
> diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
> index 7290056..fa7937a 100644
> --- a/gdb/python/py-frame.c
> +++ b/gdb/python/py-frame.c
> @@ -504,6 +504,7 @@ frapy_read_var (PyObject *self, PyObject *args)
> struct frame_info *frame;
> PyObject *sym_obj, *block_obj = NULL;
> struct symbol *var = NULL; /* gcc-4.3.2 false warning. */
> + const struct block *block = NULL;
> struct value *val = NULL;
>
> if (!PyArg_ParseTuple (args, "O|O", &sym_obj, &block_obj))
> @@ -514,7 +515,6 @@ frapy_read_var (PyObject *self, PyObject *args)
> else if (gdbpy_is_string (sym_obj))
> {
> char *var_name;
> - const struct block *block = NULL;
> struct cleanup *cleanup;
>
> var_name = python_string_to_target_string (sym_obj);
> @@ -536,11 +536,14 @@ frapy_read_var (PyObject *self, PyObject *args)
>
> TRY
> {
> + struct symbol_in_block lookup_sym;
> FRAPY_REQUIRE_VALID (self, frame);
>
> if (!block)
> block = get_frame_block (frame, NULL);
> - var = lookup_symbol (var_name, block, VAR_DOMAIN, NULL).symbol;
> + lookup_sym = lookup_symbol (var_name, block, VAR_DOMAIN, NULL);
> + var = lookup_sym.symbol;
> + block = lookup_sym.block;
> }
> CATCH (except, RETURN_MASK_ALL)
> {
> @@ -572,7 +575,7 @@ frapy_read_var (PyObject *self, PyObject *args)
> {
> FRAPY_REQUIRE_VALID (self, frame);
>
> - val = read_var_value (var, frame);
> + val = read_var_value (var, block, frame);
> }
> CATCH (except, RETURN_MASK_ALL)
> {
> diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
> index e3336b1..397ac86 100644
> --- a/gdb/python/py-framefilter.c
> +++ b/gdb/python/py-framefilter.c
> @@ -43,16 +43,17 @@ enum mi_print_types
> NAME is a pass-through argument where the name of the symbol will
> be written. NAME is allocated in this function, but the caller is
> responsible for clean up. SYM is a pass-through argument where the
> - symbol will be written. In the case of the API returning a string,
> - this will be set to NULL. LANGUAGE is also a pass-through argument
> - denoting the language attributed to the Symbol. In the case of SYM
> - being NULL, this will be set to the current language. Returns
> - EXT_LANG_BT_ERROR on error with the appropriate Python exception set, and
> - EXT_LANG_BT_OK on success. */
> + symbol will be written and SYM_BLOCK is a pass-through argument to
> + write the block where the symbol lies in. In the case of the API
> + returning a string, this will be set to NULL. LANGUAGE is also a
> + pass-through argument denoting the language attributed to the
> + Symbol. In the case of SYM being NULL, this will be set to the
> + current language. Returns EXT_LANG_BT_ERROR on error with the
> + appropriate Python exception set, and EXT_LANG_BT_OK on success. */
>
> static enum ext_lang_bt_status
> extract_sym (PyObject *obj, char **name, struct symbol **sym,
> - const struct language_defn **language)
> + struct block **sym_block, const struct language_defn **language)
> {
> PyObject *result = PyObject_CallMethod (obj, "symbol", NULL);
>
> @@ -75,12 +76,17 @@ extract_sym (PyObject *obj, char **name, struct symbol **sym,
> python_language. */
> *language = python_language;
> *sym = NULL;
> + *sym_block = NULL;
> }
> else
> {
> /* This type checks 'result' during the conversion so we
> just call it unconditionally and check the return. */
> *sym = symbol_object_to_symbol (result);
> + /* TODO: How should we find the corresponding block for this symbol?
> + Should we lookup all blocks in the owning objfile? Should we store
> + the bloc kin the Symbol object? */
> + *sym_block = NULL;
>
> Py_DECREF (result);
>
> @@ -537,10 +543,11 @@ enumerate_args (PyObject *iter,
> const struct language_defn *language;
> char *sym_name;
> struct symbol *sym;
> + struct block *sym_block;
> struct value *val;
> enum ext_lang_bt_status success = EXT_LANG_BT_ERROR;
>
> - success = extract_sym (item, &sym_name, &sym, &language);
> + success = extract_sym (item, &sym_name, &sym, &sym_block, &language);
> if (success == EXT_LANG_BT_ERROR)
> {
> Py_DECREF (item);
> @@ -736,12 +743,13 @@ enumerate_locals (PyObject *iter,
> struct value *val;
> enum ext_lang_bt_status success = EXT_LANG_BT_ERROR;
> struct symbol *sym;
> + struct block *sym_block;
> int local_indent = 8 + (8 * indent);
> struct cleanup *locals_cleanups;
>
> locals_cleanups = make_cleanup_py_decref (item);
>
> - success = extract_sym (item, &sym_name, &sym, &language);
> + success = extract_sym (item, &sym_name, &sym, &sym_block, &language);
> if (success == EXT_LANG_BT_ERROR)
> {
> do_cleanups (locals_cleanups);
> @@ -769,7 +777,7 @@ enumerate_locals (PyObject *iter,
> {
> TRY
> {
> - val = read_var_value (sym, frame);
> + val = read_var_value (sym, sym_block, frame);
> }
> CATCH (except, RETURN_MASK_ERROR)
> {
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 401e7e9..b58e59d 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -278,7 +278,10 @@ sympy_value (PyObject *self, PyObject *args)
> if (symbol_read_needs_frame (symbol) && frame_info == NULL)
> error (_("symbol requires a frame to compute its value"));
>
> - value = read_var_value (symbol, frame_info);
> + /* TODO: How should we find the corresponding block for this symbol?
> + Should we lookup all blocks in the owning objfile? Should we store
> + the block in the Symbol object? */
> + value = read_var_value (symbol, NULL, frame_info);
> }
> CATCH (except, RETURN_MASK_ALL)
> {
> diff --git a/gdb/stack.c b/gdb/stack.c
> index b4cfdbd..5a18a06 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -318,7 +318,7 @@ read_frame_local (struct symbol *sym, struct frame_info *frame,
>
> TRY
> {
> - argp->val = read_var_value (sym, frame);
> + argp->val = read_var_value (sym, NULL, frame);
> }
> CATCH (except, RETURN_MASK_ERROR)
> {
> @@ -344,7 +344,7 @@ read_frame_arg (struct symbol *sym, struct frame_info *frame,
> {
> TRY
> {
> - val = read_var_value (sym, frame);
> + val = read_var_value (sym, NULL, frame);
> }
> CATCH (except, RETURN_MASK_ERROR)
> {
> @@ -471,7 +471,7 @@ read_frame_arg (struct symbol *sym, struct frame_info *frame,
>
> TRY
> {
> - val = read_var_value (sym, frame);
> + val = read_var_value (sym, NULL, frame);
> }
> CATCH (except, RETURN_MASK_ERROR)
> {
> @@ -2424,7 +2424,7 @@ return_command (char *retval_exp, int from_tty)
> value_fetch_lazy (return_value);
>
> if (thisfun != NULL)
> - function = read_var_value (thisfun, thisframe);
> + function = read_var_value (thisfun, NULL, thisframe);
>
> rv_conv = RETURN_VALUE_REGISTER_CONVENTION;
> if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 73026b3..509537b 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -665,6 +665,24 @@ struct symbol_block_ops
> uninitialized in such case. */
> void (*find_frame_base_location) (struct symbol *framefunc, CORE_ADDR pc,
> const gdb_byte **start, size_t *length);
> +
> + /* Return the frame base address. FRAME is the frame for which we want to
> + compute the base address while FRAMEFUNC is the symbol for the
> + corresponding function.
> +
> + This method is designed to work with static links (nested functions
> + handling). Static links are function properties whose evaluation return
====
s/return/returns/
> + the frame base address for the enclosing frame. However, there are
> + multiple definitions for "frame base": the content of the frame base
> + register, the CFA as defined by DWARF unwinding information, ...
> +
> + So this specific method is supposed to compute the frame base address such
> + as for nested fuctions, the static link computes the same address. For
> + instance, considering DWARF debugging information, the static link is
> + computed with DW_AT_static_link and this method must be used to compute
> + the corresponding DW_AT_frame_base attribute. */
> + CORE_ADDR (*get_frame_base) (struct symbol *framefunc,
> + struct frame_info *frame);
> };
>
> /* Functions used with LOC_REGISTER and LOC_REGPARM_ADDR. */
> diff --git a/gdb/testsuite/gdb.base/nested-subp1.c b/gdb/testsuite/gdb.base/nested-subp1.c
> new file mode 100644
> index 0000000..967eb2f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nested-subp1.c
> @@ -0,0 +1,37 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2015 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +int
> +foo (int i1)
> +{
> + int
> + nested (int i2)
> + {
> + /* Here with i1 and i2, we can test that GDB can fetch both a local and a
> + non-local variable in the most simple nested function situation: the
> + parent block instance is accessible as the directly upper frame. */
> + return i1 * i2; /* STOP */
> + }
> +
> + return nested (i1 + 1);
> +}
> +
> +int
> +main ()
> +{
> + return !foo (1);
> +}
> diff --git a/gdb/testsuite/gdb.base/nested-subp1.exp b/gdb/testsuite/gdb.base/nested-subp1.exp
> new file mode 100644
> index 0000000..9720f5b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nested-subp1.exp
> @@ -0,0 +1,55 @@
> +# Copyright 2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +#
> +# Test nested functions related functionality.
> +#
> +
> +standard_testfile
> +
> +
> +set testcase "nested-subp1"
> +
> +if { [gdb_compile "${srcdir}/${subdir}/${testcase}.c" \
> + [standard_output_file "${testcase}"] \
> + "${testcase}" \
> + [list debug "additional_flags=-std=gnu99"]] != "" } {
> + return -1
> +}
> +
> +
> +# Run until the variables we are interested in are visible.
> +
> +clean_restart "${testcase}"
> +if ![runto_main] {
> + perror "could not run to main"
> + continue
> +}
> +
> +set bp_location [gdb_get_line_number "STOP" "${testcase}.c"]
> +gdb_test "break ${testcase}.c:${bp_location}" \
> + "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
> + "breakpoint to the STOP marker"
> +gdb_test "continue" \
> + "Breakpoint \[0-9\]+, nested .*" \
> + "continue to the STOP marker"
> +
> +
> +# Check we get correct values for both local and non-local variable references.
> +
> +gdb_test "print i1" "1"
> +gdb_test "print i2" "2"
> diff --git a/gdb/testsuite/gdb.base/nested-subp2.c b/gdb/testsuite/gdb.base/nested-subp2.c
> new file mode 100644
> index 0000000..a6449e34
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nested-subp2.c
> @@ -0,0 +1,48 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2015 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +void
> +iter_str (const char *str, void (*callback) (char c))
> +{
> + for (; *str != '\0'; ++str)
> + callback (*str);
> +}
> +
> +int
> +length_str (const char *str)
> +{
> + int count = 0;
> +
> + void
> + increment (char c)
> + {
> + /* Here with COUNT, we can test that GDB can read a non-local variable even
> + though it's not directly in the upper stack frame. */
> + count += 1; /* STOP */
> + }
> +
> + iter_str (str, &increment);
> + return count;
> +}
> +
> +int
> +main ()
> +{
> + if (length_str ("foo") == 3)
> + return 0;
> + return 1;
> +}
> diff --git a/gdb/testsuite/gdb.base/nested-subp2.exp b/gdb/testsuite/gdb.base/nested-subp2.exp
> new file mode 100644
> index 0000000..a107d1c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nested-subp2.exp
> @@ -0,0 +1,64 @@
> +# Copyright 2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +#
> +# Test nested functions related functionality.
> +#
> +
> +standard_testfile
> +
> +
> +set testcase "nested-subp2"
> +
> +if { [gdb_compile "${srcdir}/${subdir}/${testcase}.c" \
> + [standard_output_file "${testcase}"] \
> + "${testcase}" \
> + [list debug "additional_flags=-std=gnu99"]] != "" } {
> + return -1
> +}
> +
> +
> +# Run until the variables we are interested in are visible.
> +
> +clean_restart "${testcase}"
> +if ![runto_main] {
> + perror "could not run to main"
> + continue
> +}
> +
> +set bp_location [gdb_get_line_number "STOP" "${testcase}.c"]
> +gdb_test "break ${testcase}.c:${bp_location}" \
> + "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
> + "breakpoint to the STOP marker"
> +gdb_test "continue" \
> + "Breakpoint \[0-9\]+, increment .*" \
> + "continue to the STOP marker"
> +
> +
> +# Check we get correct values for both local and non-local variable references.
> +
> +gdb_test "print c" "102 'f'"
> +gdb_test "print count" "0"
> +
> +
> +# Same but a little later: make sure we were looking at the proper places.
> +
> +gdb_test "continue" \
> + "Breakpoint \[0-9\]+, increment .*" \
> + "continue to the STOP marker"
> +gdb_test "print c" "111 'o'"
> +gdb_test "print count" "1"
> diff --git a/gdb/testsuite/gdb.base/nested-subp3.c b/gdb/testsuite/gdb.base/nested-subp3.c
> new file mode 100644
> index 0000000..a51f417
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nested-subp3.c
> @@ -0,0 +1,66 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2015 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include <stdlib.h>
> +
> +typedef void (*callback_t) (void);
> +
> +extern void process (callback_t cb);
> +extern void parent (int first, callback_t cb);
> +
> +void
> +ignore (int unused)
> +{
> + (void) unused;
> +}
> +
> +void
> +process (callback_t cb)
> +{
> + parent (0, cb);
> +}
> +
> +void
> +parent (int first, callback_t cb)
> +{
> + void child (void)
> + {
> + /* When reaching this, there are two block instances for PARENT on the
> + stack: the one that is right in the upper frame is not the one actually
> + used for non-local references, so GDB has to follow the static link in
> + order to get the correct instance, and thus in order to read the proper
> + variables.
> +
> + As a simple check, we can verify that under GDB, the following is true:
> + parent_first == first (which should be one: see the IF block below). */
> + const int parent_first = first;
> + ignore (parent_first); /* STOP */
> + ignore (first);
> + }
> +
> + if (first)
> + process (&child);
> + else
> + cb ();
> +}
> +
> +int
> +main ()
> +{
> + parent (1, NULL);
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/nested-subp3.exp b/gdb/testsuite/gdb.base/nested-subp3.exp
> new file mode 100644
> index 0000000..8f9b522
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nested-subp3.exp
> @@ -0,0 +1,55 @@
> +# Copyright 2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +#
> +# Test nested functions related functionality.
> +#
> +
> +standard_testfile
> +
> +
> +set testcase "nested-subp3"
> +
> +if { [gdb_compile "${srcdir}/${subdir}/${testcase}.c" \
> + [standard_output_file "${testcase}"] \
> + "${testcase}" \
> + [list debug "additional_flags=-std=gnu99"]] != "" } {
> + return -1
> +}
> +
> +
> +# Run until the variables we are interested in are visible.
> +
> +clean_restart "${testcase}"
> +if ![runto_main] {
> + perror "could not run to main"
> + continue
> +}
> +
> +set bp_location [gdb_get_line_number "STOP" "${testcase}.c"]
> +gdb_test "break ${testcase}.c:${bp_location}" \
> + "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
> + "breakpoint to the STOP marker"
> +gdb_test "continue" \
> + "Breakpoint \[0-9\]+, child .*" \
> + "continue to the STOP marker"
> +
> +
> +# Check we get correct values for both local and non-local variable references.
> +
> +gdb_test "print first" "1"
> +gdb_test "print parent_first" "1"
> diff --git a/gdb/valops.c b/gdb/valops.c
> index d326f93..94182c5 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -1288,27 +1288,12 @@ value_repeat (struct value *arg1, int count)
> struct value *
> value_of_variable (struct symbol *var, const struct block *b)
> {
> - struct frame_info *frame;
> + struct frame_info *frame = NULL;
>
> - if (!symbol_read_needs_frame (var))
> - frame = NULL;
> - else if (!b)
> + if (symbol_read_needs_frame (var))
> frame = get_selected_frame (_("No frame selected."));
> - else
> - {
> - frame = block_innermost_frame (b);
> - if (!frame)
> - {
> - if (BLOCK_FUNCTION (b) && !block_inlined_p (b)
> - && SYMBOL_PRINT_NAME (BLOCK_FUNCTION (b)))
> - error (_("No frame is currently executing in block %s."),
> - SYMBOL_PRINT_NAME (BLOCK_FUNCTION (b)));
> - else
> - error (_("No frame is currently executing in specified block"));
> - }
> - }
>
> - return read_var_value (var, frame);
> + return read_var_value (var, b, frame);
> }
>
> struct value *
> @@ -3459,9 +3444,9 @@ value_struct_elt_for_reference (struct type *domain, int offset,
> return NULL;
>
> if (want_address)
> - return value_addr (read_var_value (s, 0));
> + return value_addr (read_var_value (s, 0, 0));
> else
> - return read_var_value (s, 0);
> + return read_var_value (s, 0, 0);
> }
>
> if (TYPE_FN_FIELD_VIRTUAL_P (f, j))
> @@ -3489,7 +3474,7 @@ value_struct_elt_for_reference (struct type *domain, int offset,
> if (s == NULL)
> return NULL;
>
> - v = read_var_value (s, 0);
> + v = read_var_value (s, 0, 0);
> if (!want_address)
> result = v;
> else
> @@ -3725,7 +3710,7 @@ value_full_object (struct value *argp,
> struct value *
> value_of_this (const struct language_defn *lang)
> {
> - struct symbol *sym;
> + struct symbol_in_block sym;
> const struct block *b;
> struct frame_info *frame;
>
> @@ -3736,12 +3721,12 @@ value_of_this (const struct language_defn *lang)
>
> b = get_frame_block (frame, NULL);
>
> - sym = lookup_language_this (lang, b).symbol;
> - if (sym == NULL)
> + sym = lookup_language_this (lang, b);
> + if (sym.symbol == NULL)
> error (_("current stack frame does not contain a variable named `%s'"),
> lang->la_name_of_this);
>
> - return read_var_value (sym, frame);
> + return read_var_value (sym.symbol, sym.block, frame);
> }
>
> /* Return the value of the local variable, if one exists. Return NULL
> diff --git a/gdb/value.h b/gdb/value.h
> index 7ff6aa8..c35a876 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -670,9 +670,11 @@ struct value *value_of_register_lazy (struct frame_info *frame, int regnum);
> extern int symbol_read_needs_frame (struct symbol *);
>
> extern struct value *read_var_value (struct symbol *var,
> + const struct block *var_block,
> struct frame_info *frame);
>
> extern struct value *default_read_var_value (struct symbol *var,
> + const struct block *var_block,
> struct frame_info *frame);
>
> extern struct value *allocate_value (struct type *type);
> diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
> index b5b2a1d..dea24a4 100644
> --- a/gdb/xcoffread.c
> +++ b/gdb/xcoffread.c
> @@ -1388,8 +1388,8 @@ read_xcoff_symtab (struct objfile *objfile, struct partial_symtab *pst)
> break;
> }
>
> - finish_block (newobj->name, &local_symbols, newobj->old_blocks,
> - newobj->start_addr,
> + finish_block (newobj->name, NULL, &local_symbols,
> + newobj->old_blocks, newobj->start_addr,
> (fcn_cs_saved.c_value
> + fcn_aux_saved.x_sym.x_misc.x_fsize
> + ANOFFSET (objfile->section_offsets,