Notes on a frame_unwind_address_in_block problem

Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

I'm just going to write up this problem, which I've been working on for the
last couple of hours, in hopes that someone else has a bright idea - or so
I can at least find my notes next time it bugs me.  Mark, if you have a
moment, there's a question for you all the way down at the bottom, in
the 32-bit section :-)


I. 64-bit

On my AMD64 GNU/Linux system, sigaltstack.exp is currently failing both
32-bit and 64-bit with similar symptoms.  Saying "finish" from a signal
handler fails to stop in the trampoline.  We correctly insert the
breakpoint, but the frame ID doesn't match: get_frame_id (get_prev_frame
(get_current_frame ())) when in the signal handler does not equal
get_frame_id (get_current_frame ()) once we've returned to the trampoline.

The causes are quite different despite the similar symptoms.  On 64-bit,
it appears to be partly related to bogus call frame information in glibc's
__restore_rt - there's dwarf2 info but (A) it doesn't start until the first
instruction of the trampoline, when for signal trampolines it ought to start
one byte earlier, and (B) it doesn't describe the signal frame at all.

Another problem is that sometimes we get the amd64-specific frame unwinder
for this code and sometimes we get the dwarf2 unwinder.  When $rip is in
__restore_rt, the dwarf2 frame sniffer successfully attaches to it.  At that
point, because of the bogus CFI, we can't backtrace.  But when it's up the
frame, the dwarf2 sniffer doesn't get it (because the unwind information
doesn't start one byte too early, as we seem to have concluded that it ought
to).  So instead the amd64 fallback sniffer gets control, identifies it as a
sigtramp, and sets up for real backtraces.

I think that correct CFI in glibc for the signal restore trampolines will
sort out the 64-bit case.  Or no CFI, but glibc would probably want correct
CFI for other reasons.


II. 32-bit.

For 32-bit, though, it gets even more interesting.  Here's where $SUBJECT
comes into play.  I have a loaded vDSO (virtual shared object), which
exports __kernel_sigreturn.  This points at the first instruction of the
trampoline, i.e. one byte after the start of the dwarf2 FDE.  When I am
stopped in the signal handler, frame_unwind_address_in_block decides to
subtract one.  That points before the symbol, so the frame ID's function
ends up being NULL; there's no symbol covering that address.  Then when we
arrive at the signal trampoline during "finish", we no longer subtract one
- since we're at an executable instruction in the topmost frame - and thus
we do find the symbol.  The two frame IDs don't compare equal.

One possible solution is the nasty patch I have in my working directory,
which boils down to this:

   if (next_frame->level >= 0
-      && get_frame_type (next_frame) == NORMAL_FRAME)
+      && get_frame_type (next_frame) == NORMAL_FRAME
+      && (next_frame->prev == NULL
+         || next_frame->prev->unwind == NULL
+         || get_frame_type (next_frame->prev) == NORMAL_FRAME))
     --pc;

But this makes frame_unwind_address_in_block change its behavior over time
for the same frame, which is awful.

Another solution would be to use the FDE start address as the code address
for dwarf2 signal frame IDs, instead of the function.  This would work on
the assumption that a single FDE would generally cover the entire trampoline
- a reasonable assumption, I think, and the consequences for the frame ID
changing while single-stepping are less disruptive here than the
alternative.

Mark, what do you think of that idea?  It seems to work.  It looks like the
patch at the end of this message.

-- 
Daniel Jacobowitz
CodeSourcery

2006-07-06  Daniel Jacobowitz  <dan@codesourcery.com>

	* dwarf2-frame.c (struct dwarf2_frame_cache): Add fde_start.
	(dwarf2_frame_cache): Set it.
	(dwarf2_signal_frame_this_id): New function.
	(dwarf2_signal_frame_unwind): Use it.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.63
diff -u -p -r1.63 dwarf2-frame.c
--- dwarf2-frame.c	28 May 2006 05:56:50 -0000	1.63
+++ dwarf2-frame.c	6 Jul 2006 22:20:10 -0000
@@ -725,6 +725,9 @@ struct dwarf2_frame_cache
   /* Set if the return address column was marked as undefined.  */
   int undefined_retaddr;
 
+  /* The FDE start address.  */
+  CORE_ADDR fde_start;
+
   /* Saved registers, indexed by GDB register number, not by DWARF
      register number.  */
   struct dwarf2_frame_state_reg *reg;
@@ -775,6 +778,7 @@ dwarf2_frame_cache (struct frame_info *n
   /* Find the correct FDE.  */
   fde = dwarf2_frame_find_fde (&fs->pc);
   gdb_assert (fde != NULL);
+  cache->fde_start = fde->initial_location;
 
   /* Extract any interesting information from the CIE.  */
   fs->data_align = fde->cie->data_alignment_factor;
@@ -931,6 +935,19 @@ dwarf2_frame_this_id (struct frame_info 
 }
 
 static void
+dwarf2_signal_frame_this_id (struct frame_info *next_frame, void **this_cache,
+			     struct frame_id *this_id)
+{
+  struct dwarf2_frame_cache *cache =
+    dwarf2_frame_cache (next_frame, this_cache);
+
+  if (cache->undefined_retaddr)
+    return;
+
+  (*this_id) = frame_id_build (cache->cfa, cache->fde_start);
+}
+
+static void
 dwarf2_frame_prev_register (struct frame_info *next_frame, void **this_cache,
 			    int regnum, int *optimizedp,
 			    enum lval_type *lvalp, CORE_ADDR *addrp,
@@ -1095,7 +1112,7 @@ static const struct frame_unwind dwarf2_
 static const struct frame_unwind dwarf2_signal_frame_unwind =
 {
   SIGTRAMP_FRAME,
-  dwarf2_frame_this_id,
+  dwarf2_signal_frame_this_id,
   dwarf2_frame_prev_register
 };
 

Re: Notes on a frame_unwind_address_in_block problem

[Mark Kettenis]

> Date: Thu, 6 Jul 2006 18:21:57 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> I. 64-bit
> 
> On my AMD64 GNU/Linux system, sigaltstack.exp is currently failing both
> 32-bit and 64-bit with similar symptoms.  Saying "finish" from a signal
> handler fails to stop in the trampoline.  We correctly insert the
> breakpoint, but the frame ID doesn't match: get_frame_id (get_prev_frame
> (get_current_frame ())) when in the signal handler does not equal
> get_frame_id (get_current_frame ()) once we've returned to the trampoline.

That's bad.

> The causes are quite different despite the similar symptoms.  On 64-bit,
> it appears to be partly related to bogus call frame information in glibc's
> __restore_rt - there's dwarf2 info but (A) it doesn't start until the first
> instruction of the trampoline, when for signal trampolines it ought to start
> one byte earlier, and (B) it doesn't describe the signal frame at all.

It depends a bit on how the implementation of the trampoline.  If the
trampoline itself calls the signal handler, there's no problem, but if
the kernel calls the signal handler directly and sets up the stack to
return to the signal handler, then yes, the unwind info should really
start before the "entry point" of the trampoline.

Looking at sysdeps/unix/sysv/linux/x86_64/sigaction.c in the glibc sources:

asm                                             \
  (                                             \
   ".align 16\n"                                \
   CFI_STARTPROC "\n"                           \
   "__" #name ":\n"                             \
   "    movq $" #syscall ", %rax\n"             \
   "    syscall\n"                              \
   CFI_ENDPROC "\n"                             \
   );

Someone should either add the proper unwind info or remove the unwind
info altogether.

> Another problem is that sometimes we get the amd64-specific frame unwinder
> for this code and sometimes we get the dwarf2 unwinder.  When $rip is in
> __restore_rt, the dwarf2 frame sniffer successfully attaches to it.  At that
> point, because of the bogus CFI, we can't backtrace.  But when it's up the
> frame, the dwarf2 sniffer doesn't get it (because the unwind information
> doesn't start one byte too early, as we seem to have concluded that it ought
> to).  So instead the amd64 fallback sniffer gets control, identifies it as a
> sigtramp, and sets up for real backtraces.

The proper way to fix this is to fix glibc.

> I think that correct CFI in glibc for the signal restore trampolines will
> sort out the 64-bit case.  Or no CFI, but glibc would probably want correct
> CFI for other reasons.

I think the ABI calls for CFI, but really, no CFI would be much better
than the bogus CFI.

> II. 32-bit.
> 
> For 32-bit, though, it gets even more interesting.  Here's where $SUBJECT
> comes into play.  I have a loaded vDSO (virtual shared object), which
> exports __kernel_sigreturn.  This points at the first instruction of the
> trampoline, i.e. one byte after the start of the dwarf2 FDE.  When I am
> stopped in the signal handler, frame_unwind_address_in_block decides to
> subtract one.  That points before the symbol, so the frame ID's function
> ends up being NULL; there's no symbol covering that address.  Then when we
> arrive at the signal trampoline during "finish", we no longer subtract one
> - since we're at an executable instruction in the topmost frame - and thus
> we do find the symbol.  The two frame IDs don't compare equal.

Ouch.  So in the end having the signal trampoline call the signal
handler *is* a better design.

> One possible solution is the nasty patch I have in my working directory,
> which boils down to this:
> 
>    if (next_frame->level >= 0
> -      && get_frame_type (next_frame) == NORMAL_FRAME)
> +      && get_frame_type (next_frame) == NORMAL_FRAME
> +      && (next_frame->prev == NULL
> +         || next_frame->prev->unwind == NULL
> +         || get_frame_type (next_frame->prev) == NORMAL_FRAME))
>      --pc;
> 
> But this makes frame_unwind_address_in_block change its behavior over time
> for the same frame, which is awful.

I'm always very suspicious of such multi-condition if statements.

> Another solution would be to use the FDE start address as the code address
> for dwarf2 signal frame IDs, instead of the function.  This would work on
> the assumption that a single FDE would generally cover the entire trampoline
> - a reasonable assumption, I think, and the consequences for the frame ID
> changing while single-stepping are less disruptive here than the
> alternative.
> 
> Mark, what do you think of that idea?  It seems to work.  It looks like the
> patch at the end of this message.

In general, I think it's a bad idea to do this, but for the special
case of a signal frame, especially in the presence of the 'S"
augmentation, that might be a reasonable thing to do.  However, I
think we can do better than that.  What about checking whether the
address returned by frame_unwind_address_in_block() is equal to the
FDE start address and add one bytes if that's the case before looking
up the function corresponding to that address?

Mark

Re: Notes on a frame_unwind_address_in_block problem

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

Mark Kettenis <mark.kettenis@xs4all.nl> writes:

> [...]
> Looking at sysdeps/unix/sysv/linux/x86_64/sigaction.c in the glibc sources:
>
> asm                                             \
>   (                                             \
>    ".align 16\n"                                \
>    CFI_STARTPROC "\n"                           \
>    "__" #name ":\n"                             \
>    "    movq $" #syscall ", %rax\n"             \
>    "    syscall\n"                              \
>    CFI_ENDPROC "\n"                             \
>    );
>
> Someone should either add the proper unwind info or remove the unwind
> info altogether.

What do you suggest to add here?

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj/
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Notes on a frame_unwind_address_in_block problem

[Mark Kettenis]

>  Mark Kettenis <mark.kettenis@xs4all.nl> writes:
>
> > [...]
> > Looking at sysdeps/unix/sysv/linux/x86_64/sigaction.c in the glibc
> > sources:
> >
> > asm                                             \
> >   (                                             \
> >    ".align 16\n"                                \
> >    CFI_STARTPROC "\n"                           \
> >    "__" #name ":\n"                             \
> >    "    movq $" #syscall ", %rax\n"             \
> >    "    syscall\n"                              \
> >    CFI_ENDPROC "\n"                             \
> >    );
> >
> > Someone should either add the proper unwind info or remove the unwind
> > info altogether.
>
>  What do you suggest to add here?

Something like what's done in the kernel (arch/x86_64/kernel/vsyscall.S).
Hmm, I wonder why Daniel's box uses the trampoline from libc instead of
the trampoline in the vsyscall page.  Looking a bit closer, it seems my
amd64 machine here at work has the same issue (SuSE 9.2 with a 2.6.8-24.20
kernel and glibc 2.3.3).  Is it just my glibc that's out of date, or is
something busted?

Anyway, if with the current libc, the trampoline provided by the kernel is
supposed to be used, then it's probably not worth bothering to add CFI
to libc, and I'd just remove the CFI_STARTPROC and CFI_ENDPROC statements.

Mark

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Mon, Jul 17, 2006 at 09:29:46AM +0200, Mark Kettenis wrote:
> Something like what's done in the kernel (arch/x86_64/kernel/vsyscall.S).
> Hmm, I wonder why Daniel's box uses the trampoline from libc instead of
> the trampoline in the vsyscall page.

Ah, now, this is a very interesting question.  I'm glad you asked :-)

__libc_sigaction (int sig, const struct sigaction *act, struct
sigaction *oact)
{
  int result;
  struct kernel_sigaction kact, koact;

  if (act)
    {
      kact.k_sa_handler = act->sa_handler;
      memcpy (&kact.sa_mask, &act->sa_mask, sizeof (sigset_t));
      kact.sa_flags = act->sa_flags | SA_RESTORER;

      kact.sa_restorer = &restore_rt;
    }

That's how we end up at the trampoline: through use of SA_RESTORER.
I didn't respond to this earlier because I wanted to find some time to
check whether that was necessary.

Andreas, looking at the i386 version, I guess that using SA_RESTORER
this way is not necessary.  Simply a performance optimization because
the old trampolines (written to the stack) were so slow, or maybe
because they required an executable stack.  i386 has
"if (GLRO(dl_sysinfo_dso) == NULL)" around it.  Can x86_64 do the same
thing?

The existing unwind information would still be wrong, but on systems
with a vDSO it wouldn't matter any more.

> Anyway, if with the current libc, the trampoline provided by the kernel is
> supposed to be used, then it's probably not worth bothering to add CFI
> to libc, and I'd just remove the CFI_STARTPROC and CFI_ENDPROC statements.

Either way seems reasonable.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Notes on a frame_unwind_address_in_block problem

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

Daniel Jacobowitz <drow@false.org> writes:

> On Mon, Jul 17, 2006 at 09:29:46AM +0200, Mark Kettenis wrote:
>> Something like what's done in the kernel (arch/x86_64/kernel/vsyscall.S).
>> Hmm, I wonder why Daniel's box uses the trampoline from libc instead of
>> the trampoline in the vsyscall page.
>
> Ah, now, this is a very interesting question.  I'm glad you asked :-)
>
> __libc_sigaction (int sig, const struct sigaction *act, struct
> sigaction *oact)
> {
>   int result;
>   struct kernel_sigaction kact, koact;
>
>   if (act)
>     {
>       kact.k_sa_handler = act->sa_handler;
>       memcpy (&kact.sa_mask, &act->sa_mask, sizeof (sigset_t));
>       kact.sa_flags = act->sa_flags | SA_RESTORER;
>
>       kact.sa_restorer = &restore_rt;
>     }
>
> That's how we end up at the trampoline: through use of SA_RESTORER.
> I didn't respond to this earlier because I wanted to find some time to
> check whether that was necessary.
>
> Andreas, looking at the i386 version, I guess that using SA_RESTORER
> this way is not necessary.  Simply a performance optimization because
> the old trampolines (written to the stack) were so slow, or maybe
> because they required an executable stack.  i386 has
> "if (GLRO(dl_sysinfo_dso) == NULL)" around it.  Can x86_64 do the same
> thing?

i386 is the only platform doing it.  I don't know the history of the
change and whether this is the right thing to do.  Is somebody willing
to test this?

> The existing unwind information would still be wrong, but on systems
> with a vDSO it wouldn't matter any more.
>
>> Anyway, if with the current libc, the trampoline provided by the kernel is
>> supposed to be used, then it's probably not worth bothering to add CFI
>> to libc, and I'd just remove the CFI_STARTPROC and CFI_ENDPROC statements.
>
> Either way seems reasonable.

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj/
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Tue, Jul 18, 2006 at 09:24:39AM +0200, Andreas Jaeger wrote:
> > Andreas, looking at the i386 version, I guess that using SA_RESTORER
> > this way is not necessary.  Simply a performance optimization because
> > the old trampolines (written to the stack) were so slow, or maybe
> > because they required an executable stack.  i386 has
> > "if (GLRO(dl_sysinfo_dso) == NULL)" around it.  Can x86_64 do the same
> > thing?
> 
> i386 is the only platform doing it.  I don't know the history of the
> change and whether this is the right thing to do.  Is somebody willing
> to test this?

Well, I tried.  But it looks like I was just wrong about the situation.
i386 has a vDSO and uses it for signal return trampolines, but x86_64
doesn't have a vDSO and in fact requires the use of SA_RESTORER.  All
x86_64 has is the vsyscall page, which is older.

(There's something claiming to be [vdso] mapped in, but I think it's
just the vsyscalls.)

So I came up with this alternative patch.  Glibc tests fine, and GDB
(combined with a patch for the function-address-in-block issue mentioned
upthread) improves quite a lot.

The only caveat is that it is in glibc, but knows about the kernel
layout of struct rt_sigframe - specifically that a struct ucontext
lives just above the SA_RESTORER return address.  Andi, what do you
think of that?  If it's a bad idea, we may need a different approach
(i.e. a vdso).

-- 
Daniel Jacobowitz
CodeSourcery

2006-07-18  Daniel Jacobowitz  <dan@codesourcery.com>

	* sysdeps/unix/sysv/linux/x86_64/sigaction.c (restore_rt): Add correct
	unwind information.

Index: sysdeps/unix/sysv/linux/x86_64/sigaction.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/unix/sysv/linux/x86_64/sigaction.c,v
retrieving revision 1.9
diff -u -p -r1.9 sigaction.c
--- sysdeps/unix/sysv/linux/x86_64/sigaction.c	29 Dec 2005 16:32:17 -0000	1.9
+++ sysdeps/unix/sysv/linux/x86_64/sigaction.c	18 Jul 2006 17:52:21 -0000
@@ -95,18 +95,94 @@ weak_alias (__libc_sigaction, sigaction)
    signal handlers work right.  Important are both the names
    (__restore_rt) and the exact instruction sequence.
    If you ever feel the need to make any changes, please notify the
-   appropriate GDB maintainer.  */
+   appropriate GDB maintainer.
+
+   The unwind information starts a byte before __restore_rt, so that
+   it is found when unwinding, to get an address the unwinder assumes
+   will be in the middle of a call instruction.  See the Linux kernel
+   (the i386 vsyscall, in particular) for an explanation of the complex
+   unwind information used here in order to get the traditional CFA.
+   We do not restore cs - it's only stored as two bytes here so that's
+   a bit tricky.  We don't use the gas cfi directives, so that we can
+   reliably add .cfi_signal_frame.  */
+
+#define do_cfa_expr \
+  ".byte 0x0f\n"	/* DW_CFA_def_cfa_expression */	\
+  ".byte 0x04\n"	/* length */		\
+  ".byte 0x77\n"   	/* DW_OP_breg8 */	\
+  ".byte 0xa0\n"				\
+  ".byte 0x01\n"	/* 160 */		\
+  ".byte 0x06\n"	/* DW_OP_deref */
+
+#define do_expr_1(regno, offset) \
+  ".byte 0x10\n"	/* DW_CFA_expression */	\
+  ".byte " CFI_STRINGIFY(regno) "\n"		\
+  ".byte 0x02\n"	/* length */		\
+  ".byte 0x77\n"   	/* DW_OP_breg8 */	\
+  ".byte " CFI_STRINGIFY(offset) "\n"		\
+
+#define do_expr_2(regno, offset1, offset2) \
+  ".byte 0x10\n"	/* DW_CFA_expression */	\
+  ".byte " CFI_STRINGIFY(regno) "\n"		\
+  ".byte 0x03\n"	/* length */		\
+  ".byte 0x77\n"   	/* DW_OP_breg8 */	\
+  ".byte " CFI_STRINGIFY(offset1) "\n"		\
+  ".byte " CFI_STRINGIFY(offset2) "\n"		\
 
 #define RESTORE(name, syscall) RESTORE2 (name, syscall)
 # define RESTORE2(name, syscall) \
 asm						\
   (						\
    ".align 16\n"				\
-   CFI_STARTPROC "\n"				\
+   ".LSTART_" #name ":\n"			\
+   "	nop\n"					\
    "__" #name ":\n"				\
    "	movq $" #syscall ", %rax\n"		\
    "	syscall\n"				\
-   CFI_ENDPROC "\n"				\
+   ".LEND_" #name ":\n"				\
+   ".section .eh_frame,\"a\",@progbits\n"				\
+   ".LSTARTFRAME_" #name ":\n"						\
+   "	.long .LENDCIE_" #name "-.LSTARTCIE_" #name "\n"		\
+   ".LSTARTCIE_" #name ":\n"						\
+   "	.long 0\n"	/* CIE ID */					\
+   "	.byte 1\n"	/* Version number */				\
+   "	.string \"zRS\"\n" /* NUL-terminated augmentation string */	\
+   "	.uleb128 1\n"	/* Code alignment factor */			\
+   "	.sleb128 -8\n"	/* Data alignment factor */			\
+   "	.byte 0x10\n"	/* Return address register column */		\
+   "	.uleb128 1\n"	/* Augmentation value length */			\
+   "	.byte 0x1b\n"	/* DW_EH_PE_pcrel|DW_EH_PE_sdata4. */		\
+   "	.byte 0\n"	/* DW_CFA_nop */				\
+   "	.align 8\n"							\
+   ".LENDCIE_" #name ":\n"						\
+   "	.long .LENDFDE_" #name "-.LSTARTFDE_" #name "\n" /* FDE len */	\
+   ".LSTARTFDE_" #name ":\n"						\
+   "	.long .LSTARTFDE_" #name "-.LSTARTFRAME_" #name "\n" /* CIE */	\
+   "	.long .LSTART_" #name "-.\n"	/* PC-relative start address */	\
+   "	.long .LEND_" #name "-.LSTART_" #name "\n"			\
+   "	.uleb128 0\n"			/* Augmentation */		\
+   do_cfa_expr					\
+   do_expr_1 (8, 0x28)				\
+   do_expr_1 (9, 0x30)				\
+   do_expr_1 (10, 0x38)				\
+   do_expr_2 (11, 0xc0, 0x00)			\
+   do_expr_2 (12, 0xc8, 0x00)			\
+   do_expr_2 (13, 0xd0, 0x00)			\
+   do_expr_2 (14, 0xd8, 0x00)			\
+   do_expr_2 (15, 0xe0, 0x00)			\
+   do_expr_2 (5, 0xe8, 0x00)			\
+   do_expr_2 (4, 0xf0, 0x00)			\
+   do_expr_2 (6, 0xf8, 0x00)			\
+   do_expr_2 (3, 0x80, 0x01)			\
+   do_expr_2 (1, 0x88, 0x01)			\
+   do_expr_2 (0, 0x90, 0x01)			\
+   do_expr_2 (2, 0x98, 0x01)			\
+   do_expr_2 (7, 0xa0, 0x01)			\
+   do_expr_2 (16, 0xa8, 0x01)			\
+   do_expr_2 (41, 0xb0, 0x01)			\
+   ".align 4\n"					\
+   ".LENDFDE_" #name ":\n"			\
+   ".previous\n"				\
    );
 /* The return code for realtime-signals.  */
 RESTORE (restore_rt, __NR_rt_sigreturn)

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Tue, Jul 18, 2006 at 02:35:20PM -0400, Daniel Jacobowitz wrote:
> The only caveat is that it is in glibc, but knows about the kernel
> layout of struct rt_sigframe - specifically that a struct ucontext
> lives just above the SA_RESTORER return address.  Andi, what do you
> think of that?  If it's a bad idea, we may need a different approach
> (i.e. a vdso).

Andreas, Andi, did either of you have an opinion on this?  I'd really
like to get the "GDB bug" fixed one way or the other.  But I'm not
sure yet if we can do it with this glibc patch, or whether we need a
true ELF vDSO - or what other x86_64 specific issues that might have.

> 2006-07-18  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* sysdeps/unix/sysv/linux/x86_64/sigaction.c (restore_rt): Add correct
> 	unwind information.
> 
> Index: sysdeps/unix/sysv/linux/x86_64/sigaction.c
> ===================================================================
> RCS file: /cvs/glibc/libc/sysdeps/unix/sysv/linux/x86_64/sigaction.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 sigaction.c
> --- sysdeps/unix/sysv/linux/x86_64/sigaction.c	29 Dec 2005 16:32:17 -0000	1.9
> +++ sysdeps/unix/sysv/linux/x86_64/sigaction.c	18 Jul 2006 17:52:21 -0000
> @@ -95,18 +95,94 @@ weak_alias (__libc_sigaction, sigaction)
>     signal handlers work right.  Important are both the names
>     (__restore_rt) and the exact instruction sequence.
>     If you ever feel the need to make any changes, please notify the
> -   appropriate GDB maintainer.  */
> +   appropriate GDB maintainer.
> +
> +   The unwind information starts a byte before __restore_rt, so that
> +   it is found when unwinding, to get an address the unwinder assumes
> +   will be in the middle of a call instruction.  See the Linux kernel
> +   (the i386 vsyscall, in particular) for an explanation of the complex
> +   unwind information used here in order to get the traditional CFA.
> +   We do not restore cs - it's only stored as two bytes here so that's
> +   a bit tricky.  We don't use the gas cfi directives, so that we can
> +   reliably add .cfi_signal_frame.  */
> +
> +#define do_cfa_expr \
> +  ".byte 0x0f\n"	/* DW_CFA_def_cfa_expression */	\
> +  ".byte 0x04\n"	/* length */		\
> +  ".byte 0x77\n"   	/* DW_OP_breg8 */	\
> +  ".byte 0xa0\n"				\
> +  ".byte 0x01\n"	/* 160 */		\
> +  ".byte 0x06\n"	/* DW_OP_deref */
> +
> +#define do_expr_1(regno, offset) \
> +  ".byte 0x10\n"	/* DW_CFA_expression */	\
> +  ".byte " CFI_STRINGIFY(regno) "\n"		\
> +  ".byte 0x02\n"	/* length */		\
> +  ".byte 0x77\n"   	/* DW_OP_breg8 */	\
> +  ".byte " CFI_STRINGIFY(offset) "\n"		\
> +
> +#define do_expr_2(regno, offset1, offset2) \
> +  ".byte 0x10\n"	/* DW_CFA_expression */	\
> +  ".byte " CFI_STRINGIFY(regno) "\n"		\
> +  ".byte 0x03\n"	/* length */		\
> +  ".byte 0x77\n"   	/* DW_OP_breg8 */	\
> +  ".byte " CFI_STRINGIFY(offset1) "\n"		\
> +  ".byte " CFI_STRINGIFY(offset2) "\n"		\
>  
>  #define RESTORE(name, syscall) RESTORE2 (name, syscall)
>  # define RESTORE2(name, syscall) \
>  asm						\
>    (						\
>     ".align 16\n"				\
> -   CFI_STARTPROC "\n"				\
> +   ".LSTART_" #name ":\n"			\
> +   "	nop\n"					\
>     "__" #name ":\n"				\
>     "	movq $" #syscall ", %rax\n"		\
>     "	syscall\n"				\
> -   CFI_ENDPROC "\n"				\
> +   ".LEND_" #name ":\n"				\
> +   ".section .eh_frame,\"a\",@progbits\n"				\
> +   ".LSTARTFRAME_" #name ":\n"						\
> +   "	.long .LENDCIE_" #name "-.LSTARTCIE_" #name "\n"		\
> +   ".LSTARTCIE_" #name ":\n"						\
> +   "	.long 0\n"	/* CIE ID */					\
> +   "	.byte 1\n"	/* Version number */				\
> +   "	.string \"zRS\"\n" /* NUL-terminated augmentation string */	\
> +   "	.uleb128 1\n"	/* Code alignment factor */			\
> +   "	.sleb128 -8\n"	/* Data alignment factor */			\
> +   "	.byte 0x10\n"	/* Return address register column */		\
> +   "	.uleb128 1\n"	/* Augmentation value length */			\
> +   "	.byte 0x1b\n"	/* DW_EH_PE_pcrel|DW_EH_PE_sdata4. */		\
> +   "	.byte 0\n"	/* DW_CFA_nop */				\
> +   "	.align 8\n"							\
> +   ".LENDCIE_" #name ":\n"						\
> +   "	.long .LENDFDE_" #name "-.LSTARTFDE_" #name "\n" /* FDE len */	\
> +   ".LSTARTFDE_" #name ":\n"						\
> +   "	.long .LSTARTFDE_" #name "-.LSTARTFRAME_" #name "\n" /* CIE */	\
> +   "	.long .LSTART_" #name "-.\n"	/* PC-relative start address */	\
> +   "	.long .LEND_" #name "-.LSTART_" #name "\n"			\
> +   "	.uleb128 0\n"			/* Augmentation */		\
> +   do_cfa_expr					\
> +   do_expr_1 (8, 0x28)				\
> +   do_expr_1 (9, 0x30)				\
> +   do_expr_1 (10, 0x38)				\
> +   do_expr_2 (11, 0xc0, 0x00)			\
> +   do_expr_2 (12, 0xc8, 0x00)			\
> +   do_expr_2 (13, 0xd0, 0x00)			\
> +   do_expr_2 (14, 0xd8, 0x00)			\
> +   do_expr_2 (15, 0xe0, 0x00)			\
> +   do_expr_2 (5, 0xe8, 0x00)			\
> +   do_expr_2 (4, 0xf0, 0x00)			\
> +   do_expr_2 (6, 0xf8, 0x00)			\
> +   do_expr_2 (3, 0x80, 0x01)			\
> +   do_expr_2 (1, 0x88, 0x01)			\
> +   do_expr_2 (0, 0x90, 0x01)			\
> +   do_expr_2 (2, 0x98, 0x01)			\
> +   do_expr_2 (7, 0xa0, 0x01)			\
> +   do_expr_2 (16, 0xa8, 0x01)			\
> +   do_expr_2 (41, 0xb0, 0x01)			\
> +   ".align 4\n"					\
> +   ".LENDFDE_" #name ":\n"			\
> +   ".previous\n"				\
>     );
>  /* The return code for realtime-signals.  */
>  RESTORE (restore_rt, __NR_rt_sigreturn)
> 

-- 
Daniel Jacobowitz
CodeSourcery

Re: Notes on a frame_unwind_address_in_block problem

[Andi Kleen]

On Thursday 03 August 2006 04:04, Daniel Jacobowitz wrote:
> On Tue, Jul 18, 2006 at 02:35:20PM -0400, Daniel Jacobowitz wrote:
> > The only caveat is that it is in glibc, but knows about the kernel
> > layout of struct rt_sigframe - specifically that a struct ucontext
> > lives just above the SA_RESTORER return address.  Andi, what do you
> > think of that?  If it's a bad idea, we may need a different approach
> > (i.e. a vdso).

Missing context a bit. What exactly is the problem?

> Andreas, Andi, did either of you have an opinion on this?  I'd really
> like to get the "GDB bug" fixed one way or the other.  But I'm not
> sure yet if we can do it with this glibc patch, or whether we need a
> true ELF vDSO - or what other x86_64 specific issues that might have.

We'll get a vDSO with kernel supplied unwind sectins sooner or later, but 
you'll have to handle the old vsyscall without unwinding anyways because 
it's not going to go away.

Also even the vDSO might end up without unwind information when compiled
with old compilers because I don't plan to support it without .cfi_*
support in binutils.

-Andi

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Thu, Aug 03, 2006 at 04:38:18AM +0200, Andi Kleen wrote:
> On Thursday 03 August 2006 04:04, Daniel Jacobowitz wrote:
> > On Tue, Jul 18, 2006 at 02:35:20PM -0400, Daniel Jacobowitz wrote:
> > > The only caveat is that it is in glibc, but knows about the kernel
> > > layout of struct rt_sigframe - specifically that a struct ucontext
> > > lives just above the SA_RESTORER return address.  Andi, what do you
> > > think of that?  If it's a bad idea, we may need a different approach
> > > (i.e. a vdso).
> 
> Missing context a bit. What exactly is the problem?

Here's some context:
  http://sourceware.org/ml/gdb/2006-07/msg00131.html

Basically, right now x86_64 signal delivery always uses SA_RESTORER.
Glibc provides the restorer.  It has some minimal, incorrect unwind
information.  If I remove the unwind information entirely from glibc,
GDB will know how to do the right thing through a signal handler - but
other unwinding scenarios like _Unwind_Backtrace won't.

I can add correct unwinding information but it would know about the
layout of rt_sigframe, and that's not always considered a public ABI.

Alternatively, I could do this the long way: add an ELF vDSO in
addition to the vsyscall pages, put syscall return trampolines there,
have glibc use those if available.

But I don't want to do either of those without checking with you;
one is an ABI issue, the other is a huge amount more work than I was
planning to do to fix the bug.

> We'll get a vDSO with kernel supplied unwind sectins sooner or later, but 
> you'll have to handle the old vsyscall without unwinding anyways because 
> it's not going to go away.
> 
> Also even the vDSO might end up without unwind information when compiled
> with old compilers because I don't plan to support it without .cfi_*
> support in binutils.

Fortunately I don't have to worry about this.  The vsyscall pages
aren't on the signal path and if you use an old compiler to build
your kernel, this will just break.  Considering how long it's already
been broken...

But, FYI, you can't actually write the unwind tables for these using
.cfi_* directives.  I tried.  I'd need at least three new directives
to do it sanely (for uleb128 escapes, sleb128 escapes, and adding the
"S" augmentation).  So I did it by hand, basically copied from the
i386 vDSO, but simpler since we don't need any pushes or pops.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Notes on a frame_unwind_address_in_block problem

[Andi Kleen]

On Thursday 03 August 2006 04:48, Daniel Jacobowitz wrote:

> Basically, right now x86_64 signal delivery always uses SA_RESTORER.

It will always. The kernel errors out if SA_RESTORER is not set.

> Glibc provides the restorer.  It has some minimal, incorrect unwind
> information.  If I remove the unwind information entirely from glibc,
> GDB will know how to do the right thing through a signal handler - but
> other unwinding scenarios like _Unwind_Backtrace won't.
> 
> I can add correct unwinding information but it would know about the
> layout of rt_sigframe, and that's not always considered a public ABI.

in practice it is - lots of programs assume it. I guess it's the best
you can do for now.
 
> Alternatively, I could do this the long way: add an ELF vDSO in
> addition to the vsyscall pages, put syscall return trampolines there,
> have glibc use those if available.

I plan to add a vDSO myself.

> > We'll get a vDSO with kernel supplied unwind sectins sooner or later, but 
> > you'll have to handle the old vsyscall without unwinding anyways because 
> > it's not going to go away.
> > 
> > Also even the vDSO might end up without unwind information when compiled
> > with old compilers because I don't plan to support it without .cfi_*
> > support in binutils.
> 
> Fortunately I don't have to worry about this.  The vsyscall pages
> aren't on the signal path 

The signal trampolines are in the vsyscall pages.

x86-64 doesn't actually have a gate page like i386.

> But, FYI, you can't actually write the unwind tables for these using
> .cfi_* directives.  I tried.  I'd need at least three new directives
> to do it sanely (for uleb128 escapes, sleb128 escapes, and adding the
> "S" augmentation).  So I did it by hand, basically copied from the
> i386 vDSO, but simpler since we don't need any pushes or pops.

If it's not possible to do sanely there won't be any unwind annotation.
I refuse to deal with any more of this binary mess that the compat
vsyscalls use because it's imho totally unmaintainable.

-Andi

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Thu, Aug 03, 2006 at 05:11:46AM +0200, Andi Kleen wrote:
> On Thursday 03 August 2006 04:48, Daniel Jacobowitz wrote:
> 
> > Basically, right now x86_64 signal delivery always uses SA_RESTORER.
> 
> It will always. The kernel errors out if SA_RESTORER is not set.

I figured you'd do what i386 did - if SA_RESTORER isn't set, use a
trampoline in the vDSO.

> > Glibc provides the restorer.  It has some minimal, incorrect unwind
> > information.  If I remove the unwind information entirely from glibc,
> > GDB will know how to do the right thing through a signal handler - but
> > other unwinding scenarios like _Unwind_Backtrace won't.
> > 
> > I can add correct unwinding information but it would know about the
> > layout of rt_sigframe, and that's not always considered a public ABI.
> 
> in practice it is - lots of programs assume it. I guess it's the best
> you can do for now.

Good enough for me.  Andreas, in that case, is the patch in
http://sourceware.org/ml/gdb/2006-07/msg00131.html OK?

> > Fortunately I don't have to worry about this.  The vsyscall pages
> > aren't on the signal path 
> 
> The signal trampolines are in the vsyscall pages.
> 
> x86-64 doesn't actually have a gate page like i386.

I'm confused now.  x86-64 doesn't have signal trmapolines in its
vsyscall pages, unless they've been added in the last week or two.
The only vsyscalls on x86-64 are vgettimeofday and vtime, in the
git pull I've got here.

> > But, FYI, you can't actually write the unwind tables for these using
> > .cfi_* directives.  I tried.  I'd need at least three new directives
> > to do it sanely (for uleb128 escapes, sleb128 escapes, and adding the
> > "S" augmentation).  So I did it by hand, basically copied from the
> > i386 vDSO, but simpler since we don't need any pushes or pops.
> 
> If it's not possible to do sanely there won't be any unwind annotation.
> I refuse to deal with any more of this binary mess that the compat
> vsyscalls use because it's imho totally unmaintainable.

Not being able to annotate them correctly would suck.  Roland (I think)
did the hard work of describing them correctly; given the macros and
comments he used, I think it's pretty straightforward.

It would be possible to add the necessary bits to gas, but I wouldn't
hold my breath.

There was a third problem other than the two I mentioned above but
I'm afraid I can't remember what it was now.  I'd have to try it again.
I did work around both the uleb128 and sleb128 problems, and there's
actually a signal frame marker in sufficiently recent gas, but I
ran into another problem that made me give up.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Notes on a frame_unwind_address_in_block problem

[Andi Kleen]

On Thursday 03 August 2006 05:21, Daniel Jacobowitz wrote:
> On Thu, Aug 03, 2006 at 05:11:46AM +0200, Andi Kleen wrote:
> > On Thursday 03 August 2006 04:48, Daniel Jacobowitz wrote:
> > 
> > > Basically, right now x86_64 signal delivery always uses SA_RESTORER.
> > 
> > It will always. The kernel errors out if SA_RESTORER is not set.
> 
> I figured you'd do what i386 did - if SA_RESTORER isn't set, use a
> trampoline in the vDSO.

No, it will printk and SIGSEGV in this case. When x86-64 was done
a whole lot of old signal stuff was dropped as legacy.

> > > Fortunately I don't have to worry about this.  The vsyscall pages
> > > aren't on the signal path 
> > 
> > The signal trampolines are in the vsyscall pages.
> > 
> > x86-64 doesn't actually have a gate page like i386.
> 
> I'm confused now.  x86-64 doesn't have signal trmapolines in its
> vsyscall pages, unless they've been added in the last week or two.
> The only vsyscalls on x86-64 are vgettimeofday and vtime, in the
> git pull I've got here.

Yes, sorry for the confusion - you're right the vstubs only exist
for compat. I nearly added them at some point, but didn't because
there was really no advantage of keeping them in glibc.

With a vDSO this will be different.

> There was a third problem other than the two I mentioned above but
> I'm afraid I can't remember what it was now.  I'd have to try it again.
> I did work around both the uleb128 and sleb128 problems, and there's
> actually a signal frame marker in sufficiently recent gas, but I
> ran into another problem that made me give up.

Which parts exactly of the signal frame does gas have trouble with?

-Andi

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Thu, Aug 03, 2006 at 05:29:13AM +0200, Andi Kleen wrote:
> > There was a third problem other than the two I mentioned above but
> > I'm afraid I can't remember what it was now.  I'd have to try it again.
> > I did work around both the uleb128 and sleb128 problems, and there's
> > actually a signal frame marker in sufficiently recent gas, but I
> > ran into another problem that made me give up.
> 
> Which parts exactly of the signal frame does gas have trouble with?

This is the same thing that required the complicated macros in i386. 
The problem is that the CFA is not at a fixed offset from the final
stack pointer, and the saved registers are fixed relative to the final
stack pointer - not the CFA.  So every single register has a
complicated expression, instead of just the CFA being complicated.

They're not fixed relative to the CFA because the CFA is the address
before any increased alignment.  It's the round_down in get_stack().

So each register is "current sp plus offset", and there's no way to
write that sort of expression directly from gas.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Notes on a frame_unwind_address_in_block problem

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]

Daniel Jacobowitz <drow@false.org> writes:

> [...]
> Good enough for me.  Andreas, in that case, is the patch in
> http://sourceware.org/ml/gdb/2006-07/msg00131.html OK?

Why are you using your own cf macros?  We have e.g. CFI_STARTPROC (see
sysdeps/generic/sysdep.h) and those should be used,

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj/
  SUSE Linux Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Fri, Aug 18, 2006 at 05:07:47PM +0200, Andreas Jaeger wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> 
> > [...]
> > Good enough for me.  Andreas, in that case, is the patch in
> > http://sourceware.org/ml/gdb/2006-07/msg00131.html OK?
> 
> Why are you using your own cf macros?  We have e.g. CFI_STARTPROC (see
> sysdeps/generic/sysdep.h) and those should be used,

I wrote earlier:

> But, FYI, you can't actually write the unwind tables for these using
> .cfi_* directives.  I tried.  I'd need at least three new directives
> to do it sanely (for uleb128 escapes, sleb128 escapes, and adding the
> "S" augmentation).  So I did it by hand, basically copied from the
> i386 vDSO, but simpler since we don't need any pushes or pops.

Even if I assume a brand new binutils which supports the "S"
augmentation, I would still need to hand-expand uleb128 and sleb128.
I thought there was another reason beyond that one too, but now I
can't remember it.  I could try again (I did this but didn't save the
patch).  But I really don't like having to assume the "S" support
is present and generating bogus unwind info if it isn't.

I suppose I could simply omit the unwind info if it isn't.  Want
me to try that?

-- 
Daniel Jacobowitz
CodeSourcery

Re: Notes on a frame_unwind_address_in_block problem

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]

Daniel Jacobowitz <drow@false.org> writes:

> On Fri, Aug 18, 2006 at 05:07:47PM +0200, Andreas Jaeger wrote:
>> Daniel Jacobowitz <drow@false.org> writes:
>> 
>> > [...]
>> > Good enough for me.  Andreas, in that case, is the patch in
>> > http://sourceware.org/ml/gdb/2006-07/msg00131.html OK?
>> 
>> Why are you using your own cf macros?  We have e.g. CFI_STARTPROC (see
>> sysdeps/generic/sysdep.h) and those should be used,
>
> I wrote earlier:
>
>> But, FYI, you can't actually write the unwind tables for these using
>> .cfi_* directives.  I tried.  I'd need at least three new directives
>> to do it sanely (for uleb128 escapes, sleb128 escapes, and adding the
>> "S" augmentation).  So I did it by hand, basically copied from the
>> i386 vDSO, but simpler since we don't need any pushes or pops.
>
> Even if I assume a brand new binutils which supports the "S"
> augmentation, I would still need to hand-expand uleb128 and sleb128.
> I thought there was another reason beyond that one too, but now I
> can't remember it.  I could try again (I did this but didn't save the
> patch).  But I really don't like having to assume the "S" support
> is present and generating bogus unwind info if it isn't.
>
> I suppose I could simply omit the unwind info if it isn't.  Want
> me to try that?

Ah I see.  Ulrich, Roland, what do you suggest here?

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj/
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Notes on a frame_unwind_address_in_block problem

[Ulrich Drepper]

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

Andreas Jaeger wrote:
> Ah I see.  Ulrich, Roland, what do you suggest here?

Nothig but the latest binutils is in any case supported.  We just don't
enforce using the latest version if older versions "by accident" still
work.  If there is something to require the new tools, definitely use
them and update the binutils version check in the configure script.

This doesn't mean I've looked at this patch sufficiently yet.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Mon, Aug 21, 2006 at 07:18:30AM -0700, Ulrich Drepper wrote:
> Andreas Jaeger wrote:
> > Ah I see.  Ulrich, Roland, what do you suggest here?
> 
> Nothig but the latest binutils is in any case supported.  We just don't
> enforce using the latest version if older versions "by accident" still
> work.  If there is something to require the new tools, definitely use
> them and update the binutils version check in the configure script.

Thanks.  I'll revise the patch.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Notes on a frame_unwind_address_in_block problem

[Daniel Jacobowitz]

On Thu, Jul 13, 2006 at 10:20:12PM +0200, Mark Kettenis wrote:
> > Date: Thu, 6 Jul 2006 18:21:57 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > II. 32-bit.
> > 
> > For 32-bit, though, it gets even more interesting.  Here's where $SUBJECT
> > comes into play.  I have a loaded vDSO (virtual shared object), which
> > exports __kernel_sigreturn.  This points at the first instruction of the
> > trampoline, i.e. one byte after the start of the dwarf2 FDE.  When I am
> > stopped in the signal handler, frame_unwind_address_in_block decides to
> > subtract one.  That points before the symbol, so the frame ID's function
> > ends up being NULL; there's no symbol covering that address.  Then when we
> > arrive at the signal trampoline during "finish", we no longer subtract one
> > - since we're at an executable instruction in the topmost frame - and thus
> > we do find the symbol.  The two frame IDs don't compare equal.
> 
> Ouch.  So in the end having the signal trampoline call the signal
> handler *is* a better design.

Um, I guess it depends what your goal is.  Given the rest of the system
architecture, for GNU/Linux, that'd waste a call instruction and some
code space; not much difference really.

> > Another solution would be to use the FDE start address as the code address
> > for dwarf2 signal frame IDs, instead of the function.  This would work on
> > the assumption that a single FDE would generally cover the entire trampoline
> > - a reasonable assumption, I think, and the consequences for the frame ID
> > changing while single-stepping are less disruptive here than the
> > alternative.
> > 
> > Mark, what do you think of that idea?  It seems to work.  It looks like the
> > patch at the end of this message.
> 
> In general, I think it's a bad idea to do this, but for the special
> case of a signal frame, especially in the presence of the 'S"
> augmentation, that might be a reasonable thing to do.  However, I
> think we can do better than that.  What about checking whether the
> address returned by frame_unwind_address_in_block() is equal to the
> FDE start address and add one bytes if that's the case before looking
> up the function corresponding to that address?

Unfortunately we can't readily; we'd have to add a bit of new code,
since all that's there is a call to frame_func_unwind (next_frame)
and that has no way to do the right thing.

I'd like to trigger off the signal augmentation.  If you'd prefer to
check the FDE start address, could you show me exactly what you have in
mind?

With that patch plus the fixed unwind information gdb.base/*.exp
results are basically clean for x86_64 - best I've ever seen them.
There is a corefile issue which I think is kernel related, and two
annotation tests fail because they don't account for a glibc with debug
information enabled.

-- 
Daniel Jacobowitz
CodeSourcery