This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfa] frame address size incorrect if address size != ptr size
On Aug 5 18:51, Ulrich Weigand wrote:
> Corinna Vinschen wrote:
>
> > Index: dwarf2-frame.c
> > [...]
>
> Ah, wait a minute -- now we're missing the distinction between .eh_frame
> and .debug_frame again. For .eh_frame, FDE encodings needs to keep using
> ptr_bit as discussed earlier, even on targets that do define a non-default
> gdbarch_dwarf2_addr_size. Sorry for not noticing earlier.
>
> In fact, this means we might as well fix the inconsistent use between
> addr_size as input to read_encoded_value (which needs to be ptr_size
> on .eh_frame and addr_size on .debug_frame), and addr_size as input
> to execute_stack_op (which needs to be addr_size always).
>
> Could you add a second variable ptr_size to struct dwarf2_cie, and
> initialize it (after the addr_size initialization) along the lines of:
>
> if (eh_frame_p)
> cie->ptr_size = gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT;
> else
> cie->ptr_size = cie->addr_size;
>
> and then change all calls to read_encoded_value throughout dwarf2-frame.c
> to pass cie->ptr_size instead of cie->addr_size.
No big deal, patch attached.
However, here's a question:
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.
And...
> It would then be interesting to see whether *both* .debug_frame and
> .eh_frame work on xstormy16 ... you might be able to try the latter
> by building with -fasynchronous-unwind-tables.
... .eh_frame sections are never generated for XStormy16. The
-fasynchronous-unwind-tables option is a no-op.
Given all that, doesn't that mean that the .eh_frame sections for those
targets would *have* to use the address size rather than the pointer
size, as soon as .eh_frame sections are to be defined for those targets?
Their unwinders will never have a chance to do their job right otherwise.
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.
It's your choice, but I'm quite certain that the previous approach/patch
will result in less headache in the long run.
Corinna
* 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.
Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.114
diff -u -p -r1.114 dwarf2-frame.c
--- dwarf2-frame.c 7 Jul 2010 17:26:38 -0000 1.114
+++ dwarf2-frame.c 6 Aug 2010 10:41:26 -0000
@@ -77,6 +77,9 @@ struct dwarf2_cie
/* Target address size in bytes. */
int addr_size;
+ /* Target pointer size in bytes. */
+ int ptr_size;
+
/* True if a 'z' augmentation existed. */
unsigned char saw_z_augmentation;
@@ -452,7 +455,7 @@ execute_cfa_program (struct dwarf2_fde *
{
case DW_CFA_set_loc:
fs->pc = read_encoded_value (fde->cie->unit, fde->cie->encoding,
- fde->cie->addr_size, insn_ptr,
+ fde->cie->ptr_size, insn_ptr,
&bytes_read, fde->initial_location);
/* Apply the objfile offset for relocatable objects. */
fs->pc += ANOFFSET (fde->cie->unit->objfile->section_offsets,
@@ -1732,13 +1735,6 @@ decode_frame_entry_1 (struct comp_unit *
depends on the target address size. */
cie->encoding = DW_EH_PE_absptr;
- /* The target address size. For .eh_frame FDEs this is considered
- equal to the size of a target pointer. For .dwarf_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;
-
/* We'll determine the final value later, but we need to
initialize it conservatively. */
cie->signal_frame = 0;
@@ -1779,9 +1775,17 @@ decode_frame_entry_1 (struct comp_unit *
}
else
{
- cie->addr_size = gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT;
+ cie->addr_size = gdbarch_dwarf2_addr_size (gdbarch);
cie->segment_size = 0;
}
+ /* 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. */
+ if (eh_frame_p)
+ cie->ptr_size = gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT;
+ else
+ cie->ptr_size = cie->addr_size;
cie->code_alignment_factor =
read_unsigned_leb128 (unit->abfd, buf, &bytes_read);
@@ -1841,7 +1845,7 @@ decode_frame_entry_1 (struct comp_unit *
{
/* Skip. Avoid indirection since we throw away the result. */
gdb_byte encoding = (*buf++) & ~DW_EH_PE_indirect;
- read_encoded_value (unit, encoding, cie->addr_size,
+ read_encoded_value (unit, encoding, cie->ptr_size,
buf, &bytes_read, 0);
buf += bytes_read;
augmentation++;
@@ -1907,13 +1911,13 @@ decode_frame_entry_1 (struct comp_unit *
gdb_assert (fde->cie != NULL);
fde->initial_location =
- read_encoded_value (unit, fde->cie->encoding, fde->cie->addr_size,
+ read_encoded_value (unit, fde->cie->encoding, fde->cie->ptr_size,
buf, &bytes_read, 0);
buf += bytes_read;
fde->address_range =
read_encoded_value (unit, fde->cie->encoding & 0x0f,
- fde->cie->addr_size, buf, &bytes_read, 0);
+ fde->cie->ptr_size, buf, &bytes_read, 0);
buf += bytes_read;
/* A 'z' augmentation in the CIE implies the presence of an
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.514
diff -u -p -r1.514 gdbarch.sh
--- gdbarch.sh 19 Jul 2010 17:51:23 -0000 1.514
+++ gdbarch.sh 6 Aug 2010 10:41:27 -0000
@@ -384,14 +384,27 @@ v:const struct floatformat **:long_doubl
# / addr_bit will be set from it.
#
# If gdbarch_ptr_bit and gdbarch_addr_bit are different, you'll probably
-# also need to set gdbarch_pointer_to_address and gdbarch_address_to_pointer
-# as well.
+# also need to set gdbarch_dwarf2_addr_size, gdbarch_pointer_to_address and
+# gdbarch_address_to_pointer as well.
#
# ptr_bit is the size of a pointer on the target
v:int:ptr_bit:::8 * sizeof (void*):gdbarch->int_bit::0
# addr_bit is the size of a target address as represented in gdb
v:int:addr_bit:::8 * sizeof (void*):0:gdbarch_ptr_bit (gdbarch):
#
+# 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.
+#
+# Note that dwarf2_addr_size only needs to be redefined by a target if the
+# GCC back-end defines a DWARF2_ADDR_SIZE other than the target pointer size,
+# and if Dwarf versions < 4 need to be supported.
+v:int:dwarf2_addr_size:::sizeof (void*):0:gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT:
+#
# One if \`char' acts like \`signed char', zero if \`unsigned char'.
v:int:char_signed:::1:-1:1
#
Index: xstormy16-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/xstormy16-tdep.c,v
retrieving revision 1.110
diff -u -p -r1.110 xstormy16-tdep.c
--- xstormy16-tdep.c 1 Jan 2010 07:31:46 -0000 1.110
+++ xstormy16-tdep.c 6 Aug 2010 10:41:27 -0000
@@ -809,6 +809,7 @@ xstormy16_gdbarch_init (struct gdbarch_i
set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
set_gdbarch_addr_bit (gdbarch, 4 * TARGET_CHAR_BIT);
+ set_gdbarch_dwarf2_addr_size (gdbarch, 4);
set_gdbarch_address_to_pointer (gdbarch, xstormy16_address_to_pointer);
set_gdbarch_pointer_to_address (gdbarch, xstormy16_pointer_to_address);
--
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat