This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [rfa] frame address size incorrect if address size != ptr size


On Aug  5 12:04, Ulrich Weigand wrote:
> Corinna Vinschen wrote:
> > On Aug  5 00:39, Jan Kratochvil wrote:
> > > I have not built gcc for the xstrormy16 target to check it more.  Still
> > > I believe .eh_frame and .dwarf_frame address size should be treated the same.
> 
> That is actually not the case; the comment in GDB is correct.
> 
> For .debug_frame, the standard refers to "the size of an address on the
> target architecture", and uses this value to define sizes of several
> FDE/CFI fields holding address values, without clearly specifying what
> that value means.  This has now been clarified in version 4, but for
> older versions you simply have to know what the appropriate value for a
> given architecture is (i.e. what value the compiler has chosen to use).

I'm wondering if that shouldn't always be the address size, though.  The
pointer size could be any arbitrary size which fits a given scenario,
but the address size is what can safely be used to address the entire
address range of the target.  As far as I understand the address size.

> GCC defines a target macro DWARF2_ADDR_SIZE to provide this value;
> it defaults to the size of a pointer (POINTER_SIZE), but can (and is)
> overridden by a target back-end to some other explicit value.
> 
> For .eh_frame, on the other hand, there is no standard.  On some platforms,
> GCC now chooses to produce data that looks like a variant of DWARF CFI,
> but there are certain differences.  In particular, it uses different
> encodings to represent addresses in the those FDE/CFI fields mentioned
> above; for example they may in fact be represented as offsets relative to
> the text section or the eh_frame section itself.  Depending on the particular
> encoding, the size of the data used to encode addresses is either hard-coded,
> or is equal to the size of a *pointer* (i.e. GCC uses POINTER_SIZE to
> represent this value, not DWARF2_ADDR_SIZE).

Which is a bug, IMHO, even if this only potentially affects targets
where address and pointer size differ.

The description at
http://refspecs.freestandards.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
just contains something like

  "PC Begin

     An encoded constant that indicates the address of the initial
     location associated with this FDE."

To indicate the address, the compiler has to use the address size
for these values.  It can't rely on the size of pointers.

> To capture this difference, the GDB dwarf2-frame.c code sets the
> cie->addr_size field accordingly.  Looking at this code more carefully,
> the usage of this field may actually not be fully correct, because it
> is used both to pass to read_encoded_value (which needs the address size
> in .debug_frame, but the pointer size in .eh_frame sections), *and* to
> pass to dwarf_expr_eval when evaluating complex expressions (which *always*
> needs the address size, even in .eh_frame sections).

Yeah, as I said, in theory the .eh_frame sections have to use the
address size, otherwise .eh_frame sections will never be useful
for small targets as the below mentioned ones.

> > I made the difference because the comment explicitely states that
> > .eh_frame sections are defined to use the target *pointer* size and
> > .dwarf_frame (sic) sections are using the *address* size as given in
> > the CU header.
> 
> Indeed, see above.
> 
> > This address size is exactly what is defined by gdbarch_addr_bit().
> 
> This, however, is not so clear to me.  gdbarch_addr_bit defines the size
> of an address *in GDB internal representation*, i.e. what the contents
> of a CORE_ADDR value mean.  On some platforms, target addresses may be
> encoded into GDB internal addresses in complex ways, e.g. for platforms
> that use segments or different code/data address spaces.  It is not
> obvious that the size of GDB internal addresses always agrees with what
> GCC chooses as Dwarf address size.
> 
> 
> Looking at the (few) platforms where GCC defines a DWARF2_ADDR_SIZE
> that differs from the default, we have:
> 
>  avr:       POINTER_SIZE 16         /   DWARF2_ADDR_SIZE 4
>  m32c:      POINTER_SIZE 16 or 32   /   DWARF2_ADDR_SIZE 4
>  m68hc11:   POINTER_SIZE 16         /   DWARF2_ADDR_SIZE 4
>  mips:      POINTER_SIZE 32 or 64   /   DWARF2_ADDR_SIZE 4 or 8  (**)
>  xstormy16: POINTER_SIZE 16         /   DWARF2_ADDR_SIZE 4
> 
> (**) but those may not always match:
>    "Note that the default POINTER_SIZE test is not appropriate for MIPS.
>     EABI64 has 64-bit pointers but uses 32-bit ELF."
> 
> On the other hand, in GDB we have these platforms that set addr_bit
> to a different value than ptr_bit:
> 
>  avr:       ptr_bit 16         /   addr_bit 32
>  m32c:      ptr_bit 16 or 32   /   addr_bit 32   (**)
>  m68hc11:   ptr_bit 16         /   addr_bit 16 or 32
>  xstormy16: ptr_bit 16         /   addr_bit 32
> 
> (**) but the back-end would have prefered to use 24:
>    "GCC uses 32 bits for addrs in the dwarf info, even though
>     only 16/24 bits are used.  Setting addr_bit to 24 causes
>     errors in reading the dwarf addresses."
> 
> So it seems of the five platforms, on three of them GDB's addr_bit
> always agrees with GCC's DWARF2_ADDR_SIZE (even though on one of
> them the match is forced), and on two of them they do not always
> agree ...

So, right now evaluating .debug_frame sections is broken for at least
avr, m32c/16, and xstormy16.

> I'm not completely sure how to proceed here.  One way out could
> certainly be to remove the overloaded semantics of addr_bit by
> simply adding a *new* gdbarch callback gdbarch_dwarf2_addr_size
> which encodes exactly what this target uses as address size
> in .dwarf_frame sections (i.e. always equal to GCC's DWARF2_ADDR_SIZE
> setting).
> 
> I'd appreciate further comments / suggestions on this issue ...

I think that's a good idea.  However, for targets which don't define
gdbarch_dwarf2_addr_size, it's still necessary to assume a useful
default.

And then there's the important hint from the comment in dwarf2-frame.c:

  "For .debug_frame FDEs, this is supposed to be the target address size
   from the associated CU header."
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Doesn't that mean we should better try to check for this value in the
first place?  I'm not fluent in GDBs dwarf2 code, but is that really
*that* tricky?

If not, I would prefer a solution like this:

  - If version > 4, use addr_size from .debug_frame section
  - Otherwise, if we can fetch the target address size from the CU
    header, use it.
  - Otherwise, if the target defined gdbarch_dwarf2_addr_size, use it.
  - Otherwise, default to gdbarch_addr_bit for .debug_frame sections
    and to gdbarch_ptr_bit for .eh_frame sections.

> >        /* The target address size.  For .eh_frame FDEs this is considered
> > -	 equal to the size of a target pointer.  For .dwarf_frame FDEs, 
> > +	 equal to the size of a target pointer.  For .debug_frame FDEs, 
> >  	 this is supposed to be the target address size from the associated
> >  	 CU header.  FIXME: We do not have a good way to determine the 
> > -	 latter.  Always use the target pointer size for now.  */
> > -      cie->addr_size = gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT;
> > +	 latter.  Always use the target address size for now.  */
> > +      cie->addr_size = gdbarch_addr_bit (gdbarch) / TARGET_CHAR_BIT;
> >  
> >        /* We'll determine the final value later, but we need to
> >  	 initialize it conservatively.  */
> > @@ -1779,7 +1779,7 @@ decode_frame_entry_1 (struct comp_unit *
> >  	}
> >        else
> >  	{
> > -	  cie->addr_size = gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT;
> > +	  cie->addr_size = gdbarch_addr_bit (gdbarch) / TARGET_CHAR_BIT;
> >  	  cie->segment_size = 0;
> >  	}
> 
> As a side note, it seems odd that add_size is set in those two
> different locations here.  The first one is always overwritten
> by the second one anyway, isn't it?

There's an early return statement after checking the version number.
That indicates a failure anyway, so it might be ok to set addr_size
only once, at the second spot (lines 1779ff).


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]