Re: [PATCH 2/2] handle an unspecified return address column

[Joel Brobecker <brobecker at adacore dot com>]

Hi Tom,

On Wed, Nov 13, 2013 at 01:51:13PM -0700, Tom Tromey wrote:
> Debugging PR 16155 further, I found that the DWARF unwinder found the
> function in question, but thought it had no registers saved
> (fs->regs.num_regs == 0).
> 
> It seems to me that if a frame does not specify the return address
> column, or if the return address column is explicitly marked as
> DWARF2_FRAME_REG_UNSPECIFIED, then we should set the
> "undefined_retaddr" flag and let the DWARF unwinder gracefully stop.
> 
> This patch implements that idea.
> 
> With this patch the backtrace works properly:
> 
>     (gdb) bt
>     #0  0x0000007fb7ed485c in nanosleep () from /lib64/libc.so.6
>     #1  0x0000007fb7ed4508 in sleep () from /lib64/libc.so.6
>     #2  0x00000000004008bc in thread_function (arg=0x4) at threadapply.c:73
>     #3  0x0000007fb7fad950 in start_thread () from /lib64/libpthread.so.0
>     #4  0x0000007fb7f0956c in clone () from /lib64/libc.so.6
> 
> 2013-11-13  Tom Tromey  <tromey@redhat.com>
> 
> 	PR backtrace/16155:
> 	* dwarf2-frame.c (dwarf2_frame_cache): Set undefined_retaddr if
> 	the return address column is unspecified.

I just found out that this patch causes some problems on at least
arm-elf and ppc-elf. Attached is a proposed patch, with associated
analysis.

gdb/ChangeLog:

        PR backtrace/16155:
        * dwarf2-frame.c (dwarf2_frame_cache): Remove condition that
        sets cache->undefined_retaddr to 1 if there is no column in
        the frame info for the return register.

Tested on x86_64-linux, no regression. It also fixes all regressions
observed on arm-elf and ppc-elf.

Can you tell me what you think?

Thank you!
-- 
Joel

>From f01867f9f4fdd55d7692d09ae1d4622968e2bb04 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 26 Nov 2013 17:08:03 +0400
Subject: [PATCH] DWARF: cannot unwind from leaf functions that do not touch
 the link register

I noticed this problem on arm-eabi and on ppc-elf. Consider this
simple function:

   function F return Num is
   begin
      return 1.0 + 2.0;
   end F;

Trying to unwind from it on ppc-elf, no longer works. Consider for
instance the following setup, where we are about to call our function
F above:

    (gdb) tar sim -e bug -r 0x400000
    (gdb) load x
    (gdb) start
    [...]
    Temporary breakpoint 1, x () at x.adb:4
    4          Z : constant Num := F;

At this point, let's see what happens to our backtrace once we've
stepped inside F:

    (gdb) s
    pck.f () at pck.adb:2
    2          function F return Num is
    (gdb) bt
    #0  pck.f () at pck.adb:2

As you can see, the backtrace is truncated. We're missing the frame
corresponding to the main subprogram "x".

This is due to a recent change, which added the following check:

    +  else if (fs->retaddr_column >= fs->regs.num_regs
    +     || (fs->regs.reg[fs->retaddr_column].how
    +         == DWARF2_FRAME_REG_UNSPECIFIED))
    +    cache->undefined_retaddr = 1;

The rationale was:

    It seems to me that if a frame does not specify the return address
    column, or if the return address column is explicitly marked as
    DWARF2_FRAME_REG_UNSPECIFIED, then we should set the
    "undefined_retaddr" flag and let the DWARF unwinder gracefully stop.

In our case, we're tripping the first half of the condition.
Looking at the frame info with readelf, for instance, we see:

| 00000000 0000000c ffffffff CIE "" cf=4 df=-4 ra=65
|    LOC   CFA
| 00000000 r1+0
|
| 00000010 00000020 00000000 FDE cie=00000000 pc=00000000..0000002c
|    LOC   CFA      r31
| 00000000 r1+0     u
| 00000004 r1+16    u
| [etc]

Here, the return address register is number 65, and there is no
column for it. I think that this meant to be decoded as "ra never
changes throughout the lifetime of the function, so no column
needed for it in the CFI". Regardless of correctness, this appears
to be a practice that's established enough that we should handle it
gracefully, as we used to.

This patch therefore changes a bit the logic to remove the part
of the condition that would cause the unwinding to terminate when
the CFI has no column corresponding to the return address register.

gdb/ChangeLog:

        PR backtrace/16155:
        * dwarf2-frame.c (dwarf2_frame_cache): Remove condition that
        sets cache->undefined_retaddr to 1 if there is no column in
        the frame info for the return register.
---
 gdb/dwarf2-frame.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index f185ca6..9e9c32b 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1243,11 +1243,9 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
   }
 
   if (fs->retaddr_column < fs->regs.num_regs
-      && fs->regs.reg[fs->retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
-    cache->undefined_retaddr = 1;
-  else if (fs->retaddr_column >= fs->regs.num_regs
-  	  || (fs->regs.reg[fs->retaddr_column].how
-  	      == DWARF2_FRAME_REG_UNSPECIFIED))
+      && (fs->regs.reg[fs->retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED
+	  || (fs->regs.reg[fs->retaddr_column].how
+	      == DWARF2_FRAME_REG_UNSPECIFIED)))
     cache->undefined_retaddr = 1;
 
   do_cleanups (old_chain);
-- 
1.8.1.2