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

Kevin Buettner
2018-09-04T21:54:56Z

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