Re: Testing Call frame information in .debug_frame section

Re: Testing Call frame information in .debug_frame section

[Richard Henderson]

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~

Re: Testing Call frame information in .debug_frame section

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 393 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.

Committed as follows.


r~


	* dw2gencfi.c (dot_cfi_dummy): New.
	(cfi_pseudo_table) [!TARGET_USE_CFIPOP]: New.
	* read.c (pobegin): Unconditionally call cfi_pop_insert.

[-- Attachment #2: z --]
[-- Type: text/plain, Size: 2199 bytes --]

Index: dw2gencfi.c
===================================================================
RCS file: /cvs/src/src/gas/dw2gencfi.c,v
retrieving revision 1.49
diff -u -p -r1.49 dw2gencfi.c
--- dw2gencfi.c	2 Sep 2010 22:43:45 -0000	1.49
+++ dw2gencfi.c	15 Feb 2011 17:58:37 -0000
@@ -1757,6 +1757,43 @@ cfi_finish (void)
 }
 
 #else /* TARGET_USE_CFIPOP */
+
+/* Emit an intelligable error message for missing support.  */
+
+static void
+dot_cfi_dummy (int ignored ATTRIBUTE_UNUSED)
+{
+  as_bad (_("CFI is not supported for this target"));
+  ignore_rest_of_line ();
+}
+
+const pseudo_typeS cfi_pseudo_table[] =
+  {
+    { "cfi_sections", dot_cfi_dummy, 0 },
+    { "cfi_startproc", dot_cfi_dummy, 0 },
+    { "cfi_endproc", dot_cfi_dummy, 0 },
+    { "cfi_def_cfa", dot_cfi_dummy, 0 },
+    { "cfi_def_cfa_register", dot_cfi_dummy, 0 },
+    { "cfi_def_cfa_offset", dot_cfi_dummy, 0 },
+    { "cfi_adjust_cfa_offset", dot_cfi_dummy, 0 },
+    { "cfi_offset", dot_cfi_dummy, 0 },
+    { "cfi_rel_offset", dot_cfi_dummy, 0 },
+    { "cfi_register", dot_cfi_dummy, 0 },
+    { "cfi_return_column", dot_cfi_dummy, 0 },
+    { "cfi_restore", dot_cfi_dummy, 0 },
+    { "cfi_undefined", dot_cfi_dummy, 0 },
+    { "cfi_same_value", dot_cfi_dummy, 0 },
+    { "cfi_remember_state", dot_cfi_dummy, 0 },
+    { "cfi_restore_state", dot_cfi_dummy, 0 },
+    { "cfi_window_save", dot_cfi_dummy, 0 },
+    { "cfi_escape", dot_cfi_dummy, 0 },
+    { "cfi_signal_frame", dot_cfi_dummy, 0 },
+    { "cfi_personality", dot_cfi_dummy, 0 },
+    { "cfi_lsda", dot_cfi_dummy, 0 },
+    { "cfi_val_encoded_addr", dot_cfi_dummy, 0 },
+    { NULL, NULL, 0 }
+  };
+
 void
 cfi_finish (void)
 {
Index: read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.168
diff -u -p -r1.168 read.c
--- read.c	1 Dec 2010 21:34:10 -0000	1.168
+++ read.c	15 Feb 2011 17:58:38 -0000
@@ -521,11 +521,10 @@ pobegin (void)
   pop_table_name = "standard";
   pop_insert (potable);
 
-#ifdef TARGET_USE_CFIPOP
+  /* Now CFI ones.  */
   pop_table_name = "cfi";
   pop_override_ok = 1;
   cfi_pop_insert ();
-#endif
 }
 \f
 #define HANDLE_CONDITIONAL_ASSEMBLY()					\

Re: Testing Call frame information in .debug_frame section

[Petr Hluzín]

On 15 February 2011 19:02, 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.
>
> Committed as follows.
>
>
> r~
>
>
>        * dw2gencfi.c (dot_cfi_dummy): New.
>        (cfi_pseudo_table) [!TARGET_USE_CFIPOP]: New.
>        * read.c (pobegin): Unconditionally call cfi_pop_insert.
>

Great, thanks.

Note: The submitted code code does not print the platform GAS is
configured for. If makefile/IDE is accidentally launching wrong build
of GAS then user will have hard time figuring this out. If it is not
an accident then the extra information is useful for googling since
recovery steps may be different. ("CFI on arch FOO? You have to
upgrade to version XY" vs "CFI on arch BAR? You have to apply this
patch.")

-as_bad (_("CFI is not supported for this target"));
+as_bad (_("CFI is not supported for target '" TARGET_FORMAT "'"));

Also "intelligable" looks like a typo. Perhaps "intelligible"?

-- 
Petr Hluzin

Re: Testing Call frame information in .debug_frame section

[Richard Henderson]

On 02/15/2011 02:13 PM, Petr Hluzín wrote:
> Note: The submitted code code does not print the platform GAS is
> configured for. If makefile/IDE is accidentally launching wrong build
> of GAS then user will have hard time figuring this out. If it is not
> an accident then the extra information is useful for googling since
> recovery steps may be different. ("CFI on arch FOO? You have to
> upgrade to version XY" vs "CFI on arch BAR? You have to apply this
> patch.")
> 
> -as_bad (_("CFI is not supported for this target"));
> +as_bad (_("CFI is not supported for target '" TARGET_FORMAT "'"));

String substitution doesn't work for i18n.  We could have put it in
a %s argument, but no other error message in gas includes this info;
I can't see why we need to use it here.

> Also "intelligable" looks like a typo. Perhaps "intelligible"?

Oops.  Fixing.


r~

Re: Testing Call frame information in .debug_frame section

[Anitha Boyapati]

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

Re: Testing Call frame information in .debug_frame section

[Richard Henderson]

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~

Re: Testing Call frame information in .debug_frame section

[Anitha Boyapati]

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.

[avr] gas support for cfi info

[Richard Henderson]

[-- 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

Re: [avr] gas support for cfi info

[Petr Hluzín]

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

Re: [avr] gas support for cfi info

[Richard Henderson]

[-- 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 */

Re: [avr] gas support for cfi info

[Petr Hluzín]

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

Re: [avr] gas support for cfi info

[Richard Henderson]

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~

Re: [avr] gas support for cfi info

[Tristan Gingold]

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).

Re: [avr] gas support for cfi info

[Anitha Boyapati]

[-- 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

Re: [avr] gas support for cfi info

[Richard Henderson]

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~