dwarf2loc.c: Perform a pointer to address conversion for DWARF_VALUE_MEMORY

Message ID 20151124231719.50585289@pinnacle.lan
State New, archived
Headers

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

Kevin Buettner Dec. 8, 2015, 6:57 p.m. UTC | #1
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)
>
  
Pedro Alves Dec. 9, 2015, 11:19 a.m. UTC | #2
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
  
Kevin Buettner Dec. 9, 2015, 4:34 p.m. UTC | #3
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
  

Patch

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)