RFC: DW_OP_call_frame_cfa, again

RFC: DW_OP_call_frame_cfa, again

[Tom Tromey]

Here is a new version of my patch to implement DW_OP_call_frame_cfa.
Previous thread here:
    http://sourceware.org/ml/gdb-patches/2009-06/msg00191.html
With followup the next month (I wish our mailing list archive handled
this more nicely):
    http://sourceware.org/ml/gdb-patches/2009-07/msg00570.html


This version implements Daniel's idea of checking the frame unwinder.
If you read the followup message (above) you'll see that this patch is
different from what I talked about there; that approach doesn't work
since the DWARF frame base sniffer is not registered for many
architectures.  Jan suggested the current approach.

This version includes a test case.

Built and regtested on x86-64 (compile farm).
I also ran the tests locally, since the new test is x86-only.

FWIW, you can also easily generate this opcode using svn gcc with
-gdwarf-3.


One further issue is that this interacts poorly with the i386 epilogue
unwinder.  I punted on this issue for the time being.  However, with GCC
4.5, I think the DWARF unwinder is supposed to work fine for epilogues
-- so can we make the epilogue unwinder conditional?  Is this special
unwinder only needed for reverse execution?  If so, could we make it
conditional on when that mode is enabled?

I don't think this particular issue affects whether this patch should go
in.

Let me know what you think.

Tom

2009-08-11  Tom Tromey  <tromey@redhat.com>

	* frame.h (frame_unwinder_is): Rename from frame_base_is.
	* frame.c (frame_unwinder_is): Rename from frame_base_is.
	Rewrite.
	* dwarf2-frame.c (dwarf2_frame_cfa): Use frame_unwinder_is and
	get_frame_base.

2009-08-11  Tom Tromey  <tromey@redhat.com>

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

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 0f6da40..db53c75 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -310,6 +310,13 @@ no_get_frame_base (void *baton, gdb_byte **start, size_t *length)
 }
 
 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)
 {
   internal_error (__FILE__, __LINE__,
@@ -360,6 +367,7 @@ execute_stack_op (gdb_byte *exp, ULONGEST len, int addr_size,
   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);
@@ -1247,6 +1255,14 @@ dwarf2_frame_base_sniffer (struct frame_info *this_frame)
 
   return NULL;
 }
+
+CORE_ADDR
+dwarf2_frame_cfa (struct frame_info *this_frame)
+{
+  if (! frame_unwinder_is (this_frame, &dwarf2_frame_unwind))
+    error (_("can't compute CFA for this frame"));
+  return get_frame_base (this_frame);
+}
 \f
 const struct objfile_data *dwarf2_frame_objfile_data;
 
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index b203661..dd03d59 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -118,4 +118,8 @@ extern const struct frame_base *
 
 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 */
diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c
index 8dbf976..8a962a4 100644
--- a/gdb/dwarf2expr.c
+++ b/gdb/dwarf2expr.c
@@ -700,6 +700,10 @@ execute_stack_op (struct dwarf_expr_context *ctx,
 	  }
 	  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
diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h
index 7047922..97edf6c 100644
--- a/gdb/dwarf2expr.h
+++ b/gdb/dwarf2expr.h
@@ -55,6 +55,9 @@ struct dwarf_expr_context
      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);
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 071b5ac..930ba18 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -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,13 @@ dwarf_expr_frame_base (void *baton, gdb_byte **start, size_t * length)
 	   SYMBOL_NATURAL_NAME (framefunc));
 }
 
+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
@@ -234,6 +242,7 @@ dwarf2_evaluate_loc_desc (struct symbol *var, struct frame_info *frame,
   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);
@@ -327,6 +336,15 @@ needs_frame_frame_base (void *baton, gdb_byte **start, size_t * length)
   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)
@@ -356,6 +374,7 @@ dwarf2_loc_desc_needs_frame (gdb_byte *data, unsigned short size,
   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);
diff --git a/gdb/frame.c b/gdb/frame.c
index 67e0607..2b5a5b6 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1843,6 +1843,15 @@ get_frame_args_address (struct frame_info *fi)
   return fi->base->this_args (fi, &fi->base_cache);
 }
 
+/* Return true if the frame base for frame FI is BASE; false
+   otherwise.  */
+
+int
+frame_unwinder_is (struct frame_info *fi, const struct frame_unwind *unwinder)
+{
+  return fi->unwind == unwinder;
+}
+
 /* Level of the selected frame: 0 for innermost, 1 for its caller, ...
    or -1 for a NULL frame.  */
 
diff --git a/gdb/frame.h b/gdb/frame.h
index febef5c..611c6d3 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -696,4 +696,10 @@ extern struct frame_info *deprecated_safe_get_selected_frame (void);
 
 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)  */

Re: RFC: DW_OP_call_frame_cfa, again

[Tom Tromey]

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

Tom> Here is a new version of my patch to implement DW_OP_call_frame_cfa.
Tom> Previous thread here:
Tom>     http://sourceware.org/ml/gdb-patches/2009-06/msg00191.html
Tom> With followup the next month (I wish our mailing list archive handled
Tom> this more nicely):
Tom>     http://sourceware.org/ml/gdb-patches/2009-07/msg00570.html

Here is an updated version.  This fixes a couple of bugs found by
testing it in Fedora rawhide.

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

Tom

2009-09-01  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.

2009-09-01  Tom Tromey  <tromey@redhat.com>

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

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 427f58f..f32a44f 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -309,6 +309,14 @@ no_get_frame_base (void *baton, gdb_byte **start, size_t *length)
 		  _("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 +371,7 @@ execute_stack_op (gdb_byte *exp, ULONGEST len, int addr_size,
   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 +1259,20 @@ dwarf2_frame_base_sniffer (struct frame_info *this_frame)
 
   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);
+  if (! frame_unwinder_is (this_frame, &dwarf2_frame_unwind))
+    error (_("can't compute CFA for this frame"));
+  return get_frame_base (this_frame);
+}
 \f
 const struct objfile_data *dwarf2_frame_objfile_data;
 
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index b203661..dd03d59 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -118,4 +118,8 @@ extern const struct frame_base *
 
 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 */
diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c
index 2721065..6401e72 100644
--- a/gdb/dwarf2expr.c
+++ b/gdb/dwarf2expr.c
@@ -716,6 +716,10 @@ execute_stack_op (struct dwarf_expr_context *ctx,
 	  }
 	  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
diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h
index 2306e49..6b3a068 100644
--- a/gdb/dwarf2expr.h
+++ b/gdb/dwarf2expr.h
@@ -55,6 +55,9 @@ struct dwarf_expr_context
      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);
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index b6c9b11..95ee2f5 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -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,15 @@ dwarf_expr_frame_base (void *baton, gdb_byte **start, size_t * length)
 	   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 +247,7 @@ dwarf2_evaluate_loc_desc (struct symbol *var, struct frame_info *frame,
   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 +342,15 @@ needs_frame_frame_base (void *baton, gdb_byte **start, size_t * length)
   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 +383,7 @@ dwarf2_loc_desc_needs_frame (gdb_byte *data, unsigned short size,
   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);
diff --git a/gdb/frame.c b/gdb/frame.c
index 67e0607..3f32471 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1843,6 +1843,17 @@ get_frame_args_address (struct frame_info *fi)
   return fi->base->this_args (fi, &fi->base_cache);
 }
 
+/* Return true if the frame base for frame FI is BASE; 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.  */
 
diff --git a/gdb/frame.h b/gdb/frame.h
index febef5c..611c6d3 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -696,4 +696,10 @@ extern struct frame_info *deprecated_safe_get_selected_frame (void);
 
 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)  */

Re: RFC: DW_OP_call_frame_cfa, again

[Daniel Jacobowitz]

On Tue, Aug 11, 2009 at 02:49:02PM -0600, Tom Tromey wrote:
> This version implements Daniel's idea of checking the frame unwinder.
> If you read the followup message (above) you'll see that this patch is
> different from what I talked about there; that approach doesn't work
> since the DWARF frame base sniffer is not registered for many
> architectures.  Jan suggested the current approach.

Am I correct that this works only if all unwinders for this
architecture use the same CFA notion that DWARF-2 would?  If so -
probably already a requirement - it deserves a comment.

> One further issue is that this interacts poorly with the i386 epilogue
> unwinder.  I punted on this issue for the time being.  However, with GCC
> 4.5, I think the DWARF unwinder is supposed to work fine for epilogues
> -- so can we make the epilogue unwinder conditional?  Is this special
> unwinder only needed for reverse execution?  If so, could we make it
> conditional on when that mode is enabled?

It's useful at other times.  My canonical torture test for this is a
software watchpoint on a local variable, and then "next" over the
application's first call to printf - with glibc debug info.  You'll go
through the PLT, ld.so, printf itself, nested functions, et cetera.
Without an epilogue unwinder the only reason this survives is an
in_epilogue_p or similar hack in the watchpoint checking code.

With GCC 4.5 the DWARF unwinder ought to work fine for epilogues on
specific platforms, not all.

On Tue, Sep 01, 2009 at 12:08:25PM -0600, Tom Tromey wrote:
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
> 
> Tom> Here is a new version of my patch to implement DW_OP_call_frame_cfa.
> Tom> Previous thread here:
> Tom>     http://sourceware.org/ml/gdb-patches/2009-06/msg00191.html
> Tom> With followup the next month (I wish our mailing list archive handled
> Tom> this more nicely):
> Tom>     http://sourceware.org/ml/gdb-patches/2009-07/msg00570.html
> 
> Here is an updated version.  This fixes a couple of bugs found by
> testing it in Fedora rawhide.
> 
> I'm happy with this version, so I plan to check it in next week or so.
> Comment soon if you think it needs some change.

Aside from above, this version seems fine.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFC: DW_OP_call_frame_cfa, again

[Doug Evans]

On Tue, Sep 1, 2009 at 11:08 AM, Tom Tromey<tromey@redhat.com> wrote:
>>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
>
> Tom> Here is a new version of my patch to implement DW_OP_call_frame_cfa.
> Tom> Previous thread here:
> Tom>     http://sourceware.org/ml/gdb-patches/2009-06/msg00191.html
> Tom> With followup the next month (I wish our mailing list archive handled
> Tom> this more nicely):
> Tom>     http://sourceware.org/ml/gdb-patches/2009-07/msg00570.html
>
> Here is an updated version.  This fixes a couple of bugs found by
> testing it in Fedora rawhide.
>
> I'm happy with this version, so I plan to check it in next week or so.
> Comment soon if you think it needs some change.
>
> Tom


I hate to nitpick on style issues (much :-)) but since this one
bothers me, and I'm not clear on what the rules are, and there are
clear rules for other things, I thought I'd bring it up.

IWBN IMHO to require a blank line between a function's comment and its
definition.  It helps readability (to me anyway).
I notice this patch has a mixture of both having the blank line and
not having it.
[I don't mind the absence of a blank line in declarations as much,
I've left that for a separate discussion.]


>
> 2009-09-01  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.
>
> 2009-09-01  Tom Tromey  <tromey@redhat.com>
>
>        * gdb.dwarf2/callframecfa.exp: New file.
>        * gdb.dwarf2/callframecfa.S: New file.
>
> diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
> index 427f58f..f32a44f 100644
> --- a/gdb/dwarf2-frame.c
> +++ b/gdb/dwarf2-frame.c
> @@ -309,6 +309,14 @@ no_get_frame_base (void *baton, gdb_byte **start, size_t *length)
>                  _("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 +371,7 @@ execute_stack_op (gdb_byte *exp, ULONGEST len, int addr_size,
>   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 +1259,20 @@ dwarf2_frame_base_sniffer (struct frame_info *this_frame)
>
>   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);
> +  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;
>
> diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
> index b203661..dd03d59 100644
> --- a/gdb/dwarf2-frame.h
> +++ b/gdb/dwarf2-frame.h
> @@ -118,4 +118,8 @@ extern const struct frame_base *
>
>  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 */
> diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c
> index 2721065..6401e72 100644
> --- a/gdb/dwarf2expr.c
> +++ b/gdb/dwarf2expr.c
> @@ -716,6 +716,10 @@ execute_stack_op (struct dwarf_expr_context *ctx,
>          }
>          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
> diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h
> index 2306e49..6b3a068 100644
> --- a/gdb/dwarf2expr.h
> +++ b/gdb/dwarf2expr.h
> @@ -55,6 +55,9 @@ struct dwarf_expr_context
>      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);
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index b6c9b11..95ee2f5 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -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,15 @@ dwarf_expr_frame_base (void *baton, gdb_byte **start, size_t * length)
>           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 +247,7 @@ dwarf2_evaluate_loc_desc (struct symbol *var, struct frame_info *frame,
>   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 +342,15 @@ needs_frame_frame_base (void *baton, gdb_byte **start, size_t * length)
>   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 +383,7 @@ dwarf2_loc_desc_needs_frame (gdb_byte *data, unsigned short size,
>   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);
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 67e0607..3f32471 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1843,6 +1843,17 @@ get_frame_args_address (struct frame_info *fi)
>   return fi->base->this_args (fi, &fi->base_cache);
>  }
>
> +/* Return true if the frame base for frame FI is BASE; 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.  */
>
> diff --git a/gdb/frame.h b/gdb/frame.h
> index febef5c..611c6d3 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -696,4 +696,10 @@ extern struct frame_info *deprecated_safe_get_selected_frame (void);
>
>  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)  */
>

Re: RFC: DW_OP_call_frame_cfa, again

[Pedro Alves]

On Tuesday 01 September 2009 19:08:25, Tom Tromey wrote:
> 
> +/* 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;
> +}
> +

Return type is CORE_ADDR, but it returns 1?

On Tuesday 01 September 2009 19:08:25, Tom Tromey wrote:
> +/* 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);


> +/* Return true if the frame base for frame FI is BASE; false
> +   otherwise.  */
> +
> +int
> +frame_unwinder_is (struct frame_info *fi, const struct frame_unwind *unwinder)
> +{

Stale comment.

-- 
Pedro Alves

Re: RFC: DW_OP_call_frame_cfa, again

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> I hate to nitpick on style issues (much :-)) but since this one
Doug> bothers me, and I'm not clear on what the rules are, and there are
Doug> clear rules for other things, I thought I'd bring it up.

Doug> IWBN IMHO to require a blank line between a function's comment and its
Doug> definition.  It helps readability (to me anyway).
Doug> I notice this patch has a mixture of both having the blank line and
Doug> not having it.

Yeah, I'm inconsistent about that.
I will fix it up.

Tom

Re: RFC: DW_OP_call_frame_cfa, again

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

>> +/* 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;
>> +}
>> +

Pedro> Return type is CORE_ADDR, but it returns 1?

Yes, just like needs_frame_read_reg and needs_frame_tls_address in the
same section.  The result is arbitrary, in this code the expression is
being evaluated for its side effect of computing needs_frame.

I am not really certain that this code always works.  It seems to me
that it won't handle branches properly.  But, that is a not a bug
introduced by my change.

Pedro> Stale comment.

Thanks, I'll fix it.

Tom

Re: RFC: DW_OP_call_frame_cfa, again

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> Am I correct that this works only if all unwinders for this
Daniel> architecture use the same CFA notion that DWARF-2 would?  If so -
Daniel> probably already a requirement - it deserves a comment.

It actually just checks, and if the frame was not unwound using the
DWARF unwinder, it gives an error.  So, if the architecture has multiple
unwinders, and a non-DWARF one makes the frame, then you can't print
locals there.

We could expand this check if other unwinders compute "compatible" frame
bases somehow.

Daniel> With GCC 4.5 the DWARF unwinder ought to work fine for epilogues on
Daniel> specific platforms, not all.

Thanks.

Tom

Re: RFC: DW_OP_call_frame_cfa, again

[Tom Tromey]

>>>>> "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);
+}
 \f
 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"
+}

Re: RFC: DW_OP_call_frame_cfa, again

[Pedro Alves]

On Wednesday 02 September 2009 15:53:34, Tom Tromey wrote:
> +gdb_test "run" "" "run for call-frame-cfa"

Please don't hardcode "run".  It won't work with remote targets (e.g., gdbserver).

-- 
Pedro Alves

Re: RFC: DW_OP_call_frame_cfa, again

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Please don't hardcode "run".  It won't work with remote targets
Pedro> (e.g., gdbserver).

Sorry about that.

This uses runto_main and gdb_continue_to_breakpoint, which should work
everywhere, at least if other tests are any indication.

Tom

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

	* gdb.dwarf2/callframecfa.exp: Use gdb_continue_to_breakpoint.

Index: gdb.dwarf2/callframecfa.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.dwarf2/callframecfa.exp,v
retrieving revision 1.1
diff -u -r1.1 callframecfa.exp
--- gdb.dwarf2/callframecfa.exp	2 Sep 2009 14:53:57 -0000	1.1
+++ gdb.dwarf2/callframecfa.exp	2 Sep 2009 16:32:22 -0000
@@ -44,8 +44,12 @@
 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"
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "break *func" "Breakpoint 2.*" "set breakpoint for call-frame-cfa"
+gdb_continue_to_breakpoint "continue to breakpoint 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

Re: RFC: DW_OP_call_frame_cfa, again

[Pedro Alves]

On Wednesday 02 September 2009 17:47:56, Tom Tromey wrote:

> This uses runto_main and gdb_continue_to_breakpoint, which should work
> everywhere, at least if other tests are any indication.
> 

Thanks, this is OK.

-- 
Pedro Alves