From patchwork Sat Aug 18 20:31:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Buettner X-Patchwork-Id: 28962 Received: (qmail 85353 invoked by alias); 18 Aug 2018 20:32:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 85254 invoked by uid 89); 18 Aug 2018 20:32:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=indicated, Until, nonzero, non-zero X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 18 Aug 2018 20:32:02 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D40887C6A9 for ; Sat, 18 Aug 2018 20:32:00 +0000 (UTC) Received: from pinnacle.lan (ovpn-117-83.phx2.redhat.com [10.3.117.83]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 896E82026D69 for ; Sat, 18 Aug 2018 20:32:00 +0000 (UTC) Date: Sat, 18 Aug 2018 13:31:58 -0700 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] Add support for DW_OP_GNU_variable_value Message-ID: <20180818133158.7e5b4dcb@pinnacle.lan> In-Reply-To: <87o9ej1emk.fsf@tromey.com> References: <20180802210405.5c04ca7a@pinnacle.lan> <20180802211754.40a529c2@pinnacle.lan> <87o9ej1emk.fsf@tromey.com> MIME-Version: 1.0 X-IsSubscribed: yes On Fri, 03 Aug 2018 12:36:35 -0600 Tom Tromey 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 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>: Abbrev Number: 12 (DW_TAG_variable) DW_AT_name : (indirect string, offset: 0x115): span.0 DW_AT_type : <0x2e> DW_AT_artificial : 1 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 + + * 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 * 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");