Re: [PATCH] DW_CFA_GNU_negative_offset_extended support

[Jan Kratochvil <jan.kratochvil@redhat.com>]

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

Hi Daniel,

the patch is updated according to your advices.


Thanks,
Jan


On Wed, 03 Jan 2007 19:27:42 +0100, Daniel Jacobowitz wrote:
> On Sat, Dec 30, 2006 at 08:06:15PM +0100, Jan Kratochvil wrote:
...
> > +	    case DW_CFA_GNU_negative_offset_extended:
> > +	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &reg);
> > +	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &offset);
> > +	      offset *= fs->data_align;
> > +	      dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
> > +	      fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_OFFSET;
> > +	      fs->regs.reg[reg].loc.offset = -offset;
> > +	      break;
> > +
> 
> What about the eh_frame_p check in all the other cases?

Fixed.  I see you also probably solved me some other Bug (PPC CFI unwinding),
thanks.


> Do you know if DW_CFA_GNU_negative_offset_extended was a factored
> offset?

Yes it is (svn diff -c 49334 gcc/dwarf2out.c).
OTOH I just trusted the code patch author (Andrew Cagney).


> > +# We should stop in abort(3).
> > +
> > +gdb_test "run" \
> > +         "Program received signal SIGABRT,.*" \
> > +         "run"
> 
> Please use gdb_run_cmd instead.  See other tests for an example.
> ("run" doesn't work for remote targets.)

Fixed.  Copying other code just is not enough for GDB...


> > +send_gdb "info frame\n";
> > +gdb_expect {
> > +    -re "Stack level 3, frame at (0x\[0-9a-f\]+):.*Saved registers:.* ecx at (0x\[0-9a-f\]+),.*" {
> > +	pass "Value of the CFI inserted register"
> > +	if { [string compare $expect_out(1,string) $expect_out(2,string)] } then {
> > +	    fail "Value of the CFI inserted register"
> > +	} else {
> > +	    pass "Value of the CFI inserted register"
> > +	}
> > +    }
> > +    default {
> > +	fail "Existence of the CFI inserted register"
> > +    }
> > +}
> 
> Please use gdb_test_multiple instead of send_gdb / gdb_expect.  Then
> you won't need the default case, which was wrong above anyway - it
> has to consume the prompt.

Good hint, the code is now cleaner (and fixed).


> Also, I think you have a typo - didn't mean for that to pass twice,
> right?

Unfortunately right.

[-- Attachment #2: gdb-6.5-DW_CFA_GNU_negative_offset_extended.patch --]
[-- Type: text/plain, Size: 11726 bytes --]

2007-01-11  Andrew Cagney <cagney@redhat.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2-frame.c (execute_cfa_program): New support of
	`DW_CFA_GNU_negative_offset_extended'.

2007-01-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.arch/i386-gnu-cfi.exp, gdb.arch/i386-gnu-cfi.c,
	gdb.arch/i386-gnu-cfi-asm.S: New files.


--- gdb/dwarf2-frame.c	9 Jan 2007 17:58:50 -0000	1.66
+++ gdb/dwarf2-frame.c	11 Jan 2007 18:57:40 -0000
@@ -579,6 +579,17 @@ bad CFI data; mismatched DW_CFA_restore_
 	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &utmp);
 	      break;
 
+	    case DW_CFA_GNU_negative_offset_extended:
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &reg);
+	      if (eh_frame_p)
+		reg = dwarf2_frame_eh_frame_regnum (gdbarch, reg);
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &offset);
+	      offset *= fs->data_align;
+	      dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+	      fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_OFFSET;
+	      fs->regs.reg[reg].loc.offset = -offset;
+	      break;
+
 	    default:
 	      internal_error (__FILE__, __LINE__, _("Unknown CFI encountered."));
 	    }
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.arch/i386-gnu-cfi-asm.S	11 Jan 2007 18:57:47 -0000
@@ -0,0 +1,357 @@
+/* Copyright 2007 Free Software Foundation, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+ *
+ * Please email any bugs, comments, and/or additions to this file to:
+ * bug-gdb@gnu.org
+ *
+ * This file is part of the gdb testsuite.
+ *
+ * It was generated using
+ * "gcc -g -O9 -S -o gate.s gate.c -fasynchronous-unwind-tables",
+ * using the following source file:
+ *
+ *
+ *	void *gate (void *(*gate) (void *data), void *data)
+ *	{
+ *	  return 1 + (*gate) (data);
+ *	}
+ *
+ * Changes are marked as `CHANGE' below.
+ */
+
+	.file	"gate.c"
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.text
+.Ltext0:
+	.p2align 4,,15
+.globl gate
+	.type	gate, @function
+gate:
+.LFB2:
+	.file 1 "gate.c"
+	.loc 1 2 0
+.LVL0:
+	pushl	%ebp
+.LCFI0:
+	movl	%esp, %ebp
+.LCFI1:
+	subl	$8, %esp
+.LCFI2:
+	.loc 1 3 0
+	movl	12(%ebp), %eax
+	movl	%eax, (%esp)
+	call	*8(%ebp)
+	.loc 1 4 0
+	leave
+	.loc 1 3 0
+	addl	$1, %eax
+	.loc 1 4 0
+	ret
+.LFE2:
+	.size	gate, .-gate
+	.section	.debug_frame,"",@progbits
+.Lframe0:
+	.long	.LECIE0-.LSCIE0
+.LSCIE0:
+	.long	0xffffffff
+	.byte	0x1
+	.string	""
+	.uleb128 0x1
+	.sleb128 -4
+	.byte	0x8
+	.byte	0xc
+	.uleb128 0x4
+	.uleb128 0x4
+	.byte	0x88
+	.uleb128 0x1
+	.align 4
+.LECIE0:
+.LSFDE0:
+	.long	.LEFDE0-.LASFDE0
+.LASFDE0:
+	.long	.Lframe0
+	.long	.LFB2
+	.long	.LFE2-.LFB2
+	.byte	0x4
+	.long	.LCFI0-.LFB2
+	.byte	0xe
+	.uleb128 0x8
+	.byte	0x85
+	.uleb128 0x2
+
+	/* CHANGE */
+	.byte	0x2f
+	.uleb128 0x1
+	.uleb128 0x0
+	/* CHANGE */
+
+	.byte	0x4
+	.long	.LCFI1-.LCFI0
+	.byte	0xd
+	.uleb128 0x5
+	.align 4
+.LEFDE0:
+	.section	.eh_frame,"a",@progbits
+.Lframe1:
+	.long	.LECIE1-.LSCIE1
+.LSCIE1:
+	.long	0x0
+	.byte	0x1
+	.string	""
+	.uleb128 0x1
+	.sleb128 -4
+	.byte	0x8
+	.byte	0xc
+	.uleb128 0x4
+	.uleb128 0x4
+	.byte	0x88
+	.uleb128 0x1
+	.align 4
+.LECIE1:
+.LSFDE1:
+	.long	.LEFDE1-.LASFDE1
+.LASFDE1:
+	.long	.LASFDE1-.Lframe1
+	.long	.LFB2
+	.long	.LFE2-.LFB2
+	.byte	0x4
+	.long	.LCFI0-.LFB2
+	.byte	0xe
+	.uleb128 0x8
+	.byte	0x85
+	.uleb128 0x2
+
+	/* CHANGE */
+	.byte	0x2f
+	.uleb128 0x1
+	.uleb128 0x0
+	/* CHANGE */
+
+	.byte	0x4
+	.long	.LCFI1-.LCFI0
+	.byte	0xd
+	.uleb128 0x5
+	.align 4
+.LEFDE1:
+	.text
+.Letext0:
+	.section	.debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+	.long	.LFB2-.Ltext0
+	.long	.LCFI0-.Ltext0
+	.value	0x2
+	.byte	0x74
+	.sleb128 4
+	.long	.LCFI0-.Ltext0
+	.long	.LCFI1-.Ltext0
+	.value	0x2
+	.byte	0x74
+	.sleb128 8
+	.long	.LCFI1-.Ltext0
+	.long	.LFE2-.Ltext0
+	.value	0x2
+	.byte	0x75
+	.sleb128 8
+	.long	0x0
+	.long	0x0
+	.section	.debug_info
+	.long	0x74
+	.value	0x2
+	.long	.Ldebug_abbrev0
+	.byte	0x4
+	.uleb128 0x1
+	.long	.Ldebug_line0
+	.long	.Letext0
+	.long	.Ltext0
+	.long	.LASF2
+	.byte	0x1
+	.long	.LASF3
+	.long	.LASF4
+	.uleb128 0x2
+	.long	0x5f
+	.byte	0x1
+	.long	.LASF0
+	.byte	0x1
+	.byte	0x2
+	.byte	0x1
+	.long	0x5f
+	.long	.LFB2
+	.long	.LFE2
+	.long	.LLST0
+	.uleb128 0x3
+	.long	.LASF0
+	.byte	0x1
+	.byte	0x1
+	.long	0x71
+	.byte	0x2
+	.byte	0x91
+	.sleb128 0
+	.uleb128 0x3
+	.long	.LASF1
+	.byte	0x1
+	.byte	0x1
+	.long	0x5f
+	.byte	0x2
+	.byte	0x91
+	.sleb128 4
+	.byte	0x0
+	.uleb128 0x4
+	.byte	0x4
+	.uleb128 0x5
+	.long	0x71
+	.byte	0x1
+	.long	0x5f
+	.uleb128 0x6
+	.long	0x5f
+	.byte	0x0
+	.uleb128 0x7
+	.byte	0x4
+	.long	0x61
+	.byte	0x0
+	.section	.debug_abbrev
+	.uleb128 0x1
+	.uleb128 0x11
+	.byte	0x1
+	.uleb128 0x10
+	.uleb128 0x6
+	.uleb128 0x12
+	.uleb128 0x1
+	.uleb128 0x11
+	.uleb128 0x1
+	.uleb128 0x25
+	.uleb128 0xe
+	.uleb128 0x13
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0xe
+	.uleb128 0x1b
+	.uleb128 0xe
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x2
+	.uleb128 0x2e
+	.byte	0x1
+	.uleb128 0x1
+	.uleb128 0x13
+	.uleb128 0x3f
+	.uleb128 0xc
+	.uleb128 0x3
+	.uleb128 0xe
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x27
+	.uleb128 0xc
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x11
+	.uleb128 0x1
+	.uleb128 0x12
+	.uleb128 0x1
+	.uleb128 0x40
+	.uleb128 0x6
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x3
+	.uleb128 0x5
+	.byte	0x0
+	.uleb128 0x3
+	.uleb128 0xe
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x2
+	.uleb128 0xa
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x4
+	.uleb128 0xf
+	.byte	0x0
+	.uleb128 0xb
+	.uleb128 0xb
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x5
+	.uleb128 0x15
+	.byte	0x1
+	.uleb128 0x1
+	.uleb128 0x13
+	.uleb128 0x27
+	.uleb128 0xc
+	.uleb128 0x49
+	.uleb128 0x13
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x6
+	.uleb128 0x5
+	.byte	0x0
+	.uleb128 0x49
+	.uleb128 0x13
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x7
+	.uleb128 0xf
+	.byte	0x0
+	.uleb128 0xb
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.byte	0x0
+	.byte	0x0
+	.byte	0x0
+	.section	.debug_pubnames,"",@progbits
+	.long	0x17
+	.value	0x2
+	.long	.Ldebug_info0
+	.long	0x78
+	.long	0x25
+	.string	"gate"
+	.long	0x0
+	.section	.debug_aranges,"",@progbits
+	.long	0x1c
+	.value	0x2
+	.long	.Ldebug_info0
+	.byte	0x4
+	.byte	0x0
+	.value	0x0
+	.value	0x0
+	.long	.Ltext0
+	.long	.Letext0-.Ltext0
+	.long	0x0
+	.long	0x0
+	.section	.debug_str,"MS",@progbits,1
+.LASF1:
+	.string	"data"
+.LASF0:
+	.string	"gate"
+.LASF3:
+	.string	"gate.c"
+.LASF4:
+	.string	"/home/jkratoch/redhat/bz165025"
+.LASF2:
+	.string	"GNU C 4.1.1 20061213 (Red Hat 4.1.1-46)"
+	.ident	"GCC: (GNU) 4.1.1 20061213 (Red Hat 4.1.1-46)"
+	.section	.note.GNU-stack,"",@progbits
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.arch/i386-gnu-cfi.c	11 Jan 2007 18:57:47 -0000
@@ -0,0 +1,31 @@
+/* Unwinding of DW_CFA_GNU_negative_offset_extended test program.
+
+   Copyright 2007, Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+   
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+   
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include <stdlib.h>
+
+/* i386-gnu-cfi-asm.S:  */
+extern void *gate (void *(*gate) (void *data), void *data);
+
+int main (void)
+{
+  gate ((void *(*) (void *data)) abort, NULL);
+  return 0;
+}
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.arch/i386-gnu-cfi.exp	11 Jan 2007 18:57:47 -0000
@@ -0,0 +1,94 @@
+# Copyright 2007 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+# Unwinding of DW_CFA_GNU_negative_offset_extended test program.
+
+if $tracelevel {
+    strace $tracelevel
+}
+
+# Test i386 unwinder.
+
+set prms_id 0
+set bug_id 0
+
+if ![istarget "i?86-*-*"] then {
+    verbose "Skipping i386 unwinder tests."
+    return
+}
+
+set testfile "i386-gnu-cfi"
+set srcfilec ${testfile}.c
+set srcfileasm ${testfile}-asm.S
+set binfile ${objdir}/${subdir}/${testfile}
+
+# some targets have leading underscores on assembly symbols.
+# TODO: detect this automatically
+set additional_flags ""
+if [istarget "i?86-*-cygwin*"] then {
+  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfilec} ${srcdir}/${subdir}/${srcfileasm}" "${binfile}" executable [list debug $additional_flags]] != "" } {
+    untested i386-gnu-cfi.exp
+    return -1
+}
+
+# Get things started.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# We should stop in abort(3).
+
+gdb_run_cmd
+
+gdb_test_multiple {} "continue to abort()" {
+    -re ".*Program received signal SIGABRT,.*$gdb_prompt $" {
+	pass "continue to abort()"
+    }
+}
+
+# Backtrace would crash before.
+
+gdb_test "bt" \
+         "main \\(\\) at .*${testfile}.c.*" \
+         "backtrace"
+
+# Check we see the inserted `DW_CFA_GNU_negative_offset_extended' CFI.
+# We see there the original CFI-stored filename `gate.c'.
+
+gdb_test "up 3" \
+         "gate \\(\[^()\]*\\) at .*gate.c.*" \
+         "shift up to the modified frame"
+
+gdb_test_multiple "info frame" "Existence of the CFI inserted register" {
+    -re "Stack level 3, frame at (0x\[0-9a-f\]+):.*Saved registers:.* ecx at (0x\[0-9a-f\]+),.*" {
+	pass "Existence of the CFI inserted register"
+	if { [string compare $expect_out(1,string) $expect_out(2,string)] } then {
+	    fail "Value of the CFI inserted register"
+	} else {
+	    pass "Value of the CFI inserted register"
+	}
+    }
+}