* [PATCH 0/3] Add support for DW_OP_GNU_variable_value
@ 2018-08-03 4:04 Kevin Buettner
2018-08-03 4:18 ` [PATCH 1/3] " Kevin Buettner
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Kevin Buettner @ 2018-08-03 4:04 UTC (permalink / raw)
To: gdb-patches
This patch set adds support and a test case for
DW_OP_GNU_variable_value.
I've posted a patch to binutils adding readelf support for this
DWARF expression opcode:
https://sourceware.org/ml/binutils/2018-08/msg00059.html
I'm keeping this introductory message short. There's a lot more
info in the first patch...
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/3] Add support for DW_OP_GNU_variable_value 2018-08-03 4:04 [PATCH 0/3] Add support for DW_OP_GNU_variable_value Kevin Buettner @ 2018-08-03 4:18 ` Kevin Buettner 2018-08-03 18:36 ` Tom Tromey 2018-08-03 18:43 ` [PATCH 1/3] Add support for DW_OP_GNU_variable_value Tom Tromey 2018-08-03 4:19 ` [PATCH 2/3] Add support of DW_OP_GNU_variable_value to DWARF assembler Kevin Buettner 2018-08-03 4:22 ` [PATCH 3/3] Test case for DW_OP_GNU_variable_value Kevin Buettner 2 siblings, 2 replies; 22+ messages in thread From: Kevin Buettner @ 2018-08-03 4:18 UTC (permalink / raw) To: gdb-patches This patch adds support for DW_OP_GNU_variable_value to GDB. Jakub Jelinek provides a fairly expansive discussion of this DWARF expression opcode in his GCC patch... https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html It has also been proposed for addition to the DWARF Standard: http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 If compiled with a suitable version of GCC, the test case associated with GCC Bug 77589 uses DW_OP_GNU_variable_value in a DW_AT_byte_stride expression. Here's a link to the bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77589 This is what the DWARF looks like. Look at the last line, which has the DW_AT_byte_stride expression: <2><e1>: Abbrev Number: 12 (DW_TAG_variable) <e2> DW_AT_name : (indirect string, offset: 0x115): span.0 <e6> DW_AT_type : <0x2e> <ea> DW_AT_artificial : 1 <ea> DW_AT_location : 3 byte block: 91 b0 7f (DW_OP_fbreg: -80) ... <2><178>: Abbrev Number: 18 (DW_TAG_subrange_type) <179> DW_AT_lower_bound : 4 byte block: 97 23 20 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref) <17e> DW_AT_upper_bound : 4 byte block: 97 23 28 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 40; DW_OP_deref) <183> DW_AT_byte_stride : 10 byte block: 97 23 18 6 fd e1 0 0 0 1e (DW_OP_push_object_address; DW_OP_plus_uconst: 24; DW_OP_deref; DW_OP_GNU_variable_value: <0xe1>; DW_OP_mul) A patch to readelf, which I'm also submitting, is required to do this decoding. I found that GDB gave me the correct answer for "p c40pt(2)" once I (correctly) implemented DW_OP_GNU_variable_value. I also have test case (later in this series) which uses the DWARF assembler and, therefore, does not rely on having a compiler with this support. gdb/ChangeLog: * dwarf2expr.h (struct dwarf_expr_context): Add virtual method dwarf_variable_value. * dwarf2-frame.c (class dwarf_expr_executor): Add override for dwarf_variable_value. * dwarf2loc.c (class dwarf_evaluate_loc_desc): Likewise. (class symbol_needs_eval_context): Likewise. (indirect_synthetic_pointer): Add forward declaration. (sect_variable_value): New function. (dwarf2_compile_expr_to_ax): Add case for DW_OP_GNU_variable_value. * dwarf2expr.c (dwarf_expr_context::execute_stack_op): Add case for DW_OP_GNU_variable_value. --- gdb/dwarf2-frame.c | 5 +++++ gdb/dwarf2expr.c | 11 +++++++++++ gdb/dwarf2expr.h | 3 +++ gdb/dwarf2loc.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+) diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c index 58f1ba4..f7dc820 100644 --- a/gdb/dwarf2-frame.c +++ b/gdb/dwarf2-frame.c @@ -284,6 +284,11 @@ class dwarf_expr_executor : public dwarf_expr_context invalid ("DW_OP_call*"); } + struct value *dwarf_variable_value (sect_offset sect_off) override + { + invalid ("DW_OP_GNU_variable_value"); + } + CORE_ADDR get_addr_index (unsigned int index) override { invalid ("DW_OP_GNU_addr_index"); diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c index 445f857..f1ca033 100644 --- a/gdb/dwarf2expr.c +++ b/gdb/dwarf2expr.c @@ -1259,6 +1259,17 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr, this->dwarf_call (cu_off); } goto no_push; + + case DW_OP_GNU_variable_value: + { + sect_offset sect_off + = (sect_offset) extract_unsigned_integer (op_ptr, + this->ref_addr_size, + byte_order); + op_ptr += this->ref_addr_size; + result_val = this->dwarf_variable_value (sect_off); + } + break; case DW_OP_entry_value: case DW_OP_GNU_entry_value: diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h index c746bfe..c702a19 100644 --- a/gdb/dwarf2expr.h +++ b/gdb/dwarf2expr.h @@ -221,6 +221,9 @@ struct dwarf_expr_context subroutine. */ virtual void dwarf_call (cu_offset die_cu_off) = 0; + /* Execute "variable value" operation on DIED at SECT_OFF. */ + virtual struct value *dwarf_variable_value (sect_offset sect_off) = 0; + /* Return the base type given by the indicated DIE at DIE_CU_OFF. This can throw an exception if the DIE is invalid or does not represent a base type. SIZE is non-zero if this function should diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index a98b676..5c7c4ba 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -61,6 +61,12 @@ static struct call_site_parameter *dwarf_expr_reg_to_entry_parameter union call_site_parameter_u kind_u, struct dwarf2_per_cu_data **per_cu_return); +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); + /* Until these have formal names, we define these here. ref: http://gcc.gnu.org/wiki/DebugFission Each entry in .debug_loc.dwo begins with a byte that describes the entry, @@ -546,6 +552,25 @@ per_cu_dwarf_call (struct dwarf_expr_context *ctx, cu_offset die_offset, ctx->eval (block.data, block.size); } +struct value * +sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off, + struct dwarf2_per_cu_data *per_cu) +{ + struct type *die_type = dwarf2_fetch_die_type_sect_off (sect_off, per_cu); + + if (die_type == NULL) + error (_("Bad DW_OP_GNU_variable_value DIE.")); + + /* Note: Things still work when the following test is removed. This + test and error is here to conform to the proposed specification. */ + if (!is_scalar_type (die_type) || TYPE_CODE (die_type) == TYPE_CODE_VOID) + error (_("Type of DW_OP_GNU_variable_value DIE must be an integer or pointer.")); + + struct type *type = lookup_pointer_type (die_type); + struct frame_info *frame = get_selected_frame (_("No frame selected.")); + return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type); +} + class dwarf_evaluate_loc_desc : public dwarf_expr_context { public: @@ -587,6 +612,14 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context per_cu_dwarf_call (this, die_offset, per_cu); } + /* Helper interface of sect_variable_value for + dwarf2_evaluate_loc_desc. */ + + struct value *dwarf_variable_value (sect_offset sect_off) override + { + return sect_variable_value (this, sect_off, per_cu); + } + struct type *get_base_type (cu_offset die_offset, int size) override { struct type *result = dwarf2_get_die_type (die_offset, per_cu); @@ -2815,6 +2848,14 @@ class symbol_needs_eval_context : public dwarf_expr_context per_cu_dwarf_call (this, die_offset, per_cu); } + /* Helper interface of sect_variable_value for + dwarf2_loc_desc_get_symbol_read_needs. */ + + struct value *dwarf_variable_value (sect_offset sect_off) override + { + return sect_variable_value (this, sect_off, per_cu); + } + /* DW_OP_entry_value accesses require a caller, therefore a frame. */ @@ -3655,6 +3696,9 @@ dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc, case DW_OP_call_ref: unimplemented (op); + case DW_OP_GNU_variable_value: + unimplemented (op); + default: unimplemented (op); } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value 2018-08-03 4:18 ` [PATCH 1/3] " Kevin Buettner @ 2018-08-03 18:36 ` Tom Tromey 2018-08-18 20:32 ` Kevin Buettner 2018-08-03 18:43 ` [PATCH 1/3] Add support for DW_OP_GNU_variable_value Tom Tromey 1 sibling, 1 reply; 22+ messages in thread From: Tom Tromey @ 2018-08-03 18:36 UTC (permalink / raw) To: Kevin Buettner; +Cc: gdb-patches >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: Kevin> This patch adds support for DW_OP_GNU_variable_value to GDB. Kevin> Jakub Jelinek provides a fairly expansive discussion of this DWARF Kevin> expression opcode in his GCC patch... Thank you for doing this. A few minor comments below. Kevin> I also have test case (later in this series) which uses the DWARF Kevin> assembler and, therefore, does not rely on having a compiler with this Kevin> support. Very nice. Kevin> + /* Execute "variable value" operation on DIED at SECT_OFF. */ I think that should probably read "... on the DIE at ...". Kevin> +struct value * Kevin> +sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off, Kevin> + struct dwarf2_per_cu_data *per_cu) A new function should have an introductory comment explaining the arguments, return value, and semantics. Kevin> @@ -3655,6 +3696,9 @@ dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc, Kevin> case DW_OP_call_ref: Kevin> unimplemented (op); Kevin> + case DW_OP_GNU_variable_value: Kevin> + unimplemented (op); Is it possible to implement this? Just curious, it doesn't seem super important to me. In fact, is it ever even useful in an agent expression? I think the patch should also update disassemble_dwarf_expression. It's an oddity that there are two DWARF disassembler in the tree, but not your problem :) Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value 2018-08-03 18:36 ` Tom Tromey @ 2018-08-18 20:32 ` Kevin Buettner 2018-08-22 15:35 ` Tom de Vries 0 siblings, 1 reply; 22+ messages in thread From: Kevin Buettner @ 2018-08-18 20:32 UTC (permalink / raw) To: gdb-patches On Fri, 03 Aug 2018 12:36:35 -0600 Tom Tromey <tom@tromey.com> wrote: > Kevin> @@ -3655,6 +3696,9 @@ dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc, > Kevin> case DW_OP_call_ref: > Kevin> unimplemented (op); > > Kevin> + case DW_OP_GNU_variable_value: > Kevin> + unimplemented (op); > > Is it possible to implement this? Just curious, it doesn't seem super > important to me. In fact, is it ever even useful in an agent > expression? When I first started looking at it, it seemed to me that DW_OP_GNU_variable_value was similar to DW_OP_call_ref. Implementing the latter using existing machinery should be possible by compiling an agent expression. It might be possible to do something similar for DW_OP_GNU_variable_value, but it's more complicated since the referenced DIE might not have DW_AT_location, but instead have DW_AT_const_value instead. In any case, my understanding of agent expressions isn't good enough to attempt an implementation. > I think the patch should also update disassemble_dwarf_expression. > It's an oddity that there are two DWARF disassembler in the tree, but > not your problem :) I fixed this, plus the other things that you mentioned. (Thanks for the quick review!) This is what I pushed... commit a6b786da4e3353bf634ec7d9c7bffbd7569e73c6 Author: Kevin Buettner <kevinb@redhat.com> Date: Mon Jul 30 15:41:56 2018 -0700 Add support for DW_OP_GNU_variable_value This patch adds support for DW_OP_GNU_variable_value to GDB. Jakub Jelinek provides a fairly expansive discussion of this DWARF expression opcode in his GCC patch... https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html It has also been proposed for addition to the DWARF Standard: http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 If compiled with a suitable version of GCC, the test case associated with GCC Bug 77589 uses DW_OP_GNU_variable_value in a DW_AT_byte_stride expression. Here's a link to the bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77589 This is what the DWARF looks like. Look at the last line, which has the DW_AT_byte_stride expression: <2><e1>: Abbrev Number: 12 (DW_TAG_variable) <e2> DW_AT_name : (indirect string, offset: 0x115): span.0 <e6> DW_AT_type : <0x2e> <ea> DW_AT_artificial : 1 <ea> DW_AT_location : 3 byte block: 91 b0 7f (DW_OP_fbreg: -80) ... <2><178>: Abbrev Number: 18 (DW_TAG_subrange_type) <179> DW_AT_lower_bound : 4 byte block: 97 23 20 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref) <17e> DW_AT_upper_bound : 4 byte block: 97 23 28 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 40; DW_OP_deref) <183> DW_AT_byte_stride : 10 byte block: 97 23 18 6 fd e1 0 0 0 1e (DW_OP_push_object_address; DW_OP_plus_uconst: 24; DW_OP_deref; DW_OP_GNU_variable_value: <0xe1>; DW_OP_mul) A patch to readelf, which I'm also submitting, is required to do this decoding. I found that GDB gave me the correct answer for "p c40pt(2)" once I (correctly) implemented DW_OP_GNU_variable_value. I also have test case (later in this series) which uses the DWARF assembler and, therefore, do not rely on having a compiler with this support. gdb/ChangeLog: * dwarf2expr.h (struct dwarf_expr_context): Add virtual method dwarf_variable_value. * dwarf2-frame.c (class dwarf_expr_executor): Add override for dwarf_variable_value. * dwarf2loc.c (class dwarf_evaluate_loc_desc): Likewise. (class symbol_needs_eval_context): Likewise. (indirect_synthetic_pointer): Add forward declaration. (sect_variable_value): New function. (dwarf2_compile_expr_to_ax): Add case for DW_OP_GNU_variable_value. * dwarf2expr.c (dwarf_expr_context::execute_stack_op): Add case for DW_OP_GNU_variable_value. --- gdb/ChangeLog | 14 ++++++++++++++ gdb/dwarf2-frame.c | 5 +++++ gdb/dwarf2expr.c | 11 +++++++++++ gdb/dwarf2expr.h | 3 +++ gdb/dwarf2loc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 89 insertions(+) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 9ae0810..c75c0e1 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,17 @@ +2018-08-18 Kevin Buettner <kevinb@redhat.com> + + * dwarf2expr.h (struct dwarf_expr_context): Add virtual method + dwarf_variable_value. + * dwarf2-frame.c (class dwarf_expr_executor): + Add override for dwarf_variable_value. + * dwarf2loc.c (class dwarf_evaluate_loc_desc): Likewise. + (class symbol_needs_eval_context): Likewise. + (indirect_synthetic_pointer): Add forward declaration. + (sect_variable_value): New function. + (dwarf2_compile_expr_to_ax): Add case for DW_OP_GNU_variable_value. + * dwarf2expr.c (dwarf_expr_context::execute_stack_op): Add case + for DW_OP_GNU_variable_value. + 2018-08-16 Tom Tromey <tom@tromey.com> * top.c (read_command_file): Update. diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c index 58f1ba4..f7dc820 100644 --- a/gdb/dwarf2-frame.c +++ b/gdb/dwarf2-frame.c @@ -284,6 +284,11 @@ class dwarf_expr_executor : public dwarf_expr_context invalid ("DW_OP_call*"); } + struct value *dwarf_variable_value (sect_offset sect_off) override + { + invalid ("DW_OP_GNU_variable_value"); + } + CORE_ADDR get_addr_index (unsigned int index) override { invalid ("DW_OP_GNU_addr_index"); diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c index 445f857..f1ca033 100644 --- a/gdb/dwarf2expr.c +++ b/gdb/dwarf2expr.c @@ -1259,6 +1259,17 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr, this->dwarf_call (cu_off); } goto no_push; + + case DW_OP_GNU_variable_value: + { + sect_offset sect_off + = (sect_offset) extract_unsigned_integer (op_ptr, + this->ref_addr_size, + byte_order); + op_ptr += this->ref_addr_size; + result_val = this->dwarf_variable_value (sect_off); + } + break; case DW_OP_entry_value: case DW_OP_GNU_entry_value: diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h index c746bfe..a98edc9 100644 --- a/gdb/dwarf2expr.h +++ b/gdb/dwarf2expr.h @@ -221,6 +221,9 @@ struct dwarf_expr_context subroutine. */ virtual void dwarf_call (cu_offset die_cu_off) = 0; + /* Execute "variable value" operation on the DIE at SECT_OFF. */ + virtual struct value *dwarf_variable_value (sect_offset sect_off) = 0; + /* Return the base type given by the indicated DIE at DIE_CU_OFF. This can throw an exception if the DIE is invalid or does not represent a base type. SIZE is non-zero if this function should diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index a98b676..df2f231 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -61,6 +61,12 @@ static struct call_site_parameter *dwarf_expr_reg_to_entry_parameter union call_site_parameter_u kind_u, struct dwarf2_per_cu_data **per_cu_return); +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); + /* Until these have formal names, we define these here. ref: http://gcc.gnu.org/wiki/DebugFission Each entry in .debug_loc.dwo begins with a byte that describes the entry, @@ -546,6 +552,30 @@ per_cu_dwarf_call (struct dwarf_expr_context *ctx, cu_offset die_offset, ctx->eval (block.data, block.size); } +/* Given context CTX, section offset SECT_OFF, and compilation unit + data PER_CU, execute the "variable value" operation on the DIE + found at SECT_OFF. */ + +static struct value * +sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off, + struct dwarf2_per_cu_data *per_cu) +{ + struct type *die_type = dwarf2_fetch_die_type_sect_off (sect_off, per_cu); + + if (die_type == NULL) + error (_("Bad DW_OP_GNU_variable_value DIE.")); + + /* Note: Things still work when the following test is removed. This + test and error is here to conform to the proposed specification. */ + if (TYPE_CODE (die_type) != TYPE_CODE_INT + && TYPE_CODE (die_type) != TYPE_CODE_PTR) + error (_("Type of DW_OP_GNU_variable_value DIE must be an integer or pointer.")); + + struct type *type = lookup_pointer_type (die_type); + struct frame_info *frame = get_selected_frame (_("No frame selected.")); + return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type); +} + class dwarf_evaluate_loc_desc : public dwarf_expr_context { public: @@ -587,6 +617,14 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context per_cu_dwarf_call (this, die_offset, per_cu); } + /* Helper interface of sect_variable_value for + dwarf2_evaluate_loc_desc. */ + + struct value *dwarf_variable_value (sect_offset sect_off) override + { + return sect_variable_value (this, sect_off, per_cu); + } + struct type *get_base_type (cu_offset die_offset, int size) override { struct type *result = dwarf2_get_die_type (die_offset, per_cu); @@ -2815,6 +2853,14 @@ class symbol_needs_eval_context : public dwarf_expr_context per_cu_dwarf_call (this, die_offset, per_cu); } + /* Helper interface of sect_variable_value for + dwarf2_loc_desc_get_symbol_read_needs. */ + + struct value *dwarf_variable_value (sect_offset sect_off) override + { + return sect_variable_value (this, sect_off, per_cu); + } + /* DW_OP_entry_value accesses require a caller, therefore a frame. */ @@ -3655,6 +3701,9 @@ dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc, case DW_OP_call_ref: unimplemented (op); + case DW_OP_GNU_variable_value: + unimplemented (op); + default: unimplemented (op); } @@ -4280,6 +4329,13 @@ disassemble_dwarf_expression (struct ui_file *stream, ul = dwarf2_read_addr_index (per_cu, ul); fprintf_filtered (stream, " %s", pulongest (ul)); break; + + case DW_OP_GNU_variable_value: + ul = extract_unsigned_integer (data, offset_size, + gdbarch_byte_order (arch)); + data += offset_size; + fprintf_filtered (stream, " offset %s", phex_nz (ul, offset_size)); + break; } fprintf_filtered (stream, "\n"); ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value 2018-08-18 20:32 ` Kevin Buettner @ 2018-08-22 15:35 ` Tom de Vries 2018-08-23 21:12 ` Kevin Buettner 0 siblings, 1 reply; 22+ messages in thread From: Tom de Vries @ 2018-08-22 15:35 UTC (permalink / raw) To: Kevin Buettner; +Cc: gdb-patches, Richard Biener, Jakub Jelinek On 08/18/2018 10:31 PM, Kevin Buettner wrote: > This patch adds support for DW_OP_GNU_variable_value to GDB. > > Jakub Jelinek provides a fairly expansive discussion of this DWARF > expression opcode in his GCC patch... > > https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html > > It has also been proposed for addition to the DWARF Standard: > > http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 Hi, AFAIU from the discussion here ( https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if: - a DW_OP_GNU_variable_value refers to a die 'a', and - there's a die 'b' with abstract_origin 'a' that does have a DW_AT_location, and - die 'b' is 'in scope' in an evaluation context, then the evaluation of DW_OP_GNU_variable_value 'a' should return the value found at the DW_AT_location of die 'b'. I've written a gcc demonstrator patch to generate code like this for VLAs, and found that gdb master (containing this patch series) does not support this. Is this further support of DW_OP_GNU_variable_value something you're currently working on, or plan to work on? Thanks, - Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value 2018-08-22 15:35 ` Tom de Vries @ 2018-08-23 21:12 ` Kevin Buettner 2018-08-24 13:09 ` [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Tom de Vries 0 siblings, 1 reply; 22+ messages in thread From: Kevin Buettner @ 2018-08-23 21:12 UTC (permalink / raw) To: gdb-patches; +Cc: Tom de Vries, Richard Biener, Jakub Jelinek On Wed, 22 Aug 2018 17:35:23 +0200 Tom de Vries <tdevries@suse.de> wrote: > On 08/18/2018 10:31 PM, Kevin Buettner wrote: > > This patch adds support for DW_OP_GNU_variable_value to GDB. > > > > Jakub Jelinek provides a fairly expansive discussion of this DWARF > > expression opcode in his GCC patch... > > > > https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html > > > > It has also been proposed for addition to the DWARF Standard: > > > > http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 > > Hi, > > AFAIU from the discussion here ( > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if: > - a DW_OP_GNU_variable_value refers to a die 'a', and > - there's a die 'b' with abstract_origin 'a' that does have a > DW_AT_location, and > - die 'b' is 'in scope' in an evaluation context, > then the evaluation of DW_OP_GNU_variable_value 'a' should return the > value found at the DW_AT_location of die 'b'. > > I've written a gcc demonstrator patch to generate code like this for > VLAs, and found that gdb master (containing this patch series) does not > support this. > > Is this further support of DW_OP_GNU_variable_value something you're > currently working on, or plan to work on? Tom and I discussed this briefly on IRC. Tom says that he'll take a look at it... Kevin ^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-08-23 21:12 ` Kevin Buettner @ 2018-08-24 13:09 ` Tom de Vries 2018-08-24 13:18 ` Richard Biener 2018-09-04 10:17 ` [PATCH, " Tom de Vries 0 siblings, 2 replies; 22+ messages in thread From: Tom de Vries @ 2018-08-24 13:09 UTC (permalink / raw) To: Kevin Buettner, gdb-patches; +Cc: Richard Biener, Jakub Jelinek [-- Attachment #1: Type: text/plain, Size: 1759 bytes --] [ was: Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value ] On 08/23/2018 11:12 PM, Kevin Buettner wrote: > On Wed, 22 Aug 2018 17:35:23 +0200 > Tom de Vries <tdevries@suse.de> wrote: > >> On 08/18/2018 10:31 PM, Kevin Buettner wrote: >>> This patch adds support for DW_OP_GNU_variable_value to GDB. >>> >>> Jakub Jelinek provides a fairly expansive discussion of this DWARF >>> expression opcode in his GCC patch... >>> >>> https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html >>> >>> It has also been proposed for addition to the DWARF Standard: >>> >>> http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 >> >> Hi, >> >> AFAIU from the discussion here ( >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if: >> - a DW_OP_GNU_variable_value refers to a die 'a', and >> - there's a die 'b' with abstract_origin 'a' that does have a >> DW_AT_location, and >> - die 'b' is 'in scope' in an evaluation context, >> then the evaluation of DW_OP_GNU_variable_value 'a' should return the >> value found at the DW_AT_location of die 'b'. >> >> I've written a gcc demonstrator patch to generate code like this for >> VLAs, and found that gdb master (containing this patch series) does not >> support this. >> >> Is this further support of DW_OP_GNU_variable_value something you're >> currently working on, or plan to work on? > > Tom and I discussed this briefly on IRC. Tom says that he'll take > a look at it... > Hi, sofar I've written: - a hack that fixes the test-case I have, without regressing anything, and - the rationale for the change, hoping that this should sufficiently explain what we're trying to implement. Hints on how to implement are welcome. Thanks, - Tom [-- Attachment #2: 0001-gdb-exp-Handle-DW_OP_GNU_variable_value-refs-to-abstract-dies.patch --] [-- Type: text/x-patch, Size: 7391 bytes --] [gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Consider a vla variable 'a' in function f1: ... <2><1a7>: Abbrev Number: 11 (DW_TAG_variable) <1a8> DW_AT_description : a <1aa> DW_AT_abstract_origin: <0x311> ... with abstract origin 'a': ... <2><311>: Abbrev Number: 3 (DW_TAG_variable) <312> DW_AT_name : a <317> DW_AT_type : <0x325> ... and inherited abstract vla type: ... <1><325>: Abbrev Number: 9 (DW_TAG_array_type) <326> DW_AT_type : <0x33a> <2><32e>: Abbrev Number: 10 (DW_TAG_subrange_type) <32f> DW_AT_type : <0x2ea> <333> DW_AT_upper_bound : 5 byte block: fd 1b 3 0 0 (DW_OP_GNU_variable_value: <0x31b>) ... where the upper bound refers to this artificial variable D.1922 without location attribute: ... <2><31b>: Abbrev Number: 8 (DW_TAG_variable) <31c> DW_AT_description : (indirect string, offset: 0x39a): D.1922 <320> DW_AT_type : <0x2ea> <324> DW_AT_artificial : 1 ... Currently, when we execute "p sizeof (a)" in f1, the upper bound is calculated by evaluating the DW_OP_GNU_variable_value expression referring to D.1922, but since that die doesn't have a location attribute, we get: ... value has been optimized out ... However, there's also artificial variable D.4283 that is sibling of vla variable 'a', has artificial variable D.1922 as abstract origin, and has a location attribute: ... <2><1ae>: Abbrev Number: 12 (DW_TAG_variable) <1af> DW_AT_description : (indirect string, offset: 0x1f8): D.4283 <1b3> DW_AT_abstract_origin: <0x31b> <1b7> DW_AT_location : 11 byte block: 75 1 8 20 24 8 20 26 31 1c 9f (DW_OP_breg5 (rdi):1; DW_OP_const1u: 32; DW_OP_shl; DW_OP_const1u: 32; DW_OP_shra; DW_OP_lit1; DW_OP_minus; DW_OP_stack_value) ... The intended behaviour for DW_OP_GNU_variable_value is to find a die that refers to D.1922 as abstract origin, has a location attribute and is 'in scope', so the expected behaviour is: ... $1 = 6 ... The 'in scope' concept can be thought of as variable D.1922 having name attribute "D.1922", and variable D.4283 inheriting that attribute, resulting in D.4283 being declared with name "D.1922" alongside vla a in f1, and when we lookup "DW_OP_GNU_variable_value D.1922", it should work as if we try to find the value of a variable named "D.1922" on the gdb command line using "p D.1922", and we should return the value of D.4283. This patch contains a hack that handles the case described above. Build and reg-tested on x86_64-linux. --- gdb/dwarf2loc.c | 10 ++++++---- gdb/dwarf2loc.h | 2 +- gdb/dwarf2read.c | 28 ++++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 7 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); /* Until these have formal names, we define these here. ref: http://gcc.gnu.org/wiki/DebugFission @@ -573,7 +573,7 @@ sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off, struct type *type = lookup_pointer_type (die_type); struct frame_info *frame = get_selected_frame (_("No frame selected.")); - return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type); + return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type, true); } class dwarf_evaluate_loc_desc : public dwarf_expr_context @@ -2181,12 +2181,14 @@ fetch_const_value_from_synthetic_pointer (sect_offset die, LONGEST byte_offset, 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 frame_info *frame, struct type *type, + bool resolve_abstract_p) { /* Fetch the location expression of the DIE we're pointing to. */ struct dwarf2_locexpr_baton baton = dwarf2_fetch_die_loc_sect_off (die, per_cu, - get_frame_address_in_block_wrapper, frame); + get_frame_address_in_block_wrapper, frame, + resolve_abstract_p); /* Get type of pointed-to DIE. */ struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu); 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); struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_cu_off (cu_offset offset_in_cu, struct dwarf2_per_cu_data *per_cu, diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 8834d08a1c..3d214b7cee 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -92,6 +92,8 @@ #include "rust-lang.h" #include "common/pathstuff.h" +std::unordered_map<struct die_info *,struct die_info *> abstract_to_concrete; + /* When == 1, print basic high level tracing messages. When > 1, be more verbose. This is in contrast to the low level DIE reading of dwarf_die_debug. */ @@ -14283,7 +14285,23 @@ read_variable (struct die_info *die, struct dwarf2_cu *cu) } } - new_symbol (die, NULL, cu, storage); + struct symbol *res = new_symbol (die, NULL, cu, storage); + if (res == NULL && dwarf2_attr (die, DW_AT_location, cu)) + { + struct attribute *attr = dwarf2_attr (die, DW_AT_abstract_origin, cu); + if (attr) + { + struct dwarf2_cu *origin_cu = cu; + struct die_info *origin_die = follow_die_ref (die, attr, &origin_cu); + /* We have a variable without a name, but with a location and an + abstract origin. This may be a concrete instance of an abstract + variable referenced from an DW_OP_GNU_variable_value, so save it to + find it back later. Hack: assume 1-on-1 relationship. Of course, + in reality one abstract variable may have many concrete instances. + TODO: Fix this hack by storing into a 1-to-many data structure. */ + abstract_to_concrete[origin_die] = die; + } + } } /* Call CALLBACK from DW_AT_ranges attribute value OFFSET @@ -22998,7 +23016,7 @@ struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, struct dwarf2_per_cu_data *per_cu, CORE_ADDR (*get_frame_pc) (void *baton), - void *baton) + void *baton, bool resolve_abstract_p) { struct dwarf2_cu *cu; struct die_info *die; @@ -23023,6 +23041,12 @@ dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, error (_("Dwarf Error: Cannot find DIE at %s referenced in module %s"), sect_offset_str (sect_off), objfile_name (objfile)); + if (resolve_abstract_p + && abstract_to_concrete.find (die) != abstract_to_concrete.end ()) + /* TODO: Rewrite abstract_to_concrete into a 1-to-many data structure, and + find the appropriate concrete instance. */ + die = abstract_to_concrete[die]; + attr = dwarf2_attr (die, DW_AT_location, cu); if (!attr) { ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-08-24 13:09 ` [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Tom de Vries @ 2018-08-24 13:18 ` Richard Biener 2018-09-04 10:17 ` [PATCH, " Tom de Vries 1 sibling, 0 replies; 22+ messages in thread From: Richard Biener @ 2018-08-24 13:18 UTC (permalink / raw) To: Tom de Vries; +Cc: Kevin Buettner, gdb-patches, Jakub Jelinek On Fri, 24 Aug 2018, Tom de Vries wrote: > [ was: Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value ] > > On 08/23/2018 11:12 PM, Kevin Buettner wrote: > > On Wed, 22 Aug 2018 17:35:23 +0200 > > Tom de Vries <tdevries@suse.de> wrote: > > > >> On 08/18/2018 10:31 PM, Kevin Buettner wrote: > >>> This patch adds support for DW_OP_GNU_variable_value to GDB. > >>> > >>> Jakub Jelinek provides a fairly expansive discussion of this DWARF > >>> expression opcode in his GCC patch... > >>> > >>> https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html > >>> > >>> It has also been proposed for addition to the DWARF Standard: > >>> > >>> http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 > >> > >> Hi, > >> > >> AFAIU from the discussion here ( > >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if: > >> - a DW_OP_GNU_variable_value refers to a die 'a', and > >> - there's a die 'b' with abstract_origin 'a' that does have a > >> DW_AT_location, and > >> - die 'b' is 'in scope' in an evaluation context, > >> then the evaluation of DW_OP_GNU_variable_value 'a' should return the > >> value found at the DW_AT_location of die 'b'. > >> > >> I've written a gcc demonstrator patch to generate code like this for > >> VLAs, and found that gdb master (containing this patch series) does not > >> support this. > >> > >> Is this further support of DW_OP_GNU_variable_value something you're > >> currently working on, or plan to work on? > > > > Tom and I discussed this briefly on IRC. Tom says that he'll take > > a look at it... > > > > Hi, > > sofar I've written: > - a hack that fixes the test-case I have, without regressing anything, > and > - the rationale for the change, > hoping that this should sufficiently explain what we're trying to implement. > > Hints on how to implement are welcome. I wonder if there isn't already support to lookup 'd' when the DIE of the concrete instance just has the location and refers to 'd' via an abstract origin. So the idea is to do this kind of lookup from the original context of the conrete instance we came from when evaluating DW_OP_GNU_variable_value on the abstract instance DIE. Btw, apart from the use with early debug / LTO this would make it possible to remove repeating VLA types in inline instances. If you build the following with -O -g you get three instances of int[n] DIEs, one in the abstract copy and two generated for the DW_TAG_inlined_subroutine in bar. Ideally we could elide those, having the type of a fully specified in the abstract instance and the concrete instances providing locations for n. static inline void foo (int n) { int a[n]; } int bar(int n1, int n2) { foo (n1); foo (n2); } Richard. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-08-24 13:09 ` [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Tom de Vries 2018-08-24 13:18 ` Richard Biener @ 2018-09-04 10:17 ` Tom de Vries 2018-09-04 13:06 ` Tom de Vries 1 sibling, 1 reply; 22+ messages in thread From: Tom de Vries @ 2018-09-04 10:17 UTC (permalink / raw) To: Kevin Buettner, gdb-patches; +Cc: Richard Biener, Jakub Jelinek [-- Attachment #1: Type: text/plain, Size: 1433 bytes --] [ was: [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies ] On 08/24/2018 03:09 PM, Tom de Vries wrote: > [ was: Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value ] > > On 08/23/2018 11:12 PM, Kevin Buettner wrote: >> On Wed, 22 Aug 2018 17:35:23 +0200 >> Tom de Vries <tdevries@suse.de> wrote: >> >>> On 08/18/2018 10:31 PM, Kevin Buettner wrote: >>>> This patch adds support for DW_OP_GNU_variable_value to GDB. >>>> >>>> Jakub Jelinek provides a fairly expansive discussion of this DWARF >>>> expression opcode in his GCC patch... >>>> >>>> https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html >>>> >>>> It has also been proposed for addition to the DWARF Standard: >>>> >>>> http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 >>> Hi, >>> >>> AFAIU from the discussion here ( >>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if: >>> - a DW_OP_GNU_variable_value refers to a die 'a', and >>> - there's a die 'b' with abstract_origin 'a' that does have a >>> DW_AT_location, and >>> - die 'b' is 'in scope' in an evaluation context, >>> then the evaluation of DW_OP_GNU_variable_value 'a' should return the >>> value found at the DW_AT_location of die 'b'. Hi, this patch implements the handling of DW_OP_GNU_variable_value refs to abstract dies. There's no test-case yet, I'll try to write one. OK for trunk? Thanks, - Tom [-- Attachment #2: 0001-gdb-exp-Handle-DW_OP_GNU_variable_value-refs-to-abstract-dies.patch --] [-- Type: text/x-patch, Size: 10031 bytes --] [gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Consider a vla variable 'a' in function f1: ... <2><1a7>: Abbrev Number: 11 (DW_TAG_variable) <1a8> DW_AT_description : a <1aa> DW_AT_abstract_origin: <0x311> ... with abstract origin 'a': ... <2><311>: Abbrev Number: 3 (DW_TAG_variable) <312> DW_AT_name : a <317> DW_AT_type : <0x325> ... and inherited abstract vla type: ... <1><325>: Abbrev Number: 9 (DW_TAG_array_type) <326> DW_AT_type : <0x33a> <2><32e>: Abbrev Number: 10 (DW_TAG_subrange_type) <32f> DW_AT_type : <0x2ea> <333> DW_AT_upper_bound : 5 byte block: fd 1b 3 0 0 (DW_OP_GNU_variable_value: <0x31b>) ... where the upper bound refers to this artificial variable D.1922 without location attribute: ... <2><31b>: Abbrev Number: 8 (DW_TAG_variable) <31c> DW_AT_description : (indirect string, offset: 0x39a): D.1922 <320> DW_AT_type : <0x2ea> <324> DW_AT_artificial : 1 ... Currently, when we execute "p sizeof (a)" in f1, the upper bound is calculated by evaluating the DW_OP_GNU_variable_value expression referring to D.1922, but since that die doesn't have a location attribute, we get: ... value has been optimized out ... However, there's also artificial variable D.4283 that is sibling of vla variable 'a', has artificial variable D.1922 as abstract origin, and has a location attribute: ... <2><1ae>: Abbrev Number: 12 (DW_TAG_variable) <1af> DW_AT_description : (indirect string, offset: 0x1f8): D.4283 <1b3> DW_AT_abstract_origin: <0x31b> <1b7> DW_AT_location : 11 byte block: 75 1 8 20 24 8 20 26 31 1c 9f (DW_OP_breg5 (rdi):1; DW_OP_const1u: 32; DW_OP_shl; DW_OP_const1u: 32; DW_OP_shra; DW_OP_lit1; DW_OP_minus; DW_OP_stack_value) ... The intended behaviour for DW_OP_GNU_variable_value is to find a die that refers to D.1922 as abstract origin, has a location attribute and is 'in scope', so the expected behaviour is: ... $1 = 6 ... The 'in scope' concept can be thought of as variable D.1922 having name attribute "D.1922", and variable D.4283 inheriting that attribute, resulting in D.4283 being declared with name "D.1922" alongside vla a in f1, and when we lookup "DW_OP_GNU_variable_value D.1922", it should work as if we try to find the value of a variable named "D.1922" on the gdb command line using "p D.1922", and we should return the value of D.4283. This patch fixes the case described above, by: - adding a field abstract_to_concrete to struct dwarf2_per_objfile, - using that field to keep track of which concrete dies are instances of an abstract die, and - using that information when getting the value DW_OP_GNU_variable_value. Build and reg-tested on x86_64-linux. 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/dwarf2loc.c | 10 ++++++---- gdb/dwarf2loc.h | 2 +- gdb/dwarf2read.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- gdb/dwarf2read.h | 6 ++++++ 4 files changed, 67 insertions(+), 7 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); /* Until these have formal names, we define these here. ref: http://gcc.gnu.org/wiki/DebugFission @@ -573,7 +573,7 @@ sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off, struct type *type = lookup_pointer_type (die_type); struct frame_info *frame = get_selected_frame (_("No frame selected.")); - return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type); + return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type, true); } class dwarf_evaluate_loc_desc : public dwarf_expr_context @@ -2181,12 +2181,14 @@ fetch_const_value_from_synthetic_pointer (sect_offset die, LONGEST byte_offset, 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 frame_info *frame, struct type *type, + bool resolve_abstract_p) { /* Fetch the location expression of the DIE we're pointing to. */ struct dwarf2_locexpr_baton baton = dwarf2_fetch_die_loc_sect_off (die, per_cu, - get_frame_address_in_block_wrapper, frame); + get_frame_address_in_block_wrapper, frame, + resolve_abstract_p); /* Get type of pointed-to DIE. */ struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu); 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); struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_cu_off (cu_offset offset_in_cu, struct dwarf2_per_cu_data *per_cu, diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index d66dfeaf2d..419858bb98 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -2159,6 +2159,13 @@ dwarf2_per_objfile::~dwarf2_per_objfile () VEC_free (dwarf2_section_info_def, types); + for (std::unordered_map<die_info_ptr, VEC (die_info_ptr) *>::iterator it + = abstract_to_concrete.begin (); + it != abstract_to_concrete.end (); + it++) + VEC_free (die_info_ptr, it->second); + abstract_to_concrete.clear (); + if (dwo_files != NULL) free_dwo_files (dwo_files, objfile); @@ -14283,7 +14290,23 @@ read_variable (struct die_info *die, struct dwarf2_cu *cu) } } - new_symbol (die, NULL, cu, storage); + struct symbol *res = new_symbol (die, NULL, cu, storage); + struct attribute *abstract_origin + = dwarf2_attr (die, DW_AT_abstract_origin, cu); + struct attribute *loc = dwarf2_attr (die, DW_AT_location, cu); + if (res == NULL && loc && abstract_origin) + { + /* We have a variable without a name, but with a location and an abstract + origin. This may be a concrete instance of an abstract variable + referenced from an DW_OP_GNU_variable_value, so save it to find it back + later. */ + struct dwarf2_cu *origin_cu = cu; + struct die_info *origin_die + = follow_die_ref (die, abstract_origin, &origin_cu); + dwarf2_per_objfile *dpo = cu->per_cu->dwarf2_per_objfile; + VEC_safe_push (die_info_ptr, dpo->abstract_to_concrete[origin_die], + die); + } } /* Call CALLBACK from DW_AT_ranges attribute value OFFSET @@ -23010,7 +23033,7 @@ struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, struct dwarf2_per_cu_data *per_cu, CORE_ADDR (*get_frame_pc) (void *baton), - void *baton) + void *baton, bool resolve_abstract_p) { struct dwarf2_cu *cu; struct die_info *die; @@ -23036,6 +23059,35 @@ dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, sect_offset_str (sect_off), objfile_name (objfile)); attr = dwarf2_attr (die, DW_AT_location, cu); + if (!attr && resolve_abstract_p + && (dwarf2_per_objfile->abstract_to_concrete.find (die) + != dwarf2_per_objfile->abstract_to_concrete.end ())) + { + CORE_ADDR pc = (*get_frame_pc) (baton); + + unsigned i; + struct die_info *cand; + for (i = 0; + VEC_iterate (die_info_ptr, + dwarf2_per_objfile->abstract_to_concrete[die], i, cand); + i++) + { + if (!cand->parent + || cand->parent->tag != DW_TAG_subprogram) + continue; + + CORE_ADDR pc_low, pc_high; + get_scope_pc_bounds (cand->parent, &pc_low, &pc_high, cu); + if (pc_low == ((CORE_ADDR) -1) + || !(pc_low <= pc && pc < pc_high)) + continue; + + die = cand; + attr = dwarf2_attr (die, DW_AT_location, cu); + break; + } + } + if (!attr) { /* DWARF: "If there is no such attribute, then there is no effect.". 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; }; /* Get the dwarf2_per_objfile associated to OBJFILE. */ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-09-04 10:17 ` [PATCH, " Tom de Vries @ 2018-09-04 13:06 ` Tom de Vries 2018-09-04 21:54 ` Kevin Buettner 0 siblings, 1 reply; 22+ messages in thread From: Tom de Vries @ 2018-09-04 13:06 UTC (permalink / raw) To: Kevin Buettner, gdb-patches; +Cc: Richard Biener, Jakub Jelinek [-- Attachment #1: Type: text/plain, Size: 1547 bytes --] On 09/04/2018 12:16 PM, Tom de Vries wrote: > [ was: [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract > dies ] > > On 08/24/2018 03:09 PM, Tom de Vries wrote: >> [ was: Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value ] >> >> On 08/23/2018 11:12 PM, Kevin Buettner wrote: >>> On Wed, 22 Aug 2018 17:35:23 +0200 >>> Tom de Vries <tdevries@suse.de> wrote: >>> >>>> On 08/18/2018 10:31 PM, Kevin Buettner wrote: >>>>> This patch adds support for DW_OP_GNU_variable_value to GDB. >>>>> >>>>> Jakub Jelinek provides a fairly expansive discussion of this DWARF >>>>> expression opcode in his GCC patch... >>>>> >>>>> https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html >>>>> >>>>> It has also been proposed for addition to the DWARF Standard: >>>>> >>>>> http://www.dwarfstd.org/ShowIssue.php?issue=161109.2 >>>> Hi, >>>> >>>> AFAIU from the discussion here ( >>>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01351.html ) if: >>>> - a DW_OP_GNU_variable_value refers to a die 'a', and >>>> - there's a die 'b' with abstract_origin 'a' that does have a >>>> DW_AT_location, and >>>> - die 'b' is 'in scope' in an evaluation context, >>>> then the evaluation of DW_OP_GNU_variable_value 'a' should return the >>>> value found at the DW_AT_location of die 'b'. > > Hi, > > this patch implements the handling of DW_OP_GNU_variable_value refs to > abstract dies. > > There's no test-case yet, I'll try to write one. > Test-case added. OK for trunk? Thanks, - Tom [-- Attachment #2: 0001-gdb-exp-Handle-DW_OP_GNU_variable_value-refs-to-abstract-dies.patch --] [-- Type: text/x-patch, Size: 11976 bytes --] [gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Consider a vla variable 'a' in function f1: ... <2><1a7>: Abbrev Number: 11 (DW_TAG_variable) <1a8> DW_AT_description : a <1aa> DW_AT_abstract_origin: <0x311> ... with abstract origin 'a': ... <2><311>: Abbrev Number: 3 (DW_TAG_variable) <312> DW_AT_name : a <317> DW_AT_type : <0x325> ... and inherited abstract vla type: ... <1><325>: Abbrev Number: 9 (DW_TAG_array_type) <326> DW_AT_type : <0x33a> <2><32e>: Abbrev Number: 10 (DW_TAG_subrange_type) <32f> DW_AT_type : <0x2ea> <333> DW_AT_upper_bound : 5 byte block: fd 1b 3 0 0 (DW_OP_GNU_variable_value: <0x31b>) ... where the upper bound refers to this artificial variable D.1922 without location attribute: ... <2><31b>: Abbrev Number: 8 (DW_TAG_variable) <31c> DW_AT_description : (indirect string, offset: 0x39a): D.1922 <320> DW_AT_type : <0x2ea> <324> DW_AT_artificial : 1 ... Currently, when we execute "p sizeof (a)" in f1, the upper bound is calculated by evaluating the DW_OP_GNU_variable_value expression referring to D.1922, but since that die doesn't have a location attribute, we get: ... value has been optimized out ... However, there's also artificial variable D.4283 that is sibling of vla variable 'a', has artificial variable D.1922 as abstract origin, and has a location attribute: ... <2><1ae>: Abbrev Number: 12 (DW_TAG_variable) <1af> DW_AT_description : (indirect string, offset: 0x1f8): D.4283 <1b3> DW_AT_abstract_origin: <0x31b> <1b7> DW_AT_location : 11 byte block: 75 1 8 20 24 8 20 26 31 1c 9f (DW_OP_breg5 (rdi):1; DW_OP_const1u: 32; DW_OP_shl; DW_OP_const1u: 32; DW_OP_shra; DW_OP_lit1; DW_OP_minus; DW_OP_stack_value) ... The intended behaviour for DW_OP_GNU_variable_value is to find a die that refers to D.1922 as abstract origin, has a location attribute and is 'in scope', so the expected behaviour is: ... $1 = 6 ... The 'in scope' concept can be thought of as variable D.1922 having name attribute "D.1922", and variable D.4283 inheriting that attribute, resulting in D.4283 being declared with name "D.1922" alongside vla a in f1, and when we lookup "DW_OP_GNU_variable_value D.1922", it should work as if we try to find the value of a variable named "D.1922" on the gdb command line using "p D.1922", and we should return the value of D.4283. This patch fixes the case described above, by: - adding a field abstract_to_concrete to struct dwarf2_per_objfile, - using that field to keep track of which concrete dies are instances of an abstract die, and - using that information when getting the value DW_OP_GNU_variable_value. Build and reg-tested on x86_64-linux. 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); /* Until these have formal names, we define these here. ref: http://gcc.gnu.org/wiki/DebugFission @@ -573,7 +573,7 @@ sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off, struct type *type = lookup_pointer_type (die_type); struct frame_info *frame = get_selected_frame (_("No frame selected.")); - return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type); + return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type, true); } class dwarf_evaluate_loc_desc : public dwarf_expr_context @@ -2181,12 +2181,14 @@ fetch_const_value_from_synthetic_pointer (sect_offset die, LONGEST byte_offset, 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 frame_info *frame, struct type *type, + bool resolve_abstract_p) { /* Fetch the location expression of the DIE we're pointing to. */ struct dwarf2_locexpr_baton baton = dwarf2_fetch_die_loc_sect_off (die, per_cu, - get_frame_address_in_block_wrapper, frame); + get_frame_address_in_block_wrapper, frame, + resolve_abstract_p); /* Get type of pointed-to DIE. */ struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu); 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); struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_cu_off (cu_offset offset_in_cu, struct dwarf2_per_cu_data *per_cu, diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index d66dfeaf2d..419858bb98 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -2159,6 +2159,13 @@ dwarf2_per_objfile::~dwarf2_per_objfile () VEC_free (dwarf2_section_info_def, types); + for (std::unordered_map<die_info_ptr, VEC (die_info_ptr) *>::iterator it + = abstract_to_concrete.begin (); + it != abstract_to_concrete.end (); + it++) + VEC_free (die_info_ptr, it->second); + abstract_to_concrete.clear (); + if (dwo_files != NULL) free_dwo_files (dwo_files, objfile); @@ -14283,7 +14290,23 @@ read_variable (struct die_info *die, struct dwarf2_cu *cu) } } - new_symbol (die, NULL, cu, storage); + struct symbol *res = new_symbol (die, NULL, cu, storage); + struct attribute *abstract_origin + = dwarf2_attr (die, DW_AT_abstract_origin, cu); + struct attribute *loc = dwarf2_attr (die, DW_AT_location, cu); + if (res == NULL && loc && abstract_origin) + { + /* We have a variable without a name, but with a location and an abstract + origin. This may be a concrete instance of an abstract variable + referenced from an DW_OP_GNU_variable_value, so save it to find it back + later. */ + struct dwarf2_cu *origin_cu = cu; + struct die_info *origin_die + = follow_die_ref (die, abstract_origin, &origin_cu); + dwarf2_per_objfile *dpo = cu->per_cu->dwarf2_per_objfile; + VEC_safe_push (die_info_ptr, dpo->abstract_to_concrete[origin_die], + die); + } } /* Call CALLBACK from DW_AT_ranges attribute value OFFSET @@ -23010,7 +23033,7 @@ struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, struct dwarf2_per_cu_data *per_cu, CORE_ADDR (*get_frame_pc) (void *baton), - void *baton) + void *baton, bool resolve_abstract_p) { struct dwarf2_cu *cu; struct die_info *die; @@ -23036,6 +23059,35 @@ dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, sect_offset_str (sect_off), objfile_name (objfile)); attr = dwarf2_attr (die, DW_AT_location, cu); + if (!attr && resolve_abstract_p + && (dwarf2_per_objfile->abstract_to_concrete.find (die) + != dwarf2_per_objfile->abstract_to_concrete.end ())) + { + CORE_ADDR pc = (*get_frame_pc) (baton); + + unsigned i; + struct die_info *cand; + for (i = 0; + VEC_iterate (die_info_ptr, + dwarf2_per_objfile->abstract_to_concrete[die], i, cand); + i++) + { + if (!cand->parent + || cand->parent->tag != DW_TAG_subprogram) + continue; + + CORE_ADDR pc_low, pc_high; + get_scope_pc_bounds (cand->parent, &pc_low, &pc_high, cu); + if (pc_low == ((CORE_ADDR) -1) + || !(pc_low <= pc && pc < pc_high)) + continue; + + die = cand; + attr = dwarf2_attr (die, DW_AT_location, cu); + break; + } + } + if (!attr) { /* DWARF: "If there is no such attribute, then there is no effect.". 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; }; /* Get the dwarf2_per_objfile associated to OBJFILE. */ diff --git a/gdb/testsuite/gdb.dwarf2/varval.exp b/gdb/testsuite/gdb.dwarf2/varval.exp index f4319ae7d2..400ad60873 100644 --- a/gdb/testsuite/gdb.dwarf2/varval.exp +++ b/gdb/testsuite/gdb.dwarf2/varval.exp @@ -49,7 +49,9 @@ Dwarf::assemble ${asm_file} { } { declare_labels int_label ptr_label struct_label var_a_label \ var_b_label var_c_label var_p_label var_bad_label \ - varval_label var_s_label var_untyped_label + varval_label var_s_label var_untyped_label \ + var_a_abstract_label var_a_concrete_label \ + varval2_label set int_size [get_sizeof "int" -1] @@ -73,6 +75,11 @@ Dwarf::assemble ${asm_file} { {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr} } + var_a_abstract_label: DW_TAG_variable { + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + } + var_b_label: DW_TAG_variable { {DW_AT_name "var_b"} {DW_AT_type :${int_label}} @@ -171,6 +178,18 @@ Dwarf::assemble ${asm_file} { DW_OP_stack_value } SPECIAL_expr} } + varval2_label: DW_TAG_variable { + {DW_AT_name "varval2"} + {DW_AT_type :${int_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_a_abstract_label} + DW_OP_stack_value + } SPECIAL_expr} + } + var_a_concrete_label: DW_TAG_variable { + {DW_AT_abstract_origin :${var_a_abstract_label}} + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr} + } DW_TAG_variable { {DW_AT_name "constval"} {DW_AT_type :${int_label}} @@ -255,6 +274,7 @@ if ![runto_main] { } gdb_test "print varval" "= 8" +gdb_test "print varval2" "= 8" gdb_test "print constval" "= 53" gdb_test "print mixedval" "= 42" gdb_test "print pointerval" "= \\(int \\*\\) $hex <var_b>" ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-09-04 13:06 ` Tom de Vries @ 2018-09-04 21:54 ` Kevin Buettner 2018-09-05 10:53 ` Tom de Vries 2018-09-06 3:46 ` Tom Tromey 0 siblings, 2 replies; 22+ messages in thread From: Kevin Buettner @ 2018-09-04 21:54 UTC (permalink / raw) To: gdb-patches; +Cc: Tom de Vries 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-09-04 21:54 ` Kevin Buettner @ 2018-09-05 10:53 ` Tom de Vries 2018-09-06 3:46 ` Tom Tromey 1 sibling, 0 replies; 22+ messages in thread From: Tom de Vries @ 2018-09-05 10:53 UTC (permalink / raw) To: Kevin Buettner, gdb-patches [-- Attachment #1: Type: text/plain, Size: 3952 bytes --] On 09/04/2018 11:54 PM, Kevin Buettner wrote: > 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. > Done. >> 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. > Done. >> 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? No, I just used what was most familiar to me. > 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. > Done. > Also, can you add a comment for abstract_to_concrete? > Done. Committed as attached. Thanks, - Tom [-- Attachment #2: 0001-gdb-exp-Handle-DW_OP_GNU_variable_value-refs-to-abstract-dies.patch --] [-- Type: text/x-patch, Size: 11457 bytes --] [gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Consider a vla variable 'a' in function f1: ... <2><1a7>: Abbrev Number: 11 (DW_TAG_variable) <1a8> DW_AT_description : a <1aa> DW_AT_abstract_origin: <0x311> ... with abstract origin 'a': ... <2><311>: Abbrev Number: 3 (DW_TAG_variable) <312> DW_AT_name : a <317> DW_AT_type : <0x325> ... and inherited abstract vla type: ... <1><325>: Abbrev Number: 9 (DW_TAG_array_type) <326> DW_AT_type : <0x33a> <2><32e>: Abbrev Number: 10 (DW_TAG_subrange_type) <32f> DW_AT_type : <0x2ea> <333> DW_AT_upper_bound : 5 byte block: fd 1b 3 0 0 (DW_OP_GNU_variable_value: <0x31b>) ... where the upper bound refers to this artificial variable D.1922 without location attribute: ... <2><31b>: Abbrev Number: 8 (DW_TAG_variable) <31c> DW_AT_description : (indirect string, offset: 0x39a): D.1922 <320> DW_AT_type : <0x2ea> <324> DW_AT_artificial : 1 ... Currently, when we execute "p sizeof (a)" in f1, the upper bound is calculated by evaluating the DW_OP_GNU_variable_value expression referring to D.1922, but since that die doesn't have a location attribute, we get: ... value has been optimized out ... However, there's also artificial variable D.4283 that is sibling of vla variable 'a', has artificial variable D.1922 as abstract origin, and has a location attribute: ... <2><1ae>: Abbrev Number: 12 (DW_TAG_variable) <1af> DW_AT_description : (indirect string, offset: 0x1f8): D.4283 <1b3> DW_AT_abstract_origin: <0x31b> <1b7> DW_AT_location : 11 byte block: 75 1 8 20 24 8 20 26 31 1c 9f (DW_OP_breg5 (rdi):1; DW_OP_const1u: 32; DW_OP_shl; DW_OP_const1u: 32; DW_OP_shra; DW_OP_lit1; DW_OP_minus; DW_OP_stack_value) ... The intended behaviour for DW_OP_GNU_variable_value is to find a die that refers to D.1922 as abstract origin, has a location attribute and is 'in scope', so the expected behaviour is: ... $1 = 6 ... The 'in scope' concept can be thought of as variable D.1922 having name attribute "D.1922", and variable D.4283 inheriting that attribute, resulting in D.4283 being declared with name "D.1922" alongside vla a in f1, and when we lookup "DW_OP_GNU_variable_value D.1922", it should work as if we try to find the value of a variable named "D.1922" on the gdb command line using "p D.1922", and we should return the value of D.4283. This patch fixes the case described above, by: - adding a field abstract_to_concrete to struct dwarf2_per_objfile, - using that field to keep track of which concrete dies are instances of an abstract die, and - using that information when getting the value DW_OP_GNU_variable_value. Build and reg-tested on x86_64-linux. 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 (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. (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 | 43 +++++++++++++++++++++++++++++++++++-- gdb/dwarf2read.h | 8 +++++++ gdb/testsuite/gdb.dwarf2/varval.exp | 22 ++++++++++++++++++- 5 files changed, 77 insertions(+), 8 deletions(-) diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 200fa03f46..84e3c3ca69 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 resolve_abstract_p = false); /* Until these have formal names, we define these here. ref: http://gcc.gnu.org/wiki/DebugFission @@ -573,7 +573,7 @@ sect_variable_value (struct dwarf_expr_context *ctx, sect_offset sect_off, struct type *type = lookup_pointer_type (die_type); struct frame_info *frame = get_selected_frame (_("No frame selected.")); - return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type); + return indirect_synthetic_pointer (sect_off, 0, per_cu, frame, type, true); } class dwarf_evaluate_loc_desc : public dwarf_expr_context @@ -2181,12 +2181,14 @@ fetch_const_value_from_synthetic_pointer (sect_offset die, LONGEST byte_offset, 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 frame_info *frame, struct type *type, + bool resolve_abstract_p) { /* Fetch the location expression of the DIE we're pointing to. */ struct dwarf2_locexpr_baton baton = dwarf2_fetch_die_loc_sect_off (die, per_cu, - get_frame_address_in_block_wrapper, frame); + get_frame_address_in_block_wrapper, frame, + resolve_abstract_p); /* Get type of pointed-to DIE. */ struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu); diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h index f82e7b2d11..d02e3cd354 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 resolve_abstract_p = false); struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_cu_off (cu_offset offset_in_cu, struct dwarf2_per_cu_data *per_cu, diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index d66dfeaf2d..4a35e389e9 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -14283,7 +14283,22 @@ read_variable (struct die_info *die, struct dwarf2_cu *cu) } } - new_symbol (die, NULL, cu, storage); + struct symbol *res = new_symbol (die, NULL, cu, storage); + struct attribute *abstract_origin + = dwarf2_attr (die, DW_AT_abstract_origin, cu); + struct attribute *loc = dwarf2_attr (die, DW_AT_location, cu); + if (res == NULL && loc && abstract_origin) + { + /* We have a variable without a name, but with a location and an abstract + origin. This may be a concrete instance of an abstract variable + referenced from an DW_OP_GNU_variable_value, so save it to find it back + later. */ + struct dwarf2_cu *origin_cu = cu; + struct die_info *origin_die + = follow_die_ref (die, abstract_origin, &origin_cu); + dwarf2_per_objfile *dpo = cu->per_cu->dwarf2_per_objfile; + dpo->abstract_to_concrete[origin_die].push_back (die); + } } /* Call CALLBACK from DW_AT_ranges attribute value OFFSET @@ -23010,7 +23025,7 @@ struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, struct dwarf2_per_cu_data *per_cu, CORE_ADDR (*get_frame_pc) (void *baton), - void *baton) + void *baton, bool resolve_abstract_p) { struct dwarf2_cu *cu; struct die_info *die; @@ -23036,6 +23051,30 @@ dwarf2_fetch_die_loc_sect_off (sect_offset sect_off, sect_offset_str (sect_off), objfile_name (objfile)); attr = dwarf2_attr (die, DW_AT_location, cu); + if (!attr && resolve_abstract_p + && (dwarf2_per_objfile->abstract_to_concrete.find (die) + != dwarf2_per_objfile->abstract_to_concrete.end ())) + { + CORE_ADDR pc = (*get_frame_pc) (baton); + + for (const auto &cand : dwarf2_per_objfile->abstract_to_concrete[die]) + { + if (!cand->parent + || cand->parent->tag != DW_TAG_subprogram) + continue; + + CORE_ADDR pc_low, pc_high; + get_scope_pc_bounds (cand->parent, &pc_low, &pc_high, cu); + if (pc_low == ((CORE_ADDR) -1) + || !(pc_low <= pc && pc < pc_high)) + continue; + + die = cand; + attr = dwarf2_attr (die, DW_AT_location, cu); + break; + } + } + if (!attr) { /* DWARF: "If there is no such attribute, then there is no effect.". diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h index 13855bcd54..67ebc1902b 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,8 @@ struct dwarf2_debug_sections; struct mapped_index; struct mapped_debug_names; struct signatured_type; +struct die_info; +typedef struct die_info *die_info_ptr; /* Collection of data recorded per objfile. This hangs off of dwarf2_objfile_data_key. */ @@ -250,6 +253,11 @@ 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; + + /* Mapping from abstract origin DIE to concrete DIEs that reference it as + DW_AT_abstract_origin. */ + std::unordered_map<die_info_ptr, std::vector<die_info_ptr>> + abstract_to_concrete; }; /* Get the dwarf2_per_objfile associated to OBJFILE. */ diff --git a/gdb/testsuite/gdb.dwarf2/varval.exp b/gdb/testsuite/gdb.dwarf2/varval.exp index f4319ae7d2..400ad60873 100644 --- a/gdb/testsuite/gdb.dwarf2/varval.exp +++ b/gdb/testsuite/gdb.dwarf2/varval.exp @@ -49,7 +49,9 @@ Dwarf::assemble ${asm_file} { } { declare_labels int_label ptr_label struct_label var_a_label \ var_b_label var_c_label var_p_label var_bad_label \ - varval_label var_s_label var_untyped_label + varval_label var_s_label var_untyped_label \ + var_a_abstract_label var_a_concrete_label \ + varval2_label set int_size [get_sizeof "int" -1] @@ -73,6 +75,11 @@ Dwarf::assemble ${asm_file} { {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr} } + var_a_abstract_label: DW_TAG_variable { + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + } + var_b_label: DW_TAG_variable { {DW_AT_name "var_b"} {DW_AT_type :${int_label}} @@ -171,6 +178,18 @@ Dwarf::assemble ${asm_file} { DW_OP_stack_value } SPECIAL_expr} } + varval2_label: DW_TAG_variable { + {DW_AT_name "varval2"} + {DW_AT_type :${int_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_a_abstract_label} + DW_OP_stack_value + } SPECIAL_expr} + } + var_a_concrete_label: DW_TAG_variable { + {DW_AT_abstract_origin :${var_a_abstract_label}} + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr} + } DW_TAG_variable { {DW_AT_name "constval"} {DW_AT_type :${int_label}} @@ -255,6 +274,7 @@ if ![runto_main] { } gdb_test "print varval" "= 8" +gdb_test "print varval2" "= 8" gdb_test "print constval" "= 53" gdb_test "print mixedval" "= 42" gdb_test "print pointerval" "= \\(int \\*\\) $hex <var_b>" ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-09-04 21:54 ` Kevin Buettner 2018-09-05 10:53 ` Tom de Vries @ 2018-09-06 3:46 ` Tom Tromey 2018-09-06 6:40 ` Tom de Vries 1 sibling, 1 reply; 22+ messages in thread From: Tom Tromey @ 2018-09-06 3:46 UTC (permalink / raw) To: Kevin Buettner; +Cc: gdb-patches, Tom de Vries >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: Kevin> Is there a good reason to use VEC instead of std::vector? I know that Kevin> there have been a number of patches which have been replacing VEC Kevin> with std:vector. So, unless there's a compelling reason to use VEC, Kevin> we might as well use std:vector here and save someone else the effort Kevin> of changing this use of VEC later on. Thanks for this note. One of my gdb c++-ification/cleanup goals is to get rid of VEC. Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-09-06 3:46 ` Tom Tromey @ 2018-09-06 6:40 ` Tom de Vries 2018-09-06 13:11 ` Tom Tromey 0 siblings, 1 reply; 22+ messages in thread From: Tom de Vries @ 2018-09-06 6:40 UTC (permalink / raw) To: Tom Tromey, Kevin Buettner; +Cc: gdb-patches On 09/06/2018 05:45 AM, Tom Tromey wrote: >>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: > > Kevin> Is there a good reason to use VEC instead of std::vector? I know that > Kevin> there have been a number of patches which have been replacing VEC > Kevin> with std:vector. So, unless there's a compelling reason to use VEC, > Kevin> we might as well use std:vector here and save someone else the effort > Kevin> of changing this use of VEC later on. > > Thanks for this note. One of my gdb c++-ification/cleanup goals is to > get rid of VEC. It would be nice if the sources reflected that fact. F.i., we could move the contents from vec.{c,h} to vec-deprecated.{c.h} and include the new files in vec.{c,h}, and add a note there why they're deprecated. I can prepare a patch implementing this or another approach, if there are better suggestions. Thanks, - Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies 2018-09-06 6:40 ` Tom de Vries @ 2018-09-06 13:11 ` Tom Tromey 0 siblings, 0 replies; 22+ messages in thread From: Tom Tromey @ 2018-09-06 13:11 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, Kevin Buettner, gdb-patches >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> It would be nice if the sources reflected that fact. Tom> F.i., we could move the contents from vec.{c,h} to vec-deprecated.{c.h} Tom> and include the new files in vec.{c,h}, and add a note there why they're Tom> deprecated. Tom> I can prepare a patch implementing this or another approach, if there Tom> are better suggestions. I think that probably would not prevent the introduction of new uses, because normally code wouldn't be including this header directly anyway. Even adding a deprecated_ onto a name isn't really enough, I think on occasion we let a new use of a deprecated function in -- maybe a sign that the function shouldn't be deprecated, or maybe just an admission that nobody is working on finishing those transitions. Still, no objections from me if you want to do this! But I tend to think the current situation is fine. Finally FWIW this isn't the only such project that is ongoing. Cleanup removal is the other big one, but I'd also eventually like to be able to remove queue.h (I can't recall but I think I sent some patches); and eventually also buffer.h. Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value 2018-08-03 4:18 ` [PATCH 1/3] " Kevin Buettner 2018-08-03 18:36 ` Tom Tromey @ 2018-08-03 18:43 ` Tom Tromey 1 sibling, 0 replies; 22+ messages in thread From: Tom Tromey @ 2018-08-03 18:43 UTC (permalink / raw) To: Kevin Buettner; +Cc: gdb-patches >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: I forgot a note earlier... Kevin> + /* Note: Things still work when the following test is removed. This Kevin> + test and error is here to conform to the proposed specification. */ Kevin> + if (!is_scalar_type (die_type) || TYPE_CODE (die_type) == TYPE_CODE_VOID) Kevin> + error (_("Type of DW_OP_GNU_variable_value DIE must be an integer or pointer.")); I suspect is_scalar_type isn't what you want here. Maybe instead either "is_integral_type || TYPE_CODE == TYPE_CODE_PTR", or just checking for TYPE_CODE_PTR and TYPE_CODE_INT (since, bizarrely to me at least, is_integral_type includes range types). Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/3] Add support of DW_OP_GNU_variable_value to DWARF assembler 2018-08-03 4:04 [PATCH 0/3] Add support for DW_OP_GNU_variable_value Kevin Buettner 2018-08-03 4:18 ` [PATCH 1/3] " Kevin Buettner @ 2018-08-03 4:19 ` Kevin Buettner 2018-08-03 18:37 ` Tom Tromey 2018-08-03 4:22 ` [PATCH 3/3] Test case for DW_OP_GNU_variable_value Kevin Buettner 2 siblings, 1 reply; 22+ messages in thread From: Kevin Buettner @ 2018-08-03 4:19 UTC (permalink / raw) To: gdb-patches gdb/testsuite/ChangeLog: * lib/dwarf.exp: Add support for DW_OP_GNU_variable_value. --- gdb/testsuite/lib/dwarf.exp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp index 82ec29b..0c3f50a 100644 --- a/gdb/testsuite/lib/dwarf.exp +++ b/gdb/testsuite/lib/dwarf.exp @@ -962,6 +962,20 @@ namespace eval Dwarf { _op .sleb128 [lindex $line 2] } + DW_OP_GNU_variable_value { + if {[llength $line] != 2} { + error "usage: $opcode LABEL" + } + + # Here label is a section offset. + set label [lindex $line 1] + if { $_cu_version == 2 } { + _op .${_cu_addr_size}byte $label + } else { + _op .${_cu_offset_size}byte $label + } + } + DW_OP_deref_size { if {[llength $line] != 2} { error "usage: DW_OP_deref_size SIZE" ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] Add support of DW_OP_GNU_variable_value to DWARF assembler 2018-08-03 4:19 ` [PATCH 2/3] Add support of DW_OP_GNU_variable_value to DWARF assembler Kevin Buettner @ 2018-08-03 18:37 ` Tom Tromey 2018-08-18 20:32 ` Kevin Buettner 0 siblings, 1 reply; 22+ messages in thread From: Tom Tromey @ 2018-08-03 18:37 UTC (permalink / raw) To: Kevin Buettner; +Cc: gdb-patches >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: Kevin> gdb/testsuite/ChangeLog: Kevin> * lib/dwarf.exp: Add support for DW_OP_GNU_variable_value. This is ok. Thank you. Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/3] Add support of DW_OP_GNU_variable_value to DWARF assembler 2018-08-03 18:37 ` Tom Tromey @ 2018-08-18 20:32 ` Kevin Buettner 0 siblings, 0 replies; 22+ messages in thread From: Kevin Buettner @ 2018-08-18 20:32 UTC (permalink / raw) To: gdb-patches On Fri, 03 Aug 2018 12:37:14 -0600 Tom Tromey <tom@tromey.com> wrote: > >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: > > Kevin> gdb/testsuite/ChangeLog: > Kevin> * lib/dwarf.exp: Add support for DW_OP_GNU_variable_value. > > This is ok. Thank you. > > Tom Pushed. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] Test case for DW_OP_GNU_variable_value 2018-08-03 4:04 [PATCH 0/3] Add support for DW_OP_GNU_variable_value Kevin Buettner 2018-08-03 4:18 ` [PATCH 1/3] " Kevin Buettner 2018-08-03 4:19 ` [PATCH 2/3] Add support of DW_OP_GNU_variable_value to DWARF assembler Kevin Buettner @ 2018-08-03 4:22 ` Kevin Buettner 2018-08-03 18:40 ` Tom Tromey 2 siblings, 1 reply; 22+ messages in thread From: Kevin Buettner @ 2018-08-03 4:22 UTC (permalink / raw) To: gdb-patches gdb/testsuite/ChangeLog: * gdb.dwarf2/varval.c: New file. * gdb.dwarf2/varval.exp: New file. --- gdb/testsuite/gdb.dwarf2/varval.c | 30 ++++ gdb/testsuite/gdb.dwarf2/varval.exp | 279 ++++++++++++++++++++++++++++++++++++ 2 files changed, 309 insertions(+) diff --git a/gdb/testsuite/gdb.dwarf2/varval.c b/gdb/testsuite/gdb.dwarf2/varval.c new file mode 100644 index 0000000..ce5592e --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/varval.c @@ -0,0 +1,30 @@ +/* Copyright (C) 2018 Free Software Foundation, Inc. + + This file is part of GDB. + + 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/>. */ + +/* Test program for DW_OP_GNU_variable_value. */ + +int var_a = 8; +int var_b = 3; +int *var_p = &var_b; +struct { int a, b, c, d, e, f, g, h; } var_s = { 101, 102, 103, 104, 105, 106, 107, 108 }; + +int +main (void) +{ + asm ("main_label: .globl main_label"); + return 0; +} diff --git a/gdb/testsuite/gdb.dwarf2/varval.exp b/gdb/testsuite/gdb.dwarf2/varval.exp new file mode 100644 index 0000000..f4319ae --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/varval.exp @@ -0,0 +1,279 @@ +# Copyright 2018 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/>. + +# Test support for DW_OP_GNU_variable_value. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +if ![dwarf2_support] { + return 0 +} + +# We'll place the output of Dwarf::assemble in varval.S. +standard_testfile .c .S + +# ${testfile} is now "varval". srcfile2 is "varval.S". +set executable ${testfile} +set asm_file [standard_output_file ${srcfile2}] + +# We need to know the size of integer and address types in order +# to write some of the debugging info we'd like to generate. +# +# For that, we ask GDB by debugging our varval program. +# Any program would do, but since we already have varval +# specifically for this testcase, might as well use that. +if [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] { + return -1 +} + +# Create the DWARF. +Dwarf::assemble ${asm_file} { + global srcdir subdir srcfile + + cu {} { + DW_TAG_compile_unit { + {DW_AT_language @DW_LANG_C_plus_plus} + } { + declare_labels int_label ptr_label struct_label var_a_label \ + var_b_label var_c_label var_p_label var_bad_label \ + varval_label var_s_label var_untyped_label + + set int_size [get_sizeof "int" -1] + + # gdb always assumes references are implemented as pointers. + set addr_size [get_sizeof "void *" -1] + + int_label: DW_TAG_base_type { + {DW_AT_byte_size ${int_size} DW_FORM_udata} + {DW_AT_encoding @DW_ATE_signed} + {DW_AT_name "int"} + } + + ptr_label: DW_TAG_pointer_type { + {DW_AT_type :$int_label} + } + + var_a_label: DW_TAG_variable { + {DW_AT_name "var_a"} + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr} + } + + var_b_label: DW_TAG_variable { + {DW_AT_name "var_b"} + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_b"]} SPECIAL_expr} + } + + var_c_label: DW_TAG_variable { + {DW_AT_name "var_c"} + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_const_value 53 DW_FORM_sdata} + } + + var_p_label: DW_TAG_variable { + {DW_AT_name "var_p"} + {DW_AT_type :${ptr_label}} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_p"]} SPECIAL_expr} + } + + var_bad_label: DW_TAG_variable { + {DW_AT_name "var_bad"} + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + } + + struct_label: DW_TAG_structure_type { + {DW_AT_byte_size 8*$int_size DW_FORM_sdata} + } { + DW_TAG_member { + {DW_AT_name "a"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 0*$int_size DW_FORM_udata} + } + DW_TAG_member { + {DW_AT_name "b"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 1*$int_size DW_FORM_udata} + } + DW_TAG_member { + {DW_AT_name "c"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 2*$int_size DW_FORM_udata} + } + DW_TAG_member { + {DW_AT_name "d"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 3*$int_size DW_FORM_udata} + } + DW_TAG_member { + {DW_AT_name "e"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 4*$int_size DW_FORM_udata} + } + DW_TAG_member { + {DW_AT_name "f"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 5*$int_size DW_FORM_udata} + } + DW_TAG_member { + {DW_AT_name "g"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 6*$int_size DW_FORM_udata} + } + DW_TAG_member { + {DW_AT_name "h"} + {DW_AT_type :$int_label} + {DW_AT_data_member_location 7*$int_size DW_FORM_udata} + } + } + + var_s_label: DW_TAG_variable { + {DW_AT_name "var_s"} + {DW_AT_type :${struct_label}} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_s"]} SPECIAL_expr} + } + + var_untyped_label: DW_TAG_variable { + {DW_AT_name "var_untyped"} + {DW_AT_external 1 DW_FORM_flag} + {DW_AT_location {DW_OP_addr [gdb_target_symbol "var_b"]} SPECIAL_expr} + } + + DW_TAG_subprogram { + {MACRO_AT_func { "main" "${srcdir}/${subdir}/${srcfile}" }} + {DW_AT_type :${int_label}} + {DW_AT_external 1 DW_FORM_flag} + } { + varval_label: DW_TAG_variable { + {DW_AT_name "varval"} + {DW_AT_type :${int_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_a_label} + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "constval"} + {DW_AT_type :${int_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_c_label} + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "mixedval"} + {DW_AT_type :${int_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_c_label} + DW_OP_GNU_variable_value ${var_b_label} + DW_OP_div + DW_OP_GNU_variable_value ${varval_label} + DW_OP_plus + DW_OP_dup + DW_OP_plus + DW_OP_GNU_variable_value ${varval_label} + DW_OP_minus + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "pointerval"} + {DW_AT_type :${ptr_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_p_label} + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "badval"} + {DW_AT_type :${int_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_bad_label} + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "structval"} + {DW_AT_type :${struct_label}} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_s_label} + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "untypedval"} + {DW_AT_location { + DW_OP_GNU_variable_value ${var_untyped_label} + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "bad_die_val1"} + {DW_AT_location { + DW_OP_GNU_variable_value 0xabcdef11 + DW_OP_stack_value + } SPECIAL_expr} + } + DW_TAG_variable { + {DW_AT_name "bad_die_val2"} + {DW_AT_location { + DW_OP_GNU_variable_value ${ptr_label}+1 + DW_OP_stack_value + } SPECIAL_expr} + } + } + } + } +} + +if [prepare_for_testing "failed to prepare" ${executable} [list ${asm_file} ${srcfile}] {}] { + return -1 +} + +# DW_OP_GNU_variable_value implementation requires a valid frame. +if ![runto_main] { + return -1 +} + +gdb_test "print varval" "= 8" +gdb_test "print constval" "= 53" +gdb_test "print mixedval" "= 42" +gdb_test "print pointerval" "= \\(int \\*\\) $hex <var_b>" +gdb_test "print *pointerval" "= 3" +gdb_test "print badval" "value has been optimized out" + +# Jakub says: "The intended behavior is that the debug info consumer +# computes the value of that referenced variable at the current PC, +# and if it can compute it and pushes the value as a generic type +# integer into the DWARF stack (it is really only meaningful when +# referring to integral/pointer typed variables)." + +gdb_test "print structval" \ + "Type of DW_OP_GNU_variable_value DIE must be an integer or pointer\\." + +gdb_test "print untypedval" \ + "Type of DW_OP_GNU_variable_value DIE must be an integer or pointer\\." + +gdb_test "print bad_die_val1" \ + "invalid dwarf2 offset 0xabcdef11" +gdb_test "print bad_die_val2" \ + "Bad DW_OP_GNU_variable_value DIE\\." ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] Test case for DW_OP_GNU_variable_value 2018-08-03 4:22 ` [PATCH 3/3] Test case for DW_OP_GNU_variable_value Kevin Buettner @ 2018-08-03 18:40 ` Tom Tromey 2018-08-18 20:32 ` Kevin Buettner 0 siblings, 1 reply; 22+ messages in thread From: Tom Tromey @ 2018-08-03 18:40 UTC (permalink / raw) To: Kevin Buettner; +Cc: gdb-patches >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: Kevin> gdb/testsuite/ChangeLog: Kevin> * gdb.dwarf2/varval.c: New file. Kevin> * gdb.dwarf2/varval.exp: New file. This is a really great test case, wow. Thank you. This is ok. Tom ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/3] Test case for DW_OP_GNU_variable_value 2018-08-03 18:40 ` Tom Tromey @ 2018-08-18 20:32 ` Kevin Buettner 0 siblings, 0 replies; 22+ messages in thread From: Kevin Buettner @ 2018-08-18 20:32 UTC (permalink / raw) To: gdb-patches On Fri, 03 Aug 2018 12:40:42 -0600 Tom Tromey <tom@tromey.com> wrote: > >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes: > > Kevin> gdb/testsuite/ChangeLog: > Kevin> * gdb.dwarf2/varval.c: New file. > Kevin> * gdb.dwarf2/varval.exp: New file. > > This is a really great test case, wow. Thank you. > This is ok. > > Tom Pushed. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-09-06 13:11 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-03 4:04 [PATCH 0/3] Add support for DW_OP_GNU_variable_value Kevin Buettner 2018-08-03 4:18 ` [PATCH 1/3] " Kevin Buettner 2018-08-03 18:36 ` Tom Tromey 2018-08-18 20:32 ` Kevin Buettner 2018-08-22 15:35 ` Tom de Vries 2018-08-23 21:12 ` Kevin Buettner 2018-08-24 13:09 ` [RFC, gdb/exp] Handle DW_OP_GNU_variable_value refs to abstract dies Tom de Vries 2018-08-24 13:18 ` Richard Biener 2018-09-04 10:17 ` [PATCH, " Tom de Vries 2018-09-04 13:06 ` Tom de Vries 2018-09-04 21:54 ` Kevin Buettner 2018-09-05 10:53 ` Tom de Vries 2018-09-06 3:46 ` Tom Tromey 2018-09-06 6:40 ` Tom de Vries 2018-09-06 13:11 ` Tom Tromey 2018-08-03 18:43 ` [PATCH 1/3] Add support for DW_OP_GNU_variable_value Tom Tromey 2018-08-03 4:19 ` [PATCH 2/3] Add support of DW_OP_GNU_variable_value to DWARF assembler Kevin Buettner 2018-08-03 18:37 ` Tom Tromey 2018-08-18 20:32 ` Kevin Buettner 2018-08-03 4:22 ` [PATCH 3/3] Test case for DW_OP_GNU_variable_value Kevin Buettner 2018-08-03 18:40 ` Tom Tromey 2018-08-18 20:32 ` Kevin Buettner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).