Message ID | 20151124231719.50585289@pinnacle.lan |
---|---|
State | New, archived |
Headers |
Received: (qmail 110332 invoked by alias); 25 Nov 2015 06:17:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 110317 invoked by uid 89); 25 Nov 2015 06:17:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 25 Nov 2015 06:17:24 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 0032171 for <gdb-patches@sourceware.org>; Wed, 25 Nov 2015 06:17:22 +0000 (UTC) Received: from pinnacle.lan (ovpn-113-161.phx2.redhat.com [10.3.113.161]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAP6HLcS028551 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO) for <gdb-patches@sourceware.org>; Wed, 25 Nov 2015 01:17:22 -0500 Date: Tue, 24 Nov 2015 23:17:19 -0700 From: Kevin Buettner <kevinb@redhat.com> To: gdb-patches@sourceware.org Subject: [PATCH] dwarf2loc.c: Perform a pointer to address conversion for DWARF_VALUE_MEMORY Message-ID: <20151124231719.50585289@pinnacle.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes |
Commit Message
Kevin Buettner
Nov. 25, 2015, 6:17 a.m. UTC
This patch fixes the following failures for rl78-elf: FAIL: gdb.base/vla-datatypes.exp: print int_vla FAIL: gdb.base/vla-datatypes.exp: print unsigned_int_vla FAIL: gdb.base/vla-datatypes.exp: print double_vla FAIL: gdb.base/vla-datatypes.exp: print float_vla FAIL: gdb.base/vla-datatypes.exp: print long_vla FAIL: gdb.base/vla-datatypes.exp: print unsigned_long_vla FAIL: gdb.base/vla-datatypes.exp: print char_vla FAIL: gdb.base/vla-datatypes.exp: print short_vla FAIL: gdb.base/vla-datatypes.exp: print unsigned_short_vla FAIL: gdb.base/vla-datatypes.exp: print unsigned_char_vla FAIL: gdb.base/vla-datatypes.exp: print foo_vla FAIL: gdb.base/vla-datatypes.exp: print bar_vla FAIL: gdb.base/vla-datatypes.exp: print vla_struct_object FAIL: gdb.base/vla-datatypes.exp: print vla_union_object FAIL: gdb.base/vla-ptr.exp: print td_vla FAIL: gdb.mi/mi-vla-c99.exp: evaluate complete vla The first failure in this bunch occurs due to printing an incorrect result for a variable length array: print int_vla $1 = {-1, -1, -1, -1, -1} The result should actually be this: $1 = {0, 2, 4, 6, 8} When I started examining this bug, I found that printing an individual array element worked correctly. E.g. "print int_vla[2]" resulted in 4 being printed. I have not looked closely to see why this is the case. I found that evaluation of the location expression for int_vla was causing problems. This is the relevant DWARF entry for int_vla: <2><15a>: Abbrev Number: 10 (DW_TAG_variable) <15b> DW_AT_name : (indirect string, offset: 0xbf): int_vla <15f> DW_AT_decl_file : 1 <160> DW_AT_decl_line : 35 <161> DW_AT_type : <0x393> <165> DW_AT_location : 4 byte block: 86 7a 94 2 (DW_OP_breg22 (r22): -6; DW_OP_deref_size: 2) I found that DW_OP_breg22 was providing a correct result. DW_OP_deref_size was fetching the correct value from memory. However, the value being fetched should be considered a pointer. DW_OP_deref_size zero extends the fetched value prior to pushing it onto the evaluation stack. (The DWARF-4 document specifies this action; so GDB is faithfully implementing the DWARF-4 specification.) However, zero extending the pointer is not sufficient for converting that value to an address for rl78 and (perhaps) other architectures which define a `pointer_to_address' method. (I suspect that m32c would have the same problem.) Ideally, we would perform the pointer to address conversion in DW_OP_deref_size. We don't, however, know the type of the object that the address refers to in DW_OP_deref_size. I can't think of a way to infer the type at that point in the code. Before proceeding, I should note that there are two other DWARF operations that could be used in place of DW_OP_deref_size. One of these is DW_OP_GNU_deref_type. Current GDB implements this operation, but as is obvious from the name, it is non-standard DWARF. The other operation is DW_OP_xderef_size. Even though it's part of DWARF-2 through DWARF-4 specifications, it's not presently implemented in GDB. Present day GCC does not output dwarf expressions containing this operation either. [Of the two, I like DW_OP_GNU_deref_type better. Using it avoids the need to specify an "address space identifier". (GCC, GDB, and other non-free tools all need to agree on the meanings of these identifiers.)] Back to the bug analysis... The closest consumer of the DW_OP_deref_size result is the DWARF_VALUE_MEMORY case in dwarf2_evaluate_loc_desc_full. At that location, we do know the object type to which the address is intended to refer. I added code to perform a pointer to address conversion at this location. (See the patch.) I do have some misgivings regarding this patch. As noted earlier, it would really be better to perform the pointer to address conversion in DW_OP_deref_size. I can't, however, think of a way to make this work. Changing GCC to output one of the other aforementioned operations might be preferable but, as noted earlier, these solutions have problems as well. Long term, I think it'd be good to have something like DW_OP_GNU_deref_type become part of the standard. If that can't or won't happen, we'll need to implement DW_OP_xderef_size. But until that happens, this patch will work for expressions in which DW_OP_deref_size occurs last. It should even work for dereferences followed by adding an offset. I don't think it'll work for more than one dereference in the same expression. gdb/ChangeLog: * dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Perform a pointer to address conversion for DWARF_VALUE_MEMORY. --- gdb/dwarf2loc.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
Comments
Ping. On Tue, 24 Nov 2015 23:17:19 -0700 Kevin Buettner <kevinb@redhat.com> wrote: > This patch fixes the following failures for rl78-elf: > > FAIL: gdb.base/vla-datatypes.exp: print int_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_int_vla > FAIL: gdb.base/vla-datatypes.exp: print double_vla > FAIL: gdb.base/vla-datatypes.exp: print float_vla > FAIL: gdb.base/vla-datatypes.exp: print long_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_long_vla > FAIL: gdb.base/vla-datatypes.exp: print char_vla > FAIL: gdb.base/vla-datatypes.exp: print short_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_short_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_char_vla > FAIL: gdb.base/vla-datatypes.exp: print foo_vla > FAIL: gdb.base/vla-datatypes.exp: print bar_vla > FAIL: gdb.base/vla-datatypes.exp: print vla_struct_object > FAIL: gdb.base/vla-datatypes.exp: print vla_union_object > FAIL: gdb.base/vla-ptr.exp: print td_vla > FAIL: gdb.mi/mi-vla-c99.exp: evaluate complete vla > > The first failure in this bunch occurs due to printing an incorrect > result for a variable length array: > > print int_vla > $1 = {-1, -1, -1, -1, -1} > > The result should actually be this: > > $1 = {0, 2, 4, 6, 8} > > When I started examining this bug, I found that printing an > individual array element worked correctly. E.g. "print int_vla[2]" > resulted in 4 being printed. I have not looked closely to see why > this is the case. > > I found that evaluation of the location expression for int_vla was > causing problems. This is the relevant DWARF entry for int_vla: > > <2><15a>: Abbrev Number: 10 (DW_TAG_variable) > <15b> DW_AT_name : (indirect string, offset: 0xbf): int_vla > <15f> DW_AT_decl_file : 1 > <160> DW_AT_decl_line : 35 > <161> DW_AT_type : <0x393> > <165> DW_AT_location : 4 byte block: 86 7a 94 2 (DW_OP_breg22 (r22): -6; DW_OP_deref_size: 2) > > I found that DW_OP_breg22 was providing a correct result. > DW_OP_deref_size was fetching the correct value from memory. However, > the value being fetched should be considered a pointer. > DW_OP_deref_size zero extends the fetched value prior to pushing > it onto the evaluation stack. (The DWARF-4 document specifies this > action; so GDB is faithfully implementing the DWARF-4 specification.) > > However, zero extending the pointer is not sufficient for converting > that value to an address for rl78 and (perhaps) other architectures > which define a `pointer_to_address' method. (I suspect that m32c > would have the same problem.) > > Ideally, we would perform the pointer to address conversion in > DW_OP_deref_size. We don't, however, know the type of the object > that the address refers to in DW_OP_deref_size. I can't think > of a way to infer the type at that point in the code. > > Before proceeding, I should note that there are two other DWARF > operations that could be used in place of DW_OP_deref_size. One of > these is DW_OP_GNU_deref_type. Current GDB implements this operation, > but as is obvious from the name, it is non-standard DWARF. The other > operation is DW_OP_xderef_size. Even though it's part of DWARF-2 > through DWARF-4 specifications, it's not presently implemented in GDB. > Present day GCC does not output dwarf expressions containing this > operation either. [Of the two, I like DW_OP_GNU_deref_type better. > Using it avoids the need to specify an "address space identifier". > (GCC, GDB, and other non-free tools all need to agree on the meanings > of these identifiers.)] > > Back to the bug analysis... > > The closest consumer of the DW_OP_deref_size result is the > DWARF_VALUE_MEMORY case in dwarf2_evaluate_loc_desc_full. At that > location, we do know the object type to which the address is intended > to refer. I added code to perform a pointer to address conversion at > this location. (See the patch.) > > I do have some misgivings regarding this patch. As noted earlier, it > would really be better to perform the pointer to address conversion in > DW_OP_deref_size. I can't, however, think of a way to make this work. > Changing GCC to output one of the other aforementioned operations might > be preferable but, as noted earlier, these solutions have problems as > well. Long term, I think it'd be good to have something like > DW_OP_GNU_deref_type become part of the standard. If that can't or > won't happen, we'll need to implement DW_OP_xderef_size. > > But until that happens, this patch will work for expressions in which > DW_OP_deref_size occurs last. It should even work for dereferences > followed by adding an offset. I don't think it'll work for more than > one dereference in the same expression. > > gdb/ChangeLog: > > * dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Perform a pointer > to address conversion for DWARF_VALUE_MEMORY. > --- > gdb/dwarf2loc.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c > index a8f5c91..b8e7fa0 100644 > --- a/gdb/dwarf2loc.c > +++ b/gdb/dwarf2loc.c > @@ -2343,9 +2343,30 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, > > case DWARF_VALUE_MEMORY: > { > + struct type *ptr_type; > CORE_ADDR address = dwarf_expr_fetch_address (ctx, 0); > int in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0); > > + /* DW_OP_deref_size (and possibly other operations too) may > + create a pointer instead of an address. Ideally, the > + pointer to address conversion would be performed as part > + of those operations, but the type of the object to > + which the address refers is not known at the time of > + the operation. Therefore, we do the conversion here > + since the type is readily available. */ > + > + switch (TYPE_CODE (type)) > + { > + case TYPE_CODE_FUNC: > + case TYPE_CODE_METHOD: > + ptr_type = builtin_type (ctx->gdbarch)->builtin_func_ptr; > + break; > + default: > + ptr_type = builtin_type (ctx->gdbarch)->builtin_data_ptr; > + break; > + } > + address = value_as_address (value_from_pointer (ptr_type, address)); > + > do_cleanups (value_chain); > retval = value_at_lazy (type, address + byte_offset); > if (in_stack_memory) >
On 11/25/2015 06:17 AM, Kevin Buettner wrote: > This patch fixes the following failures for rl78-elf: > > FAIL: gdb.base/vla-datatypes.exp: print int_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_int_vla > FAIL: gdb.base/vla-datatypes.exp: print double_vla > FAIL: gdb.base/vla-datatypes.exp: print float_vla > FAIL: gdb.base/vla-datatypes.exp: print long_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_long_vla > FAIL: gdb.base/vla-datatypes.exp: print char_vla > FAIL: gdb.base/vla-datatypes.exp: print short_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_short_vla > FAIL: gdb.base/vla-datatypes.exp: print unsigned_char_vla > FAIL: gdb.base/vla-datatypes.exp: print foo_vla > FAIL: gdb.base/vla-datatypes.exp: print bar_vla > FAIL: gdb.base/vla-datatypes.exp: print vla_struct_object > FAIL: gdb.base/vla-datatypes.exp: print vla_union_object > FAIL: gdb.base/vla-ptr.exp: print td_vla > FAIL: gdb.mi/mi-vla-c99.exp: evaluate complete vla > > The first failure in this bunch occurs due to printing an incorrect > result for a variable length array: > > print int_vla > $1 = {-1, -1, -1, -1, -1} > > The result should actually be this: > > $1 = {0, 2, 4, 6, 8} > > When I started examining this bug, I found that printing an > individual array element worked correctly. E.g. "print int_vla[2]" > resulted in 4 being printed. I have not looked closely to see why > this is the case. > > I found that evaluation of the location expression for int_vla was > causing problems. This is the relevant DWARF entry for int_vla: > > <2><15a>: Abbrev Number: 10 (DW_TAG_variable) > <15b> DW_AT_name : (indirect string, offset: 0xbf): int_vla > <15f> DW_AT_decl_file : 1 > <160> DW_AT_decl_line : 35 > <161> DW_AT_type : <0x393> > <165> DW_AT_location : 4 byte block: 86 7a 94 2 (DW_OP_breg22 (r22): -6; DW_OP_deref_size: 2) > > I found that DW_OP_breg22 was providing a correct result. > DW_OP_deref_size was fetching the correct value from memory. However, > the value being fetched should be considered a pointer. > DW_OP_deref_size zero extends the fetched value prior to pushing > it onto the evaluation stack. (The DWARF-4 document specifies this > action; so GDB is faithfully implementing the DWARF-4 specification.) > > However, zero extending the pointer is not sufficient for converting > that value to an address for rl78 and (perhaps) other architectures > which define a `pointer_to_address' method. (I suspect that m32c > would have the same problem.) > > Ideally, we would perform the pointer to address conversion in > DW_OP_deref_size. We don't, however, know the type of the object > that the address refers to in DW_OP_deref_size. I can't think > of a way to infer the type at that point in the code. I'd think this would be another case of old-style untyped-dwarf, so should be handled with dwarf_expr_address_type. But that doesn't distinguish data / code pointers types, only sizes, which isn't sufficient. In that case, this seems reasonable to me. Thanks, Pedro Alves
On Wed, 09 Dec 2015 11:19:58 +0000
Pedro Alves <palves@redhat.com> wrote:
> In that case, this seems reasonable to me.
Pushed.
Thanks for looking it over.
Kevin
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index a8f5c91..b8e7fa0 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -2343,9 +2343,30 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, case DWARF_VALUE_MEMORY: { + struct type *ptr_type; CORE_ADDR address = dwarf_expr_fetch_address (ctx, 0); int in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0); + /* DW_OP_deref_size (and possibly other operations too) may + create a pointer instead of an address. Ideally, the + pointer to address conversion would be performed as part + of those operations, but the type of the object to + which the address refers is not known at the time of + the operation. Therefore, we do the conversion here + since the type is readily available. */ + + switch (TYPE_CODE (type)) + { + case TYPE_CODE_FUNC: + case TYPE_CODE_METHOD: + ptr_type = builtin_type (ctx->gdbarch)->builtin_func_ptr; + break; + default: + ptr_type = builtin_type (ctx->gdbarch)->builtin_data_ptr; + break; + } + address = value_as_address (value_from_pointer (ptr_type, address)); + do_cleanups (value_chain); retval = value_at_lazy (type, address + byte_offset); if (in_stack_memory)