From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30047 invoked by alias); 6 Aug 2010 10:48:20 -0000 Received: (qmail 30039 invoked by uid 22791); 6 Aug 2010 10:48:19 -0000 X-Spam-Check-By: sourceware.org Received: from aquarius.hirmke.de (HELO calimero.vinschen.de) (217.91.18.234) by sourceware.org (qpsmtpd/0.83/v0.83-20-g38e4449) with ESMTP; Fri, 06 Aug 2010 10:48:13 +0000 Received: by calimero.vinschen.de (Postfix, from userid 500) id E0A136D4364; Fri, 6 Aug 2010 12:48:10 +0200 (CEST) Date: Fri, 06 Aug 2010 10:48:00 -0000 From: Corinna Vinschen To: gdb-patches@sourceware.org Subject: Re: [rfa] frame address size incorrect if address size != ptr size Message-ID: <20100806104810.GJ4610@calimero.vinschen.de> Reply-To: gdb-patches@sourceware.org Mail-Followup-To: gdb-patches@sourceware.org References: <20100805153010.GG4610@calimero.vinschen.de> <201008051651.o75GpmTo002448@d12av02.megacenter.de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <201008051651.o75GpmTo002448@d12av02.megacenter.de.ibm.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-08/txt/msg00060.txt.bz2 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