[rfc][patch] Fix for gdb/10275 (mis-handling of DW_CFA_restore_state)

[rfc][patch] Fix for gdb/10275 (mis-handling of DW_CFA_restore_state)

[Paul Pluzhnikov]

Greetings,

Fixing this looks easier than I expected :-)

Regtested on Linux/x86_64 (no regressions).

I also verified that I get correct stack for my reduced test case in
gdb/10275, and in my real gcc-4.5 compiled optimized code.

If this looks correct, I'll add a test case.

Thanks,
--
Paul Pluzhnikov

2009-06-12  Paul Pluzhnikov  <ppluzhnikov@google.com>

	gdb/10275
	* dwarf2-frame.c (dwarf2_frame_state): Move cfa_offset, cfa_reg,
	cfa_how and cfa_exp into regs and adjust users.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.87
diff -u -p -u -r1.87 dwarf2-frame.c
--- dwarf2-frame.c	3 Jan 2009 05:57:51 -0000	1.87
+++ dwarf2-frame.c	13 Jun 2009 01:20:54 -0000
@@ -164,19 +164,19 @@ struct dwarf2_frame_state
     struct dwarf2_frame_state_reg *reg;
     int num_regs;
 
+    LONGEST cfa_offset;
+    ULONGEST cfa_reg;
+    enum {
+      CFA_UNSET,
+      CFA_REG_OFFSET,
+      CFA_EXP
+    } cfa_how;
+    gdb_byte *cfa_exp;
+
     /* Used to implement DW_CFA_remember_state.  */
     struct dwarf2_frame_state_reg_info *prev;
   } regs;
 
-  LONGEST cfa_offset;
-  ULONGEST cfa_reg;
-  gdb_byte *cfa_exp;
-  enum {
-    CFA_UNSET,
-    CFA_REG_OFFSET,
-    CFA_EXP
-  } cfa_how;
-
   /* The PC described by the current frame state.  */
   CORE_ADDR pc;
 
@@ -502,21 +502,21 @@ bad CFI data; mismatched DW_CFA_restore_
 	      break;
 
 	    case DW_CFA_def_cfa:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_reg);
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->regs.cfa_reg);
 	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &utmp);
 
 	      if (fs->armcc_cfa_offsets_sf)
 		utmp *= fs->data_align;
 
-	      fs->cfa_offset = utmp;
-	      fs->cfa_how = CFA_REG_OFFSET;
+	      fs->regs.cfa_offset = utmp;
+	      fs->regs.cfa_how = CFA_REG_OFFSET;
 	      break;
 
 	    case DW_CFA_def_cfa_register:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_reg);
-	      fs->cfa_reg = dwarf2_frame_adjust_regnum (gdbarch, fs->cfa_reg,
-							eh_frame_p);
-	      fs->cfa_how = CFA_REG_OFFSET;
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->regs.cfa_reg);
+	      fs->regs.cfa_reg = dwarf2_frame_adjust_regnum (gdbarch, fs->regs.cfa_reg,
+                                                             eh_frame_p);
+	      fs->regs.cfa_how = CFA_REG_OFFSET;
 	      break;
 
 	    case DW_CFA_def_cfa_offset:
@@ -525,7 +525,7 @@ bad CFI data; mismatched DW_CFA_restore_
 	      if (fs->armcc_cfa_offsets_sf)
 		utmp *= fs->data_align;
 
-	      fs->cfa_offset = utmp;
+	      fs->regs.cfa_offset = utmp;
 	      /* cfa_how deliberately not set.  */
 	      break;
 
@@ -533,10 +533,10 @@ bad CFI data; mismatched DW_CFA_restore_
 	      break;
 
 	    case DW_CFA_def_cfa_expression:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_exp_len);
-	      fs->cfa_exp = insn_ptr;
-	      fs->cfa_how = CFA_EXP;
-	      insn_ptr += fs->cfa_exp_len;
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->regs.cfa_exp_len);
+	      fs->regs.cfa_exp = insn_ptr;
+	      fs->regs.cfa_how = CFA_EXP;
+	      insn_ptr += fs->regs.cfa_exp_len;
 	      break;
 
 	    case DW_CFA_expression:
@@ -589,17 +589,18 @@ bad CFI data; mismatched DW_CFA_restore_
 	      break;
 
 	    case DW_CFA_def_cfa_sf:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_reg);
-	      fs->cfa_reg = dwarf2_frame_adjust_regnum (gdbarch, fs->cfa_reg,
-							eh_frame_p);
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->regs.cfa_reg);
+	      fs->regs.cfa_reg = dwarf2_frame_adjust_regnum (gdbarch,
+                                                             fs->regs.cfa_reg,
+                                                             eh_frame_p);
 	      insn_ptr = read_sleb128 (insn_ptr, insn_end, &offset);
-	      fs->cfa_offset = offset * fs->data_align;
-	      fs->cfa_how = CFA_REG_OFFSET;
+	      fs->regs.cfa_offset = offset * fs->data_align;
+	      fs->regs.cfa_how = CFA_REG_OFFSET;
 	      break;
 
 	    case DW_CFA_def_cfa_offset_sf:
 	      insn_ptr = read_sleb128 (insn_ptr, insn_end, &offset);
-	      fs->cfa_offset = offset * fs->data_align;
+	      fs->regs.cfa_offset = offset * fs->data_align;
 	      /* cfa_how deliberately not set.  */
 	      break;
 
@@ -932,19 +933,19 @@ dwarf2_frame_cache (struct frame_info *t
   execute_cfa_program (fde, fde->instructions, fde->end, this_frame, fs);
 
   /* Calculate the CFA.  */
-  switch (fs->cfa_how)
+  switch (fs->regs.cfa_how)
     {
     case CFA_REG_OFFSET:
-      cache->cfa = read_reg (this_frame, fs->cfa_reg);
+      cache->cfa = read_reg (this_frame, fs->regs.cfa_reg);
       if (fs->armcc_cfa_offsets_reversed)
-	cache->cfa -= fs->cfa_offset;
+	cache->cfa -= fs->regs.cfa_offset;
       else
-	cache->cfa += fs->cfa_offset;
+	cache->cfa += fs->regs.cfa_offset;
       break;
 
     case CFA_EXP:
       cache->cfa =
-	execute_stack_op (fs->cfa_exp, fs->cfa_exp_len,
+	execute_stack_op (fs->regs.cfa_exp, fs->regs.cfa_exp_len,
 			  cache->addr_size, this_frame, 0);
       break;
 

Re: [rfc][patch] Fix for gdb/10275 (mis-handling of DW_CFA_restore_state)

[Paul Pluzhnikov]

On Fri, Jun 12, 2009 at 6:28 PM, Paul Pluzhnikov<ppluzhnikov@google.com> wrote:

> 2009-06-12  Paul Pluzhnikov  <ppluzhnikov@google.com>
>
>        gdb/10275
>        * dwarf2-frame.c (dwarf2_frame_state): Move cfa_offset, cfa_reg,
>        cfa_how and cfa_exp into regs and adjust users.

Ping?

-- 
Paul Pluzhnikov

Re: [rfc][patch] Fix for gdb/10275 (mis-handling of DW_CFA_restore_state)

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> Fixing this looks easier than I expected :-)
Paul> Regtested on Linux/x86_64 (no regressions).

This looks reasonable to me.

Paul> I also verified that I get correct stack for my reduced test case in
Paul> gdb/10275, and in my real gcc-4.5 compiled optimized code.

Paul> If this looks correct, I'll add a test case.

Yes, please do.  Thanks.

Paul> +	      fs->regs.cfa_reg = dwarf2_frame_adjust_regnum (gdbarch, fs->regs.cfa_reg,

This line looks like it wraps now.

Tom

Re: [rfc][patch] Fix for gdb/10275 (mis-handling of DW_CFA_restore_state)

[Paul Pluzhnikov]

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

On Thu, Jun 25, 2009 at 9:14 AM, Tom Tromey<tromey@redhat.com> wrote:

> Paul> If this looks correct, I'll add a test case.
> Yes, please do.

Attached.

> Paul> +       fs->regs.cfa_reg = dwarf2_frame_adjust_regnum (gdbarch, fs->regs.cfa_reg,
> This line looks like it wraps now.

Yes, fixed here and in one more place.

Thanks,
-- 
Paul Pluzhnikov

ChangeLog

2009-06-25  Paul Pluzhnikov  <ppluzhnikov@google.com>

	gdb/10275
	* dwarf2-frame.c (dwarf2_frame_state): Move cfa_offset, cfa_reg,
	cfa_how and cfa_exp into regs and adjust users.

testsuite/ChangeLog

2009-06-25  Paul Pluzhnikov  <ppluzhnikov@google.com>

	gdb/10275
	* gdb.dwarf2/dw2-restore.{S,exp}: New test.

[-- Attachment #2: gdb-bug-10275-20090625.txt --]
[-- Type: text/plain, Size: 7902 bytes --]

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.89
diff -u -p -u -r1.89 dwarf2-frame.c
--- dwarf2-frame.c	18 Jun 2009 14:02:18 -0000	1.89
+++ dwarf2-frame.c	25 Jun 2009 19:37:28 -0000
@@ -164,19 +164,19 @@ struct dwarf2_frame_state
     struct dwarf2_frame_state_reg *reg;
     int num_regs;
 
+    LONGEST cfa_offset;
+    ULONGEST cfa_reg;
+    enum {
+      CFA_UNSET,
+      CFA_REG_OFFSET,
+      CFA_EXP
+    } cfa_how;
+    gdb_byte *cfa_exp;
+
     /* Used to implement DW_CFA_remember_state.  */
     struct dwarf2_frame_state_reg_info *prev;
   } regs;
 
-  LONGEST cfa_offset;
-  ULONGEST cfa_reg;
-  gdb_byte *cfa_exp;
-  enum {
-    CFA_UNSET,
-    CFA_REG_OFFSET,
-    CFA_EXP
-  } cfa_how;
-
   /* The PC described by the current frame state.  */
   CORE_ADDR pc;
 
@@ -502,21 +502,22 @@ bad CFI data; mismatched DW_CFA_restore_
 	      break;
 
 	    case DW_CFA_def_cfa:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_reg);
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->regs.cfa_reg);
 	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &utmp);
 
 	      if (fs->armcc_cfa_offsets_sf)
 		utmp *= fs->data_align;
 
-	      fs->cfa_offset = utmp;
-	      fs->cfa_how = CFA_REG_OFFSET;
+	      fs->regs.cfa_offset = utmp;
+	      fs->regs.cfa_how = CFA_REG_OFFSET;
 	      break;
 
 	    case DW_CFA_def_cfa_register:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_reg);
-	      fs->cfa_reg = dwarf2_frame_adjust_regnum (gdbarch, fs->cfa_reg,
-							eh_frame_p);
-	      fs->cfa_how = CFA_REG_OFFSET;
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->regs.cfa_reg);
+	      fs->regs.cfa_reg = dwarf2_frame_adjust_regnum (gdbarch,
+                                                             fs->regs.cfa_reg,
+                                                             eh_frame_p);
+	      fs->regs.cfa_how = CFA_REG_OFFSET;
 	      break;
 
 	    case DW_CFA_def_cfa_offset:
@@ -525,7 +526,7 @@ bad CFI data; mismatched DW_CFA_restore_
 	      if (fs->armcc_cfa_offsets_sf)
 		utmp *= fs->data_align;
 
-	      fs->cfa_offset = utmp;
+	      fs->regs.cfa_offset = utmp;
 	      /* cfa_how deliberately not set.  */
 	      break;
 
@@ -533,10 +534,11 @@ bad CFI data; mismatched DW_CFA_restore_
 	      break;
 
 	    case DW_CFA_def_cfa_expression:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_exp_len);
-	      fs->cfa_exp = insn_ptr;
-	      fs->cfa_how = CFA_EXP;
-	      insn_ptr += fs->cfa_exp_len;
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end,
+                                       &fs->regs.cfa_exp_len);
+	      fs->regs.cfa_exp = insn_ptr;
+	      fs->regs.cfa_how = CFA_EXP;
+	      insn_ptr += fs->regs.cfa_exp_len;
 	      break;
 
 	    case DW_CFA_expression:
@@ -589,17 +591,18 @@ bad CFI data; mismatched DW_CFA_restore_
 	      break;
 
 	    case DW_CFA_def_cfa_sf:
-	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->cfa_reg);
-	      fs->cfa_reg = dwarf2_frame_adjust_regnum (gdbarch, fs->cfa_reg,
-							eh_frame_p);
+	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &fs->regs.cfa_reg);
+	      fs->regs.cfa_reg = dwarf2_frame_adjust_regnum (gdbarch,
+                                                             fs->regs.cfa_reg,
+                                                             eh_frame_p);
 	      insn_ptr = read_sleb128 (insn_ptr, insn_end, &offset);
-	      fs->cfa_offset = offset * fs->data_align;
-	      fs->cfa_how = CFA_REG_OFFSET;
+	      fs->regs.cfa_offset = offset * fs->data_align;
+	      fs->regs.cfa_how = CFA_REG_OFFSET;
 	      break;
 
 	    case DW_CFA_def_cfa_offset_sf:
 	      insn_ptr = read_sleb128 (insn_ptr, insn_end, &offset);
-	      fs->cfa_offset = offset * fs->data_align;
+	      fs->regs.cfa_offset = offset * fs->data_align;
 	      /* cfa_how deliberately not set.  */
 	      break;
 
@@ -932,19 +935,19 @@ dwarf2_frame_cache (struct frame_info *t
   execute_cfa_program (fde, fde->instructions, fde->end, this_frame, fs);
 
   /* Calculate the CFA.  */
-  switch (fs->cfa_how)
+  switch (fs->regs.cfa_how)
     {
     case CFA_REG_OFFSET:
-      cache->cfa = read_reg (this_frame, fs->cfa_reg);
+      cache->cfa = read_reg (this_frame, fs->regs.cfa_reg);
       if (fs->armcc_cfa_offsets_reversed)
-	cache->cfa -= fs->cfa_offset;
+	cache->cfa -= fs->regs.cfa_offset;
       else
-	cache->cfa += fs->cfa_offset;
+	cache->cfa += fs->regs.cfa_offset;
       break;
 
     case CFA_EXP:
       cache->cfa =
-	execute_stack_op (fs->cfa_exp, fs->cfa_exp_len,
+	execute_stack_op (fs->regs.cfa_exp, fs->regs.cfa_exp_len,
 			  cache->addr_size, this_frame, 0);
       break;
 
Index: testsuite/gdb.dwarf2/dw2-restore.S
===================================================================
RCS file: testsuite/gdb.dwarf2/dw2-restore.S
diff -N testsuite/gdb.dwarf2/dw2-restore.S
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/dw2-restore.S	25 Jun 2009 19:37:28 -0000
@@ -0,0 +1,50 @@
+/*
+   Copyright 2009 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 3 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, see <http://www.gnu.org/licenses/>.
+ */
+
+/* Compile with "gcc -nostdlib dw2-restore.S" */
+
+	.text
+	.globl _start
+	.func	_start
+_start:	call foo
+	mov $0,%rax
+	ret
+	.endfunc
+
+	.func	foo
+foo:	.cfi_startproc
+	push %rbp
+	.cfi_adjust_cfa_offset 8
+	mov %rsp,%rbp
+	.cfi_def_cfa_register %rbp
+
+	.cfi_remember_state
+	jmp 2f
+	
+1:	mov %rbp,%rsp
+	.cfi_restore %rbp
+	pop %rbp
+	.cfi_adjust_cfa_offset -8
+	.cfi_def_cfa_register %rsp
+	ret
+
+	.cfi_restore_state
+2: 	movq $0,%rax
+	movq $0,(%rax)  /* crash here */
+	jmp 1b
+	.cfi_endproc
+	.endfunc
Index: testsuite/gdb.dwarf2/dw2-restore.exp
===================================================================
RCS file: testsuite/gdb.dwarf2/dw2-restore.exp
diff -N testsuite/gdb.dwarf2/dw2-restore.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/dw2-restore.exp	25 Jun 2009 19:37:28 -0000
@@ -0,0 +1,37 @@
+# Copyright 2009 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 3 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, see <http://www.gnu.org/licenses/>.
+
+# Test handling of DW_CFA_restore_state.
+
+# This test can only be run on x86_64 targets.
+if {![istarget x86_64-*]} {
+    return 0  
+}
+set testfile "dw2-restore"
+set srcfile ${testfile}.S
+set binfile ${objdir}/${subdir}/${testfile}.x
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+       [list {additional_flags=-nostdlib}]] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+gdb_test "run" ""
+gdb_test "where" ".*$hex in foo ().+$hex in _start ().*"

Re: [rfc][patch] Fix for gdb/10275 (mis-handling of DW_CFA_restore_state)

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> 2009-06-25  Paul Pluzhnikov  <ppluzhnikov@google.com>
Paul> 	gdb/10275
Paul> 	* dwarf2-frame.c (dwarf2_frame_state): Move cfa_offset, cfa_reg,
Paul> 	cfa_how and cfa_exp into regs and adjust users.

This is ok.  Thanks for fixing this.

Tom