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

[Corinna Vinschen <vinschen@redhat.com>]

On Aug  6 16:50, Ulrich Weigand wrote:
> Corinna Vinschen wrote:
> 
> > At one point in this thread you mentioned that .eh_frame isn't really
> > standarized.
> > 
> > Additionally, using the pointer size in .eh_frame sections would
> > invariably break unwinding on targets like avr and XStormy16.
> 
> No, it would make it work :-)  In .eh_frame sections, the contents
> of DW_EH_PE_absptr encoded fields are *used as pointer values* at
> run time.  See e.g. unwind-pe.h:read_encoded_value_with_base:
> [...]
>         case DW_EH_PE_absptr:
>           result = (_Unwind_Internal_Ptr) u->ptr;
>           p += sizeof (void *);
>           break;
> 
> "p" points directly into the mapped .eh_frame image.  The contents
> are simply interpreted as a native "void *" at run-time.  If GDB
> wants to use this section as well, it has to treat it like it would
> read a "void *" global variable from the target ...
> [...]
> > Consequentially, is it *really* correct to define the ptr_size as you
> > requested now, or isn't it *more* correct to use the target dependent
> > approach as my previous patch implemented?  Since it's using the pointer
> > size as fallback, it will be correct for all existing .eh_frame sections
> > anyway.
> 
> The difference between .eh_frame and .debug_frame is really fundamental;
> .eh_frame holds *pointer* values in target "void *" format; while
> .debug_frame holds *address* values using the same format as the rest
> of the DWARF debug sections.
> 
> Therefore we will definitely have to make a distinction between the two.
> If you have another suggestion how to achieve that, I'd certainly be
> interested ...

I understand what you're up to, but to me this means that the above code
from unwind-pe.h is potentially not usable for certain small targets,
unless some conditions are met.

As it is the one example I really know well, let's stick to XStormy16
for now.

The problem for XStormy16 in 16 bit pointer mode is that a pointer is
not able to point to every place in the 24 bit address space the CPU can
address.  For function pointers that means that the target potentially
has to use a jump table.  For the stack that means it is restricted to
the first 64K RAM.

So, afaics, the unwind-pe.h code only works correct for XStormy16, if
either the application fits into the first 64K of memory, or if
DW_EH_PE_absptr is not used, rather DW_EH_PE_pcrel, DW_EH_PE_textrel,
DW_EH_PE_datarel, or DW_EH_PE_funcrel.  Oh, and then there's the
type of _Unwind_Ptr, which would have to be big enough, 32 bit.

Alternatively, never use DW EH on such targets since it has very
obviously never been designed with small targets in mind.

> > 	* dwarf2-frame.c (struct dwarf2_cie): Add ptr_size member.
> > 	Throughout, call read_encoded_value with ptr_size rather than addr_size.
> > 	(decode_frame_entry_1): Remove redundant setting of
> > 	addr_size.  Call gdbarch_dwarf2_addr_size rather than gdbarch_ptr_bit
> > 	to determine addr_size in Dwarf versions < 4.  Set ptr_size dependent
> > 	on examined frame section.  Add comment to explain why.
> > 	* gdbarch.sh (dwarf2_addr_size): Define as variable.  Add lengthy
> > 	comment to explain usage.
> > 	* gdbarch.c: Regenerate.
> > 	* gdbarch.h: Regenerate.
> > 	* xstormy16-tdep.c (xstormy16_gdbarch_init): Set dwarf2_addr_size to 4.
> 
> This looks good to me, except a couple of the comments:
> 
> > +      /* Address values in .eh_frame sections are defined to have the
> > +	 target's pointer size.  Watchout: This breaks frame info for
> > +	 targets with pointer size < address size, unless a .debug_frame
> > +	 section exists as well. */
> 
> As discussed above, I don't agree that this breaks anything ...  Do you
> still feel it does?

Right now?  Yes.  It's not GDBs fault, though...

> > +# dwarf2_addr_size is the target address size as used int the Dwarf debug
> > +# info.  For .eh_frame FDEs this is considered 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, and which is equivalent to the
> > +# DWARF2_ADDR_SIZE as defined by the target specific GCC back-end.
> > +# Unfortunately there is no good way to determine this value.  Therefore
> > +# dwarf2_addr_size simply defaults to the target pointer size.
> 
> I'd reword this to make clear that this value is *not* used for .eh_frame,
> but solely for .debug_frame.

Ok, will do.  I'd just like to put the discussion to an end, first.
Just tell me what you think of what I wrote above.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat