Pedro, On 03/10/2015 04:25 PM, Pedro Alves wrote: > 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! :-) Oh indeed, sorry about that! Being used to "subprogram" in the Ada world, I thought it was a better match, but never mind: I've replaced all occurences with "function" instead. Thanks for having had a look at the patch anyway. ;-) > 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. I knew about the "block_found" alternative: I nevertheless tried to avoid it because I believed it was a better design to attach this information to symbols themselves... but of course runtime performance matters too. The updated patch switched to the "block_found" way: it makes the patch more complex but indeed it does not make the the symbol structure grow. I left TODO's in the patch, though: in the Python user interface (for instance in Frame.read_var), we sometimes accept gdb.Symbol objects and end up reading the corresponding variables. In such cases, we cannot use block_found since the lookup was done possibly a long time before; on the other hand there's almost always a gdb.Block argument around, so I guess we expect Python developpers to keep track of blocks corresponding to their symbols... which sounds wrong to me. It's okay for performance not to store blocks in GDB internal symbols data structures but on the other hand I don't think we should expose this design tweak in a public API. I did not try it because it implies a kind of API change: what about storing the origin block in gdb.Symbol instances? My guess is that the size of gdb.Symbol is not as critical as for struct symbol. The block arguments in the Python API would then be used only when a string is passed instead of a gdb.Symbol. This would also fix the problem we still have for gdb.Symbol.value, which does not take any block argument. Thoughts? > 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: This would look cleaner indeed. It's a big change itself though so if most consider this as a good idea I don't mind doing it... although it would be for another commit! > For the block->static_link case, maybe put the static link chain > in a separate hash indexed by block? This is what I did in the updated patch: I don't think there's a realistic use case for gdbarch-owned block having a static link so I added a hashed table to all objfiles. It's indexed by blocks and contains static links for these (if they have one). Then we perform lookups when reading variables... Thank you for your review, Pedro! :-) gdb/ChangeLog: 2015-03-20 Pierre-Marie de Rodat * 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. * 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. * 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): Pass the block found to read_var_value. * 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 (find_function_in_inferior): Pass the block found to value_of_variable. (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.c (value_static_field): Likewise. * value.h (read_var_value): Add a VAR_BLOCK argument. (default_read_var_value): Likewise. gdb/testsuite/ChangeLog: 2015-03-20 Pierre-Marie de Rodat * 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. -- Pierre-Marie de Rodat