* Testing Call frame information in .debug_frame section @ 2011-02-01 13:04 Anitha Boyapati 2011-02-13 2:34 ` Petr Hluzín 0 siblings, 1 reply; 24+ messages in thread From: Anitha Boyapati @ 2011-02-01 13:04 UTC (permalink / raw) To: gdb Hello, I would like to use some help with regarding to call frame information present in .debug_frame section in DWARF - 2. + Firstly, I understand that .debug_frame can be used to unwind the stack. + Secondly, I understand that .debug_frame is optional to gdb. http://sourceware.org/gdb/papers/unwind.html Now I am trying to fix a bug in AVR port of gcc to emit call-frame information (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17994). The problem is testing. Initially, I thought avr-gdb can be used to backtrace/up/down with the newly generated elf file (with a .debug_frame section in it). However, avr-gdb does not seem to use this particular section. The version is 7.0.1. I am not sure how to test the FDEs in .debug_frame section other than manual inspection. I am sort of newbie with DWARF-2. So, I could be missing some pointers. Can someone suggest a strategy? or better yet can someone shed information on which features of gdb uses .debug_frame section. Thanks Anitha ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-01 13:04 Testing Call frame information in .debug_frame section Anitha Boyapati @ 2011-02-13 2:34 ` Petr Hluzín 2011-02-13 9:57 ` Anitha Boyapati 0 siblings, 1 reply; 24+ messages in thread From: Petr Hluzín @ 2011-02-13 2:34 UTC (permalink / raw) To: Anitha Boyapati; +Cc: gdb Hi Anitha On 1 February 2011 14:03, Anitha Boyapati <anitha.boyapati@gmail.com> wrote: > Hello, > > I would like to use some help with regarding to call frame information > present in .debug_frame section in DWARF - 2. > > + Firstly, I understand that .debug_frame can be used to unwind the stack. > + Secondly, I understand that .debug_frame is optional to gdb. > http://sourceware.org/gdb/papers/unwind.html > > Now I am trying to fix a bug in AVR port of gcc to emit call-frame > information (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17994). The > problem is testing. > > Initially, I thought avr-gdb can be used to backtrace/up/down with the > newly generated elf file (with a .debug_frame section in it). However, > avr-gdb does not seem to use this particular section. The version is > 7.0.1. I am not sure how to test the FDEs in .debug_frame section > other than manual inspection. I am sort of newbie with DWARF-2. So, I > could be missing some pointers. > > Can someone suggest a strategy? or better yet can someone shed > information on which features of gdb uses .debug_frame section. > > Thanks > Anitha > GDB does not use CFI on Atmel AVR, see avr-tdep.c. There is only one call to frame_unwind_append_unwinder(). I am glad to hear that GCC can somehow emit CFI data. What is the state of the implementation? I tried to use CFI statements in GAS (binutils), but it cannot parse them and provides misleading diagnostics. (I even submitted a patch for that but never received any response.) I tried to improve GDB's ability to scan prologues a year ago, however I ran out of enthusiasm. (The better scanner would allow stack reconstruction even without .debug_frame/CFI info.) I think I know where and how to hook the new CFI-based unwinder. However I am not familiar with .debug_frame unwinding in GDB. I am interested in writing the unwinder. I will need an example ELF with trivial code run-able in simulator and lots of faith/enthusiasm. (Assuming the compiler implements this spec: http://dwarfstd.org/doc/DWARF4.pdf, or dwarf-2.0.0.pdf) -- Petr Hluzin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-13 2:34 ` Petr Hluzín @ 2011-02-13 9:57 ` Anitha Boyapati 2011-02-13 15:11 ` Petr Hluzín 2011-02-14 16:42 ` Tom Tromey 0 siblings, 2 replies; 24+ messages in thread From: Anitha Boyapati @ 2011-02-13 9:57 UTC (permalink / raw) To: Petr Hluzín; +Cc: gdb Hello Petr, On 13 February 2011 08:04, Petr Hluzín <petr.hluzin@gmail.com> wrote: > > > GDB does not use CFI on Atmel AVR, see avr-tdep.c. There is only one > call to frame_unwind_append_unwinder(). Yes. I have looked into the code and it looked like the support is not present. But my understanding is still incomplete. frame-unwind.h/c files are what I have been looking through. Even if it is supported in other targets like ARM perhaps, how do we use that? Which of the gdb commands' processing involves frame unwinding following dwarf2/3/4...? > > I am glad to hear that GCC can somehow emit CFI data. What is the > state of the implementation? It has macros which is defined can fill .debug_frame with CIE and FDE information. The implementation from target side is very minimal. * The prologue instructions that modify frame/stack pointer have to be marked with RTX_FRAME_RELATED_P(). * The hook INCOMING_RETURN_ADDR_RTX should know how to access return address on stack. The return address can be stored in memory or in some register. * Likewise, hook INCOMING_FRAME_SP_OFFSET should specify the stack pointer offset from its previous frame but before prologue of the current function. The macros are documented in stack layout and calling conventions of gcc internals. Emitting CFI is optional in GCC. I used DWARF2 standard and the output seemed to comply it. (For now I am manually verifying the dump of debug_frame section) > I tried to use CFI statements in GAS (binutils), but it cannot parse > them and provides misleading diagnostics. (I even submitted a patch > for that but never received any response.) Sounds interesting. Can you provide the link? > > I tried to improve GDB's ability to scan prologues a year ago, however > I ran out of enthusiasm. (The better scanner would allow stack > reconstruction even without .debug_frame/CFI info.) > How does GDB currently use information from .debug_frame section? From the chat rooms I came to know that current GDB can limp along without CFI information. It tries to scan the prologues and epilogues and build frame information is what I came to know. Although I did not analyze how. > I think I know where and how to hook the new CFI-based unwinder. > However I am not familiar with .debug_frame unwinding in GDB. > I am interested in writing the unwinder. I will need an example ELF > with trivial code run-able in simulator and lots of faith/enthusiasm. > (Assuming the compiler implements this spec: > http://dwarfstd.org/doc/DWARF4.pdf, or dwarf-2.0.0.pdf) > :-) Anitha > -- > Petr Hluzin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-13 9:57 ` Anitha Boyapati @ 2011-02-13 15:11 ` Petr Hluzín 2011-02-15 17:41 ` Richard Henderson 2011-02-15 18:18 ` Testing Call frame information in .debug_frame section Anitha Boyapati 2011-02-14 16:42 ` Tom Tromey 1 sibling, 2 replies; 24+ messages in thread From: Petr Hluzín @ 2011-02-13 15:11 UTC (permalink / raw) To: Anitha Boyapati; +Cc: gdb Hi Anitha On 13 February 2011 10:57, Anitha Boyapati <anitha.boyapati@gmail.com> wrote: > Hello Petr, > > On 13 February 2011 08:04, Petr Hluzín <petr.hluzin@gmail.com> wrote: >> >> >> GDB does not use CFI on Atmel AVR, see avr-tdep.c. There is only one >> call to frame_unwind_append_unwinder(). > > Yes. I have looked into the code and it looked like the support is not > present. But my understanding is still incomplete. frame-unwind.h/c > files are what I have been looking through. Even if it is supported in > other targets like ARM perhaps, how do we use that? Which of the gdb > commands' processing involves frame unwinding following dwarf2/3/4...? I do not know how is .debug_frame/dwarf/CFI used on any arch. If an architecture uses debugging info then "backtrace" and many other commands use the info. >> >> I am glad to hear that GCC can somehow emit CFI data. What is the >> state of the implementation? > > ... > The macros are documented in stack layout and calling conventions of > gcc internals. Emitting CFI is optional in GCC. I used DWARF2 standard > and the output seemed to comply it. (For now I am manually verifying > the dump of debug_frame section) So it seems to be fished in avr-gcc? >> I tried to use CFI statements in GAS (binutils), but it cannot parse >> them and provides misleading diagnostics. (I even submitted a patch >> for that but never received any response.) > > Sounds interesting. Can you provide the link? Just go to binutils web and search for my name in the patch tracker. Oh wait, they do not have one. Maybe this is the reason they forgot to reply to my patches. Here you have it: http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html (You might also like "add pretty-printing of AVR register names": http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00108.html) >> >> I tried to improve GDB's ability to scan prologues a year ago, however >> I ran out of enthusiasm. (The better scanner would allow stack >> reconstruction even without .debug_frame/CFI info.) >> > > How does GDB currently use information from .debug_frame section? From > the chat rooms I came to know that current GDB can limp along without > CFI information. It tries to scan the prologues and epilogues and > build frame information is what I came to know. Although I did not > analyze how. It does not use .debug_frame section (on AVR). It only looks up the starting address of current function (the prologue scanner needs just that). -- Petr Hluzin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-13 15:11 ` Petr Hluzín @ 2011-02-15 17:41 ` Richard Henderson 2011-02-15 18:09 ` Anitha Boyapati 2011-02-15 19:03 ` [avr] gas support for cfi info Richard Henderson 2011-02-15 18:18 ` Testing Call frame information in .debug_frame section Anitha Boyapati 1 sibling, 2 replies; 24+ messages in thread From: Richard Henderson @ 2011-02-15 17:41 UTC (permalink / raw) To: Petr Hluzín; +Cc: Anitha Boyapati, gdb, binutils On 02/13/2011 07:10 AM, Petr HluzÃn wrote: > http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html I'll agree that a better error message would be helpful. To answer a question within that message: > By the way: Why AVR target does not understand CFI? What needs to be > done in binutils? And in GDB? TARGET_USE_CFIPOP DWARF2_DEFAULT_RETURN_COLUMN DWARF2_CIE_DATA_ALIGNMENT DWARF2_LINE_MIN_INSN_LENGTH are the macros that need to be defined, tc_cfi_frame_initial_instructions may be required depending on what the state of the unwind info incoming to a function. Have a look at tc-i386.c, tc_x86_frame_initial_instructions for a typical stack-based call mechanism. For the nearly related task of dwarf2 line numbers, you need a call to dwarf2_emit_insn emitted immediately before each insn is added to the frags. Again, see tc-i386.c for ideas. r~ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-15 17:41 ` Richard Henderson @ 2011-02-15 18:09 ` Anitha Boyapati 2011-02-15 18:48 ` Richard Henderson 2011-02-15 19:03 ` [avr] gas support for cfi info Richard Henderson 1 sibling, 1 reply; 24+ messages in thread From: Anitha Boyapati @ 2011-02-15 18:09 UTC (permalink / raw) To: Richard Henderson; +Cc: gdb, binutils On 15 February 2011 23:11, Richard Henderson <rth@redhat.com> wrote: > > On 02/13/2011 07:10 AM, Petr Hluzín wrote: > > http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html > > I'll agree that a better error message would be helpful. > > To answer a question within that message: > > > By the way: Why AVR target does not understand CFI? What needs to be > > done in binutils? And in GDB? > > TARGET_USE_CFIPOP > DWARF2_DEFAULT_RETURN_COLUMN > DWARF2_CIE_DATA_ALIGNMENT > DWARF2_LINE_MIN_INSN_LENGTH > > are the macros that need to be defined, I am a little confused here. I was under the impression that changes to GCC files alone would suffice. I am missing something here. Are the above mentioned changes required for assembling CFI information in assembly files in binutils? ( I see that i386 defines them in gas) > > tc_cfi_frame_initial_instructions > > may be required depending on what the state of the unwind > info incoming to a function. Have a look at tc-i386.c, > tc_x86_frame_initial_instructions for a typical stack-based > call mechanism. > > For the nearly related task of dwarf2 line numbers, you need > a call to dwarf2_emit_insn emitted immediately before each > insn is added to the frags. Again, see tc-i386.c for ideas. > > Anitha ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-15 18:09 ` Anitha Boyapati @ 2011-02-15 18:48 ` Richard Henderson 2011-02-15 19:15 ` Anitha Boyapati 0 siblings, 1 reply; 24+ messages in thread From: Richard Henderson @ 2011-02-15 18:48 UTC (permalink / raw) To: Anitha Boyapati; +Cc: gdb, binutils On 02/15/2011 10:09 AM, Anitha Boyapati wrote: > I am a little confused here. I was under the impression that changes > to GCC files alone would suffice. I am missing something here. Are the > above mentioned changes required for assembling CFI information in > assembly files in binutils? GCC *can* produce cfi information by itself without assembler help, but can produce slightly more compact cfi information *with* help. In addition, with assembler support it's easy to write cfi info to go along with hand-written assembly. r~ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-15 18:48 ` Richard Henderson @ 2011-02-15 19:15 ` Anitha Boyapati 0 siblings, 0 replies; 24+ messages in thread From: Anitha Boyapati @ 2011-02-15 19:15 UTC (permalink / raw) To: Richard Henderson; +Cc: gdb, binutils On 16 February 2011 00:18, Richard Henderson <rth@redhat.com> wrote: > On 02/15/2011 10:09 AM, Anitha Boyapati wrote: >> I am a little confused here. I was under the impression that changes >> to GCC files alone would suffice. I am missing something here. Are the >> above mentioned changes required for assembling CFI information in >> assembly files in binutils? > > GCC *can* produce cfi information by itself without assembler help, > but can produce slightly more compact cfi information *with* help. > In addition, with assembler support it's easy to write cfi info to > go along with hand-written assembly. Ok. I was skimming through binutils ml archives for more info. Basically I have implemented CFI emission in AVR-GCC http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17994 I have no idea that somewhat similar changes are required in binutils too. Upon that the manual examination of output appeared correct. I am using binutils 2.20.1. Will see what happens on supporting the above macros. I wish there is some kind of updated binutils internals document like that of GCC's for some references. Thanks Richard! Anitha. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [avr] gas support for cfi info 2011-02-15 17:41 ` Richard Henderson 2011-02-15 18:09 ` Anitha Boyapati @ 2011-02-15 19:03 ` Richard Henderson 2011-02-15 22:45 ` Petr Hluzín 1 sibling, 1 reply; 24+ messages in thread From: Richard Henderson @ 2011-02-15 19:03 UTC (permalink / raw) To: Petr Hluzín Cc: Anitha Boyapati, binutils, gdb, GCC Patches, chertykov, aesok, eric.weddington [-- Attachment #1: Type: text/plain, Size: 1441 bytes --] On 02/15/2011 09:41 AM, Richard Henderson wrote: > On 02/13/2011 07:10 AM, Petr HluzÃn wrote: >> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html > > I'll agree that a better error message would be helpful. > > To answer a question within that message: > >> By the way: Why AVR target does not understand CFI? What needs to be >> done in binutils? And in GDB? > > TARGET_USE_CFIPOP > DWARF2_DEFAULT_RETURN_COLUMN > DWARF2_CIE_DATA_ALIGNMENT > DWARF2_LINE_MIN_INSN_LENGTH > > are the macros that need to be defined, > > tc_cfi_frame_initial_instructions > > may be required depending on what the state of the unwind > info incoming to a function. Have a look at tc-i386.c, > tc_x86_frame_initial_instructions for a typical stack-based > call mechanism. > > For the nearly related task of dwarf2 line numbers, you need > a call to dwarf2_emit_insn emitted immediately before each > insn is added to the frags. Again, see tc-i386.c for ideas. To follow up on myself, it appears as if avr already has dwarf2 line number support, and only needs a few things in order to enable cfi support. CC'd to gcc and gdb because the dwarf2 register numbers for SP and the return address column need to be coordinated. This is part of the target's ABI. I've left a ??? marker for when AVR_3_BYTE_PC would be true in gcc; I haven't tracked down how that maps into the assembler, or even if there is a simple mapping. r~ [-- Attachment #2: zz --] [-- Type: text/plain, Size: 1729 bytes --] Index: config/tc-avr.c =================================================================== RCS file: /cvs/src/src/gas/config/tc-avr.c,v retrieving revision 1.74 diff -u -p -r1.74 tc-avr.c --- config/tc-avr.c 28 Jun 2010 14:06:57 -0000 1.74 +++ config/tc-avr.c 15 Feb 2011 18:52:05 -0000 @@ -24,6 +24,8 @@ #include "as.h" #include "safe-ctype.h" #include "subsegs.h" +#include "dw2gencfi.h" + struct avr_opcodes_s { @@ -1488,3 +1490,12 @@ avr_cons_fix_new (fragS *frag, exp_mod_pm = 0; } } + +void +tc_cfi_frame_initial_instructions (void) +{ + /* ??? How do we tell if we're in 3-byte pc mode? */ + /* The CFA is immediately above the return address, which is on the stack. */ + cfi_add_CFA_def_cfa (32, 2); + cfi_add_CFA_offset (DWARF2_DEFAULT_RETURN_COLUMN, -2); +} Index: config/tc-avr.h =================================================================== RCS file: /cvs/src/src/gas/config/tc-avr.h,v retrieving revision 1.17 diff -u -p -r1.17 tc-avr.h --- config/tc-avr.h 27 Oct 2009 15:39:27 -0000 1.17 +++ config/tc-avr.h 15 Feb 2011 18:52:05 -0000 @@ -153,3 +153,17 @@ extern long md_pcrel_from_section (struc /* 32 bits pseudo-addresses are used on AVR. */ #define DWARF2_ADDR_SIZE(bfd) 4 + +/* Enable cfi directives. */ +#define TARGET_USE_CFIPOP 1 + +/* The stack grows down, and is only byte aligned. */ +#define DWARF2_CIE_DATA_ALIGNMENT -1 + +/* Define the column that represents the PC. */ +/* ??? This is an abi thing; coordinate with other projects. */ +#define DWARF2_DEFAULT_RETURN_COLUMN 36 + +/* Define a hook to setup initial CFI state. */ +extern void tc_cfi_frame_initial_instructions (void); +#define tc_cfi_frame_initial_instructions tc_cfi_frame_initial_instructions ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-15 19:03 ` [avr] gas support for cfi info Richard Henderson @ 2011-02-15 22:45 ` Petr Hluzín 2011-02-16 17:59 ` Richard Henderson 0 siblings, 1 reply; 24+ messages in thread From: Petr Hluzín @ 2011-02-15 22:45 UTC (permalink / raw) To: Richard Henderson Cc: Anitha Boyapati, binutils, gdb, GCC Patches, chertykov, aesok, eric.weddington On 15 February 2011 20:03, Richard Henderson <rth@redhat.com> wrote: > On 02/15/2011 09:41 AM, Richard Henderson wrote: >> On 02/13/2011 07:10 AM, Petr Hluzín wrote: >>> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html >> >> I'll agree that a better error message would be helpful. >> >> To answer a question within that message: >> >>> By the way: Why AVR target does not understand CFI? What needs to be >>> done in binutils? And in GDB? >> >> TARGET_USE_CFIPOP >> DWARF2_DEFAULT_RETURN_COLUMN >> DWARF2_CIE_DATA_ALIGNMENT >> DWARF2_LINE_MIN_INSN_LENGTH >> >> are the macros that need to be defined, >> >> tc_cfi_frame_initial_instructions >> >> may be required depending on what the state of the unwind >> info incoming to a function. Have a look at tc-i386.c, >> tc_x86_frame_initial_instructions for a typical stack-based >> call mechanism. >> >> For the nearly related task of dwarf2 line numbers, you need >> a call to dwarf2_emit_insn emitted immediately before each >> insn is added to the frags. Again, see tc-i386.c for ideas. > > To follow up on myself, it appears as if avr already has dwarf2 > line number support, and only needs a few things in order to > enable cfi support. > > CC'd to gcc and gdb because the dwarf2 register numbers for SP > and the return address column need to be coordinated. This is > part of the target's ABI. In avr-tdep.c [1] near avr_dwarf_reg_to_regnum(): /* Unfortunately dwarf2 register for SP is 32. */ (I can't help you with the value for #define DWARF2_DEFAULT_RETURN_COLUMN 36) AFAIK there is no written ABI. Only the calling convention is documented (and only the easy cases), the rest is in gdb/gcc/binutils sources and people's heads. > I've left a ??? marker for when AVR_3_BYTE_PC would be true in > gcc; I haven't tracked down how that maps into the assembler, > or even if there is a simple mapping. In avr_gdbarch_init() in avr-tdep.c [1]: /* Avr-6 call instructions save 3 bytes. */ switch (info.bfd_arch_info->mach) ... case bfd_mach_avr6: call_length = 3; break; [1] http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/src/gdb/avr-tdep.c?rev=1.128&content-type=text/plain&cvsroot=src -- Petr Hluzin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-15 22:45 ` Petr Hluzín @ 2011-02-16 17:59 ` Richard Henderson 2011-02-16 22:49 ` Petr Hluzín 2011-02-17 15:35 ` Anitha Boyapati 0 siblings, 2 replies; 24+ messages in thread From: Richard Henderson @ 2011-02-16 17:59 UTC (permalink / raw) To: Petr Hluzín Cc: Anitha Boyapati, binutils, gdb, GCC Patches, chertykov, aesok, eric.weddington [-- Attachment #1: Type: text/plain, Size: 1528 bytes --] On 02/15/2011 02:44 PM, Petr HluzÃn wrote: > In avr-tdep.c [1] near avr_dwarf_reg_to_regnum(): > /* Unfortunately dwarf2 register for SP is 32. */ Excellent. We're all on the same page for this. > (I can't help you with the value for #define DWARF2_DEFAULT_RETURN_COLUMN 36) > AFAIK there is no written ABI. Only the calling convention is > documented (and only the easy cases), the rest is in gdb/gcc/binutils > sources and people's heads. As I recall, GCC defaults to using FIRST_PSEUDO_REGISTER for this, so as to not overlap any hard registers. I'll continue to so the same. > /* Avr-6 call instructions save 3 bytes. */ > switch (info.bfd_arch_info->mach) Thanks. That value is readily available in the assembler as well. Anitha pointed out to me via gcc pr17994 that AVR uses post-decrement for its pushes. I had a brief read over the AVR insn manual, and it's not crystal clear how multi-byte post-decrement pushes operate. I've made an assumption that it happens as-if each byte is pushed separately. I.e. caller: callee: save rN save rM trash <- SP hi(ret) <- CFA lo(ret) trash <- SP This is the only way I can imagine that call insns interoperate with byte push/pop insns. All of which means that the return address is at a different offset from the CFA than I originally thought. This ought to be fixed in the following. Can someone please test these two patches and see if they actually agree with the hardware? r~ [-- Attachment #2: z-gas --] [-- Type: text/plain, Size: 1947 bytes --] Index: config/tc-avr.c =================================================================== RCS file: /cvs/src/src/gas/config/tc-avr.c,v retrieving revision 1.74 diff -u -p -r1.74 tc-avr.c --- config/tc-avr.c 28 Jun 2010 14:06:57 -0000 1.74 +++ config/tc-avr.c 16 Feb 2011 17:39:55 -0000 @@ -24,6 +24,8 @@ #include "as.h" #include "safe-ctype.h" #include "subsegs.h" +#include "dw2gencfi.h" + struct avr_opcodes_s { @@ -1488,3 +1490,18 @@ avr_cons_fix_new (fragS *frag, exp_mod_pm = 0; } } + +void +tc_cfi_frame_initial_instructions (void) +{ + /* AVR6 pushes 3 bytes for calls. */ + int return_size = (avr_mcu->mach == bfd_mach_avr6 ? 3 : 2); + + /* The CFA is the caller's stack location before the call insn. */ + /* Note that the stack pointer is dwarf register number 32. */ + cfi_add_CFA_def_cfa (32, return_size); + + /* Note that AVR consistently uses post-decrement, which means that things + do not line up the same way as for targers that use pre-decrement. */ + cfi_add_CFA_offset (DWARF2_DEFAULT_RETURN_COLUMN, 1-return_size); +} Index: config/tc-avr.h =================================================================== RCS file: /cvs/src/src/gas/config/tc-avr.h,v retrieving revision 1.17 diff -u -p -r1.17 tc-avr.h --- config/tc-avr.h 27 Oct 2009 15:39:27 -0000 1.17 +++ config/tc-avr.h 16 Feb 2011 17:39:55 -0000 @@ -153,3 +153,16 @@ extern long md_pcrel_from_section (struc /* 32 bits pseudo-addresses are used on AVR. */ #define DWARF2_ADDR_SIZE(bfd) 4 + +/* Enable cfi directives. */ +#define TARGET_USE_CFIPOP 1 + +/* The stack grows down, and is only byte aligned. */ +#define DWARF2_CIE_DATA_ALIGNMENT -1 + +/* Define the column that represents the PC. */ +#define DWARF2_DEFAULT_RETURN_COLUMN 36 + +/* Define a hook to setup initial CFI state. */ +extern void tc_cfi_frame_initial_instructions (void); +#define tc_cfi_frame_initial_instructions tc_cfi_frame_initial_instructions [-- Attachment #3: z-gcc --] [-- Type: text/plain, Size: 3012 bytes --] diff --git a/gcc/config/avr/avr-protos.h b/gcc/config/avr/avr-protos.h index 4569359..06c9254 100644 --- a/gcc/config/avr/avr-protos.h +++ b/gcc/config/avr/avr-protos.h @@ -109,6 +109,7 @@ extern RTX_CODE avr_normalize_condition (RTX_CODE condition); extern int compare_eq_p (rtx insn); extern void out_shift_with_cnt (const char *templ, rtx insn, rtx operands[], int *len, int t_len); +extern rtx avr_incoming_return_addr_rtx (void); #endif /* RTX_CODE */ #ifdef HAVE_MACHINE_MODES diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c index 30e4626..761eea3 100644 --- a/gcc/config/avr/avr.c +++ b/gcc/config/avr/avr.c @@ -534,6 +534,16 @@ get_sequence_length (rtx insns) return length; } +/* Implement INCOMING_RETURN_ADDR_RTX. */ + +rtx +avr_incoming_return_addr_rtx (void) +{ + /* The return address is at the top of the stack. Note that the push + was via post-decrement, which means the actual address is off by one. */ + return gen_frame_mem (HImode, plus_constant (stack_pointer_rtx, 1)); +} + /* Output function prologue. */ void diff --git a/gcc/config/avr/avr.h b/gcc/config/avr/avr.h index 9a71078..385b4e6 100644 --- a/gcc/config/avr/avr.h +++ b/gcc/config/avr/avr.h @@ -809,6 +809,9 @@ mmcu=*:-mmcu=%*}" #define OBJECT_FORMAT_ELF +#define INCOMING_RETURN_ADDR_RTX avr_incoming_return_addr_rtx () +#define INCOMING_FRAME_SP_OFFSET (AVR_3_BYTE_PC ? 3 : 2) + #define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \ avr_hard_regno_rename_ok (OLD_REG, NEW_REG) diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index fea8209..b5e660c 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2240,7 +2240,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label) cfa.base_offset = -cfa_store.offset Rule 11: - (set (mem ({pre_inc,pre_dec} sp:cfa_store.reg)) <reg>) + (set (mem ({pre_inc,pre_dec,post_dec} sp:cfa_store.reg)) <reg>) effects: cfa_store.offset += -/+ mode_size(mem) cfa.offset = cfa_store.offset if cfa.reg == sp cfa.reg = sp @@ -2259,7 +2259,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label) cfa.base_offset = -{cfa_store,cfa_temp}.offset Rule 14: - (set (mem (postinc <reg1>:cfa_temp <const_int>)) <reg2>) + (set (mem (post_inc <reg1>:cfa_temp <const_int>)) <reg2>) effects: cfa.reg = <reg1> cfa.base_offset = -cfa_temp.offset cfa_temp.offset -= mode_size(mem) @@ -2592,6 +2592,7 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label) /* Rule 11 */ case PRE_INC: case PRE_DEC: + case POST_DEC: offset = GET_MODE_SIZE (GET_MODE (dest)); if (GET_CODE (XEXP (dest, 0)) == PRE_INC) offset = -offset; @@ -2616,7 +2617,10 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label) if (cfa.reg == STACK_POINTER_REGNUM) cfa.offset = cfa_store.offset; - offset = -cfa_store.offset; + if (GET_CODE (XEXP (dest, 0)) == POST_DEC) + offset += -cfa_store.offset; + else + offset = -cfa_store.offset; break; /* Rule 12 */ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-16 17:59 ` Richard Henderson @ 2011-02-16 22:49 ` Petr Hluzín 2011-02-17 16:12 ` Richard Henderson 2011-02-17 15:35 ` Anitha Boyapati 1 sibling, 1 reply; 24+ messages in thread From: Petr Hluzín @ 2011-02-16 22:49 UTC (permalink / raw) To: Richard Henderson Cc: Anitha Boyapati, binutils, gdb, GCC Patches, chertykov, aesok, eric.weddington On 16 February 2011 18:58, Richard Henderson <rth@redhat.com> wrote: > Anitha pointed out to me via gcc pr17994 that AVR uses post-decrement > for its pushes. I had a brief read over the AVR insn manual, and it's > not crystal clear how multi-byte post-decrement pushes operate. > > I've made an assumption that it happens as-if each byte is pushed > separately. I.e. > > caller: callee: > save rN > save rM > trash <- SP hi(ret) <- CFA > lo(ret) > trash <- SP > > This is the only way I can imagine that call insns interoperate with > byte push/pop insns. Yes, except the bytes of return address are in big-endian. See avr-tdep.c in function avr_frame_prev_register(): /* ... And to confuse matters even more, the return address stored on the stack is in big endian byte order, even though most everything else about the avr is little endian. Ick! */ > All of which means that the return address is at a different offset > from the CFA than I originally thought. This ought to be fixed in > the following. > > Can someone please test these two patches and see if they actually > agree with the hardware? What should I look for when testing? The layout of stack you draw is correct (minus the endianity), see avr-tdep.c: /* Any function with a frame looks like this ... -- Petr Hluzin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-16 22:49 ` Petr Hluzín @ 2011-02-17 16:12 ` Richard Henderson 2011-02-17 16:16 ` Tristan Gingold 0 siblings, 1 reply; 24+ messages in thread From: Richard Henderson @ 2011-02-17 16:12 UTC (permalink / raw) To: Petr Hluzín Cc: Anitha Boyapati, binutils, gdb, GCC Patches, chertykov, aesok, eric.weddington On 02/16/2011 02:47 PM, Petr HluzÃn wrote: > What should I look for when testing? Run the gdb testsuite with dwarf-3 enabled. Either by editing the default in the compiler, or by some dejagnu argument that compiles the tests with -gdwarf-3. The use of Dwarf3 enables the use of DW_OP_call_frame_cfa in the DW_AT_frame_base field, which means that the .debug_frame info will be used every time local variables are referenced, as well as for unwinding the stack. If lots of tests fail, see if you can determine if the address computed for the local variable -- or even more particularly a function parameter -- is off by a byte. I have an idea that we're missing a definition of #define ARG_POINTER_CFA_OFFSET(FNDECL) -1 in GCC. r~ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-17 16:12 ` Richard Henderson @ 2011-02-17 16:16 ` Tristan Gingold 0 siblings, 0 replies; 24+ messages in thread From: Tristan Gingold @ 2011-02-17 16:16 UTC (permalink / raw) To: Richard Henderson Cc: Petr Hluzín, Anitha Boyapati, binutils, gdb, GCC Patches, chertykov, aesok, eric.weddington On Feb 17, 2011, at 5:12 PM, Richard Henderson wrote: > On 02/16/2011 02:47 PM, Petr Hluzín wrote: >> What should I look for when testing? > > Run the gdb testsuite with dwarf-3 enabled. Either by editing the > default in the compiler, or by some dejagnu argument that compiles > the tests with -gdwarf-3. Note that gdb includes an avr simulator (if this could simplify your testing). ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-16 17:59 ` Richard Henderson 2011-02-16 22:49 ` Petr Hluzín @ 2011-02-17 15:35 ` Anitha Boyapati 2011-02-17 16:05 ` Richard Henderson 1 sibling, 1 reply; 24+ messages in thread From: Anitha Boyapati @ 2011-02-17 15:35 UTC (permalink / raw) To: Richard Henderson, GCC Patches Cc: Petr Hluzín, binutils, gdb, chertykov, aesok, eric.weddington [-- Attachment #1: Type: text/plain, Size: 2931 bytes --] On 16 February 2011 23:28, Richard Henderson <rth@redhat.com> wrote: > On 02/15/2011 02:44 PM, Petr Hluzín wrote: >> In avr-tdep.c [1] near avr_dwarf_reg_to_regnum(): >> /* Unfortunately dwarf2 register for SP is 32. */ > > Excellent. We're all on the same page for this. > >> (I can't help you with the value for #define DWARF2_DEFAULT_RETURN_COLUMN 36) >> AFAIK there is no written ABI. Only the calling convention is >> documented (and only the easy cases), the rest is in gdb/gcc/binutils >> sources and people's heads. > > As I recall, GCC defaults to using FIRST_PSEUDO_REGISTER for this, > so as to not overlap any hard registers. I'll continue to so the same. > >> /* Avr-6 call instructions save 3 bytes. */ >> switch (info.bfd_arch_info->mach) > > Thanks. That value is readily available in the assembler as well. > > Anitha pointed out to me via gcc pr17994 that AVR uses post-decrement > for its pushes. I had a brief read over the AVR insn manual, and it's > not crystal clear how multi-byte post-decrement pushes operate. > > I've made an assumption that it happens as-if each byte is pushed > separately. I.e. > > caller: callee: > save rN > save rM > trash <- SP hi(ret) <- CFA > lo(ret) > trash <- SP > > This is the only way I can imagine that call insns interoperate with > byte push/pop insns. > The stack layout is correct. For call/rcall instructions, PC-low is pushed first followed by a PC-high. (I just verified by running/debugging a small app on the device) > All of which means that the return address is at a different offset > from the CFA than I originally thought. This ought to be fixed in > the following. Can you please explain the logic behind the following lines in gcc patch: - offset = -cfa_store.offset; + if (GET_CODE (XEXP (dest, 0)) == POST_DEC) + offset += -cfa_store.offset; + else + offset = -cfa_store.offset; > > Can someone please test these two patches and see if they actually > agree with the hardware? I have tried only compiler patch. Please refer to the attached output for a small testcase. (avr-objdump -WfF). It appeared correct to me. However I have one simple question with regarding the output: The CFI instructions for registers have changed only after the prologue. (For convenience I have attached disassembly too). As far as I understand, DWARF2 spec emits CFI instructions immediately. (Appendix 5 of DWARF2 specification) The other scenario is - how about functions with signals/interrupts? The compiler will give an ICE compiling a function as below: void my_interrupt_handler() __attribute__ (("interrupt")); Likewise, for signal attribute too. I am going to apply assembler patch and test it. Will get back on it shortly. Anitha > > r~ > [-- Attachment #2: call-saved.txt --] [-- Type: text/plain, Size: 5684 bytes --] call-saved: file format elf32-avr Contents of the .debug_frame section: 00000000 00000010 ffffffff CIE Version: 1 Augmentation: "" Code alignment factor: 1 Data alignment factor: -1 Return address column: 36 DW_CFA_def_cfa: r32 ofs 2 DW_CFA_offset: r36 at cfa-1 DW_CFA_nop DW_CFA_nop 00000014 0000006c 00000000 FDE cie=00000000 pc=00000000..000000ce DW_CFA_advance_loc: 2 to 00000002 DW_CFA_def_cfa_offset: 3 DW_CFA_advance_loc: 2 to 00000004 DW_CFA_def_cfa_offset: 4 DW_CFA_advance_loc: 2 to 00000006 DW_CFA_def_cfa_offset: 5 DW_CFA_advance_loc: 2 to 00000008 DW_CFA_def_cfa_offset: 6 DW_CFA_advance_loc: 2 to 0000000a DW_CFA_def_cfa_offset: 7 DW_CFA_advance_loc: 2 to 0000000c DW_CFA_def_cfa_offset: 8 DW_CFA_advance_loc: 2 to 0000000e DW_CFA_def_cfa_offset: 9 DW_CFA_advance_loc: 2 to 00000010 DW_CFA_def_cfa_offset: 10 DW_CFA_advance_loc: 2 to 00000012 DW_CFA_def_cfa_offset: 11 DW_CFA_advance_loc: 2 to 00000014 DW_CFA_def_cfa_offset: 12 DW_CFA_advance_loc: 2 to 00000016 DW_CFA_def_cfa_offset: 13 DW_CFA_advance_loc: 2 to 00000018 DW_CFA_def_cfa_offset: 14 DW_CFA_advance_loc: 2 to 0000001a DW_CFA_def_cfa_offset: 15 DW_CFA_advance_loc: 2 to 0000001c DW_CFA_def_cfa_offset: 16 DW_CFA_advance_loc: 2 to 0000001e DW_CFA_def_cfa_offset: 17 DW_CFA_advance_loc: 2 to 00000020 DW_CFA_def_cfa_offset: 18 DW_CFA_advance_loc: 4 to 00000024 DW_CFA_def_cfa_offset: 20 DW_CFA_offset: r28 at cfa-18 DW_CFA_offset: r17 at cfa-17 DW_CFA_offset: r16 at cfa-16 DW_CFA_offset: r15 at cfa-15 DW_CFA_offset: r14 at cfa-14 DW_CFA_offset: r13 at cfa-13 DW_CFA_offset: r12 at cfa-12 DW_CFA_offset: r11 at cfa-11 DW_CFA_offset: r10 at cfa-10 DW_CFA_offset: r9 at cfa-9 DW_CFA_offset: r8 at cfa-8 DW_CFA_offset: r7 at cfa-7 DW_CFA_offset: r6 at cfa-6 DW_CFA_offset: r5 at cfa-5 DW_CFA_offset: r4 at cfa-4 DW_CFA_offset: r3 at cfa-3 DW_CFA_offset: r2 at cfa-2 DW_CFA_advance_loc: 4 to 00000028 DW_CFA_def_cfa_register: r28 DW_CFA_advance_loc: 2 to 0000002a DW_CFA_def_cfa_offset: 36 DW_CFA_advance_loc: 10 to 00000034 DW_CFA_def_cfa_register: r32 DW_CFA_nop DW_CFA_nop 00000084 00000014 00000000 FDE cie=00000000 pc=000000ce..000000de DW_CFA_advance_loc: 4 to 000000d2 DW_CFA_def_cfa_offset: 4 DW_CFA_offset: r28 at cfa-2 DW_CFA_advance_loc: 4 to 000000d6 DW_CFA_def_cfa_register: r28 call-saved: file format elf32-avr Contents of the .debug_frame section: 00000000 00000010 ffffffff CIE "" cf=1 df=-1 ra=36 LOC CFA ra 00000000 r32+2 c-1 00000014 0000006c 00000000 FDE cie=00000000 pc=00000000..000000ce LOC CFA r2 r3 r4 r5 r6 r7 r8 r9 r10 r11 r12 r13 r14 r15 r16 r17 r28 ra 00000000 r32+2 u u u u u u u u u u u u u u u u u c-1 00000002 r32+3 u u u u u u u u u u u u u u u u u c-1 00000004 r32+4 u u u u u u u u u u u u u u u u u c-1 00000006 r32+5 u u u u u u u u u u u u u u u u u c-1 00000008 r32+6 u u u u u u u u u u u u u u u u u c-1 0000000a r32+7 u u u u u u u u u u u u u u u u u c-1 0000000c r32+8 u u u u u u u u u u u u u u u u u c-1 0000000e r32+9 u u u u u u u u u u u u u u u u u c-1 00000010 r32+10 u u u u u u u u u u u u u u u u u c-1 00000012 r32+11 u u u u u u u u u u u u u u u u u c-1 00000014 r32+12 u u u u u u u u u u u u u u u u u c-1 00000016 r32+13 u u u u u u u u u u u u u u u u u c-1 00000018 r32+14 u u u u u u u u u u u u u u u u u c-1 0000001a r32+15 u u u u u u u u u u u u u u u u u c-1 0000001c r32+16 u u u u u u u u u u u u u u u u u c-1 0000001e r32+17 u u u u u u u u u u u u u u u u u c-1 00000020 r32+18 u u u u u u u u u u u u u u u u u c-1 00000024 r32+20 c-2 c-3 c-4 c-5 c-6 c-7 c-8 c-9 c-10 c-11 c-12 c-13 c-14 c-15 c-16 c-17 c-18 c-1 00000028 r28+20 c-2 c-3 c-4 c-5 c-6 c-7 c-8 c-9 c-10 c-11 c-12 c-13 c-14 c-15 c-16 c-17 c-18 c-1 0000002a r28+36 c-2 c-3 c-4 c-5 c-6 c-7 c-8 c-9 c-10 c-11 c-12 c-13 c-14 c-15 c-16 c-17 c-18 c-1 00000034 r32+36 c-2 c-3 c-4 c-5 c-6 c-7 c-8 c-9 c-10 c-11 c-12 c-13 c-14 c-15 c-16 c-17 c-18 c-1 00000084 00000014 00000000 FDE cie=00000000 pc=000000ce..000000de LOC CFA r28 ra 000000ce r32+2 u c-1 000000d2 r32+4 c-2 c-1 000000d6 r28+4 c-2 c-1 [-- Attachment #3: call-saved.c --] [-- Type: text/x-csrc, Size: 281 bytes --] int foo() { register a1, b1, c1, d1, e1, f1, g1, h1; register a2, b2, c2, d2, e2, f2, g2, h2; register result1 = a1+b1+c1+d1+e1+f1+g1+h1; register result2 = a2+b2+c2+d2+e2+f2+g2+h2; register result = result1 + result2; return result; } void main() { return foo(); } [-- Attachment #4: call-saved-disas.txt --] [-- Type: text/plain, Size: 3628 bytes --] call-saved: file format elf32-avr Disassembly of section .text: 00000000 <foo>: 0: 2f 92 push r2 2: 3f 92 push r3 4: 4f 92 push r4 6: 5f 92 push r5 8: 6f 92 push r6 a: 7f 92 push r7 c: 8f 92 push r8 e: 9f 92 push r9 10: af 92 push r10 12: bf 92 push r11 14: cf 92 push r12 16: df 92 push r13 18: ef 92 push r14 1a: ff 92 push r15 1c: 0f 93 push r16 1e: 1f 93 push r17 20: df 93 push r29 22: cf 93 push r28 24: cd b7 in r28, 0x3d ; 61 26: de b7 in r29, 0x3e ; 62 28: 60 97 sbiw r28, 0x10 ; 16 2a: 0f b6 in r0, 0x3f ; 63 2c: f8 94 cli 2e: de bf out 0x3e, r29 ; 62 30: 0f be out 0x3f, r0 ; 63 32: cd bf out 0x3d, r28 ; 61 34: 8e 2d mov r24, r14 36: 9f 2d mov r25, r15 38: 80 0f add r24, r16 3a: 91 1f adc r25, r17 3c: 29 81 ldd r18, Y+1 ; 0x01 3e: 3a 81 ldd r19, Y+2 ; 0x02 40: 82 0f add r24, r18 42: 93 1f adc r25, r19 44: 2b 81 ldd r18, Y+3 ; 0x03 46: 3c 81 ldd r19, Y+4 ; 0x04 48: 82 0f add r24, r18 4a: 93 1f adc r25, r19 4c: 2d 81 ldd r18, Y+5 ; 0x05 4e: 3e 81 ldd r19, Y+6 ; 0x06 50: 82 0f add r24, r18 52: 93 1f adc r25, r19 54: 2f 81 ldd r18, Y+7 ; 0x07 56: 38 85 ldd r19, Y+8 ; 0x08 58: 82 0f add r24, r18 5a: 93 1f adc r25, r19 5c: 29 85 ldd r18, Y+9 ; 0x09 5e: 3a 85 ldd r19, Y+10 ; 0x0a 60: 82 0f add r24, r18 62: 93 1f adc r25, r19 64: eb 84 ldd r14, Y+11 ; 0x0b 66: fc 84 ldd r15, Y+12 ; 0x0c 68: e8 0e add r14, r24 6a: f9 1e adc r15, r25 6c: 8d 85 ldd r24, Y+13 ; 0x0d 6e: 9e 85 ldd r25, Y+14 ; 0x0e 70: 2f 85 ldd r18, Y+15 ; 0x0f 72: 38 89 ldd r19, Y+16 ; 0x10 74: 82 0f add r24, r18 76: 93 1f adc r25, r19 78: 82 0d add r24, r2 7a: 93 1d adc r25, r3 7c: 84 0d add r24, r4 7e: 95 1d adc r25, r5 80: 86 0d add r24, r6 82: 97 1d adc r25, r7 84: 88 0d add r24, r8 86: 99 1d adc r25, r9 88: 8a 0d add r24, r10 8a: 9b 1d adc r25, r11 8c: 08 2f mov r16, r24 8e: 19 2f mov r17, r25 90: 0c 0d add r16, r12 92: 1d 1d adc r17, r13 94: 0e 0d add r16, r14 96: 1f 1d adc r17, r15 98: 80 2f mov r24, r16 9a: 91 2f mov r25, r17 9c: 60 96 adiw r28, 0x10 ; 16 9e: 0f b6 in r0, 0x3f ; 63 a0: f8 94 cli a2: de bf out 0x3e, r29 ; 62 a4: 0f be out 0x3f, r0 ; 63 a6: cd bf out 0x3d, r28 ; 61 a8: cf 91 pop r28 aa: df 91 pop r29 ac: 1f 91 pop r17 ae: 0f 91 pop r16 b0: ff 90 pop r15 b2: ef 90 pop r14 b4: df 90 pop r13 b6: cf 90 pop r12 b8: bf 90 pop r11 ba: af 90 pop r10 bc: 9f 90 pop r9 be: 8f 90 pop r8 c0: 7f 90 pop r7 c2: 6f 90 pop r6 c4: 5f 90 pop r5 c6: 4f 90 pop r4 c8: 3f 90 pop r3 ca: 2f 90 pop r2 cc: 08 95 ret 000000ce <main>: ce: df 93 push r29 d0: cf 93 push r28 d2: cd b7 in r28, 0x3d ; 61 d4: de b7 in r29, 0x3e ; 62 d6: 94 df rcall .-216 ; 0x0 <foo> d8: cf 91 pop r28 da: df 91 pop r29 dc: 08 95 ret ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-17 15:35 ` Anitha Boyapati @ 2011-02-17 16:05 ` Richard Henderson 2011-02-17 19:53 ` Richard Henderson 0 siblings, 1 reply; 24+ messages in thread From: Richard Henderson @ 2011-02-17 16:05 UTC (permalink / raw) To: Anitha Boyapati Cc: GCC Patches, Petr Hluzín, binutils, gdb, chertykov, aesok, eric.weddington On 02/17/2011 07:35 AM, Anitha Boyapati wrote: > Can you please explain the logic behind the following lines in gcc patch: > > > - offset = -cfa_store.offset; > + if (GET_CODE (XEXP (dest, 0)) == POST_DEC) > + offset += -cfa_store.offset; > + else > + offset = -cfa_store.offset; This is differentiating between pre-dec and post-dec. pre-dec post-dec before stuff stuff stuff <-sp trash <-sp trash after stuff stuff stuff value value <-sp trash <-sp We've just decremented cfa_store.offset by the size of the pushed value, and we're computing the offset of VALUE from the CFA. For pre-decrement, the value is stored at the CFA offset (the else); for post-decrement, the value is stored just above the CFA offset. I admit the logic is confusing here, because we're storing some quantities as positive offsets, and some quantities as negative offsets, and we're also using the same variable for two different things over two sections of code. Perhaps it would have been less obtuse if I had written offset = -(cfa_store.offset - offset); > However I have one simple question with regarding the output: The CFI > instructions for registers have changed only after the prologue. (For > convenience I have attached disassembly too). As far as I understand, > DWARF2 spec emits CFI instructions immediately. (Appendix 5 of DWARF2 > specification) GCC is attempting to minimize the number of advance opcodes by grouping the DW_CFA_offset opcodes. We can delay emission of such until the registers in question are actually clobbered. In this case this delay tactic failed because of the push -- we cannot delay changes to the CFA. > The other scenario is - how about functions with signals/interrupts? > The compiler will give an ICE compiling a function as below: > > void my_interrupt_handler() __attribute__ (("interrupt")); It's a bug in the avr backend; the enable_interrupt insn should not be marked as frame-related. We should fix this separately. r~ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-17 16:05 ` Richard Henderson @ 2011-02-17 19:53 ` Richard Henderson 2011-02-22 16:18 ` Anitha Boyapati 0 siblings, 1 reply; 24+ messages in thread From: Richard Henderson @ 2011-02-17 19:53 UTC (permalink / raw) To: Anitha Boyapati Cc: GCC Patches, Petr Hluzín, gdb, chertykov, aesok, eric.weddington [-- Attachment #1: Type: text/plain, Size: 1809 bytes --] On 02/17/2011 08:04 AM, Richard Henderson wrote: >> The other scenario is - how about functions with signals/interrupts? >> The compiler will give an ICE compiling a function as below: >> >> void my_interrupt_handler() __attribute__ (("interrupt")); > > It's a bug in the avr backend; the enable_interrupt insn should not be > marked as frame-related. We should fix this separately. Looking at this closer, it seem that the FRAME_RELATED markers in avr.c were sprinkled at random without really knowing what is going on. There were 2 places where UNSPEC_VOLATILE insns were marked frame related, somehow expecting the unwinder to do something magical. The following cleans all that up. There are a couple of odd points: (1) SREG and RAMPZ cannot be annotated as saved in the frame of an interrupt function without allocating hard register numbers for these registers. Not to be confused with "real" registers, these need a number in the same way that SP_L and SP_H have register numbers. I've simply ignored unwind info for these for now. (2) At present it's possible to use epilogue_restores without having used prologue_saves. I.e. use epilogue_restores with an inline prologue. The problem being that the inline prologue uses an HImode push of REG_Y (i.e. r29 first), whereas the code in prologue_saves pushes r28 first and epilogue_restores is written to expect that. I've changed the inline prologue and epilogue to emit explicit byte push/pop for r28/r29 in numerical order to match the external routines. This has a side effect that r29 actually receives unwind info. The generic unwinder doesn't expect to see multi-register saves, and only emits one unwind opcode for each save. r~ [-- Attachment #2: z-gcc-2 --] [-- Type: text/plain, Size: 20167 bytes --] diff --git a/gcc/config/avr/avr-protos.h b/gcc/config/avr/avr-protos.h index 4569359..06c9254 100644 --- a/gcc/config/avr/avr-protos.h +++ b/gcc/config/avr/avr-protos.h @@ -109,6 +109,7 @@ extern RTX_CODE avr_normalize_condition (RTX_CODE condition); extern int compare_eq_p (rtx insn); extern void out_shift_with_cnt (const char *templ, rtx insn, rtx operands[], int *len, int t_len); +extern rtx avr_incoming_return_addr_rtx (void); #endif /* RTX_CODE */ #ifdef HAVE_MACHINE_MODES diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c index 30e4626..6eaa593 100644 --- a/gcc/config/avr/avr.c +++ b/gcc/config/avr/avr.c @@ -534,6 +534,35 @@ get_sequence_length (rtx insns) return length; } +/* Implement INCOMING_RETURN_ADDR_RTX. */ + +rtx +avr_incoming_return_addr_rtx (void) +{ + /* The return address is at the top of the stack. Note that the push + was via post-decrement, which means the actual address is off by one. */ + return gen_frame_mem (HImode, plus_constant (stack_pointer_rtx, 1)); +} + +/* Helper for expand_prologue. Emit a push of a byte register. */ + +static void +emit_push_byte (unsigned regno, bool frame_related_p) +{ + rtx mem, reg, insn; + + mem = gen_rtx_POST_DEC (HImode, stack_pointer_rtx); + mem = gen_frame_mem (QImode, mem); + reg = gen_rtx_REG (QImode, regno); + + insn = emit_insn (gen_rtx_SET (VOIDmode, mem, reg)); + if (frame_related_p) + RTX_FRAME_RELATED_P (insn) = 1; + + cfun->machine->stack_usage++; +} + + /* Output function prologue. */ void @@ -543,11 +572,6 @@ expand_prologue (void) HARD_REG_SET set; int minimize; HOST_WIDE_INT size = get_frame_size(); - /* Define templates for push instructions. */ - rtx pushbyte = gen_rtx_MEM (QImode, - gen_rtx_POST_DEC (HImode, stack_pointer_rtx)); - rtx pushword = gen_rtx_MEM (HImode, - gen_rtx_POST_DEC (HImode, stack_pointer_rtx)); rtx insn; /* Init cfun->machine. */ @@ -575,46 +599,34 @@ expand_prologue (void) if (cfun->machine->is_interrupt || cfun->machine->is_signal) { + /* Enable interrupts. */ if (cfun->machine->is_interrupt) - { - /* Enable interrupts. */ - insn = emit_insn (gen_enable_interrupt ()); - RTX_FRAME_RELATED_P (insn) = 1; - } + emit_insn (gen_enable_interrupt ()); /* Push zero reg. */ - insn = emit_move_insn (pushbyte, zero_reg_rtx); - RTX_FRAME_RELATED_P (insn) = 1; - cfun->machine->stack_usage++; + emit_push_byte (ZERO_REGNO, true); /* Push tmp reg. */ - insn = emit_move_insn (pushbyte, tmp_reg_rtx); - RTX_FRAME_RELATED_P (insn) = 1; - cfun->machine->stack_usage++; + emit_push_byte (TMP_REGNO, true); /* Push SREG. */ - insn = emit_move_insn (tmp_reg_rtx, - gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR))); - RTX_FRAME_RELATED_P (insn) = 1; - insn = emit_move_insn (pushbyte, tmp_reg_rtx); - RTX_FRAME_RELATED_P (insn) = 1; - cfun->machine->stack_usage++; + /* ??? There's no dwarf2 column reserved for SREG. */ + emit_move_insn (tmp_reg_rtx, gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR))); + emit_push_byte (TMP_REGNO, false); /* Push RAMPZ. */ - if(AVR_HAVE_RAMPZ - && (TEST_HARD_REG_BIT (set, REG_Z) && TEST_HARD_REG_BIT (set, REG_Z + 1))) + /* ??? There's no dwarf2 column reserved for RAMPZ. */ + if (AVR_HAVE_RAMPZ + && TEST_HARD_REG_BIT (set, REG_Z) + && TEST_HARD_REG_BIT (set, REG_Z + 1)) { - insn = emit_move_insn (tmp_reg_rtx, - gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR))); - RTX_FRAME_RELATED_P (insn) = 1; - insn = emit_move_insn (pushbyte, tmp_reg_rtx); - RTX_FRAME_RELATED_P (insn) = 1; - cfun->machine->stack_usage++; + emit_move_insn (tmp_reg_rtx, + gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR))); + emit_push_byte (TMP_REGNO, false); } /* Clear zero reg. */ - insn = emit_move_insn (zero_reg_rtx, const0_rtx); - RTX_FRAME_RELATED_P (insn) = 1; + emit_move_insn (zero_reg_rtx, const0_rtx); /* Prevent any attempt to delete the setting of ZERO_REG! */ emit_use (zero_reg_rtx); @@ -623,37 +635,63 @@ expand_prologue (void) || (AVR_2_BYTE_PC && live_seq > 6) || live_seq > 7)) { - insn = emit_move_insn (gen_rtx_REG (HImode, REG_X), - gen_int_mode (size, HImode)); - RTX_FRAME_RELATED_P (insn) = 1; + int first_reg, reg, offset; - insn = - emit_insn (gen_call_prologue_saves (gen_int_mode (live_seq, HImode), - gen_int_mode (size + live_seq, HImode))); + emit_move_insn (gen_rtx_REG (HImode, REG_X), + gen_int_mode (size, HImode)); + + insn = emit_insn (gen_call_prologue_saves + (gen_int_mode (live_seq, HImode), + gen_int_mode (size + live_seq, HImode))); RTX_FRAME_RELATED_P (insn) = 1; + + /* Describe the effect of the unspec_volatile call to prologue_saves. + Note that this formulation assumes that add_reg_note pushes the + notes to the front. Thus we build them in the reverse order of + how we want dwarf2out to process them. */ + + /* The function does always set frame_pointer_rtx, but whether that + is going to be permanent in the function is frame_pointer_needed. */ + add_reg_note (insn, REG_CFA_ADJUST_CFA, + gen_rtx_SET (VOIDmode, + (frame_pointer_needed + ? frame_pointer_rtx : stack_pointer_rtx), + plus_constant (stack_pointer_rtx, + -(size + live_seq)))); + + /* Note that live_seq always contains r28+r29, but the other + registers to be saved are all below 18. */ + first_reg = 18 - (live_seq - 2); + + for (reg = 29, offset = -live_seq + 1; + reg >= first_reg; + reg = (reg == 28 ? 17 : reg - 1), ++offset) + { + rtx m, r; + + m = gen_rtx_MEM (QImode, plus_constant (stack_pointer_rtx, offset)); + r = gen_rtx_REG (QImode, reg); + add_reg_note (insn, REG_CFA_OFFSET, gen_rtx_SET (VOIDmode, m, r)); + } + cfun->machine->stack_usage += size + live_seq; } else { int reg; for (reg = 0; reg < 32; ++reg) - { - if (TEST_HARD_REG_BIT (set, reg)) - { - /* Emit push of register to save. */ - insn=emit_move_insn (pushbyte, gen_rtx_REG (QImode, reg)); - RTX_FRAME_RELATED_P (insn) = 1; - cfun->machine->stack_usage++; - } - } + if (TEST_HARD_REG_BIT (set, reg)) + emit_push_byte (reg, true); + if (frame_pointer_needed) { if (!(cfun->machine->is_OS_task || cfun->machine->is_OS_main)) { - /* Push frame pointer. */ - insn = emit_move_insn (pushword, frame_pointer_rtx); - RTX_FRAME_RELATED_P (insn) = 1; - cfun->machine->stack_usage += 2; + /* Push frame pointer. Always be consistent about the + ordering of pushes -- epilogue_restores expects the + register pair to be pushed low byte first. */ + emit_push_byte (REG_Y, true); + emit_push_byte (REG_Y + 1, true); } if (!size) @@ -676,13 +714,12 @@ expand_prologue (void) is selected. */ rtx myfp; rtx fp_plus_insns; - rtx sp_plus_insns = NULL_RTX; if (AVR_HAVE_8BIT_SP) { - /* The high byte (r29) doesn't change - prefer 'subi' (1 cycle) - over 'sbiw' (2 cycles, same size). */ - myfp = gen_rtx_REG (QImode, REGNO (frame_pointer_rtx)); + /* The high byte (r29) doesn't change. Prefer 'subi' + (1 cycle) over 'sbiw' (2 cycles, same size). */ + myfp = gen_rtx_REG (QImode, FRAME_POINTER_REGNUM); } else { @@ -693,41 +730,43 @@ expand_prologue (void) /* Method 1-Adjust frame pointer. */ start_sequence (); - insn = emit_move_insn (frame_pointer_rtx, stack_pointer_rtx); - RTX_FRAME_RELATED_P (insn) = 1; + /* Normally the dwarf2out frame-related-expr interpreter does + not expect to have the CFA change once the frame pointer is + set up. Thus we avoid marking the move insn below and + instead indicate that the entire operation is complete after + the frame pointer subtraction is done. */ - insn = - emit_move_insn (myfp, - gen_rtx_PLUS (GET_MODE(myfp), myfp, - gen_int_mode (-size, - GET_MODE(myfp)))); - RTX_FRAME_RELATED_P (insn) = 1; + emit_move_insn (frame_pointer_rtx, stack_pointer_rtx); - /* Copy to stack pointer. */ + insn = emit_move_insn (myfp, plus_constant (myfp, -size)); + RTX_FRAME_RELATED_P (insn) = 1; + add_reg_note (insn, REG_CFA_ADJUST_CFA, + gen_rtx_SET (VOIDmode, frame_pointer_rtx, + plus_constant (stack_pointer_rtx, + -size))); + + /* Copy to stack pointer. Note that since we've already + changed the CFA to the frame pointer this operation + need not be annotated at all. */ if (AVR_HAVE_8BIT_SP) { - insn = emit_move_insn (stack_pointer_rtx, frame_pointer_rtx); - RTX_FRAME_RELATED_P (insn) = 1; + emit_move_insn (stack_pointer_rtx, frame_pointer_rtx); } else if (TARGET_NO_INTERRUPTS || cfun->machine->is_signal || cfun->machine->is_OS_main) { - insn = - emit_insn (gen_movhi_sp_r_irq_off (stack_pointer_rtx, - frame_pointer_rtx)); - RTX_FRAME_RELATED_P (insn) = 1; + emit_insn (gen_movhi_sp_r_irq_off (stack_pointer_rtx, + frame_pointer_rtx)); } else if (cfun->machine->is_interrupt) { - insn = emit_insn (gen_movhi_sp_r_irq_on (stack_pointer_rtx, - frame_pointer_rtx)); - RTX_FRAME_RELATED_P (insn) = 1; + emit_insn (gen_movhi_sp_r_irq_on (stack_pointer_rtx, + frame_pointer_rtx)); } else { - insn = emit_move_insn (stack_pointer_rtx, frame_pointer_rtx); - RTX_FRAME_RELATED_P (insn) = 1; + emit_move_insn (stack_pointer_rtx, frame_pointer_rtx); } fp_plus_insns = get_insns (); @@ -736,30 +775,30 @@ expand_prologue (void) /* Method 2-Adjust Stack pointer. */ if (size <= 6) { + rtx sp_plus_insns; + start_sequence (); - insn = - emit_move_insn (stack_pointer_rtx, - gen_rtx_PLUS (HImode, - stack_pointer_rtx, - gen_int_mode (-size, - HImode))); + insn = plus_constant (stack_pointer_rtx, -size); + insn = emit_move_insn (stack_pointer_rtx, insn); RTX_FRAME_RELATED_P (insn) = 1; - insn = - emit_move_insn (frame_pointer_rtx, stack_pointer_rtx); + insn = emit_move_insn (frame_pointer_rtx, stack_pointer_rtx); RTX_FRAME_RELATED_P (insn) = 1; sp_plus_insns = get_insns (); end_sequence (); - } - /* Use shortest method. */ - if (size <= 6 && (get_sequence_length (sp_plus_insns) - < get_sequence_length (fp_plus_insns))) - emit_insn (sp_plus_insns); - else + /* Use shortest method. */ + if (get_sequence_length (sp_plus_insns) + < get_sequence_length (fp_plus_insns)) + emit_insn (sp_plus_insns); + else + emit_insn (fp_plus_insns); + } + else emit_insn (fp_plus_insns); + cfun->machine->stack_usage += size; } } @@ -813,6 +852,20 @@ avr_epilogue_uses (int regno ATTRIBUTE_UNUSED) return 0; } +/* Helper for expand_epilogue. Emit a pop of a byte register. */ + +static void +emit_pop_byte (unsigned regno) +{ + rtx mem, reg; + + mem = gen_rtx_PRE_INC (HImode, stack_pointer_rtx); + mem = gen_frame_mem (QImode, mem); + reg = gen_rtx_REG (QImode, regno); + + emit_insn (gen_rtx_SET (VOIDmode, reg, mem)); +} + /* Output RTL epilogue. */ void @@ -865,13 +918,12 @@ expand_epilogue (void) /* Try two methods to adjust stack and select shortest. */ rtx myfp; rtx fp_plus_insns; - rtx sp_plus_insns = NULL_RTX; - + if (AVR_HAVE_8BIT_SP) { /* The high byte (r29) doesn't change - prefer 'subi' (1 cycle) over 'sbiw' (2 cycles, same size). */ - myfp = gen_rtx_REG (QImode, REGNO (frame_pointer_rtx)); + myfp = gen_rtx_REG (QImode, FRAME_POINTER_REGNUM); } else { @@ -882,10 +934,7 @@ expand_epilogue (void) /* Method 1-Adjust frame pointer. */ start_sequence (); - emit_move_insn (myfp, - gen_rtx_PLUS (GET_MODE (myfp), myfp, - gen_int_mode (size, - GET_MODE(myfp)))); + emit_move_insn (myfp, plus_constant (myfp, size)); /* Copy to stack pointer. */ if (AVR_HAVE_8BIT_SP) @@ -914,58 +963,63 @@ expand_epilogue (void) /* Method 2-Adjust Stack pointer. */ if (size <= 5) { + rtx sp_plus_insns; + start_sequence (); emit_move_insn (stack_pointer_rtx, - gen_rtx_PLUS (HImode, stack_pointer_rtx, - gen_int_mode (size, - HImode))); + plus_constant (stack_pointer_rtx, size)); sp_plus_insns = get_insns (); end_sequence (); - } - /* Use shortest method. */ - if (size <= 5 && (get_sequence_length (sp_plus_insns) - < get_sequence_length (fp_plus_insns))) - emit_insn (sp_plus_insns); - else + /* Use shortest method. */ + if (get_sequence_length (sp_plus_insns) + < get_sequence_length (fp_plus_insns)) + emit_insn (sp_plus_insns); + else + emit_insn (fp_plus_insns); + } + else emit_insn (fp_plus_insns); } if (!(cfun->machine->is_OS_task || cfun->machine->is_OS_main)) { - /* Restore previous frame_pointer. */ - emit_insn (gen_pophi (frame_pointer_rtx)); + /* Restore previous frame_pointer. See expand_prologue for + rationale for not using pophi. */ + emit_pop_byte (REG_Y + 1); + emit_pop_byte (REG_Y); } } + /* Restore used registers. */ for (reg = 31; reg >= 0; --reg) - { - if (TEST_HARD_REG_BIT (set, reg)) - emit_insn (gen_popqi (gen_rtx_REG (QImode, reg))); - } + if (TEST_HARD_REG_BIT (set, reg)) + emit_pop_byte (reg); + if (cfun->machine->is_interrupt || cfun->machine->is_signal) { /* Restore RAMPZ using tmp reg as scratch. */ - if(AVR_HAVE_RAMPZ - && (TEST_HARD_REG_BIT (set, REG_Z) && TEST_HARD_REG_BIT (set, REG_Z + 1))) + if (AVR_HAVE_RAMPZ + && TEST_HARD_REG_BIT (set, REG_Z) + && TEST_HARD_REG_BIT (set, REG_Z + 1)) { - emit_insn (gen_popqi (tmp_reg_rtx)); - emit_move_insn (gen_rtx_MEM(QImode, GEN_INT(RAMPZ_ADDR)), + emit_pop_byte (TMP_REGNO); + emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR)), tmp_reg_rtx); } /* Restore SREG using tmp reg as scratch. */ - emit_insn (gen_popqi (tmp_reg_rtx)); + emit_pop_byte (TMP_REGNO); - emit_move_insn (gen_rtx_MEM(QImode, GEN_INT(SREG_ADDR)), + emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)), tmp_reg_rtx); /* Restore tmp REG. */ - emit_insn (gen_popqi (tmp_reg_rtx)); + emit_pop_byte (TMP_REGNO); /* Restore zero REG. */ - emit_insn (gen_popqi (zero_reg_rtx)); + emit_pop_byte (ZERO_REGNO); } emit_jump_insn (gen_return ()); diff --git a/gcc/config/avr/avr.h b/gcc/config/avr/avr.h index 9a71078..ee36daf 100644 --- a/gcc/config/avr/avr.h +++ b/gcc/config/avr/avr.h @@ -351,9 +351,6 @@ enum reg_class { #define STATIC_CHAIN_REGNUM 2 -/* Offset from the frame pointer register value to the top of the stack. */ -#define FRAME_POINTER_CFA_OFFSET(FNDECL) 0 - #define ELIMINABLE_REGS { \ {ARG_POINTER_REGNUM, FRAME_POINTER_REGNUM}, \ {FRAME_POINTER_REGNUM, STACK_POINTER_REGNUM} \ @@ -809,6 +806,13 @@ mmcu=*:-mmcu=%*}" #define OBJECT_FORMAT_ELF +#define INCOMING_RETURN_ADDR_RTX avr_incoming_return_addr_rtx () +#define INCOMING_FRAME_SP_OFFSET (AVR_3_BYTE_PC ? 3 : 2) + +/* The caller's stack pointer value immediately before the call + is one byte below the first argument. */ +#define ARG_POINTER_CFA_OFFSET(FNDECL) -1 + #define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \ avr_hard_regno_rename_ok (OLD_REG, NEW_REG) diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md index f086e80..f4a95d8 100644 --- a/gcc/config/avr/avr.md +++ b/gcc/config/avr/avr.md @@ -182,7 +182,7 @@ (define_insn "*pushqi" - [(set (mem:QI (post_dec (reg:HI REG_SP))) + [(set (mem:QI (post_dec:HI (reg:HI REG_SP))) (match_operand:QI 0 "reg_or_0_operand" "r,L"))] "" "@ @@ -190,9 +190,8 @@ push __zero_reg__" [(set_attr "length" "1,1")]) - (define_insn "*pushhi" - [(set (mem:HI (post_dec (reg:HI REG_SP))) + [(set (mem:HI (post_dec:HI (reg:HI REG_SP))) (match_operand:HI 0 "reg_or_0_operand" "r,L"))] "" "@ @@ -201,7 +200,7 @@ [(set_attr "length" "2,2")]) (define_insn "*pushsi" - [(set (mem:SI (post_dec (reg:HI REG_SP))) + [(set (mem:SI (post_dec:HI (reg:HI REG_SP))) (match_operand:SI 0 "reg_or_0_operand" "r,L"))] "" "@ @@ -210,7 +209,7 @@ [(set_attr "length" "4,4")]) (define_insn "*pushsf" - [(set (mem:SF (post_dec (reg:HI REG_SP))) + [(set (mem:SF (post_dec:HI (reg:HI REG_SP))) (match_operand:SF 0 "register_operand" "r"))] "" "push %D0 @@ -3127,20 +3126,12 @@ (define_insn "popqi" [(set (match_operand:QI 0 "register_operand" "=r") - (mem:QI (post_inc (reg:HI REG_SP))))] + (mem:QI (pre_inc:HI (reg:HI REG_SP))))] "" "pop %0" [(set_attr "cc" "none") (set_attr "length" "1")]) -(define_insn "pophi" - [(set (match_operand:HI 0 "register_operand" "=r") - (mem:HI (post_inc (reg:HI REG_SP))))] - "" - "pop %A0\;pop %B0" - [(set_attr "cc" "none") - (set_attr "length" "2")]) - ;; Enable Interrupts (define_insn "enable_interrupt" [(unspec [(const_int 0)] UNSPEC_SEI)] diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index fea8209..b5e660c 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2240,7 +2240,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label) cfa.base_offset = -cfa_store.offset Rule 11: - (set (mem ({pre_inc,pre_dec} sp:cfa_store.reg)) <reg>) + (set (mem ({pre_inc,pre_dec,post_dec} sp:cfa_store.reg)) <reg>) effects: cfa_store.offset += -/+ mode_size(mem) cfa.offset = cfa_store.offset if cfa.reg == sp cfa.reg = sp @@ -2259,7 +2259,7 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, const char *label) cfa.base_offset = -{cfa_store,cfa_temp}.offset Rule 14: - (set (mem (postinc <reg1>:cfa_temp <const_int>)) <reg2>) + (set (mem (post_inc <reg1>:cfa_temp <const_int>)) <reg2>) effects: cfa.reg = <reg1> cfa.base_offset = -cfa_temp.offset cfa_temp.offset -= mode_size(mem) @@ -2592,6 +2592,7 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label) /* Rule 11 */ case PRE_INC: case PRE_DEC: + case POST_DEC: offset = GET_MODE_SIZE (GET_MODE (dest)); if (GET_CODE (XEXP (dest, 0)) == PRE_INC) offset = -offset; @@ -2616,7 +2617,10 @@ dwarf2out_frame_debug_expr (rtx expr, const char *label) if (cfa.reg == STACK_POINTER_REGNUM) cfa.offset = cfa_store.offset; - offset = -cfa_store.offset; + if (GET_CODE (XEXP (dest, 0)) == POST_DEC) + offset += -cfa_store.offset; + else + offset = -cfa_store.offset; break; /* Rule 12 */ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-17 19:53 ` Richard Henderson @ 2011-02-22 16:18 ` Anitha Boyapati 2011-02-22 17:51 ` Richard Henderson 0 siblings, 1 reply; 24+ messages in thread From: Anitha Boyapati @ 2011-02-22 16:18 UTC (permalink / raw) To: Richard Henderson, GCC Patches Cc: Petr Hluzín, gdb, chertykov, aesok, eric.weddington 2011/2/18 Richard Henderson <rth@redhat.com> > > On 02/17/2011 08:04 AM, Richard Henderson wrote: > >> The other scenario is - how about functions with signals/interrupts? > >> The compiler will give an ICE compiling a function as below: > >> > >> void my_interrupt_handler() __attribute__ (("interrupt")); > > > > It's a bug in the avr backend; the enable_interrupt insn should not be > > marked as frame-related. We should fix this separately. > > Looking at this closer, it seem that the FRAME_RELATED markers in avr.c > were sprinkled at random without really knowing what is going on. This is quite a clean up! I am yet to reach the changes in expand_epilogue. > There > were 2 places where UNSPEC_VOLATILE insns were marked frame related, > somehow expecting the unwinder to do something magical. > Just to be on same page, these are the 2 places: 1. gen_enable_interrupt() 2. gen_call_prologue_saves() For the latter, can you explain why adding reg notes is required? + add_reg_note (insn, REG_CFA_ADJUST_CFA, + gen_rtx_SET (VOIDmode, + (frame_pointer_needed + ? frame_pointer_rtx : stack_pointer_rtx), + plus_constant (stack_pointer_rtx, + -(size + live_seq)))); + (The comment does say that this is to describe the effect of UNSPEC_VOLATILE, but how reg notes help?) > The following cleans all that up. There are a couple of odd points: > > (1) SREG and RAMPZ cannot be annotated as saved in the frame of an > interrupt function without allocating hard register numbers for > these registers. Not to be confused with "real" registers, > these need a number in the same way that SP_L and SP_H have > register numbers. > > I've simply ignored unwind info for these for now. > In addition to the above, there are atleast 4 more registers RAMPX, RAMPY, RAMPD and EIND. (Actually the support for these registers is in the form of patches which aren't upstream yet) > (2) At present it's possible to use epilogue_restores without > having used prologue_saves. I.e. use epilogue_restores with > an inline prologue. The problem being that the inline prologue > uses an HImode push of REG_Y (i.e. r29 first), whereas the code > in prologue_saves pushes r28 first and epilogue_restores is > written to expect that. > Is this is the line being referred to ? if (frame_pointer_needed) { ... /* Push frame pointer. */ insn = emit_move_insn (pushword, frame_pointer_rtx); On the whole, when the patch is applied, an ICE is longer generated during the compilation of interrupt and signal functions. I am yet to see the changes due to ARG_POINTER_CFA_OFFSET(FNDECL) definition and the complete unwind info incase of interrupts. Anitha ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [avr] gas support for cfi info 2011-02-22 16:18 ` Anitha Boyapati @ 2011-02-22 17:51 ` Richard Henderson 0 siblings, 0 replies; 24+ messages in thread From: Richard Henderson @ 2011-02-22 17:51 UTC (permalink / raw) To: Anitha Boyapati Cc: GCC Patches, Petr Hluzín, gdb, chertykov, aesok, eric.weddington On 02/22/2011 08:17 AM, Anitha Boyapati wrote: > Just to be on same page, these are the 2 places: > > 1. gen_enable_interrupt() > 2. gen_call_prologue_saves() > > > For the latter, can you explain why adding reg notes is required? > > + add_reg_note (insn, REG_CFA_ADJUST_CFA, > + gen_rtx_SET (VOIDmode, > + (frame_pointer_needed > + ? frame_pointer_rtx : stack_pointer_rtx), > + plus_constant (stack_pointer_rtx, > + -(size + live_seq)))); > + > > (The comment does say that this is to describe the effect of > UNSPEC_VOLATILE, but how reg notes help?) The external function call represented by the prologue_saves unspec saves registers to the stack, and allocates stack space. Both of these actions are things we want to describe in unwind info. The reg notes I added describe all of the actions performed by the function call. >> (2) At present it's possible to use epilogue_restores without >> having used prologue_saves. I.e. use epilogue_restores with >> an inline prologue. The problem being that the inline prologue >> uses an HImode push of REG_Y (i.e. r29 first), whereas the code >> in prologue_saves pushes r28 first and epilogue_restores is >> written to expect that. >> > > Is this is the line being referred to ? > > if (frame_pointer_needed) > { > ... > /* Push frame pointer. */ > insn = emit_move_insn (pushword, frame_pointer_rtx); Yes. r~ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-13 15:11 ` Petr Hluzín 2011-02-15 17:41 ` Richard Henderson @ 2011-02-15 18:18 ` Anitha Boyapati 2011-02-15 22:12 ` Petr Hluzín 1 sibling, 1 reply; 24+ messages in thread From: Anitha Boyapati @ 2011-02-15 18:18 UTC (permalink / raw) To: Petr Hluzín; +Cc: gdb On 13 February 2011 20:40, Petr Hluzín <petr.hluzin@gmail.com> wrote: > > Here you have it: > http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html > (You might also like "add pretty-printing of AVR register names": > http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00108.html) > Sorry..I was busy with lot other things that I did not try these links. But why I get this -'404 Not Found' error while trying to open above messages ?! Anitha ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-15 18:18 ` Testing Call frame information in .debug_frame section Anitha Boyapati @ 2011-02-15 22:12 ` Petr Hluzín 0 siblings, 0 replies; 24+ messages in thread From: Petr Hluzín @ 2011-02-15 22:12 UTC (permalink / raw) To: Anitha Boyapati; +Cc: gdb On 15 February 2011 19:17, Anitha Boyapati <anitha.boyapati@gmail.com> wrote: > On 13 February 2011 20:40, Petr Hluzín <petr.hluzin@gmail.com> wrote: > >> >> Here you have it: >> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00109.html >> (You might also like "add pretty-printing of AVR register names": >> http://xfree86.cygwin.ru/ml/binutils/2010-08/msg00108.html) >> > Sorry..I was busy with lot other things that I did not try these links. > > But why I get this -'404 Not Found' error while trying to open above > messages ?! Wow. It was there when I copied the URL. Here you go: [PATCH] add pretty-printing of AVR register names http://sourceware.org/ml/binutils/2010-08/msg00108.html [PATCH] print better diagnostic when no CFI support http://sourceware.org/ml/binutils/2010-08/msg00109.html -- Petr Hluzin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-13 9:57 ` Anitha Boyapati 2011-02-13 15:11 ` Petr Hluzín @ 2011-02-14 16:42 ` Tom Tromey 2011-02-14 22:43 ` Petr Hluzín 1 sibling, 1 reply; 24+ messages in thread From: Tom Tromey @ 2011-02-14 16:42 UTC (permalink / raw) To: Anitha Boyapati; +Cc: Petr Hluzín, gdb Anitha> How does GDB currently use information from .debug_frame Anitha> section? IIUC, GDB can use this information when it is available and when the target requests the use of the DWARF frame sniffer. Look for calls to dwarf2_append_unwinders; this will tell you which targets are already capable of using it. Petr> Just go to binutils web and search for my name in the patch tracker. Petr> Oh wait, they do not have one. Maybe this is the reason they forgot to Petr> reply to my patches. You have to ping patches, not just for binutils but also gdb and gcc. It isn't a great system but it does have its advantages. One or two weeks after an initial submission, if there has been no answer, just send a ping message as a follow-up to your patch. Then do it every week. Tom ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-14 16:42 ` Tom Tromey @ 2011-02-14 22:43 ` Petr Hluzín 2011-02-15 15:06 ` Tom Tromey 0 siblings, 1 reply; 24+ messages in thread From: Petr Hluzín @ 2011-02-14 22:43 UTC (permalink / raw) To: Tom Tromey; +Cc: Anitha Boyapati, gdb On 14 February 2011 17:42, Tom Tromey <tromey@redhat.com> wrote: >... > Petr> Just go to binutils web and search for my name in the patch tracker. > Petr> Oh wait, they do not have one. Maybe this is the reason they forgot to > Petr> reply to my patches. > > You have to ping patches, not just for binutils but also gdb and gcc. > It isn't a great system but it does have its advantages. One advantage is that people can write replies inline into the patch. However this practice is fairly common so I bet patch-trackers can be configured to include the submitted patch when mailing a notification to a mailing list. Savannah's tools look quite capable. Are there more advantages? Are they pretty common? Is there an automatized solution for them, yet? > One or two weeks after an initial submission, if there has been no > answer, just send a ping message as a follow-up to your patch. Then do > it every week. This sounds quite mechanical, boring and common to a lot of people (submitters). Great example of task suitable for machines. (Why do you people choose such suffering?) -- Petr Hluzin ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Testing Call frame information in .debug_frame section 2011-02-14 22:43 ` Petr Hluzín @ 2011-02-15 15:06 ` Tom Tromey 0 siblings, 0 replies; 24+ messages in thread From: Tom Tromey @ 2011-02-15 15:06 UTC (permalink / raw) To: Petr Hluzín; +Cc: Anitha Boyapati, gdb >>>>> "Petr" == Petr Hluzín <petr.hluzin@gmail.com> writes: Petr> Are there more advantages? Are they pretty common? Is there an Petr> automatized solution for them, yet? >> One or two weeks after an initial submission, if there has been no >> answer, just send a ping message as a follow-up to your patch. Then do >> it every week. Petr> This sounds quite mechanical, boring and common to a lot of people Petr> (submitters). Great example of task suitable for machines. (Why do you Petr> people choose such suffering?) I am just describing the system as it actually exists, not really defending it or anything. The way I look at it is that if you want to get a patch in, you have to bear some of the burden. gdb tried a patch tracker for a while but it didn't prove to be very popular. Maybe most maintainers prefer working via email; but it is hard to know for sure. Recently some GCC developers started using Rietveld for patch tracking and review: http://gcc.gnu.org/ml/gcc/2011-01/msg00354.html Doug suggested using it for GDB as well, but AFAIK nobody has set it up. Maybe if you set it up, people would use it. Tom ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2011-02-22 17:51 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-01 13:04 Testing Call frame information in .debug_frame section Anitha Boyapati 2011-02-13 2:34 ` Petr Hluzín 2011-02-13 9:57 ` Anitha Boyapati 2011-02-13 15:11 ` Petr Hluzín 2011-02-15 17:41 ` Richard Henderson 2011-02-15 18:09 ` Anitha Boyapati 2011-02-15 18:48 ` Richard Henderson 2011-02-15 19:15 ` Anitha Boyapati 2011-02-15 19:03 ` [avr] gas support for cfi info Richard Henderson 2011-02-15 22:45 ` Petr Hluzín 2011-02-16 17:59 ` Richard Henderson 2011-02-16 22:49 ` Petr Hluzín 2011-02-17 16:12 ` Richard Henderson 2011-02-17 16:16 ` Tristan Gingold 2011-02-17 15:35 ` Anitha Boyapati 2011-02-17 16:05 ` Richard Henderson 2011-02-17 19:53 ` Richard Henderson 2011-02-22 16:18 ` Anitha Boyapati 2011-02-22 17:51 ` Richard Henderson 2011-02-15 18:18 ` Testing Call frame information in .debug_frame section Anitha Boyapati 2011-02-15 22:12 ` Petr Hluzín 2011-02-14 16:42 ` Tom Tromey 2011-02-14 22:43 ` Petr Hluzín 2011-02-15 15:06 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox