Re: [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies

[Kevin Buettner <kevinb@redhat.com>]

Hi Tom,

Thank you for doing this work.  It looks mostly okay, though I do have
a few comments...

Kevin

On Tue, 4 Sep 2018 15:06:34 +0200
Tom de Vries <tdevries@suse.de> wrote:

> 2018-09-04  Tom de Vries  <tdevries@suse.de>
> 
> 	* dwarf2loc.c (sect_variable_value): Call indirect_synthetic_pointer
> 	with resolve_abstract_p == true.
> 	(indirect_synthetic_pointer): Add resolve_abstract_p parameter,
> 	defaulting to false. Propagate resolve_abstract_p to
> 	dwarf2_fetch_die_loc_sect_off.
> 	* dwarf2loc.h (dwarf2_fetch_die_loc_sect_off): Add resolve_abstract_p
> 	parameter, defaulting to false.
> 	* dwarf2read.c (dwarf2_per_objfile::~dwarf2_per_objfile): Reset
> 	abstract_to_concrete.
> 	(read_variable): Add variable to abstract_to_concrete.
> 	(dwarf2_fetch_die_loc_sect_off): Add and handle resolve_abstract_p
> 	parameter.
> 	* dwarf2read.h (struct die_info): Forward-declare.
> 	(die_info_ptr): New typedef.
> 	(DEF_VEC_P (die_info_ptr)): Add.
> 	(struct dwarf2_per_objfile): Add abstract_to_concrete field.
> 
> 	* gdb.dwarf2/varval.exp: Add test.
> 
> ---
>  gdb/dwarf2loc.c                     | 10 ++++---
>  gdb/dwarf2loc.h                     |  2 +-
>  gdb/dwarf2read.c                    | 56 +++++++++++++++++++++++++++++++++++--
>  gdb/dwarf2read.h                    |  6 ++++
>  gdb/testsuite/gdb.dwarf2/varval.exp | 22 ++++++++++++++-
>  5 files changed, 88 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 200fa03f46..a75eb158c7 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -65,7 +65,7 @@ static struct value *indirect_synthetic_pointer
>    (sect_offset die, LONGEST byte_offset,
>     struct dwarf2_per_cu_data *per_cu,
>     struct frame_info *frame,
> -   struct type *type);
> +   struct type *type, bool = false);

Please add a name for the new bool parameter.

> diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
> index f82e7b2d11..08120651b6 100644
> --- a/gdb/dwarf2loc.h
> +++ b/gdb/dwarf2loc.h
> @@ -67,7 +67,7 @@ const gdb_byte *dwarf2_find_location_expression
>  struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off
>    (sect_offset offset_in_cu, struct dwarf2_per_cu_data *per_cu,
>     CORE_ADDR (*get_frame_pc) (void *baton),
> -   void *baton);
> +   void *baton, bool = false);

Likewise, here.

> diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
> index 13855bcd54..71ef781d17 100644
> --- a/gdb/dwarf2read.h
> +++ b/gdb/dwarf2read.h
> @@ -20,6 +20,7 @@
>  #ifndef DWARF2READ_H
>  #define DWARF2READ_H
>  
> +#include <unordered_map>
>  #include "dwarf-index-cache.h"
>  #include "filename-seen-cache.h"
>  #include "gdb_obstack.h"
> @@ -95,6 +96,9 @@ struct dwarf2_debug_sections;
>  struct mapped_index;
>  struct mapped_debug_names;
>  struct signatured_type;
> +struct die_info;
> +typedef struct die_info *die_info_ptr;
> +DEF_VEC_P (die_info_ptr);
>  
>  /* Collection of data recorded per objfile.
>     This hangs off of dwarf2_objfile_data_key.  */
> @@ -250,6 +254,8 @@ public:
>    /* If we loaded the index from an external file, this contains the
>       resources associated to the open file, memory mapping, etc.  */
>    std::unique_ptr<index_cache_resource> index_cache_res;
> +
> +  std::unordered_map<die_info_ptr, VEC (die_info_ptr) *> abstract_to_concrete;
>  };

Is there a good reason to use VEC instead of std::vector?  I know that
there have been a number of patches which have been replacing VEC
with std:vector.  So, unless there's a compelling reason to use VEC,
we might as well use std:vector here and save someone else the effort
of changing this use of VEC later on.

Also, can you add a comment for abstract_to_concrete?

Kevin