[PATCHv2,4/5] gdb: Carry default property type around with dynamic properties

Message ID 3a2c023242f902483cfdaaac9c99f1562534de7a.1557439866.git.andrew.burgess@embecosm.com
State New
Headers show
Series
  • Improve handling of negative dynamic properties
Related show

Commit Message

Andrew Burgess May 9, 2019, 10:22 p.m.
This commit is preparation for the next one, with the aim of better
supporting signed dynamic properties on targets where the address size
specified in the DWARF headers is smaller than a CORE_ADDR, for
example debugging an i386 application on x86-64.

Consider this small Fortran program 'bounds.f90':

    program test
      integer, allocatable :: array (:)
      allocate (array (-5:5))
      array(3) = 1
    end program test

Compiled with 'gfortran -m32 -g3 -O0 -o bounds bounds.f90'.  The DWARF
for 'array' looks like this:

   <2><97>: Abbrev Number: 10 (DW_TAG_variable)
      <98>   DW_AT_name        : (indirect string, offset: 0x0): array
      <9c>   DW_AT_decl_file   : 1
      <9d>   DW_AT_decl_line   : 2
      <9e>   DW_AT_type        : <0xaf>
      <a2>   DW_AT_location    : 2 byte block: 91 58              (DW_OP_fbreg: -40)
   <2><a5>: Abbrev Number: 11 (DW_TAG_lexical_block)
      <a6>   DW_AT_low_pc      : 0x80485c3
      <aa>   DW_AT_high_pc     : 0x8b
   <2><ae>: Abbrev Number: 0
   <1><af>: Abbrev Number: 12 (DW_TAG_array_type)
      <b0>   DW_AT_data_location: 2 byte block: 97 6              (DW_OP_push_object_address; DW_OP_deref)
      <b3>   DW_AT_allocated   : 4 byte block: 97 6 30 2e         (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)
      <b8>   DW_AT_type        : <0x2a>
   <2><bc>: Abbrev Number: 13 (DW_TAG_subrange_type)
      <bd>   DW_AT_lower_bound : 4 byte block: 97 23 10 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 16; DW_OP_deref)
      <c2>   DW_AT_upper_bound : 4 byte block: 97 23 14 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 20; DW_OP_deref)
      <c7>   DW_AT_byte_stride : 6 byte block: 97 23 c 6 34 1e    (DW_OP_push_object_address; DW_OP_plus_uconst: 12; DW_OP_deref; DW_OP_lit4; DW_OP_mul)
   <2><ce>: Abbrev Number: 0

If we look at the DW_AT_lower_bound attribute, which will become a
dynamic property that GDB evaluates when needed by calling
dwarf2_evaluate_property.

The process of evaluating a dynamic property requires GDB to execute
each DW_OP_* operation, the results of these operations is held on a
stack of 'struct value *'s.

When the entire expression is evaluated the result is on top of the
stack.

If we look at DW_AT_lower_bound then the last operation is
DW_OP_deref, this loads a signed address the size of which matches the
DWARF address size, and so in our i386 on x86-64 situation, the top of
the stack will be a signed 4-byte value.

The problem is how these values are fetched from the stack.  Currently
they are always fetched by a call to dwarf_expr_context::fetch_address,
which converts the value to an unsigned value with a length matching
the values current length, before converting to a CORE_ADDR.  This
means we loose the signed nature of the property.

I wonder if the best solution for dealing with signed properties will
be to move away from an over reliance on fetch_address, and instead
come up with a new solution that considers the current type of the
value on the stack, and the type that the value needs to become;
basically a solution built around casting rather than assuming we
always want an address.

However, before we can start to even think about moving away from
fetch_address, there is a more urgent issue to fix, which is we don't
currently know what type each property should be.  We just hold the
value of the property in a CORE_ADDR as returned by fetch_address, and
rely on higher level code (outside of the DWARF expression evaluation
code) to fix things up for us.  This is what this patch aims to
address.

When creating a dynamic property (see attr_to_dynamic_prop in
dwarf2read.c) we can sometimes figure out the type of a property; if
the property is a reference to another DIE then it will have a
DW_AT_type attribute.

However, the DW_AT_lower_bound case above isn't a reference to another
DIE, it's just a DWARF expression.  We don't have any indication for
what type the property should have.

Luckily, the DWARF spec helps us out, for the lower and upper bounds
5.13 of the DWARFv5 spec tells us that without any other type
information the bounds are signed integers the same size as a DWARF
address.

It is my belief that we can find a suitable default type for every
dynamic property, either specified explicitly in the DWARF spec, or we
can infer an obvious choice if the spec doesn't help us.

This commit extends the creation of all dynamic properties to include
suggesting a suitable default type, all dynamic properties now always
carry their type around with them.

In later commits we can use this property type to ensure that the
value we extract from the DWARF stack is handled in a suitable manor
to correctly maintain its sign extension.

There should be no user visible changes from this commit.  The actual
fix to correctly support negative array bounds will come later.

gdb/ChangeLog:

	* dwarf2loc.c (dwarf2_evaluate_property): Update to take account
	of changes to field names, and use new is_reference field to
	decide if a property is a reference or not.
	* dwarf2loc.h (struct dwarf2_locexpr_baton): Add 'is_reference'
	field.
	(struct dwarf2_property_baton): Update header comment, rename
	'referenced_type' to 'property_type' and update comments.
	* dwarf2read.c (attr_to_dynamic_prop): Add extra parameter to hold
	default property type, store in property baton, update to take
	accound of renamed field.
	(read_func_scope): Update call to attr_to_dynamic_prop.
	(read_array_type): Likewise.
	(dwarf2_per_cu_addr_sized_int_type): New function.
	(read_subrange_index_type): Move type finding code to
	dwarf2_per_cu_addr_sized_int_type.
	(read_subrange_type): Update calls to attr_to_dynamic_prop.
	(dwarf2_per_cu_addr_type): New function.
	(set_die_type): Update calls to attr_to_dynamic_prop.
---
 gdb/ChangeLog    |  21 ++++++++
 gdb/dwarf2loc.c  |  10 ++--
 gdb/dwarf2loc.h  |  24 ++++++---
 gdb/dwarf2read.c | 150 +++++++++++++++++++++++++++++++++++++++++--------------
 4 files changed, 155 insertions(+), 50 deletions(-)

-- 
2.14.5

Comments

Tom Tromey May 22, 2019, 7:05 p.m. | #1
Andrew>    <1><af>: Abbrev Number: 12 (DW_TAG_array_type)
Andrew>       <b0>   DW_AT_data_location: 2 byte block: 97 6              (DW_OP_push_object_address; DW_OP_deref)
Andrew>       <b3>   DW_AT_allocated   : 4 byte block: 97 6 30 2e         (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)
Andrew>       <b8>   DW_AT_type        : <0x2a>
Andrew>    <2><bc>: Abbrev Number: 13 (DW_TAG_subrange_type)
Andrew>       <bd>   DW_AT_lower_bound : 4 byte block: 97 23 10 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 16; DW_OP_deref)
Andrew>       <c2>   DW_AT_upper_bound : 4 byte block: 97 23 14 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 20; DW_OP_deref)
Andrew>       <c7>   DW_AT_byte_stride : 6 byte block: 97 23 c 6 34 1e    (DW_OP_push_object_address; DW_OP_plus_uconst: 12; DW_OP_deref; DW_OP_lit4; DW_OP_mul)

This is a funny coincidence, because we were discussing a similar case
with Ada just this week.  Gnat can generate very similar expressions in
some cases:

 <1><11b5>: Abbrev Number: 8 (DW_TAG_array_type)
    <11b6>   DW_AT_name        : (indirect string, offset: 0x1ba2): string
    <11ba>   DW_AT_data_location: 2 byte block: 97 6 	(DW_OP_push_object_address; DW_OP_deref)
    <11bd>   DW_AT_type        : <0x113c>
    <11c1>   DW_AT_sibling     : <0x11db>
 <2><11c5>: Abbrev Number: 9 (DW_TAG_subrange_type)
    <11c6>   DW_AT_type        : <0x11e0>
    <11ca>   DW_AT_lower_bound : 6 byte block: 97 23 8 6 94 4 	(DW_OP_push_object_address; DW_OP_plus_uconst: 8; DW_OP_deref; DW_OP_deref_size: 4)
    <11d1>   DW_AT_upper_bound : 8 byte block: 97 23 8 6 23 4 94 4 	(DW_OP_push_object_address; DW_OP_plus_uconst: 8; DW_OP_deref; DW_OP_plus_uconst: 4; DW_OP_deref_size: 4)


Essentially what's going on here is that the compiler represents a
pointer-to-array as a structure that holds a pointer and the array
bounds; and then chooses to represent this in DWARF as the above.

This in turn causes problems in gdb.  In the above case, it's not
possible to make a call like `do_something("string")', because gdb
doesn't know that constructing one of these arrays at runtime actually
requires two allocations and some special messing around.

So, this week we've been talking about changing the representation of
these things to be more explicit, that is, make the hidden struct type
explicit (and marked DW_AT_artificial).

I wonder how Fortran will handle this, or whether it even needs to.  Is
it possible for gdb to construct one of these arrays at runtime?

Another option, besides changing the representation, would be to teach
gdb how to create one of the above by recognizing the expressions as a
kind of special case.  This seemed uglier and more fragile, though.

For Ada at least, this sort of DWARF isn't the default yet.  The default
is this Ada-specific name encodings scheme; you have to opt-in to DWARF
using a compiler command-line flag; but one of the things I'm looking
into is getting gdb to the point where we can flip the default.

Andrew> I wonder if the best solution for dealing with signed properties will
Andrew> be to move away from an over reliance on fetch_address, and instead
Andrew> come up with a new solution that considers the current type of the
Andrew> value on the stack, and the type that the value needs to become;
Andrew> basically a solution built around casting rather than assuming we
Andrew> always want an address.

DWARF now has typed expressions as well, so gcc could just emit this
explicitly, I think.  I didn't look at gcc's dwarf2out.c for this but
maybe you need -gdwarf-5 to make this happen.

Andrew> It is my belief that we can find a suitable default type for every
Andrew> dynamic property, either specified explicitly in the DWARF spec, or we
Andrew> can infer an obvious choice if the spec doesn't help us.

Seems reasonable.

Andrew> +static struct type *
Andrew> +dwarf2_per_cu_addr_sized_int_type (struct dwarf2_per_cu_data *per_cu)
Andrew> +{
Andrew> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;
Andrew> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);
Andrew> +  struct type *int_type;
Andrew> +
Andrew> +  int_type = objfile_type (objfile)->builtin_short;
Andrew> +  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)
Andrew> +    return int_type;

dwarf2_per_cu_addr_type handles unsigned char here, but this one does not.
I doubt it matters but I wonder why the difference.

Andrew> +/* Return a type that is a generic pointer type, the size of which matches
Andrew> +   the address size given in the compilation unit header for PER_CU.  */
Andrew> +static struct type *
Andrew> +dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu)
Andrew> +{
Andrew> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;
Andrew> +  struct type *void_type = objfile_type (objfile)->builtin_void;
Andrew> +  struct type *addr_type = lookup_pointer_type (void_type);
Andrew> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);
Andrew> +
Andrew> +  if (TYPE_LENGTH (addr_type) == addr_size)
Andrew> +    return addr_type;
Andrew> +
Andrew> +  /* Yuck! We currently only support one address size per architecture in
Andrew> +     GDB, which should usually match the address size encoded into the
Andrew> +     compilation unit header.  However... we have a few tests where this is
Andrew> +     not the case, these are mostly test cases where the DWARF is hand
Andrew> +     written and includes a fixed address size, for example 8-bytes.  When
Andrew> +     we compile these tests on a 32-bit i386 target the gdbarch address
Andrew> +     size is 4-bytes and the above attempt to create a suitable address
Andrew> +     type fails.
Andrew> +
Andrew> +     As we can't currently create an address type of a different size, we
Andrew> +     instead substitute an unsigned integer for an address.
Andrew> +
Andrew> +     I don't know if there are targets that have signed addresses and if
Andrew> +     they would need a signed integer here.  I figure we'll handle that
Andrew> +     case when it presents itself as a problem.  */

gdbarch.sh makes a distinction between addresses and pointers which, I
confess, I have never understood.  However, based on this, I think at
least MIPS may have signed pointers.  From mips-tdep.c:

  set_gdbarch_pointer_to_address (gdbarch, signed_pointer_to_address);
  set_gdbarch_address_to_pointer (gdbarch, address_to_signed_pointer);

Also, I wonder whether it wouldn't be simpler to just provide a way to
create a TYPE_CODE_PTR type with a specified size.

Tom
Andrew Burgess June 10, 2019, 10:29 p.m. | #2
* Tom Tromey <tom@tromey.com> [2019-05-22 15:05:48 -0400]:

> Andrew>    <1><af>: Abbrev Number: 12 (DW_TAG_array_type)

> Andrew>       <b0>   DW_AT_data_location: 2 byte block: 97 6              (DW_OP_push_object_address; DW_OP_deref)

> Andrew>       <b3>   DW_AT_allocated   : 4 byte block: 97 6 30 2e         (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)

> Andrew>       <b8>   DW_AT_type        : <0x2a>

> Andrew>    <2><bc>: Abbrev Number: 13 (DW_TAG_subrange_type)

> Andrew>       <bd>   DW_AT_lower_bound : 4 byte block: 97 23 10 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 16; DW_OP_deref)

> Andrew>       <c2>   DW_AT_upper_bound : 4 byte block: 97 23 14 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 20; DW_OP_deref)

> Andrew>       <c7>   DW_AT_byte_stride : 6 byte block: 97 23 c 6 34 1e    (DW_OP_push_object_address; DW_OP_plus_uconst: 12; DW_OP_deref; DW_OP_lit4; DW_OP_mul)

> 

> This is a funny coincidence, because we were discussing a similar case

> with Ada just this week.  Gnat can generate very similar expressions in

> some cases:

> 

>  <1><11b5>: Abbrev Number: 8 (DW_TAG_array_type)

>     <11b6>   DW_AT_name        : (indirect string, offset: 0x1ba2): string

>     <11ba>   DW_AT_data_location: 2 byte block: 97 6 	(DW_OP_push_object_address; DW_OP_deref)

>     <11bd>   DW_AT_type        : <0x113c>

>     <11c1>   DW_AT_sibling     : <0x11db>

>  <2><11c5>: Abbrev Number: 9 (DW_TAG_subrange_type)

>     <11c6>   DW_AT_type        : <0x11e0>

>     <11ca>   DW_AT_lower_bound : 6 byte block: 97 23 8 6 94 4 	(DW_OP_push_object_address; DW_OP_plus_uconst: 8; DW_OP_deref; DW_OP_deref_size: 4)

>     <11d1>   DW_AT_upper_bound : 8 byte block: 97 23 8 6 23 4 94 4 	(DW_OP_push_object_address; DW_OP_plus_uconst: 8; DW_OP_deref; DW_OP_plus_uconst: 4; DW_OP_deref_size: 4)

> 

> 

> Essentially what's going on here is that the compiler represents a

> pointer-to-array as a structure that holds a pointer and the array

> bounds; and then chooses to represent this in DWARF as the above.

> 

> This in turn causes problems in gdb.  In the above case, it's not

> possible to make a call like `do_something("string")', because gdb

> doesn't know that constructing one of these arrays at runtime actually

> requires two allocations and some special messing around.

> 

> So, this week we've been talking about changing the representation of

> these things to be more explicit, that is, make the hidden struct type

> explicit (and marked DW_AT_artificial).

> 

> I wonder how Fortran will handle this, or whether it even needs to.  Is

> it possible for gdb to construct one of these arrays at runtime?

> 

> Another option, besides changing the representation, would be to teach

> gdb how to create one of the above by recognizing the expressions as a

> kind of special case.  This seemed uglier and more fragile, though.

> 

> For Ada at least, this sort of DWARF isn't the default yet.  The default

> is this Ada-specific name encodings scheme; you have to opt-in to DWARF

> using a compiler command-line flag; but one of the things I'm looking

> into is getting gdb to the point where we can flip the default.

> 

> Andrew> I wonder if the best solution for dealing with signed properties will

> Andrew> be to move away from an over reliance on fetch_address, and instead

> Andrew> come up with a new solution that considers the current type of the

> Andrew> value on the stack, and the type that the value needs to become;

> Andrew> basically a solution built around casting rather than assuming we

> Andrew> always want an address.

> 

> DWARF now has typed expressions as well, so gcc could just emit this

> explicitly, I think.  I didn't look at gcc's dwarf2out.c for this but

> maybe you need -gdwarf-5 to make this happen.


I've seen the typed expressions, but my immediate focus is on
debugging GCC's default DWARF as that's what most folk seem to end up
debugging with.

> 

> Andrew> It is my belief that we can find a suitable default type for every

> Andrew> dynamic property, either specified explicitly in the DWARF spec, or we

> Andrew> can infer an obvious choice if the spec doesn't help us.

> 

> Seems reasonable.

> 

> Andrew> +static struct type *

> Andrew> +dwarf2_per_cu_addr_sized_int_type (struct dwarf2_per_cu_data *per_cu)

> Andrew> +{

> Andrew> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;

> Andrew> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);

> Andrew> +  struct type *int_type;

> Andrew> +

> Andrew> +  int_type = objfile_type (objfile)->builtin_short;

> Andrew> +  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)

> Andrew> +    return int_type;

> 

> dwarf2_per_cu_addr_type handles unsigned char here, but this one does not.

> I doubt it matters but I wonder why the difference.


In the places where this is used the DWARF standard specifies that the
property is signed, so there was no need to handle unsigned.

> 

> Andrew> +/* Return a type that is a generic pointer type, the size of which matches

> Andrew> +   the address size given in the compilation unit header for PER_CU.  */

> Andrew> +static struct type *

> Andrew> +dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu)

> Andrew> +{

> Andrew> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;

> Andrew> +  struct type *void_type = objfile_type (objfile)->builtin_void;

> Andrew> +  struct type *addr_type = lookup_pointer_type (void_type);

> Andrew> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);

> Andrew> +

> Andrew> +  if (TYPE_LENGTH (addr_type) == addr_size)

> Andrew> +    return addr_type;

> Andrew> +

> Andrew> +  /* Yuck! We currently only support one address size per architecture in

> Andrew> +     GDB, which should usually match the address size encoded into the

> Andrew> +     compilation unit header.  However... we have a few tests where this is

> Andrew> +     not the case, these are mostly test cases where the DWARF is hand

> Andrew> +     written and includes a fixed address size, for example 8-bytes.  When

> Andrew> +     we compile these tests on a 32-bit i386 target the gdbarch address

> Andrew> +     size is 4-bytes and the above attempt to create a suitable address

> Andrew> +     type fails.

> Andrew> +

> Andrew> +     As we can't currently create an address type of a different size, we

> Andrew> +     instead substitute an unsigned integer for an address.

> Andrew> +

> Andrew> +     I don't know if there are targets that have signed addresses and if

> Andrew> +     they would need a signed integer here.  I figure we'll handle that

> Andrew> +     case when it presents itself as a problem.  */

> 

> gdbarch.sh makes a distinction between addresses and pointers which, I

> confess, I have never understood.  However, based on this, I think at

> least MIPS may have signed pointers.  From mips-tdep.c:

> 

>   set_gdbarch_pointer_to_address (gdbarch, signed_pointer_to_address);

>   set_gdbarch_address_to_pointer (gdbarch, address_to_signed_pointer);


I've extended dwarf2_per_cu_addr_sized_int_type to create signed or
unsigned types, and made dwarf2_per_cu_addr_type use this, taking the
sign of the default 'void *' type into account.  This should mean that
MIPS will do the right thing now.

> 

> Also, I wonder whether it wouldn't be simpler to just provide a way to

> create a TYPE_CODE_PTR type with a specified size.


I wondered about that, but I was worried that it might open a huge can
of worms that I'd rather not open.  If we did add multiple pointer
sizes later then it's easy to just change dwarf2_per_cu_addr_type to
do the right thing I think.

How would you feel with this version?

Thanks,
Andrew

--

commit 191bf42d636a33c374dbe0b65eb53b7950e49f48
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed May 8 13:16:03 2019 +0100

    gdb: Carry default property type around with dynamic properties
    
    This commit is preparation for the next one, with the aim of better
    supporting signed dynamic properties on targets where the address size
    specified in the DWARF headers is smaller than a CORE_ADDR, for
    example debugging an i386 application on x86-64.
    
    Consider this small Fortran program 'bounds.f90':
    
        program test
          integer, allocatable :: array (:)
          allocate (array (-5:5))
          array(3) = 1
        end program test
    
    Compiled with 'gfortran -m32 -g3 -O0 -o bounds bounds.f90'.  The DWARF
    for 'array' looks like this:
    
       <2><97>: Abbrev Number: 10 (DW_TAG_variable)
          <98>   DW_AT_name        : (indirect string, offset: 0x0): array
          <9c>   DW_AT_decl_file   : 1
          <9d>   DW_AT_decl_line   : 2
          <9e>   DW_AT_type        : <0xaf>
          <a2>   DW_AT_location    : 2 byte block: 91 58              (DW_OP_fbreg: -40)
       <2><a5>: Abbrev Number: 11 (DW_TAG_lexical_block)
          <a6>   DW_AT_low_pc      : 0x80485c3
          <aa>   DW_AT_high_pc     : 0x8b
       <2><ae>: Abbrev Number: 0
       <1><af>: Abbrev Number: 12 (DW_TAG_array_type)
          <b0>   DW_AT_data_location: 2 byte block: 97 6              (DW_OP_push_object_address; DW_OP_deref)
          <b3>   DW_AT_allocated   : 4 byte block: 97 6 30 2e         (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)
          <b8>   DW_AT_type        : <0x2a>
       <2><bc>: Abbrev Number: 13 (DW_TAG_subrange_type)
          <bd>   DW_AT_lower_bound : 4 byte block: 97 23 10 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 16; DW_OP_deref)
          <c2>   DW_AT_upper_bound : 4 byte block: 97 23 14 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 20; DW_OP_deref)
          <c7>   DW_AT_byte_stride : 6 byte block: 97 23 c 6 34 1e    (DW_OP_push_object_address; DW_OP_plus_uconst: 12; DW_OP_deref; DW_OP_lit4; DW_OP_mul)
       <2><ce>: Abbrev Number: 0
    
    If we look at the DW_AT_lower_bound attribute, which will become a
    dynamic property that GDB evaluates when needed by calling
    dwarf2_evaluate_property.
    
    The process of evaluating a dynamic property requires GDB to execute
    each DW_OP_* operation, the results of these operations is held on a
    stack of 'struct value *'s.
    
    When the entire expression is evaluated the result is on top of the
    stack.
    
    If we look at DW_AT_lower_bound then the last operation is
    DW_OP_deref, this loads a signed address the size of which matches the
    DWARF address size, and so in our i386 on x86-64 situation, the top of
    the stack will be a signed 4-byte value.
    
    The problem is how these values are fetched from the stack.  Currently
    they are always fetched by a call to dwarf_expr_context::fetch_address,
    which converts the value to an unsigned value with a length matching
    the values current length, before converting to a CORE_ADDR.  This
    means we loose the signed nature of the property.
    
    I wonder if the best solution for dealing with signed properties will
    be to move away from an over reliance on fetch_address, and instead
    come up with a new solution that considers the current type of the
    value on the stack, and the type that the value needs to become;
    basically a solution built around casting rather than assuming we
    always want an address.
    
    However, before we can start to even think about moving away from
    fetch_address, there is a more urgent issue to fix, which is we don't
    currently know what type each property should be.  We just hold the
    value of the property in a CORE_ADDR as returned by fetch_address, and
    rely on higher level code (outside of the DWARF expression evaluation
    code) to fix things up for us.  This is what this patch aims to
    address.
    
    When creating a dynamic property (see attr_to_dynamic_prop in
    dwarf2read.c) we can sometimes figure out the type of a property; if
    the property is a reference to another DIE then it will have a
    DW_AT_type attribute.
    
    However, the DW_AT_lower_bound case above isn't a reference to another
    DIE, it's just a DWARF expression.  We don't have any indication for
    what type the property should have.
    
    Luckily, the DWARF spec helps us out, for the lower and upper bounds
    5.13 of the DWARFv5 spec tells us that without any other type
    information the bounds are signed integers the same size as a DWARF
    address.
    
    It is my belief that we can find a suitable default type for every
    dynamic property, either specified explicitly in the DWARF spec, or we
    can infer an obvious choice if the spec doesn't help us.
    
    This commit extends the creation of all dynamic properties to include
    suggesting a suitable default type, all dynamic properties now always
    carry their type around with them.
    
    In later commits we can use this property type to ensure that the
    value we extract from the DWARF stack is handled in a suitable manor
    to correctly maintain its sign extension.
    
    There should be no user visible changes from this commit.  The actual
    fix to correctly support negative array bounds will come later.
    
    gdb/ChangeLog:
    
            * dwarf2loc.c (dwarf2_evaluate_property): Update to take account
            of changes to field names, and use new is_reference field to
            decide if a property is a reference or not.
            * dwarf2loc.h (struct dwarf2_locexpr_baton): Add 'is_reference'
            field.
            (struct dwarf2_property_baton): Update header comment, rename
            'referenced_type' to 'property_type' and update comments.
            * dwarf2read.c (attr_to_dynamic_prop): Add extra parameter to hold
            default property type, store in property baton, update to take
            accound of renamed field.
            (read_func_scope): Update call to attr_to_dynamic_prop.
            (read_array_type): Likewise.
            (dwarf2_per_cu_addr_sized_int_type): New function.
            (read_subrange_index_type): Move type finding code to
            dwarf2_per_cu_addr_sized_int_type.
            (read_subrange_type): Update calls to attr_to_dynamic_prop.
            (dwarf2_per_cu_addr_type): New function.
            (set_die_type): Update calls to attr_to_dynamic_prop.

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index c03f261f1e1..88d34eb8660 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2443,15 +2443,15 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
       {
 	const struct dwarf2_property_baton *baton
 	  = (const struct dwarf2_property_baton *) prop->data.baton;
+	gdb_assert (baton->property_type != NULL);
 
 	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
 				       addr_stack ? addr_stack->addr : 0,
 				       value))
 	  {
-	    if (baton->referenced_type)
+	    if (baton->locexpr.is_reference)
 	      {
-		struct value *val = value_at (baton->referenced_type, *value);
-
+		struct value *val = value_at (baton->property_type, *value);
 		*value = value_as_address (val);
 	      }
 	    return true;
@@ -2471,7 +2471,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	data = dwarf2_find_location_expression (&baton->loclist, &size, pc);
 	if (data != NULL)
 	  {
-	    val = dwarf2_evaluate_loc_desc (baton->referenced_type, frame, data,
+	    val = dwarf2_evaluate_loc_desc (baton->property_type, frame, data,
 					    size, baton->loclist.per_cu);
 	    if (!value_optimized_out (val))
 	      {
@@ -2497,7 +2497,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	  {
 	    /* This approach lets us avoid checking the qualifiers.  */
 	    if (TYPE_MAIN_TYPE (pinfo->type)
-		== TYPE_MAIN_TYPE (baton->referenced_type))
+		== TYPE_MAIN_TYPE (baton->property_type))
 	      break;
 	  }
 	if (pinfo == NULL)
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index ac1a771a9f3..baa5762003d 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -183,6 +183,12 @@ struct dwarf2_locexpr_baton
      zero.  */
   size_t size;
 
+  /* When true this location expression is a reference and actually
+     describes the address at which the value of the attribute can be
+     found.  When false the expression provides the value of the attribute
+     directly.  */
+  bool is_reference;
+
   /* The compilation unit containing the symbol whose location
      we're computing.  */
   struct dwarf2_per_cu_data *per_cu;
@@ -228,23 +234,27 @@ struct dwarf2_offset_baton
 
 /* A dynamic property is either expressed as a single location expression
    or a location list.  If the property is an indirection, pointing to
-   another die, keep track of the targeted type in REFERENCED_TYPE.  */
+   another die, keep track of the targeted type in PROPERTY_TYPE.
+   Alternatively, if the property location gives the property value
+   directly then it will have PROPERTY_TYPE.  */
 
 struct dwarf2_property_baton
 {
   /* If the property is an indirection, we need to evaluate the location
-     in the context of the type REFERENCED_TYPE.
-     If NULL, the location is the actual value of the property.  */
-  struct type *referenced_type;
+     in the context of the type PROPERTY_TYPE.  If the property is supplied
+     by value then it will be of PROPERTY_TYPE.  This field should never be
+     NULL.  */
+  struct type *property_type;
   union
   {
-    /* Location expression.  */
+    /* Location expression either evaluated in the context of
+       PROPERTY_TYPE, or a value of type PROPERTY_TYPE.  */
     struct dwarf2_locexpr_baton locexpr;
 
-    /* Location list to be evaluated in the context of REFERENCED_TYPE.  */
+    /* Location list to be evaluated in the context of PROPERTY_TYPE.  */
     struct dwarf2_loclist_baton loclist;
 
-    /* The location is an offset to REFERENCED_TYPE.  */
+    /* The location is an offset to PROPERTY_TYPE.  */
     struct dwarf2_offset_baton offset_info;
   };
 };
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index fc3d888fadc..e8d1e3ced18 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1833,7 +1833,7 @@ static void read_signatured_type (struct signatured_type *);
 
 static int attr_to_dynamic_prop (const struct attribute *attr,
 				 struct die_info *die, struct dwarf2_cu *cu,
-				 struct dynamic_prop *prop);
+				 struct dynamic_prop *prop, struct type *type);
 
 /* memory allocation interface */
 
@@ -1913,6 +1913,10 @@ static void queue_comp_unit (struct dwarf2_per_cu_data *per_cu,
 
 static void process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile);
 
+static struct type *dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu);
+static struct type *dwarf2_per_cu_addr_sized_int_type
+	(struct dwarf2_per_cu_data *per_cu, bool unsigned_p);
+
 /* Class, the destructor of which frees all allocated queue entries.  This
    will only have work to do if an error was thrown while processing the
    dwarf.  If no error was thrown then the queue entries should have all
@@ -13759,7 +13763,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
     {
       newobj->static_link
 	= XOBNEW (&objfile->objfile_obstack, struct dynamic_prop);
-      attr_to_dynamic_prop (attr, die, cu, newobj->static_link);
+      attr_to_dynamic_prop (attr, die, cu, newobj->static_link,
+			    dwarf2_per_cu_addr_type (cu->per_cu));
     }
 
   cu->list_in_scope = cu->get_builder ()->get_local_symbols ();
@@ -16515,10 +16520,13 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   if (attr != NULL)
     {
       int stride_ok;
+      struct type *prop_type
+	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
 
       byte_stride_prop
 	= (struct dynamic_prop *) alloca (sizeof (struct dynamic_prop));
-      stride_ok = attr_to_dynamic_prop (attr, die, cu, byte_stride_prop);
+      stride_ok = attr_to_dynamic_prop (attr, die, cu, byte_stride_prop,
+					prop_type);
       if (!stride_ok)
 	{
 	  complaint (_("unable to read array DW_AT_byte_stride "
@@ -17733,22 +17741,26 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 
 static int
 attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
-		      struct dwarf2_cu *cu, struct dynamic_prop *prop)
+		      struct dwarf2_cu *cu, struct dynamic_prop *prop,
+		      struct type *default_type)
 {
   struct dwarf2_property_baton *baton;
   struct obstack *obstack
     = &cu->per_cu->dwarf2_per_objfile->objfile->objfile_obstack;
 
+  gdb_assert (default_type != NULL);
+
   if (attr == NULL || prop == NULL)
     return 0;
 
   if (attr_form_is_block (attr))
     {
       baton = XOBNEW (obstack, struct dwarf2_property_baton);
-      baton->referenced_type = NULL;
+      baton->property_type = default_type;
       baton->locexpr.per_cu = cu->per_cu;
       baton->locexpr.size = DW_BLOCK (attr)->size;
       baton->locexpr.data = DW_BLOCK (attr)->data;
+      baton->locexpr.is_reference = false;
       prop->data.baton = baton;
       prop->kind = PROP_LOCEXPR;
       gdb_assert (prop->data.baton != NULL);
@@ -17773,7 +17785,7 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 	    if (attr_form_is_section_offset (target_attr))
 	      {
 		baton = XOBNEW (obstack, struct dwarf2_property_baton);
-		baton->referenced_type = die_type (target_die, target_cu);
+		baton->property_type = die_type (target_die, target_cu);
 		fill_in_loclist_baton (cu, &baton->loclist, target_attr);
 		prop->data.baton = baton;
 		prop->kind = PROP_LOCLIST;
@@ -17782,10 +17794,11 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 	    else if (attr_form_is_block (target_attr))
 	      {
 		baton = XOBNEW (obstack, struct dwarf2_property_baton);
-		baton->referenced_type = die_type (target_die, target_cu);
+		baton->property_type = die_type (target_die, target_cu);
 		baton->locexpr.per_cu = cu->per_cu;
 		baton->locexpr.size = DW_BLOCK (target_attr)->size;
 		baton->locexpr.data = DW_BLOCK (target_attr)->data;
+		baton->locexpr.is_reference = true;
 		prop->data.baton = baton;
 		prop->kind = PROP_LOCEXPR;
 		gdb_assert (prop->data.baton != NULL);
@@ -17806,7 +17819,7 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 		return 0;
 
 	      baton = XOBNEW (obstack, struct dwarf2_property_baton);
-	      baton->referenced_type = read_type_die (target_die->parent,
+	      baton->property_type = read_type_die (target_die->parent,
 						      target_cu);
 	      baton->offset_info.offset = offset;
 	      baton->offset_info.type = die_type (target_die, target_cu);
@@ -17831,6 +17844,37 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
   return 1;
 }
 
+/* Find an integer type the same size as the address size given in the
+   compilation unit header for PER_CU.  UNSIGNED_P controls if the integer
+   is unsigned or not.  */
+
+static struct type *
+dwarf2_per_cu_addr_sized_int_type (struct dwarf2_per_cu_data *per_cu,
+				   bool unsigned_p)
+{
+  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;
+  int addr_size = dwarf2_per_cu_addr_size (per_cu);
+  struct type *int_type;
+
+  /* Helper macro to examine the various builtin types.  */
+#define TRY_TYPE(F)						\
+  int_type = (unsigned_p					\
+	      ? objfile_type (objfile)->builtin_unsigned_ ## F	\
+	      : objfile_type (objfile)->builtin_ ## F);		\
+  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)	\
+    return int_type
+
+  TRY_TYPE (char);
+  TRY_TYPE (short);
+  TRY_TYPE (int);
+  TRY_TYPE (long);
+  TRY_TYPE (long_long);
+
+#undef TRY_TYPE
+
+  gdb_assert_not_reached ("unable to find suitable integer type");
+}
+
 /* Read the DW_AT_type attribute for a sub-range.  If this attribute is not
    present (which is valid) then compute the default type based on the
    compilation units address size.  */
@@ -17853,30 +17897,7 @@ read_subrange_index_type (struct die_info *die, struct dwarf2_cu *cu)
      FIXME: muller/2010-05-28: Possible references to object for low bound,
      high bound or count are not yet handled by this code.  */
   if (TYPE_CODE (index_type) == TYPE_CODE_VOID)
-    {
-      struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
-      struct gdbarch *gdbarch = get_objfile_arch (objfile);
-      int addr_size = gdbarch_addr_bit (gdbarch) /8;
-      struct type *int_type = objfile_type (objfile)->builtin_int;
-
-      /* Test "int", "long int", and "long long int" objfile types,
-	 and select the first one having a size above or equal to the
-	 architecture address size.  */
-      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
-	index_type = int_type;
-      else
-	{
-	  int_type = objfile_type (objfile)->builtin_long;
-	  if (int_type && TYPE_LENGTH (int_type) >= addr_size)
-	    index_type = int_type;
-	  else
-	    {
-	      int_type = objfile_type (objfile)->builtin_long_long;
-	      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
-		index_type = int_type;
-	    }
-	}
-    }
+    index_type = dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
 
   return index_type;
 }
@@ -17945,7 +17966,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 
   attr = dwarf2_attr (die, DW_AT_lower_bound, cu);
   if (attr)
-    attr_to_dynamic_prop (attr, die, cu, &low);
+    attr_to_dynamic_prop (attr, die, cu, &low, base_type);
   else if (!low_default_is_valid)
     complaint (_("Missing DW_AT_lower_bound "
 				      "- DIE at %s [in module %s]"),
@@ -17954,10 +17975,10 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 
   struct attribute *attr_ub, *attr_count;
   attr = attr_ub = dwarf2_attr (die, DW_AT_upper_bound, cu);
-  if (!attr_to_dynamic_prop (attr, die, cu, &high))
+  if (!attr_to_dynamic_prop (attr, die, cu, &high, base_type))
     {
       attr = attr_count = dwarf2_attr (die, DW_AT_count, cu);
-      if (attr_to_dynamic_prop (attr, die, cu, &high))
+      if (attr_to_dynamic_prop (attr, die, cu, &high, base_type))
 	{
 	  /* If bounds are constant do the final calculation here.  */
 	  if (low.kind == PROP_CONST && high.kind == PROP_CONST)
@@ -25310,6 +25331,24 @@ dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *per_cu)
   return ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 }
 
+/* Return a type that is a generic pointer type, the size of which matches
+   the address size given in the compilation unit header for PER_CU.  */
+static struct type *
+dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu)
+{
+  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;
+  struct type *void_type = objfile_type (objfile)->builtin_void;
+  struct type *addr_type = lookup_pointer_type (void_type);
+  int addr_size = dwarf2_per_cu_addr_size (per_cu);
+
+  if (TYPE_LENGTH (addr_type) == addr_size)
+    return addr_type;
+
+  addr_type
+    = dwarf2_per_cu_addr_sized_int_type (per_cu, TYPE_UNSIGNED (addr_type));
+  return addr_type;
+}
+
 /* Return DWARF version number of PER_CU.  */
 
 short
@@ -25576,7 +25615,9 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_allocated, cu);
   if (attr_form_is_block (attr))
     {
-      if (attr_to_dynamic_prop (attr, die, cu, &prop))
+      struct type *prop_type
+	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
+      if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))
         add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);
     }
   else if (attr != NULL)
@@ -25590,7 +25631,9 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_associated, cu);
   if (attr_form_is_block (attr))
     {
-      if (attr_to_dynamic_prop (attr, die, cu, &prop))
+      struct type *prop_type
+	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);
+      if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))
         add_dyn_prop (DYN_PROP_ASSOCIATED, prop, type);
     }
   else if (attr != NULL)
@@ -25602,7 +25645,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
-  if (attr_to_dynamic_prop (attr, die, cu, &prop))
+  if (attr_to_dynamic_prop (attr, die, cu, &prop,
+			    dwarf2_per_cu_addr_type (cu->per_cu)))
     add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
Andrew Burgess July 10, 2019, 2:13 p.m. | #3
Ping!

Was there any additional feedback on this series?

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2019-06-10 23:29:32 +0100]:

> * Tom Tromey <tom@tromey.com> [2019-05-22 15:05:48 -0400]:

> 

> > Andrew>    <1><af>: Abbrev Number: 12 (DW_TAG_array_type)

> > Andrew>       <b0>   DW_AT_data_location: 2 byte block: 97 6              (DW_OP_push_object_address; DW_OP_deref)

> > Andrew>       <b3>   DW_AT_allocated   : 4 byte block: 97 6 30 2e         (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)

> > Andrew>       <b8>   DW_AT_type        : <0x2a>

> > Andrew>    <2><bc>: Abbrev Number: 13 (DW_TAG_subrange_type)

> > Andrew>       <bd>   DW_AT_lower_bound : 4 byte block: 97 23 10 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 16; DW_OP_deref)

> > Andrew>       <c2>   DW_AT_upper_bound : 4 byte block: 97 23 14 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 20; DW_OP_deref)

> > Andrew>       <c7>   DW_AT_byte_stride : 6 byte block: 97 23 c 6 34 1e    (DW_OP_push_object_address; DW_OP_plus_uconst: 12; DW_OP_deref; DW_OP_lit4; DW_OP_mul)

> > 

> > This is a funny coincidence, because we were discussing a similar case

> > with Ada just this week.  Gnat can generate very similar expressions in

> > some cases:

> > 

> >  <1><11b5>: Abbrev Number: 8 (DW_TAG_array_type)

> >     <11b6>   DW_AT_name        : (indirect string, offset: 0x1ba2): string

> >     <11ba>   DW_AT_data_location: 2 byte block: 97 6 	(DW_OP_push_object_address; DW_OP_deref)

> >     <11bd>   DW_AT_type        : <0x113c>

> >     <11c1>   DW_AT_sibling     : <0x11db>

> >  <2><11c5>: Abbrev Number: 9 (DW_TAG_subrange_type)

> >     <11c6>   DW_AT_type        : <0x11e0>

> >     <11ca>   DW_AT_lower_bound : 6 byte block: 97 23 8 6 94 4 	(DW_OP_push_object_address; DW_OP_plus_uconst: 8; DW_OP_deref; DW_OP_deref_size: 4)

> >     <11d1>   DW_AT_upper_bound : 8 byte block: 97 23 8 6 23 4 94 4 	(DW_OP_push_object_address; DW_OP_plus_uconst: 8; DW_OP_deref; DW_OP_plus_uconst: 4; DW_OP_deref_size: 4)

> > 

> > 

> > Essentially what's going on here is that the compiler represents a

> > pointer-to-array as a structure that holds a pointer and the array

> > bounds; and then chooses to represent this in DWARF as the above.

> > 

> > This in turn causes problems in gdb.  In the above case, it's not

> > possible to make a call like `do_something("string")', because gdb

> > doesn't know that constructing one of these arrays at runtime actually

> > requires two allocations and some special messing around.

> > 

> > So, this week we've been talking about changing the representation of

> > these things to be more explicit, that is, make the hidden struct type

> > explicit (and marked DW_AT_artificial).

> > 

> > I wonder how Fortran will handle this, or whether it even needs to.  Is

> > it possible for gdb to construct one of these arrays at runtime?

> > 

> > Another option, besides changing the representation, would be to teach

> > gdb how to create one of the above by recognizing the expressions as a

> > kind of special case.  This seemed uglier and more fragile, though.

> > 

> > For Ada at least, this sort of DWARF isn't the default yet.  The default

> > is this Ada-specific name encodings scheme; you have to opt-in to DWARF

> > using a compiler command-line flag; but one of the things I'm looking

> > into is getting gdb to the point where we can flip the default.

> > 

> > Andrew> I wonder if the best solution for dealing with signed properties will

> > Andrew> be to move away from an over reliance on fetch_address, and instead

> > Andrew> come up with a new solution that considers the current type of the

> > Andrew> value on the stack, and the type that the value needs to become;

> > Andrew> basically a solution built around casting rather than assuming we

> > Andrew> always want an address.

> > 

> > DWARF now has typed expressions as well, so gcc could just emit this

> > explicitly, I think.  I didn't look at gcc's dwarf2out.c for this but

> > maybe you need -gdwarf-5 to make this happen.

> 

> I've seen the typed expressions, but my immediate focus is on

> debugging GCC's default DWARF as that's what most folk seem to end up

> debugging with.

> 

> > 

> > Andrew> It is my belief that we can find a suitable default type for every

> > Andrew> dynamic property, either specified explicitly in the DWARF spec, or we

> > Andrew> can infer an obvious choice if the spec doesn't help us.

> > 

> > Seems reasonable.

> > 

> > Andrew> +static struct type *

> > Andrew> +dwarf2_per_cu_addr_sized_int_type (struct dwarf2_per_cu_data *per_cu)

> > Andrew> +{

> > Andrew> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;

> > Andrew> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);

> > Andrew> +  struct type *int_type;

> > Andrew> +

> > Andrew> +  int_type = objfile_type (objfile)->builtin_short;

> > Andrew> +  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)

> > Andrew> +    return int_type;

> > 

> > dwarf2_per_cu_addr_type handles unsigned char here, but this one does not.

> > I doubt it matters but I wonder why the difference.

> 

> In the places where this is used the DWARF standard specifies that the

> property is signed, so there was no need to handle unsigned.

> 

> > 

> > Andrew> +/* Return a type that is a generic pointer type, the size of which matches

> > Andrew> +   the address size given in the compilation unit header for PER_CU.  */

> > Andrew> +static struct type *

> > Andrew> +dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu)

> > Andrew> +{

> > Andrew> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;

> > Andrew> +  struct type *void_type = objfile_type (objfile)->builtin_void;

> > Andrew> +  struct type *addr_type = lookup_pointer_type (void_type);

> > Andrew> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);

> > Andrew> +

> > Andrew> +  if (TYPE_LENGTH (addr_type) == addr_size)

> > Andrew> +    return addr_type;

> > Andrew> +

> > Andrew> +  /* Yuck! We currently only support one address size per architecture in

> > Andrew> +     GDB, which should usually match the address size encoded into the

> > Andrew> +     compilation unit header.  However... we have a few tests where this is

> > Andrew> +     not the case, these are mostly test cases where the DWARF is hand

> > Andrew> +     written and includes a fixed address size, for example 8-bytes.  When

> > Andrew> +     we compile these tests on a 32-bit i386 target the gdbarch address

> > Andrew> +     size is 4-bytes and the above attempt to create a suitable address

> > Andrew> +     type fails.

> > Andrew> +

> > Andrew> +     As we can't currently create an address type of a different size, we

> > Andrew> +     instead substitute an unsigned integer for an address.

> > Andrew> +

> > Andrew> +     I don't know if there are targets that have signed addresses and if

> > Andrew> +     they would need a signed integer here.  I figure we'll handle that

> > Andrew> +     case when it presents itself as a problem.  */

> > 

> > gdbarch.sh makes a distinction between addresses and pointers which, I

> > confess, I have never understood.  However, based on this, I think at

> > least MIPS may have signed pointers.  From mips-tdep.c:

> > 

> >   set_gdbarch_pointer_to_address (gdbarch, signed_pointer_to_address);

> >   set_gdbarch_address_to_pointer (gdbarch, address_to_signed_pointer);

> 

> I've extended dwarf2_per_cu_addr_sized_int_type to create signed or

> unsigned types, and made dwarf2_per_cu_addr_type use this, taking the

> sign of the default 'void *' type into account.  This should mean that

> MIPS will do the right thing now.

> 

> > 

> > Also, I wonder whether it wouldn't be simpler to just provide a way to

> > create a TYPE_CODE_PTR type with a specified size.

> 

> I wondered about that, but I was worried that it might open a huge can

> of worms that I'd rather not open.  If we did add multiple pointer

> sizes later then it's easy to just change dwarf2_per_cu_addr_type to

> do the right thing I think.

> 

> How would you feel with this version?

> 

> Thanks,

> Andrew

> 

> --

> 

> commit 191bf42d636a33c374dbe0b65eb53b7950e49f48

> Author: Andrew Burgess <andrew.burgess@embecosm.com>

> Date:   Wed May 8 13:16:03 2019 +0100

> 

>     gdb: Carry default property type around with dynamic properties

>     

>     This commit is preparation for the next one, with the aim of better

>     supporting signed dynamic properties on targets where the address size

>     specified in the DWARF headers is smaller than a CORE_ADDR, for

>     example debugging an i386 application on x86-64.

>     

>     Consider this small Fortran program 'bounds.f90':

>     

>         program test

>           integer, allocatable :: array (:)

>           allocate (array (-5:5))

>           array(3) = 1

>         end program test

>     

>     Compiled with 'gfortran -m32 -g3 -O0 -o bounds bounds.f90'.  The DWARF

>     for 'array' looks like this:

>     

>        <2><97>: Abbrev Number: 10 (DW_TAG_variable)

>           <98>   DW_AT_name        : (indirect string, offset: 0x0): array

>           <9c>   DW_AT_decl_file   : 1

>           <9d>   DW_AT_decl_line   : 2

>           <9e>   DW_AT_type        : <0xaf>

>           <a2>   DW_AT_location    : 2 byte block: 91 58              (DW_OP_fbreg: -40)

>        <2><a5>: Abbrev Number: 11 (DW_TAG_lexical_block)

>           <a6>   DW_AT_low_pc      : 0x80485c3

>           <aa>   DW_AT_high_pc     : 0x8b

>        <2><ae>: Abbrev Number: 0

>        <1><af>: Abbrev Number: 12 (DW_TAG_array_type)

>           <b0>   DW_AT_data_location: 2 byte block: 97 6              (DW_OP_push_object_address; DW_OP_deref)

>           <b3>   DW_AT_allocated   : 4 byte block: 97 6 30 2e         (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne)

>           <b8>   DW_AT_type        : <0x2a>

>        <2><bc>: Abbrev Number: 13 (DW_TAG_subrange_type)

>           <bd>   DW_AT_lower_bound : 4 byte block: 97 23 10 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 16; DW_OP_deref)

>           <c2>   DW_AT_upper_bound : 4 byte block: 97 23 14 6         (DW_OP_push_object_address; DW_OP_plus_uconst: 20; DW_OP_deref)

>           <c7>   DW_AT_byte_stride : 6 byte block: 97 23 c 6 34 1e    (DW_OP_push_object_address; DW_OP_plus_uconst: 12; DW_OP_deref; DW_OP_lit4; DW_OP_mul)

>        <2><ce>: Abbrev Number: 0

>     

>     If we look at the DW_AT_lower_bound attribute, which will become a

>     dynamic property that GDB evaluates when needed by calling

>     dwarf2_evaluate_property.

>     

>     The process of evaluating a dynamic property requires GDB to execute

>     each DW_OP_* operation, the results of these operations is held on a

>     stack of 'struct value *'s.

>     

>     When the entire expression is evaluated the result is on top of the

>     stack.

>     

>     If we look at DW_AT_lower_bound then the last operation is

>     DW_OP_deref, this loads a signed address the size of which matches the

>     DWARF address size, and so in our i386 on x86-64 situation, the top of

>     the stack will be a signed 4-byte value.

>     

>     The problem is how these values are fetched from the stack.  Currently

>     they are always fetched by a call to dwarf_expr_context::fetch_address,

>     which converts the value to an unsigned value with a length matching

>     the values current length, before converting to a CORE_ADDR.  This

>     means we loose the signed nature of the property.

>     

>     I wonder if the best solution for dealing with signed properties will

>     be to move away from an over reliance on fetch_address, and instead

>     come up with a new solution that considers the current type of the

>     value on the stack, and the type that the value needs to become;

>     basically a solution built around casting rather than assuming we

>     always want an address.

>     

>     However, before we can start to even think about moving away from

>     fetch_address, there is a more urgent issue to fix, which is we don't

>     currently know what type each property should be.  We just hold the

>     value of the property in a CORE_ADDR as returned by fetch_address, and

>     rely on higher level code (outside of the DWARF expression evaluation

>     code) to fix things up for us.  This is what this patch aims to

>     address.

>     

>     When creating a dynamic property (see attr_to_dynamic_prop in

>     dwarf2read.c) we can sometimes figure out the type of a property; if

>     the property is a reference to another DIE then it will have a

>     DW_AT_type attribute.

>     

>     However, the DW_AT_lower_bound case above isn't a reference to another

>     DIE, it's just a DWARF expression.  We don't have any indication for

>     what type the property should have.

>     

>     Luckily, the DWARF spec helps us out, for the lower and upper bounds

>     5.13 of the DWARFv5 spec tells us that without any other type

>     information the bounds are signed integers the same size as a DWARF

>     address.

>     

>     It is my belief that we can find a suitable default type for every

>     dynamic property, either specified explicitly in the DWARF spec, or we

>     can infer an obvious choice if the spec doesn't help us.

>     

>     This commit extends the creation of all dynamic properties to include

>     suggesting a suitable default type, all dynamic properties now always

>     carry their type around with them.

>     

>     In later commits we can use this property type to ensure that the

>     value we extract from the DWARF stack is handled in a suitable manor

>     to correctly maintain its sign extension.

>     

>     There should be no user visible changes from this commit.  The actual

>     fix to correctly support negative array bounds will come later.

>     

>     gdb/ChangeLog:

>     

>             * dwarf2loc.c (dwarf2_evaluate_property): Update to take account

>             of changes to field names, and use new is_reference field to

>             decide if a property is a reference or not.

>             * dwarf2loc.h (struct dwarf2_locexpr_baton): Add 'is_reference'

>             field.

>             (struct dwarf2_property_baton): Update header comment, rename

>             'referenced_type' to 'property_type' and update comments.

>             * dwarf2read.c (attr_to_dynamic_prop): Add extra parameter to hold

>             default property type, store in property baton, update to take

>             accound of renamed field.

>             (read_func_scope): Update call to attr_to_dynamic_prop.

>             (read_array_type): Likewise.

>             (dwarf2_per_cu_addr_sized_int_type): New function.

>             (read_subrange_index_type): Move type finding code to

>             dwarf2_per_cu_addr_sized_int_type.

>             (read_subrange_type): Update calls to attr_to_dynamic_prop.

>             (dwarf2_per_cu_addr_type): New function.

>             (set_die_type): Update calls to attr_to_dynamic_prop.

> 

> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c

> index c03f261f1e1..88d34eb8660 100644

> --- a/gdb/dwarf2loc.c

> +++ b/gdb/dwarf2loc.c

> @@ -2443,15 +2443,15 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,

>        {

>  	const struct dwarf2_property_baton *baton

>  	  = (const struct dwarf2_property_baton *) prop->data.baton;

> +	gdb_assert (baton->property_type != NULL);

>  

>  	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,

>  				       addr_stack ? addr_stack->addr : 0,

>  				       value))

>  	  {

> -	    if (baton->referenced_type)

> +	    if (baton->locexpr.is_reference)

>  	      {

> -		struct value *val = value_at (baton->referenced_type, *value);

> -

> +		struct value *val = value_at (baton->property_type, *value);

>  		*value = value_as_address (val);

>  	      }

>  	    return true;

> @@ -2471,7 +2471,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,

>  	data = dwarf2_find_location_expression (&baton->loclist, &size, pc);

>  	if (data != NULL)

>  	  {

> -	    val = dwarf2_evaluate_loc_desc (baton->referenced_type, frame, data,

> +	    val = dwarf2_evaluate_loc_desc (baton->property_type, frame, data,

>  					    size, baton->loclist.per_cu);

>  	    if (!value_optimized_out (val))

>  	      {

> @@ -2497,7 +2497,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,

>  	  {

>  	    /* This approach lets us avoid checking the qualifiers.  */

>  	    if (TYPE_MAIN_TYPE (pinfo->type)

> -		== TYPE_MAIN_TYPE (baton->referenced_type))

> +		== TYPE_MAIN_TYPE (baton->property_type))

>  	      break;

>  	  }

>  	if (pinfo == NULL)

> diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h

> index ac1a771a9f3..baa5762003d 100644

> --- a/gdb/dwarf2loc.h

> +++ b/gdb/dwarf2loc.h

> @@ -183,6 +183,12 @@ struct dwarf2_locexpr_baton

>       zero.  */

>    size_t size;

>  

> +  /* When true this location expression is a reference and actually

> +     describes the address at which the value of the attribute can be

> +     found.  When false the expression provides the value of the attribute

> +     directly.  */

> +  bool is_reference;

> +

>    /* The compilation unit containing the symbol whose location

>       we're computing.  */

>    struct dwarf2_per_cu_data *per_cu;

> @@ -228,23 +234,27 @@ struct dwarf2_offset_baton

>  

>  /* A dynamic property is either expressed as a single location expression

>     or a location list.  If the property is an indirection, pointing to

> -   another die, keep track of the targeted type in REFERENCED_TYPE.  */

> +   another die, keep track of the targeted type in PROPERTY_TYPE.

> +   Alternatively, if the property location gives the property value

> +   directly then it will have PROPERTY_TYPE.  */

>  

>  struct dwarf2_property_baton

>  {

>    /* If the property is an indirection, we need to evaluate the location

> -     in the context of the type REFERENCED_TYPE.

> -     If NULL, the location is the actual value of the property.  */

> -  struct type *referenced_type;

> +     in the context of the type PROPERTY_TYPE.  If the property is supplied

> +     by value then it will be of PROPERTY_TYPE.  This field should never be

> +     NULL.  */

> +  struct type *property_type;

>    union

>    {

> -    /* Location expression.  */

> +    /* Location expression either evaluated in the context of

> +       PROPERTY_TYPE, or a value of type PROPERTY_TYPE.  */

>      struct dwarf2_locexpr_baton locexpr;

>  

> -    /* Location list to be evaluated in the context of REFERENCED_TYPE.  */

> +    /* Location list to be evaluated in the context of PROPERTY_TYPE.  */

>      struct dwarf2_loclist_baton loclist;

>  

> -    /* The location is an offset to REFERENCED_TYPE.  */

> +    /* The location is an offset to PROPERTY_TYPE.  */

>      struct dwarf2_offset_baton offset_info;

>    };

>  };

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c

> index fc3d888fadc..e8d1e3ced18 100644

> --- a/gdb/dwarf2read.c

> +++ b/gdb/dwarf2read.c

> @@ -1833,7 +1833,7 @@ static void read_signatured_type (struct signatured_type *);

>  

>  static int attr_to_dynamic_prop (const struct attribute *attr,

>  				 struct die_info *die, struct dwarf2_cu *cu,

> -				 struct dynamic_prop *prop);

> +				 struct dynamic_prop *prop, struct type *type);

>  

>  /* memory allocation interface */

>  

> @@ -1913,6 +1913,10 @@ static void queue_comp_unit (struct dwarf2_per_cu_data *per_cu,

>  

>  static void process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile);

>  

> +static struct type *dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu);

> +static struct type *dwarf2_per_cu_addr_sized_int_type

> +	(struct dwarf2_per_cu_data *per_cu, bool unsigned_p);

> +

>  /* Class, the destructor of which frees all allocated queue entries.  This

>     will only have work to do if an error was thrown while processing the

>     dwarf.  If no error was thrown then the queue entries should have all

> @@ -13759,7 +13763,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)

>      {

>        newobj->static_link

>  	= XOBNEW (&objfile->objfile_obstack, struct dynamic_prop);

> -      attr_to_dynamic_prop (attr, die, cu, newobj->static_link);

> +      attr_to_dynamic_prop (attr, die, cu, newobj->static_link,

> +			    dwarf2_per_cu_addr_type (cu->per_cu));

>      }

>  

>    cu->list_in_scope = cu->get_builder ()->get_local_symbols ();

> @@ -16515,10 +16520,13 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)

>    if (attr != NULL)

>      {

>        int stride_ok;

> +      struct type *prop_type

> +	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);

>  

>        byte_stride_prop

>  	= (struct dynamic_prop *) alloca (sizeof (struct dynamic_prop));

> -      stride_ok = attr_to_dynamic_prop (attr, die, cu, byte_stride_prop);

> +      stride_ok = attr_to_dynamic_prop (attr, die, cu, byte_stride_prop,

> +					prop_type);

>        if (!stride_ok)

>  	{

>  	  complaint (_("unable to read array DW_AT_byte_stride "

> @@ -17733,22 +17741,26 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)

>  

>  static int

>  attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,

> -		      struct dwarf2_cu *cu, struct dynamic_prop *prop)

> +		      struct dwarf2_cu *cu, struct dynamic_prop *prop,

> +		      struct type *default_type)

>  {

>    struct dwarf2_property_baton *baton;

>    struct obstack *obstack

>      = &cu->per_cu->dwarf2_per_objfile->objfile->objfile_obstack;

>  

> +  gdb_assert (default_type != NULL);

> +

>    if (attr == NULL || prop == NULL)

>      return 0;

>  

>    if (attr_form_is_block (attr))

>      {

>        baton = XOBNEW (obstack, struct dwarf2_property_baton);

> -      baton->referenced_type = NULL;

> +      baton->property_type = default_type;

>        baton->locexpr.per_cu = cu->per_cu;

>        baton->locexpr.size = DW_BLOCK (attr)->size;

>        baton->locexpr.data = DW_BLOCK (attr)->data;

> +      baton->locexpr.is_reference = false;

>        prop->data.baton = baton;

>        prop->kind = PROP_LOCEXPR;

>        gdb_assert (prop->data.baton != NULL);

> @@ -17773,7 +17785,7 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,

>  	    if (attr_form_is_section_offset (target_attr))

>  	      {

>  		baton = XOBNEW (obstack, struct dwarf2_property_baton);

> -		baton->referenced_type = die_type (target_die, target_cu);

> +		baton->property_type = die_type (target_die, target_cu);

>  		fill_in_loclist_baton (cu, &baton->loclist, target_attr);

>  		prop->data.baton = baton;

>  		prop->kind = PROP_LOCLIST;

> @@ -17782,10 +17794,11 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,

>  	    else if (attr_form_is_block (target_attr))

>  	      {

>  		baton = XOBNEW (obstack, struct dwarf2_property_baton);

> -		baton->referenced_type = die_type (target_die, target_cu);

> +		baton->property_type = die_type (target_die, target_cu);

>  		baton->locexpr.per_cu = cu->per_cu;

>  		baton->locexpr.size = DW_BLOCK (target_attr)->size;

>  		baton->locexpr.data = DW_BLOCK (target_attr)->data;

> +		baton->locexpr.is_reference = true;

>  		prop->data.baton = baton;

>  		prop->kind = PROP_LOCEXPR;

>  		gdb_assert (prop->data.baton != NULL);

> @@ -17806,7 +17819,7 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,

>  		return 0;

>  

>  	      baton = XOBNEW (obstack, struct dwarf2_property_baton);

> -	      baton->referenced_type = read_type_die (target_die->parent,

> +	      baton->property_type = read_type_die (target_die->parent,

>  						      target_cu);

>  	      baton->offset_info.offset = offset;

>  	      baton->offset_info.type = die_type (target_die, target_cu);

> @@ -17831,6 +17844,37 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,

>    return 1;

>  }

>  

> +/* Find an integer type the same size as the address size given in the

> +   compilation unit header for PER_CU.  UNSIGNED_P controls if the integer

> +   is unsigned or not.  */

> +

> +static struct type *

> +dwarf2_per_cu_addr_sized_int_type (struct dwarf2_per_cu_data *per_cu,

> +				   bool unsigned_p)

> +{

> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;

> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);

> +  struct type *int_type;

> +

> +  /* Helper macro to examine the various builtin types.  */

> +#define TRY_TYPE(F)						\

> +  int_type = (unsigned_p					\

> +	      ? objfile_type (objfile)->builtin_unsigned_ ## F	\

> +	      : objfile_type (objfile)->builtin_ ## F);		\

> +  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)	\

> +    return int_type

> +

> +  TRY_TYPE (char);

> +  TRY_TYPE (short);

> +  TRY_TYPE (int);

> +  TRY_TYPE (long);

> +  TRY_TYPE (long_long);

> +

> +#undef TRY_TYPE

> +

> +  gdb_assert_not_reached ("unable to find suitable integer type");

> +}

> +

>  /* Read the DW_AT_type attribute for a sub-range.  If this attribute is not

>     present (which is valid) then compute the default type based on the

>     compilation units address size.  */

> @@ -17853,30 +17897,7 @@ read_subrange_index_type (struct die_info *die, struct dwarf2_cu *cu)

>       FIXME: muller/2010-05-28: Possible references to object for low bound,

>       high bound or count are not yet handled by this code.  */

>    if (TYPE_CODE (index_type) == TYPE_CODE_VOID)

> -    {

> -      struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;

> -      struct gdbarch *gdbarch = get_objfile_arch (objfile);

> -      int addr_size = gdbarch_addr_bit (gdbarch) /8;

> -      struct type *int_type = objfile_type (objfile)->builtin_int;

> -

> -      /* Test "int", "long int", and "long long int" objfile types,

> -	 and select the first one having a size above or equal to the

> -	 architecture address size.  */

> -      if (int_type && TYPE_LENGTH (int_type) >= addr_size)

> -	index_type = int_type;

> -      else

> -	{

> -	  int_type = objfile_type (objfile)->builtin_long;

> -	  if (int_type && TYPE_LENGTH (int_type) >= addr_size)

> -	    index_type = int_type;

> -	  else

> -	    {

> -	      int_type = objfile_type (objfile)->builtin_long_long;

> -	      if (int_type && TYPE_LENGTH (int_type) >= addr_size)

> -		index_type = int_type;

> -	    }

> -	}

> -    }

> +    index_type = dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);

>  

>    return index_type;

>  }

> @@ -17945,7 +17966,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)

>  

>    attr = dwarf2_attr (die, DW_AT_lower_bound, cu);

>    if (attr)

> -    attr_to_dynamic_prop (attr, die, cu, &low);

> +    attr_to_dynamic_prop (attr, die, cu, &low, base_type);

>    else if (!low_default_is_valid)

>      complaint (_("Missing DW_AT_lower_bound "

>  				      "- DIE at %s [in module %s]"),

> @@ -17954,10 +17975,10 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)

>  

>    struct attribute *attr_ub, *attr_count;

>    attr = attr_ub = dwarf2_attr (die, DW_AT_upper_bound, cu);

> -  if (!attr_to_dynamic_prop (attr, die, cu, &high))

> +  if (!attr_to_dynamic_prop (attr, die, cu, &high, base_type))

>      {

>        attr = attr_count = dwarf2_attr (die, DW_AT_count, cu);

> -      if (attr_to_dynamic_prop (attr, die, cu, &high))

> +      if (attr_to_dynamic_prop (attr, die, cu, &high, base_type))

>  	{

>  	  /* If bounds are constant do the final calculation here.  */

>  	  if (low.kind == PROP_CONST && high.kind == PROP_CONST)

> @@ -25310,6 +25331,24 @@ dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *per_cu)

>    return ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));

>  }

>  

> +/* Return a type that is a generic pointer type, the size of which matches

> +   the address size given in the compilation unit header for PER_CU.  */

> +static struct type *

> +dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu)

> +{

> +  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;

> +  struct type *void_type = objfile_type (objfile)->builtin_void;

> +  struct type *addr_type = lookup_pointer_type (void_type);

> +  int addr_size = dwarf2_per_cu_addr_size (per_cu);

> +

> +  if (TYPE_LENGTH (addr_type) == addr_size)

> +    return addr_type;

> +

> +  addr_type

> +    = dwarf2_per_cu_addr_sized_int_type (per_cu, TYPE_UNSIGNED (addr_type));

> +  return addr_type;

> +}

> +

>  /* Return DWARF version number of PER_CU.  */

>  

>  short

> @@ -25576,7 +25615,9 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)

>    attr = dwarf2_attr (die, DW_AT_allocated, cu);

>    if (attr_form_is_block (attr))

>      {

> -      if (attr_to_dynamic_prop (attr, die, cu, &prop))

> +      struct type *prop_type

> +	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);

> +      if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))

>          add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);

>      }

>    else if (attr != NULL)

> @@ -25590,7 +25631,9 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)

>    attr = dwarf2_attr (die, DW_AT_associated, cu);

>    if (attr_form_is_block (attr))

>      {

> -      if (attr_to_dynamic_prop (attr, die, cu, &prop))

> +      struct type *prop_type

> +	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu, false);

> +      if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))

>          add_dyn_prop (DYN_PROP_ASSOCIATED, prop, type);

>      }

>    else if (attr != NULL)

> @@ -25602,7 +25645,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)

>  

>    /* Read DW_AT_data_location and set in type.  */

>    attr = dwarf2_attr (die, DW_AT_data_location, cu);

> -  if (attr_to_dynamic_prop (attr, die, cu, &prop))

> +  if (attr_to_dynamic_prop (attr, die, cu, &prop,

> +			    dwarf2_per_cu_addr_type (cu->per_cu)))

>      add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);

>  

>    if (dwarf2_per_objfile->die_type_hash == NULL)
Tom Tromey July 10, 2019, 3:06 p.m. | #4
Andrew> Was there any additional feedback on this series?

Nope, sorry about that!  I think it is fine.

Tom

Patch

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index c03f261f1e1..88d34eb8660 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2443,15 +2443,15 @@  dwarf2_evaluate_property (const struct dynamic_prop *prop,
       {
 	const struct dwarf2_property_baton *baton
 	  = (const struct dwarf2_property_baton *) prop->data.baton;
+	gdb_assert (baton->property_type != NULL);
 
 	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
 				       addr_stack ? addr_stack->addr : 0,
 				       value))
 	  {
-	    if (baton->referenced_type)
+	    if (baton->locexpr.is_reference)
 	      {
-		struct value *val = value_at (baton->referenced_type, *value);
-
+		struct value *val = value_at (baton->property_type, *value);
 		*value = value_as_address (val);
 	      }
 	    return true;
@@ -2471,7 +2471,7 @@  dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	data = dwarf2_find_location_expression (&baton->loclist, &size, pc);
 	if (data != NULL)
 	  {
-	    val = dwarf2_evaluate_loc_desc (baton->referenced_type, frame, data,
+	    val = dwarf2_evaluate_loc_desc (baton->property_type, frame, data,
 					    size, baton->loclist.per_cu);
 	    if (!value_optimized_out (val))
 	      {
@@ -2497,7 +2497,7 @@  dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	  {
 	    /* This approach lets us avoid checking the qualifiers.  */
 	    if (TYPE_MAIN_TYPE (pinfo->type)
-		== TYPE_MAIN_TYPE (baton->referenced_type))
+		== TYPE_MAIN_TYPE (baton->property_type))
 	      break;
 	  }
 	if (pinfo == NULL)
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index ac1a771a9f3..baa5762003d 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -183,6 +183,12 @@  struct dwarf2_locexpr_baton
      zero.  */
   size_t size;
 
+  /* When true this location expression is a reference and actually
+     describes the address at which the value of the attribute can be
+     found.  When false the expression provides the value of the attribute
+     directly.  */
+  bool is_reference;
+
   /* The compilation unit containing the symbol whose location
      we're computing.  */
   struct dwarf2_per_cu_data *per_cu;
@@ -228,23 +234,27 @@  struct dwarf2_offset_baton
 
 /* A dynamic property is either expressed as a single location expression
    or a location list.  If the property is an indirection, pointing to
-   another die, keep track of the targeted type in REFERENCED_TYPE.  */
+   another die, keep track of the targeted type in PROPERTY_TYPE.
+   Alternatively, if the property location gives the property value
+   directly then it will have PROPERTY_TYPE.  */
 
 struct dwarf2_property_baton
 {
   /* If the property is an indirection, we need to evaluate the location
-     in the context of the type REFERENCED_TYPE.
-     If NULL, the location is the actual value of the property.  */
-  struct type *referenced_type;
+     in the context of the type PROPERTY_TYPE.  If the property is supplied
+     by value then it will be of PROPERTY_TYPE.  This field should never be
+     NULL.  */
+  struct type *property_type;
   union
   {
-    /* Location expression.  */
+    /* Location expression either evaluated in the context of
+       PROPERTY_TYPE, or a value of type PROPERTY_TYPE.  */
     struct dwarf2_locexpr_baton locexpr;
 
-    /* Location list to be evaluated in the context of REFERENCED_TYPE.  */
+    /* Location list to be evaluated in the context of PROPERTY_TYPE.  */
     struct dwarf2_loclist_baton loclist;
 
-    /* The location is an offset to REFERENCED_TYPE.  */
+    /* The location is an offset to PROPERTY_TYPE.  */
     struct dwarf2_offset_baton offset_info;
   };
 };
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index e9c53148961..76f017daa1a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1816,7 +1816,7 @@  static void read_signatured_type (struct signatured_type *);
 
 static int attr_to_dynamic_prop (const struct attribute *attr,
 				 struct die_info *die, struct dwarf2_cu *cu,
-				 struct dynamic_prop *prop);
+				 struct dynamic_prop *prop, struct type *type);
 
 /* memory allocation interface */
 
@@ -1896,6 +1896,10 @@  static void queue_comp_unit (struct dwarf2_per_cu_data *per_cu,
 
 static void process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile);
 
+static struct type *dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu);
+static struct type *dwarf2_per_cu_addr_sized_int_type
+  (struct dwarf2_per_cu_data *per_cu);
+
 /* Class, the destructor of which frees all allocated queue entries.  This
    will only have work to do if an error was thrown while processing the
    dwarf.  If no error was thrown then the queue entries should have all
@@ -13737,7 +13741,8 @@  read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
     {
       newobj->static_link
 	= XOBNEW (&objfile->objfile_obstack, struct dynamic_prop);
-      attr_to_dynamic_prop (attr, die, cu, newobj->static_link);
+      attr_to_dynamic_prop (attr, die, cu, newobj->static_link,
+			    dwarf2_per_cu_addr_type (cu->per_cu));
     }
 
   cu->list_in_scope = cu->get_builder ()->get_local_symbols ();
@@ -16493,10 +16498,13 @@  read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   if (attr != NULL)
     {
       int stride_ok;
+      struct type *prop_type
+	= dwarf2_per_cu_addr_sized_int_type (cu->per_cu);
 
       byte_stride_prop
 	= (struct dynamic_prop *) alloca (sizeof (struct dynamic_prop));
-      stride_ok = attr_to_dynamic_prop (attr, die, cu, byte_stride_prop);
+      stride_ok = attr_to_dynamic_prop (attr, die, cu, byte_stride_prop,
+					prop_type);
       if (!stride_ok)
 	{
 	  complaint (_("unable to read array DW_AT_byte_stride "
@@ -17711,22 +17719,26 @@  read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 
 static int
 attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
-		      struct dwarf2_cu *cu, struct dynamic_prop *prop)
+		      struct dwarf2_cu *cu, struct dynamic_prop *prop,
+		      struct type *default_type)
 {
   struct dwarf2_property_baton *baton;
   struct obstack *obstack
     = &cu->per_cu->dwarf2_per_objfile->objfile->objfile_obstack;
 
+  gdb_assert (default_type != NULL);
+
   if (attr == NULL || prop == NULL)
     return 0;
 
   if (attr_form_is_block (attr))
     {
       baton = XOBNEW (obstack, struct dwarf2_property_baton);
-      baton->referenced_type = NULL;
+      baton->property_type = default_type;
       baton->locexpr.per_cu = cu->per_cu;
       baton->locexpr.size = DW_BLOCK (attr)->size;
       baton->locexpr.data = DW_BLOCK (attr)->data;
+      baton->locexpr.is_reference = false;
       prop->data.baton = baton;
       prop->kind = PROP_LOCEXPR;
       gdb_assert (prop->data.baton != NULL);
@@ -17751,7 +17763,7 @@  attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 	    if (attr_form_is_section_offset (target_attr))
 	      {
 		baton = XOBNEW (obstack, struct dwarf2_property_baton);
-		baton->referenced_type = die_type (target_die, target_cu);
+		baton->property_type = die_type (target_die, target_cu);
 		fill_in_loclist_baton (cu, &baton->loclist, target_attr);
 		prop->data.baton = baton;
 		prop->kind = PROP_LOCLIST;
@@ -17760,10 +17772,11 @@  attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 	    else if (attr_form_is_block (target_attr))
 	      {
 		baton = XOBNEW (obstack, struct dwarf2_property_baton);
-		baton->referenced_type = die_type (target_die, target_cu);
+		baton->property_type = die_type (target_die, target_cu);
 		baton->locexpr.per_cu = cu->per_cu;
 		baton->locexpr.size = DW_BLOCK (target_attr)->size;
 		baton->locexpr.data = DW_BLOCK (target_attr)->data;
+		baton->locexpr.is_reference = true;
 		prop->data.baton = baton;
 		prop->kind = PROP_LOCEXPR;
 		gdb_assert (prop->data.baton != NULL);
@@ -17784,7 +17797,7 @@  attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 		return 0;
 
 	      baton = XOBNEW (obstack, struct dwarf2_property_baton);
-	      baton->referenced_type = read_type_die (target_die->parent,
+	      baton->property_type = read_type_die (target_die->parent,
 						      target_cu);
 	      baton->offset_info.offset = offset;
 	      baton->offset_info.type = die_type (target_die, target_cu);
@@ -17809,6 +17822,35 @@  attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
   return 1;
 }
 
+/* Find a signed integer type the same size as the address size given in
+   the compilation unit header for PER_CU.  */
+
+static struct type *
+dwarf2_per_cu_addr_sized_int_type (struct dwarf2_per_cu_data *per_cu)
+{
+  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;
+  int addr_size = dwarf2_per_cu_addr_size (per_cu);
+  struct type *int_type;
+
+  int_type = objfile_type (objfile)->builtin_short;
+  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)
+    return int_type;
+
+  int_type = objfile_type (objfile)->builtin_int;
+  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)
+    return int_type;
+
+  int_type = objfile_type (objfile)->builtin_long;
+  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)
+    return int_type;
+
+  int_type = objfile_type (objfile)->builtin_long_long;
+  if (int_type != NULL && TYPE_LENGTH (int_type) == addr_size)
+    return int_type;
+
+  gdb_assert_not_reached ("unable to find suitable integer type");
+}
+
 /* Read the DW_AT_type attribute for a sub-range.  If this attribute is not
    present (which is valid) then compute the default type based on the
    compilation units address size.  */
@@ -17831,30 +17873,7 @@  read_subrange_index_type (struct die_info *die, struct dwarf2_cu *cu)
      FIXME: muller/2010-05-28: Possible references to object for low bound,
      high bound or count are not yet handled by this code.  */
   if (TYPE_CODE (index_type) == TYPE_CODE_VOID)
-    {
-      struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
-      struct gdbarch *gdbarch = get_objfile_arch (objfile);
-      int addr_size = gdbarch_addr_bit (gdbarch) /8;
-      struct type *int_type = objfile_type (objfile)->builtin_int;
-
-      /* Test "int", "long int", and "long long int" objfile types,
-	 and select the first one having a size above or equal to the
-	 architecture address size.  */
-      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
-	index_type = int_type;
-      else
-	{
-	  int_type = objfile_type (objfile)->builtin_long;
-	  if (int_type && TYPE_LENGTH (int_type) >= addr_size)
-	    index_type = int_type;
-	  else
-	    {
-	      int_type = objfile_type (objfile)->builtin_long_long;
-	      if (int_type && TYPE_LENGTH (int_type) >= addr_size)
-		index_type = int_type;
-	    }
-	}
-    }
+    index_type = dwarf2_per_cu_addr_sized_int_type (cu->per_cu);
 
   return index_type;
 }
@@ -17923,7 +17942,7 @@  read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 
   attr = dwarf2_attr (die, DW_AT_lower_bound, cu);
   if (attr)
-    attr_to_dynamic_prop (attr, die, cu, &low);
+    attr_to_dynamic_prop (attr, die, cu, &low, base_type);
   else if (!low_default_is_valid)
     complaint (_("Missing DW_AT_lower_bound "
 				      "- DIE at %s [in module %s]"),
@@ -17932,10 +17951,10 @@  read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 
   struct attribute *attr_ub, *attr_count;
   attr = attr_ub = dwarf2_attr (die, DW_AT_upper_bound, cu);
-  if (!attr_to_dynamic_prop (attr, die, cu, &high))
+  if (!attr_to_dynamic_prop (attr, die, cu, &high, base_type))
     {
       attr = attr_count = dwarf2_attr (die, DW_AT_count, cu);
-      if (attr_to_dynamic_prop (attr, die, cu, &high))
+      if (attr_to_dynamic_prop (attr, die, cu, &high, base_type))
 	{
 	  /* If bounds are constant do the final calculation here.  */
 	  if (low.kind == PROP_CONST && high.kind == PROP_CONST)
@@ -25256,6 +25275,58 @@  dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *per_cu)
   return ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 }
 
+/* Return a type that is a generic pointer type, the size of which matches
+   the address size given in the compilation unit header for PER_CU.  */
+static struct type *
+dwarf2_per_cu_addr_type (struct dwarf2_per_cu_data *per_cu)
+{
+  struct objfile *objfile = per_cu->dwarf2_per_objfile->objfile;
+  struct type *void_type = objfile_type (objfile)->builtin_void;
+  struct type *addr_type = lookup_pointer_type (void_type);
+  int addr_size = dwarf2_per_cu_addr_size (per_cu);
+
+  if (TYPE_LENGTH (addr_type) == addr_size)
+    return addr_type;
+
+  /* Yuck! We currently only support one address size per architecture in
+     GDB, which should usually match the address size encoded into the
+     compilation unit header.  However... we have a few tests where this is
+     not the case, these are mostly test cases where the DWARF is hand
+     written and includes a fixed address size, for example 8-bytes.  When
+     we compile these tests on a 32-bit i386 target the gdbarch address
+     size is 4-bytes and the above attempt to create a suitable address
+     type fails.
+
+     As we can't currently create an address type of a different size, we
+     instead substitute an unsigned integer for an address.
+
+     I don't know if there are targets that have signed addresses and if
+     they would need a signed integer here.  I figure we'll handle that
+     case when it presents itself as a problem.  */
+
+  addr_type = objfile_type (objfile)->builtin_unsigned_char;
+  if (addr_type != NULL && TYPE_LENGTH (addr_type) == addr_size)
+    return addr_type;
+
+  addr_type = objfile_type (objfile)->builtin_unsigned_short;
+  if (addr_type != NULL && TYPE_LENGTH (addr_type) == addr_size)
+    return addr_type;
+
+  addr_type = objfile_type (objfile)->builtin_unsigned_int;
+  if (addr_type != NULL && TYPE_LENGTH (addr_type) == addr_size)
+    return addr_type;
+
+  addr_type = objfile_type (objfile)->builtin_unsigned_long;
+  if (addr_type != NULL && TYPE_LENGTH (addr_type) == addr_size)
+    return addr_type;
+
+  addr_type = objfile_type (objfile)->builtin_unsigned_long_long;
+  if (addr_type != NULL && TYPE_LENGTH (addr_type) == addr_size)
+    return addr_type;
+
+  gdb_assert_not_reached ("failed to find address type");
+}
+
 /* Return DWARF version number of PER_CU.  */
 
 short
@@ -25522,7 +25593,8 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_allocated, cu);
   if (attr_form_is_block (attr))
     {
-      if (attr_to_dynamic_prop (attr, die, cu, &prop))
+      struct type *prop_type = dwarf2_per_cu_addr_sized_int_type (cu->per_cu);
+      if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))
         add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);
     }
   else if (attr != NULL)
@@ -25536,7 +25608,8 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_associated, cu);
   if (attr_form_is_block (attr))
     {
-      if (attr_to_dynamic_prop (attr, die, cu, &prop))
+      struct type *prop_type = dwarf2_per_cu_addr_sized_int_type (cu->per_cu);
+      if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))
         add_dyn_prop (DYN_PROP_ASSOCIATED, prop, type);
     }
   else if (attr != NULL)
@@ -25548,7 +25621,8 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
-  if (attr_to_dynamic_prop (attr, die, cu, &prop))
+  if (attr_to_dynamic_prop (attr, die, cu, &prop,
+			    dwarf2_per_cu_addr_type (cu->per_cu)))
     add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)