[PATCH] Fix unwind info in x86 interrupt functions (PR debug/83728)

[PATCH] Fix unwind info in x86 interrupt functions (PR debug/83728)

[Jakub Jelinek]

Hi!

Last summer i386 INCOMING_FRAME_SP_OFFSET macro has been changed, so that it
is one word for most of the functions (as previously always), but 2 words
for functions with interrupt attribute.
Unfortunately this breaks the unwind info, as can be seen on the
gcc.dg/guality/pr68037-1.c testcase.  Our infrastructure assumes we have
just one set of cfi instructions in the CIE (we can have multiple CIEs, but
only for various flags, not for different cfi instructions), so if
the first function being assembled is an interrupt function, we start
with the 2*word initial offset in the CIE we emit (when
-fno-dwarf2-cfi-asm), or just assume the CIE has that (with -fdwarf2-cfi-asm
when it is GAS that emits it) even when it doesn't.  For -fdwarf2-cfi-asm
the effect is that until first CFA adjustment the offset is off by a word,
after that correct.  In both cases all CFA offsets are off by a word in
non-interrupt functions.
Or, if the first function doesn't have interrupt attribute, CFI in
non-interrupt functions is correct, but interrupt functions are wrong.

I've looked around and it seems stormy16 is the only other target that
bases the INCOMING_FRAME_SP_OFFSET value not just on ABI changing switches,
but on properties of the current function.

For these 2 targets the following patch introduces another macro,
DEFAULT_INCOMING_FRAME_SP_OFFSET, that is meant to be the same for all
functions of the same ABI and where GAS supports .cfi_startproc it should
also match what GAS does.  For the hopefully minority of functions that
need something else (i.e. interrupt functions) we emit a CFI instruction
right at the start of the function.

Bootstrapped/regtested on x86_64-linux and i686-linux, fixes the pr68037-1.c
FAILs, ok for trunk?

2018-01-18  Jakub Jelinek  <jakub@redhat.com>

	PR debug/81570
	PR debug/83728
	* dwarf2cfi.c (DEFAULT_INCOMING_FRAME_SP_OFFSET): Define to
	INCOMING_FRAME_SP_OFFSET if not defined.
	(scan_trace): Add ENTRY argument.  If true and
	DEFAULT_INCOMING_FRAME_SP_OFFSET != INCOMING_FRAME_SP_OFFSET,
	emit a note to adjust the CFA offset.
	(create_cfi_notes): Adjust scan_trace callers.
	(create_cie_data): Use DEFAULT_INCOMING_FRAME_SP_OFFSET rather than
	INCOMING_FRAME_SP_OFFSET in the CIE.
	* config/i386/i386.h (DEFAULT_INCOMING_FRAME_SP_OFFSET): Define.
	* config/stormy16/stormy16.h (DEFAULT_INCOMING_FRAME_SP_OFFSET):
	Likewise.
	* doc/tm.texi.in (DEFAULT_INCOMING_FRAME_SP_OFFSET): Document.
	* doc/tm.texi: Regenerated.

--- gcc/dwarf2cfi.c.jj	2018-01-04 00:43:17.632703231 +0100
+++ gcc/dwarf2cfi.c	2018-01-18 15:57:21.165911020 +0100
@@ -52,6 +52,10 @@ along with GCC; see the file COPYING3.
 #ifndef INCOMING_RETURN_ADDR_RTX
 #define INCOMING_RETURN_ADDR_RTX  (gcc_unreachable (), NULL_RTX)
 #endif
+
+#ifndef DEFAULT_INCOMING_FRAME_SP_OFFSET
+#define DEFAULT_INCOMING_FRAME_SP_OFFSET INCOMING_FRAME_SP_OFFSET
+#endif
 \f
 /* A collected description of an entire row of the abstract CFI table.  */
 struct GTY(()) dw_cfi_row
@@ -2484,7 +2488,7 @@ scan_insn_after (rtx_insn *insn)
    instructions therein.  */
 
 static void
-scan_trace (dw_trace_info *trace)
+scan_trace (dw_trace_info *trace, bool entry)
 {
   rtx_insn *prev, *insn = trace->head;
   dw_cfa_location this_cfa;
@@ -2503,6 +2507,17 @@ scan_trace (dw_trace_info *trace)
   this_cfa = cur_row->cfa;
   cur_cfa = &this_cfa;
 
+  /* If the current function starts with a non-standard incoming frame
+     sp offset, emit a note before the first instruction.  */
+  if (entry
+      && DEFAULT_INCOMING_FRAME_SP_OFFSET != INCOMING_FRAME_SP_OFFSET)
+    {
+      add_cfi_insn = insn;
+      gcc_assert (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_DELETED);
+      this_cfa.offset = INCOMING_FRAME_SP_OFFSET;
+      def_cfa_1 (&this_cfa);
+    }
+
   for (prev = insn, insn = NEXT_INSN (insn);
        insn;
        prev = insn, insn = NEXT_INSN (insn))
@@ -2671,12 +2686,12 @@ create_cfi_notes (void)
 
   /* Always begin at the entry trace.  */
   ti = &trace_info[0];
-  scan_trace (ti);
+  scan_trace (ti, true);
 
   while (!trace_work_list.is_empty ())
     {
       ti = trace_work_list.pop ();
-      scan_trace (ti);
+      scan_trace (ti, false);
     }
 
   queued_reg_saves.release ();
@@ -2980,7 +2995,12 @@ create_cie_data (void)
   /* On entry, the Canonical Frame Address is at SP.  */
   memset (&loc, 0, sizeof (loc));
   loc.reg = dw_stack_pointer_regnum;
-  loc.offset = INCOMING_FRAME_SP_OFFSET;
+  /* create_cie_data is called just once per TU, and when using .cfi_startproc
+     is even done by the assembler rather than the compiler.  If the target
+     has different incoming frame sp offsets depending on what kind of
+     function it is, use a single constant offset for the target and
+     if needed, adjust before the first instruction in insn stream.  */
+  loc.offset = DEFAULT_INCOMING_FRAME_SP_OFFSET;
   def_cfa_1 (&loc);
 
   if (targetm.debug_unwind_info () == UI_DWARF2
--- gcc/config/i386/i386.h.jj	2018-01-14 17:16:55.694836137 +0100
+++ gcc/config/i386/i386.h	2018-01-14 17:16:55.694836137 +0100
@@ -2110,6 +2110,10 @@ extern int const svr4_dbx_register_map[F
   (cfun->machine->func_type == TYPE_EXCEPTION \
    ? 2 * UNITS_PER_WORD : UNITS_PER_WORD)
 
+/* The value of INCOMING_FRAME_SP_OFFSET the assembler assumes in
+   .cfi_startproc.  */
+#define DEFAULT_INCOMING_FRAME_SP_OFFSET UNITS_PER_WORD
+
 /* Describe how we implement __builtin_eh_return.  */
 #define EH_RETURN_DATA_REGNO(N)	((N) <= DX_REG ? (N) : INVALID_REGNUM)
 #define EH_RETURN_STACKADJ_RTX	gen_rtx_REG (Pmode, CX_REG)
--- gcc/config/stormy16/stormy16.h.jj	2018-01-03 10:20:15.243537170 +0100
+++ gcc/config/stormy16/stormy16.h	2018-01-18 16:10:52.677954368 +0100
@@ -228,6 +228,7 @@ enum reg_class
 
 #define INCOMING_FRAME_SP_OFFSET (xstormy16_interrupt_function_p () ? -6 : -4)
 
+#define DEFAULT_INCOMING_FRAME_SP_OFFSET -4
 \f
 /* Register That Address the Stack Frame.  */
 
--- gcc/doc/tm.texi.in.jj	2018-01-14 17:16:48.573836465 +0100
+++ gcc/doc/tm.texi.in	2018-01-18 16:16:40.702976121 +0100
@@ -2559,6 +2559,15 @@ You only need to define this macro if yo
 debugging information like that provided by DWARF 2.
 @end defmac
 
+@defmac DEFAULT_INCOMING_FRAME_SP_OFFSET
+Like @code{INCOMING_FRAME_SP_OFFSET}, but must be the same for all
+functions of the same ABI, and when using GAS @code{.cfi_*} directives
+must also agree with the default CFI GAS emits.  Define this macro
+only if @code{INCOMING_FRAME_SP_OFFSET} can have different values
+between different functions of the same ABI or when
+@code{INCOMING_FRAME_SP_OFFSET} does not agree with GAS default CFI.
+@end defmac
+
 @defmac ARG_POINTER_CFA_OFFSET (@var{fundecl})
 A C expression whose value is an integer giving the offset, in bytes,
 from the argument pointer to the canonical frame address (cfa).  The
--- gcc/doc/tm.texi.jj	2018-01-14 17:16:48.529836466 +0100
+++ gcc/doc/tm.texi	2018-01-18 16:16:54.849977107 +0100
@@ -3168,6 +3168,15 @@ You only need to define this macro if yo
 debugging information like that provided by DWARF 2.
 @end defmac
 
+@defmac DEFAULT_INCOMING_FRAME_SP_OFFSET
+Like @code{INCOMING_FRAME_SP_OFFSET}, but must be the same for all
+functions of the same ABI, and when using GAS @code{.cfi_*} directives
+must also agree with the default CFI GAS emits.  Define this macro
+only if @code{INCOMING_FRAME_SP_OFFSET} can have different values
+between different functions of the same ABI or when
+@code{INCOMING_FRAME_SP_OFFSET} does not agree with GAS default CFI.
+@end defmac
+
 @defmac ARG_POINTER_CFA_OFFSET (@var{fundecl})
 A C expression whose value is an integer giving the offset, in bytes,
 from the argument pointer to the canonical frame address (cfa).  The

	Jakub

Re: [PATCH] Fix unwind info in x86 interrupt functions (PR debug/83728)

[Jason Merrill]

On Thu, Jan 18, 2018 at 6:28 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Last summer i386 INCOMING_FRAME_SP_OFFSET macro has been changed, so that it
> is one word for most of the functions (as previously always), but 2 words
> for functions with interrupt attribute.
> Unfortunately this breaks the unwind info, as can be seen on the
> gcc.dg/guality/pr68037-1.c testcase.  Our infrastructure assumes we have
> just one set of cfi instructions in the CIE (we can have multiple CIEs, but
> only for various flags, not for different cfi instructions), so if
> the first function being assembled is an interrupt function, we start
> with the 2*word initial offset in the CIE we emit (when
> -fno-dwarf2-cfi-asm), or just assume the CIE has that (with -fdwarf2-cfi-asm
> when it is GAS that emits it) even when it doesn't.  For -fdwarf2-cfi-asm
> the effect is that until first CFA adjustment the offset is off by a word,
> after that correct.  In both cases all CFA offsets are off by a word in
> non-interrupt functions.
> Or, if the first function doesn't have interrupt attribute, CFI in
> non-interrupt functions is correct, but interrupt functions are wrong.
>
> I've looked around and it seems stormy16 is the only other target that
> bases the INCOMING_FRAME_SP_OFFSET value not just on ABI changing switches,
> but on properties of the current function.
>
> For these 2 targets the following patch introduces another macro,
> DEFAULT_INCOMING_FRAME_SP_OFFSET, that is meant to be the same for all
> functions of the same ABI and where GAS supports .cfi_startproc it should
> also match what GAS does.  For the hopefully minority of functions that
> need something else (i.e. interrupt functions) we emit a CFI instruction
> right at the start of the function.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, fixes the pr68037-1.c
> FAILs, ok for trunk?

OK, thanks.

Jason

Re: [PATCH] Fix unwind info in x86 interrupt functions (PR debug/83728)

[Andreas Krebbel]

On 1/19/18 12:28 AM, Jakub Jelinek wrote:
+#ifndef DEFAULT_INCOMING_FRAME_SP_OFFSET
+#define DEFAULT_INCOMING_FRAME_SP_OFFSET INCOMING_FRAME_SP_OFFSET
+#endif
...
+  /* If the current function starts with a non-standard incoming frame
+     sp offset, emit a note before the first instruction.  */
+  if (entry
+      && DEFAULT_INCOMING_FRAME_SP_OFFSET != INCOMING_FRAME_SP_OFFSET)
+    {

This triggers a warning on s390x:

/home/andreas/build/../gcc/gcc/dwarf2cfi.c: In function ‘void scan_trace(dw_trace_info*, bool)’:
/home/andreas/build/../gcc/gcc/dwarf2cfi.c:2541:43: error: self-comparison always evaluates to false
[-Werror=tautol
ogical-compare]
 2541 |       && DEFAULT_INCOMING_FRAME_SP_OFFSET != INCOMING_FRAME_SP_OFFSET)
      |                                           ^~


Andreas