[PATCH] DWARF-2 unwinder off-by-one problem with signal frames

[Ulrich Weigand]

Hello,

better prolog optimizations have exposed a common code
DWARF-2 CFI unwinder problem on s390x.

The problem is shown by the gcc.dg/cleanup-8.c test case,
specifically the function fn2 (which gets fn3 inlined):

static void fn3 ()
{
  abort ();
}

static int __attribute__((noinline)) fn2 ()
{
  *null = 0;
  fn3 ();
  return 0;
}

This now results in the following code:

fn2:
.LFB26:
        larl    %r1,null                # &null -> %r1
        stmg    %r14,%r15,112(%r15)	# %r14 (ra), %r15 (sp) -> stack
.LCFI6:
        lg      %r2,0(%r1)		# null -> %r2
        aghi    %r15,-160		# allocate stack frame
.LCFI7:
        mvi     0(%r2),0		# 0 -> *null
        brasl   %r14,abort		# call abort
.LFE26:

Note in particular that the insn generating the trap ('mvi') is
immediately adjacent to the insn that allocates the stack frame
(by decrementing the stack pointer %r15).

Now, once the trap happens, the signal handler gets called, which 
performs a DWARF-2 unwind.  This works up to the signal handler
frame.  When trying to unwind up into the fn2 frame, however,
we have a problem.

This is because the return address from the signal handler frame
points to the start address of the instruction that trapped,
i.e. in this case context->ra equals the .LCFI7 symbol above.

When uw_frame_state_for now tries to get the frame state for that
return address, it uses execute_cfa_program to process the FDE
instructions up to the return address; however, that routine 
contains the following code:

  /* The comparison with the return address uses < rather than <= because
     we are only interested in the effects of code before the call; for a
     noreturn function, the return address may point to unrelated code with
     a different stack configuration that we are not interested in.  We
     assume that the call itself is unwind info-neutral; if not, or if
     there are delay instructions that adjust the stack, these must be
     reflected at the point immediately before the call insn.  */
  while (insn_ptr < insn_end && fs->pc < context->ra)

so it actually scans the FDE only up to *before* .LCFI7.  This means 
that the frame state's assumption of where the stack pointer points to
is off by 160 bytes, and subsequently the return address is restored
from an incorrect location.


The question is how to fix this.  Going back one byte makes perfect
sense for a normal 'return address' pointing to *after* a call insn.
However, for the special case of a signal handler 'return address',
which may well point *to* the trapping insn, it is wrong.

GDB has the same problem, but there it is solved by explicitly
distinguishing the two cases, see frame_unwind_address_in_block:

  /* If THIS frame is not inner most (i.e., NEXT isn't the sentinel),
     and NEXT is `normal' (i.e., not a sigtramp, dummy, ....) THIS
     frame's PC ends up pointing at the instruction fallowing the
     "call".  Adjust that PC value so that it falls on the call
     instruction (which, hopefully, falls within THIS frame's code
     block.  So far it's proved to be a very good approximation.  See
     get_frame_type() for why ->type can't be used.  */
  if (next_frame->level >= 0
      && get_frame_type (next_frame) == NORMAL_FRAME)
    --pc;


Unfortunately, in GCC's unwinder we don't actually have the concept
of a 'frame type'; signal frames are simply made up to appear like
call frames using MD_FALLBACK_FRAME_STATE_FOR.

One solution I've tried (and which fixes my test case) is to handle
this situation in MD_FROB_UPDATE_CONTEXT: when we just unwound from
a signal frame, *increment* the return address by one (so that when
common code decrements again, we get the result we want).

This requires moving the call to MD_FROB_UPDATE_CONTEXT from 
uw_update_context_1 to uw_update_context until after the return
address field is actually filled in.


Bootstrapped/regtested on s390-ibm-linux and s390x-ibm-linux.
OK?

Bye,
Ulrich


ChangeLog:

	* unwind-dw2.c (uw_update_context_1): Move call to 
	MD_FROB_UPDATE_CONTEXT from here ...
	(uw_update_context): ... to here.

	* config/s390/linux.h (MD_FROB_UPDATE_CONTEXT): Define.

Index: gcc/unwind-dw2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/unwind-dw2.c,v
retrieving revision 1.41
diff -c -p -r1.41 unwind-dw2.c
*** gcc/unwind-dw2.c	20 May 2004 22:34:58 -0000	1.41
--- gcc/unwind-dw2.c	30 Jun 2004 00:45:39 -0000
*************** uw_update_context_1 (struct _Unwind_Cont
*** 1230,1237 ****
  	}
  	break;
        }
- 
-   MD_FROB_UPDATE_CONTEXT (context, fs);
  }
  
  /* CONTEXT describes the unwind state for a frame, and FS describes the FDE
--- 1230,1235 ----
*************** uw_update_context (struct _Unwind_Contex
*** 1248,1253 ****
--- 1246,1253 ----
       can change from frame to frame.  */
    context->ra = __builtin_extract_return_addr
      (_Unwind_GetPtr (context, fs->retaddr_column));
+ 
+   MD_FROB_UPDATE_CONTEXT (context, fs);
  }
  
  /* Fill in CONTEXT for top-of-stack.  The only valid registers at this
Index: gcc/config/s390/linux.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/linux.h,v
retrieving revision 1.34
diff -c -p -r1.34 linux.h
*** gcc/config/s390/linux.h	29 Nov 2003 03:08:12 -0000	1.34
--- gcc/config/s390/linux.h	30 Jun 2004 00:45:40 -0000
*************** Software Foundation, 59 Temple Place - S
*** 170,173 ****
--- 170,186 ----
      goto SUCCESS;							\
    } while (0)
  
+ /* unwind-dw2.c decrements context->ra by one in order not to fall
+    off the end of the caller of a noreturn function.  This breaks
+    if the inner frame was a signal frame, as the return address 
+    will point to the failing instruction; decrementing by one may
+    thus yield an incorrect CFI.  We work around this by *incrementing*
+    the return address here if we came from a signal frame.  */
+ 
+ #define MD_FROB_UPDATE_CONTEXT(CONTEXT, FS)				\
+   do {									\
+     if ((FS)->retaddr_column == 32)					\
+       (CONTEXT)->ra++;							\
+   } while (0)
+ 
  #endif
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de