Lift DWARF unwinder restriction in dwarf2-frame.c::dwarf2_frame_cfa

Message ID 1416542375-742-1-git-send-email-brobecker@adacore.com
State New, archived
Headers

Commit Message

Joel Brobecker Nov. 21, 2014, 3:59 a.m. UTC
  Hello,

GDB is currently broken on all SPARC targets when using GCC 4.9.
When trying to print any local variable:

    (gdb) p x
    can't compute CFA for this frame

This is related to the fact that the compiler now generates DWARF 4
debugging info by default, and in particular that it now emits
DW_OP_call_frame_cfa, which triggers a limitation in dwarf2_frame_cfa:

   /* This restriction could be lifted if other unwinders are known to
      compute the frame base in a way compatible with the DWARF
      unwinder.  */
   if (!frame_unwinder_is (this_frame, &dwarf2_frame_unwind)
       && !frame_unwinder_is (this_frame, &dwarf2_tailcall_frame_unwind))
     error (_("can't compute CFA for this frame"));

We couldn't append the dwarf2 unwinder to all SPARC targets because
it does not work properly with StackGhost:
    https://www.sourceware.org/ml/gdb-patches/2014-07/msg00012.html

We also later discovered that using the DWARF2 unwinder means
using it for computing the function's return address, which
is buggy when it comes to functions returning a struct (where
the return address is saved-pc+12 instead of saved-pc+8).
This is because GCC is emitting the info about the return address
as %o7/%i7 instead of the actual return address.  For functions
that have debugging info, we compensate by looking at the function's
return type and add the extra +4, but for function without debug
info, we're stuck.

EricB and I twisted the issue in all the directions we could think of,
and unfortunately couldn't find a way to make it work without
introduction one regression or another.

But, stepping back a little, just removing the restriction seems to work
well for us on all both sparc-elf and {sparc,sparc64}-solaris.
After reviewing the previous discussions about this test, I could
not figure out whether some unwinders were already known to have
incompatible CFAs or if the concern was purely theoretical:
    https://www.sourceware.org/ml/gdb-patches/2009-06/msg00191.html
    https://www.sourceware.org/ml/gdb-patches/2009-07/msg00570.html
    https://www.sourceware.org/ml/gdb-patches/2009-09/msg00027.html

At the moment, we took the approach of trying it out, and see what
happens...

gdb/ChangeLog:

        PR backtrace/16215:
        * dwarf2-frame.c (dwarf2_frame_cfa): Remove the restriction
        the frame unwinder must either be the dwarf2_frame_unwind
        or the dwarf2_tailcall_frame_unwind.  Verify that this_frame's
        stack_addr is valid before calling get_frame_base.  Throw
        an error if not valid.

Tested on sparc-solaris and sparc-elf with AdaCore's testsuite
(the FSF testsuite crashes all of AdaCore's solaris machines).

OK to commit?

Thank you,
  

Comments

Joel Brobecker Dec. 13, 2014, 1:27 p.m. UTC | #1
> gdb/ChangeLog:
> 
>         PR backtrace/16215:
>         * dwarf2-frame.c (dwarf2_frame_cfa): Remove the restriction
>         the frame unwinder must either be the dwarf2_frame_unwind
>         or the dwarf2_tailcall_frame_unwind.  Verify that this_frame's
>         stack_addr is valid before calling get_frame_base.  Throw
>         an error if not valid.

Pushed!
  
Joel Brobecker Dec. 31, 2014, 7:23 a.m. UTC | #2
> > gdb/ChangeLog:
> > 
> >         PR backtrace/16215:
> >         * dwarf2-frame.c (dwarf2_frame_cfa): Remove the restriction
> >         the frame unwinder must either be the dwarf2_frame_unwind
> >         or the dwarf2_tailcall_frame_unwind.  Verify that this_frame's
> >         stack_addr is valid before calling get_frame_base.  Throw
> >         an error if not valid.
> 
> Pushed!

And pushed to the gdb-7.8-branch also.
  

Patch

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 80e5903..30a24c8 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1512,12 +1512,12 @@  dwarf2_frame_cfa (struct frame_info *this_frame)
     throw_error (NOT_AVAILABLE_ERROR,
                 _("can't compute CFA for this frame: "
                   "required registers or memory are unavailable"));
-  /* This restriction could be lifted if other unwinders are known to
-     compute the frame base in a way compatible with the DWARF
-     unwinder.  */
-  if (!frame_unwinder_is (this_frame, &dwarf2_frame_unwind)
-      && !frame_unwinder_is (this_frame, &dwarf2_tailcall_frame_unwind))
-    error (_("can't compute CFA for this frame"));
+
+  if (get_frame_id (this_frame).stack_status != FID_STACK_VALID)
+    throw_error (NOT_AVAILABLE_ERROR,
+                _("can't compute CFA for this frame: "
+                  "frame base not available"));
+
   return get_frame_base (this_frame);
 }