Patchwork RFA: Fix DWARF2 CFI for m68k

login
register
mail settings
Submitter Daniel Jacobowitz
Date July 23, 2010, 2:18 a.m.
Message ID <20100723021849.GA20358@caradoc.them.org>
Download mbox | patch
Permalink /patch/59721/
State New
Headers show

Comments

Daniel Jacobowitz - July 23, 2010, 2:18 a.m.
I traced a large number of GDB test failures on m68k-elf to bogus CFI
data.  Here's a typical example:

foo:
.LFB0:
        .file 1 "gdb.base/advance.c"
        .loc 1 5 0
        .cfi_startproc
        link.w %fp,#-4
.LCFI0:
        .cfi_def_cfa 14, 8
        .loc 1 6 0
        move.l 8(%fp),%a0
        lea (10,%a0),%a0
        move.l %a0,-4(%fp)
        .loc 1 7 0
        move.l -4(%fp),%d0
        .loc 1 8 0
        unlk %fp
        .cfi_offset 14, -8
        rts
        .cfi_endproc

Take a look at that .cfi_offset.  It represents the save of %fp at
CFA-8, by the link.w instruction.  But it's way down at the end, and
the link.w instruction modified %fp.  So GDB's idea of the saved value
of %fp is wrong except right before the rts (at which point it's wrong
again, because we're after the unlk - but that's a different issue).

The problem was the parallel used by the link.w pattern.
dwarf2out_frame_debug outputs queued saves if the current insn would
interfere, before processing it.  But it doesn't output saves after
the insn if the insn itself both queued new saves and clobbered the
register they were relative to.

This shows up for m68k because the link instruction does a save and a
frame pointer update in parallel.  I believe it doesn't show up for
other architectures, e.g. ARM, because the frame-related epxr on ARM
is a SEQUENCE rather than a PARALLEL.

Regression tested on an m68k-elf cross compiler (three ColdFire
multilibs with QEMU).  We've also had it in our tree for a while to
test on other platforms.

OK to commit?
Richard Guenther - July 23, 2010, 9:20 a.m.
On Fri, Jul 23, 2010 at 4:18 AM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
> I traced a large number of GDB test failures on m68k-elf to bogus CFI
> data.  Here's a typical example:
>
> foo:
> .LFB0:
>        .file 1 "gdb.base/advance.c"
>        .loc 1 5 0
>        .cfi_startproc
>        link.w %fp,#-4
> .LCFI0:
>        .cfi_def_cfa 14, 8
>        .loc 1 6 0
>        move.l 8(%fp),%a0
>        lea (10,%a0),%a0
>        move.l %a0,-4(%fp)
>        .loc 1 7 0
>        move.l -4(%fp),%d0
>        .loc 1 8 0
>        unlk %fp
>        .cfi_offset 14, -8
>        rts
>        .cfi_endproc
>
> Take a look at that .cfi_offset.  It represents the save of %fp at
> CFA-8, by the link.w instruction.  But it's way down at the end, and
> the link.w instruction modified %fp.  So GDB's idea of the saved value
> of %fp is wrong except right before the rts (at which point it's wrong
> again, because we're after the unlk - but that's a different issue).
>
> The problem was the parallel used by the link.w pattern.
> dwarf2out_frame_debug outputs queued saves if the current insn would
> interfere, before processing it.  But it doesn't output saves after
> the insn if the insn itself both queued new saves and clobbered the
> register they were relative to.
>
> This shows up for m68k because the link instruction does a save and a
> frame pointer update in parallel.  I believe it doesn't show up for
> other architectures, e.g. ARM, because the frame-related epxr on ARM
> is a SEQUENCE rather than a PARALLEL.
>
> Regression tested on an m68k-elf cross compiler (three ColdFire
> multilibs with QEMU).  We've also had it in our tree for a while to
> test on other platforms.
>
> OK to commit?

Ok.

Thanks,
Richard.

> --
> Daniel Jacobowitz
> CodeSourcery
>
> 2010-07-22  Daniel Jacobowitz  <dan@codesourcery.com>
>
>        * dwarf2out.c (dwarf2out_frame_debug): Check for queued saves
>        again after processing insn.
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c     (revision 162416)
> +++ gcc/dwarf2out.c     (working copy)
> @@ -2791,6 +2791,12 @@ dwarf2out_frame_debug (rtx insn, bool af
>   insn = PATTERN (insn);
>  found:
>   dwarf2out_frame_debug_expr (insn, label);
> +
> +  /* Check again.  A parallel can save and update the same register.
> +     We could probably check just once, here, but this is safer than
> +     removing the check above.  */
> +  if (clobbers_queued_reg_save (insn))
> +    flush_queued_reg_saves ();
>  }
>
>  /* Determine if we need to save and restore CFI information around this
>

Patch

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 162416)
+++ gcc/dwarf2out.c	(working copy)
@@ -2791,6 +2791,12 @@  dwarf2out_frame_debug (rtx insn, bool af
   insn = PATTERN (insn);
  found:
   dwarf2out_frame_debug_expr (insn, label);
+
+  /* Check again.  A parallel can save and update the same register.
+     We could probably check just once, here, but this is safer than
+     removing the check above.  */
+  if (clobbers_queued_reg_save (insn))
+    flush_queued_reg_saves ();
 }
 
 /* Determine if we need to save and restore CFI information around this