Re: RFC: DW_OP_call_frame_cfa, again

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> I'm happy with this version, so I plan to check it in next week or so.
Tom> Comment soon if you think it needs some change.

Since I got comments right away, I am going ahead with this now.

I've addressed all the comments in this version.
Also I remembered to include the test case in the diff this time.

Tom

ChangeLog:
2009-09-02  Tom Tromey  <tromey@redhat.com>

	* frame.h (frame_unwinder_is): Declare.
	* frame.c (frame_unwinder_is): New function.
	* dwarf2loc.c: Include dwarf2-frame.h.
	(dwarf_expr_frame_cfa): New function.
	(dwarf2_evaluate_loc_desc): Use it.
	(needs_frame_frame_cfa): New function.
	(dwarf2_loc_desc_needs_frame): Use it.
	* dwarf2expr.h (struct dwarf_expr_context) <get_frame_cfa>: New
	field.
	* dwarf2expr.c (execute_stack_op) <DW_OP_call_frame_cfa>: New
	case.
	* dwarf2-frame.h (dwarf2_frame_cfa): Declare.
	* dwarf2-frame.c (no_get_frame_cfa): New function.
	(execute_stack_op): Use it.
	(dwarf2_frame_cfa): New function.

testsuite/ChangeLog:
2009-09-02  Tom Tromey  <tromey@redhat.com>

	* gdb.dwarf2/callframecfa.exp: New file.
	* gdb.dwarf2/callframecfa.S: New file.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.96
diff -u -r1.96 dwarf2-frame.c
--- dwarf2-frame.c	11 Aug 2009 20:36:49 -0000	1.96
+++ dwarf2-frame.c	2 Sep 2009 14:51:32 -0000
@@ -309,6 +309,15 @@
 		  _("Support for DW_OP_fbreg is unimplemented"));
 }
 
+/* Helper function for execute_stack_op.  */
+
+static CORE_ADDR
+no_get_frame_cfa (void *baton)
+{
+  internal_error (__FILE__, __LINE__,
+		  _("Support for DW_OP_call_frame_cfa is unimplemented"));
+}
+
 static CORE_ADDR
 no_get_tls_address (void *baton, CORE_ADDR offset)
 {
@@ -363,6 +372,7 @@
   ctx->read_reg = read_reg;
   ctx->read_mem = read_mem;
   ctx->get_frame_base = no_get_frame_base;
+  ctx->get_frame_cfa = no_get_frame_cfa;
   ctx->get_tls_address = no_get_tls_address;
 
   dwarf_expr_push (ctx, initial);
@@ -1250,6 +1260,23 @@
 
   return NULL;
 }
+
+/* Compute the CFA for THIS_FRAME, but only if THIS_FRAME came from
+   the DWARF unwinder.  This is used to implement
+   DW_OP_call_frame_cfa.  */
+
+CORE_ADDR
+dwarf2_frame_cfa (struct frame_info *this_frame)
+{
+  while (get_frame_type (this_frame) == INLINE_FRAME)
+    this_frame = get_prev_frame (this_frame);
+  /* 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))
+    error (_("can't compute CFA for this frame"));
+  return get_frame_base (this_frame);
+}
 
 const struct objfile_data *dwarf2_frame_objfile_data;
 
Index: dwarf2-frame.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.h,v
retrieving revision 1.20
diff -u -r1.20 dwarf2-frame.h
--- dwarf2-frame.h	3 Jan 2009 05:57:51 -0000	1.20
+++ dwarf2-frame.h	2 Sep 2009 14:51:32 -0000
@@ -118,4 +118,8 @@
 
 void dwarf2_frame_build_info (struct objfile *objfile);
 
+/* Compute the DWARF CFA for a frame.  */
+
+CORE_ADDR dwarf2_frame_cfa (struct frame_info *this_frame);
+
 #endif /* dwarf2-frame.h */
Index: dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.34
diff -u -r1.34 dwarf2expr.c
--- dwarf2expr.c	11 Aug 2009 20:36:49 -0000	1.34
+++ dwarf2expr.c	2 Sep 2009 14:51:32 -0000
@@ -716,6 +716,10 @@
 	  }
 	  break;
 
+	case DW_OP_call_frame_cfa:
+	  result = (ctx->get_frame_cfa) (ctx->baton);
+	  break;
+
 	case DW_OP_GNU_push_tls_address:
 	  /* Variable is at a constant offset in the thread-local
 	  storage block into the objfile for the current thread and
Index: dwarf2expr.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
retrieving revision 1.16
diff -u -r1.16 dwarf2expr.h
--- dwarf2expr.h	11 Aug 2009 20:36:49 -0000	1.16
+++ dwarf2expr.h	2 Sep 2009 14:51:32 -0000
@@ -55,6 +55,9 @@
      expression evaluation is complete.  */
   void (*get_frame_base) (void *baton, gdb_byte **start, size_t *length);
 
+  /* Return the CFA for the frame.  */
+  CORE_ADDR (*get_frame_cfa) (void *baton);
+
   /* Return the thread-local storage address for
      DW_OP_GNU_push_tls_address.  */
   CORE_ADDR (*get_tls_address) (void *baton, CORE_ADDR offset);
Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.64
diff -u -r1.64 dwarf2loc.c
--- dwarf2loc.c	31 Aug 2009 20:18:45 -0000	1.64
+++ dwarf2loc.c	2 Sep 2009 14:51:32 -0000
@@ -36,6 +36,7 @@
 #include "dwarf2.h"
 #include "dwarf2expr.h"
 #include "dwarf2loc.h"
+#include "dwarf2-frame.h"
 
 #include "gdb_string.h"
 #include "gdb_assert.h"
@@ -194,6 +195,16 @@
 	   SYMBOL_NATURAL_NAME (framefunc));
 }
 
+/* Helper function for dwarf2_evaluate_loc_desc.  Computes the CFA for
+   the frame in BATON.  */
+
+static CORE_ADDR
+dwarf_expr_frame_cfa (void *baton)
+{
+  struct dwarf_expr_baton *debaton = (struct dwarf_expr_baton *) baton;
+  return dwarf2_frame_cfa (debaton->frame);
+}
+
 /* Using the objfile specified in BATON, find the address for the
    current thread's thread-local storage with offset OFFSET.  */
 static CORE_ADDR
@@ -237,6 +248,7 @@
   ctx->read_reg = dwarf_expr_read_reg;
   ctx->read_mem = dwarf_expr_read_mem;
   ctx->get_frame_base = dwarf_expr_frame_base;
+  ctx->get_frame_cfa = dwarf_expr_frame_cfa;
   ctx->get_tls_address = dwarf_expr_tls_address;
 
   dwarf_expr_eval (ctx, data, size);
@@ -331,6 +343,16 @@
   nf_baton->needs_frame = 1;
 }
 
+/* CFA accesses require a frame.  */
+
+static CORE_ADDR
+needs_frame_frame_cfa (void *baton)
+{
+  struct needs_frame_baton *nf_baton = baton;
+  nf_baton->needs_frame = 1;
+  return 1;
+}
+
 /* Thread-local accesses do require a frame.  */
 static CORE_ADDR
 needs_frame_tls_address (void *baton, CORE_ADDR offset)
@@ -363,6 +385,7 @@
   ctx->read_reg = needs_frame_read_reg;
   ctx->read_mem = needs_frame_read_mem;
   ctx->get_frame_base = needs_frame_frame_base;
+  ctx->get_frame_cfa = needs_frame_frame_cfa;
   ctx->get_tls_address = needs_frame_tls_address;
 
   dwarf_expr_eval (ctx, data, size);
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.271
diff -u -r1.271 frame.c
--- frame.c	2 Jul 2009 17:25:53 -0000	1.271
+++ frame.c	2 Sep 2009 14:51:33 -0000
@@ -1843,6 +1843,17 @@
   return fi->base->this_args (fi, &fi->base_cache);
 }
 
+/* Return true if the frame unwinder for frame FI is UNWINDER; false
+   otherwise.  */
+
+int
+frame_unwinder_is (struct frame_info *fi, const struct frame_unwind *unwinder)
+{
+  if (fi->unwind == NULL)
+    fi->unwind = frame_unwind_find_by_frame (fi, &fi->prologue_cache);
+  return fi->unwind == unwinder;
+}
+
 /* Level of the selected frame: 0 for innermost, 1 for its caller, ...
    or -1 for a NULL frame.  */
 
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.178
diff -u -r1.178 frame.h
--- frame.h	2 Jul 2009 17:09:28 -0000	1.178
+++ frame.h	2 Sep 2009 14:51:33 -0000
@@ -696,4 +696,10 @@
 
 extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
 
+/* Return true if the frame unwinder for frame FI is UNWINDER; false
+   otherwise.  */
+
+extern int frame_unwinder_is (struct frame_info *fi,
+			      const struct frame_unwind *unwinder);
+
 #endif /* !defined (FRAME_H)  */
Index: testsuite/gdb.dwarf2/callframecfa.S
===================================================================
RCS file: testsuite/gdb.dwarf2/callframecfa.S
diff -N testsuite/gdb.dwarf2/callframecfa.S
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/callframecfa.S	2 Sep 2009 14:51:34 -0000
@@ -0,0 +1,309 @@
+/*
+   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/>.
+ */
+
+/* This was compiled from a trivial program just to test the
+   DW_OP_call_frame_cfa operator:
+
+    int func (int arg) {
+      return arg + 23;
+    }
+
+    int main(int argc, char *argv[]) {
+      func (77);
+    }
+*/
+
+	.file	"q.c"
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.text
+.Ltext0:
+.globl func
+	.type	func, @function
+func:
+.LFB0:
+	.file 1 "q.c"
+	.loc 1 2 0
+	.cfi_startproc
+	pushl	%ebp
+	.cfi_def_cfa_offset 8
+	movl	%esp, %ebp
+	.cfi_offset 5, -8
+	.cfi_def_cfa_register 5
+	.loc 1 3 0
+	movl	8(%ebp), %eax
+	addl	$23, %eax
+	.loc 1 4 0
+	popl	%ebp
+	.cfi_restore 5
+	.cfi_def_cfa 4, 4
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	func, .-func
+.globl _start
+	.type	_start, @function
+_start:
+.LFB1:
+	.loc 1 6 0
+	.cfi_startproc
+	pushl	%ebp
+	.cfi_def_cfa_offset 8
+	movl	%esp, %ebp
+	.cfi_offset 5, -8
+	.cfi_def_cfa_register 5
+	subl	$4, %esp
+	.loc 1 7 0
+	movl	$77, (%esp)
+	call	func
+	.loc 1 8 0
+	leave
+	.cfi_restore 5
+	.cfi_def_cfa 4, 4
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	_start, .-_start
+.Letext0:
+	.section	.debug_info
+	.long	0x9e
+	.value	0x3
+	.long	.Ldebug_abbrev0
+	.byte	0x4
+	.uleb128 0x1
+	.long	.LASF5
+	.byte	0x1
+	.string	"q.c"
+	.long	.LASF6
+	.long	.Ltext0
+	.long	.Letext0
+	.long	.Ldebug_line0
+	.uleb128 0x2
+	.byte	0x1
+	.long	.LASF0
+	.byte	0x1
+	.byte	0x1
+	.byte	0x1
+	.long	0x4f
+	.long	.LFB0
+	.long	.LFE0
+	.byte	0x1
+	.byte	0x9c
+	.long	0x4f
+	.uleb128 0x3
+	.string	"arg"
+	.byte	0x1
+	.byte	0x1
+	.long	0x4f
+	.byte	0x2
+	.byte	0x91
+	.sleb128 0
+	.byte	0x0
+	.uleb128 0x4
+	.byte	0x4
+	.byte	0x5
+	.string	"int"
+	.uleb128 0x2
+	.byte	0x1
+	.long	.LASF1
+	.byte	0x1
+	.byte	0x6
+	.byte	0x1
+	.long	0x4f
+	.long	.LFB1
+	.long	.LFE1
+	.byte	0x1
+	.byte	0x9c
+	.long	0x8e
+	.uleb128 0x5
+	.long	.LASF2
+	.byte	0x1
+	.byte	0x6
+	.long	0x4f
+	.byte	0x2
+	.byte	0x91
+	.sleb128 0
+	.uleb128 0x5
+	.long	.LASF3
+	.byte	0x1
+	.byte	0x6
+	.long	0x8e
+	.byte	0x2
+	.byte	0x91
+	.sleb128 4
+	.byte	0x0
+	.uleb128 0x6
+	.byte	0x4
+	.long	0x94
+	.uleb128 0x6
+	.byte	0x4
+	.long	0x9a
+	.uleb128 0x7
+	.byte	0x1
+	.byte	0x6
+	.long	.LASF4
+	.byte	0x0
+	.section	.debug_abbrev
+	.uleb128 0x1
+	.uleb128 0x11
+	.byte	0x1
+	.uleb128 0x25
+	.uleb128 0xe
+	.uleb128 0x13
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0x1b
+	.uleb128 0xe
+	.uleb128 0x11
+	.uleb128 0x1
+	.uleb128 0x12
+	.uleb128 0x1
+	.uleb128 0x10
+	.uleb128 0x6
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x2
+	.uleb128 0x2e
+	.byte	0x1
+	.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 0xa
+	.uleb128 0x1
+	.uleb128 0x13
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x3
+	.uleb128 0x5
+	.byte	0x0
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x2
+	.uleb128 0xa
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x4
+	.uleb128 0x24
+	.byte	0x0
+	.uleb128 0xb
+	.uleb128 0xb
+	.uleb128 0x3e
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0x8
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x5
+	.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 0x6
+	.uleb128 0xf
+	.byte	0x0
+	.uleb128 0xb
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.byte	0x0
+	.byte	0x0
+	.uleb128 0x7
+	.uleb128 0x24
+	.byte	0x0
+	.uleb128 0xb
+	.uleb128 0xb
+	.uleb128 0x3e
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0xe
+	.byte	0x0
+	.byte	0x0
+	.byte	0x0
+	.section	.debug_pubnames,"",@progbits
+	.long	0x20
+	.value	0x2
+	.long	.Ldebug_info0
+	.long	0xa2
+	.long	0x25
+	.string	"func"
+	.long	0x56
+	.string	"main"
+	.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
+.LASF5:
+	.string	"GNU C 4.5.0 20090810 (experimental) [trunk revision 150633]"
+.LASF2:
+	.string	"argc"
+.LASF6:
+	.string	"/tmp"
+.LASF0:
+	.string	"func"
+.LASF3:
+	.string	"argv"
+.LASF1:
+	.string	"main"
+.LASF4:
+	.string	"char"
+	.ident	"GCC: (GNU) 4.5.0 20090810 (experimental) [trunk revision 150633]"
+	.section	.note.GNU-stack,"",@progbits
Index: testsuite/gdb.dwarf2/callframecfa.exp
===================================================================
RCS file: testsuite/gdb.dwarf2/callframecfa.exp
diff -N testsuite/gdb.dwarf2/callframecfa.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/callframecfa.exp	2 Sep 2009 14:51:34 -0000
@@ -0,0 +1,55 @@
+# 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 DW_OP_call_frame_cfa.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0  
+}
+# This test can only be run on x86 targets.
+if {![istarget i?86-*]} {
+    return 0  
+}
+
+set testfile "callframecfa"
+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 "break *func" "Breakpoint 1.*" "set breakpoint for call-frame-cfa"
+gdb_test "run" "" "run for call-frame-cfa"
+gdb_test "display arg" "arg = 77" "set display for call-frame-cfa"
+
+# We know how many instructions are in the function.  Note that we
+# can't handle the "ret" instruction due to the epilogue unwinder.
+for {set i 1} {$i < 5} {incr i} {
+    gdb_test "si" "arg = 77" "step $i for call-frame-cfa"
+}