[RFC] better dwarf checking for values on the stack

[RFC] better dwarf checking for values on the stack

[Doug Evans]

This patch builds on the patch here:

http://sourceware.org/ml/gdb-patches/2009-09/msg00106.html

The purpose is to call set_value_stack only for values actually on the stack.
This addresses the issues identified here:

http://sourceware.org/ml/gdb-patches/2009-09/msg00069.html

I'd like to get this in 7.0, otherwise we have to consider
pulling "set stack-cache" out.

2009-09-10  Doug Evans  <dje@google.com>

	Add better checking for values on stack.
	* dwarf2expr.h (dwarf_value_location): Rename DWARF_VALUE_STACK to
	DWARF_VALUE_DWARF_STACK, all uses updated.
	New enum DWARF_VALUE_MEMORY_STACK.
	* dwarf2expr.c (execute_stack_op, case DW_OP_fbreg): Mark location
	as DWARF_VALUE_MEMORY_STACK.
	(execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
	(execute_stack_op, cases DW_OP_deref, DW_OP_deref_size): Mark
	location as DWARF_VALUE_MEMORY.
	(execute_stack_op, case DW_OP_piece): Remove unused addr_or_regnum.
	* dwarf2loc.c (read_pieced_value): Handle DWARF_VALUE_MEMORY_STACK.
	(write_pieced_value): Ditto.
	(dwarf2_evaluate_loc_desc): Ditto.  Only call set_value_stack for
	values known to be on the stack.
	* dwarf2-frame.c (execute_stack_op): Handle DWARF_VALUE_MEMORY_STACK.

--- ../../../dcache-for-stack2/src/gdb/dwarf2expr.h	2009-09-10 11:23:40.000000000 -0700
+++ ./dwarf2expr.h	2009-09-10 11:50:25.000000000 -0700
@@ -26,12 +26,25 @@
 /* The location of a value.  */
 enum dwarf_value_location
 {
-  /* The piece is in memory.  */
+  /* The piece is in memory.
+     The value on the dwarf stack is its address.  */
   DWARF_VALUE_MEMORY,
-  /* The piece is in a register.  */
+
+  /* The piece is in memory and known to be on the stack.
+     The value on the dwarf stack is its address.
+     Stack memory is treated specially as it allows optimized access
+     for remote targets.
+     NOTE: This is an optimization.  It is always ok to use
+     DWARF_VALUE_MEMORY instead.  */
+  DWARF_VALUE_MEMORY_STACK,
+
+  /* The piece is in a register.
+     The value on the dwarf stack is the register number.  */
   DWARF_VALUE_REGISTER,
-  /* The piece is on the stack.  */
-  DWARF_VALUE_STACK,
+
+  /* The piece is on the dwarf stack.  */
+  DWARF_VALUE_DWARF_STACK,
+
   /* The piece is a literal.  */
   DWARF_VALUE_LITERAL
 };
@@ -111,7 +124,7 @@ struct dwarf_expr_context
      Each time DW_OP_piece is executed, we add a new element to the
      end of this array, recording the current top of the stack, the
      current location, and the size given as the operand to
-     DW_OP_piece.  We then pop the top value from the stack, rest the
+     DW_OP_piece.  We then pop the top value from the stack, reset the
      location, and resume evaluation.
 
      The Dwarf spec doesn't say whether DW_OP_piece pops the top value
--- ../../../dcache-for-stack2/src/gdb/dwarf2expr.c	2009-09-10 11:23:40.000000000 -0700
+++ ./dwarf2expr.c	2009-09-10 15:37:20.000000000 -0700
@@ -483,7 +483,7 @@ execute_stack_op (struct dwarf_expr_cont
 	  goto no_push;
 
 	case DW_OP_stack_value:
-	  ctx->location = DWARF_VALUE_STACK;
+	  ctx->location = DWARF_VALUE_DWARF_STACK;
 	  require_composition (op_ptr, op_end, "DW_OP_stack_value");
 	  goto no_push;
 
@@ -551,14 +551,14 @@ execute_stack_op (struct dwarf_expr_cont
 	    (ctx->get_frame_base) (ctx->baton, &datastart, &datalen);
 	    dwarf_expr_eval (ctx, datastart, datalen);
 	    if (ctx->location == DWARF_VALUE_LITERAL
-		|| ctx->location == DWARF_VALUE_STACK)
+		|| ctx->location == DWARF_VALUE_DWARF_STACK)
 	      error (_("Not implemented: computing frame base using explicit value operator"));
 	    result = dwarf_expr_fetch (ctx, 0);
 	    if (ctx->location == DWARF_VALUE_REGISTER)
 	      result = (ctx->read_reg) (ctx->baton, result);
 	    result = result + offset;
 	    ctx->stack_len = before_stack_len;
-	    ctx->location = DWARF_VALUE_MEMORY;
+	    ctx->location = DWARF_VALUE_MEMORY_STACK;
 	  }
 	  break;
 	case DW_OP_dup:
@@ -627,6 +627,7 @@ execute_stack_op (struct dwarf_expr_cont
 		result = dwarf2_read_address (ctx->gdbarch,
 					      buf, buf + ctx->addr_size,
 					      ctx->addr_size);
+		ctx->location = DWARF_VALUE_MEMORY;
 	      }
 	      break;
 
@@ -638,6 +639,7 @@ execute_stack_op (struct dwarf_expr_cont
 		result = dwarf2_read_address (ctx->gdbarch,
 					      buf, buf + addr_size,
 					      addr_size);
+		ctx->location = DWARF_VALUE_MEMORY;
 	      }
 	      break;
 
@@ -758,6 +760,7 @@ execute_stack_op (struct dwarf_expr_cont
 
 	case DW_OP_call_frame_cfa:
 	  result = (ctx->get_frame_cfa) (ctx->baton);
+	  ctx->location = DWARF_VALUE_MEMORY_STACK;
 	  break;
 
 	case DW_OP_GNU_push_tls_address:
@@ -794,7 +797,6 @@ execute_stack_op (struct dwarf_expr_cont
         case DW_OP_piece:
           {
             ULONGEST size;
-            CORE_ADDR addr_or_regnum;
 
             /* Record the piece.  */
             op_ptr = read_uleb128 (op_ptr, op_end, &size);
--- ../../../dcache-for-stack2/src/gdb/dwarf2loc.c	2009-09-10 11:23:40.000000000 -0700
+++ ./dwarf2loc.c	2009-09-10 11:57:01.000000000 -0700
@@ -220,7 +220,7 @@ struct piece_closure
   /* The number of pieces used to describe this variable.  */
   int n_pieces;
 
-  /* The architecture, used only for DWARF_VALUE_STACK.  */
+  /* The architecture, used only for DWARF_VALUE_DWARF_STACK.  */
   struct gdbarch *arch;
 
   /* The pieces themselves.  */
@@ -275,7 +275,11 @@ read_pieced_value (struct value *v)
 	  read_memory (p->v.value, contents + offset, p->size);
 	  break;
 
-	case DWARF_VALUE_STACK:
+	case DWARF_VALUE_MEMORY_STACK:
+	  read_stack (p->v.value, contents + offset, p->size);
+	  break;
+
+	case DWARF_VALUE_DWARF_STACK:
 	  {
 	    gdb_byte bytes[sizeof (ULONGEST)];
 	    size_t n;
@@ -335,6 +339,7 @@ write_pieced_value (struct value *to, st
 	  }
 	  break;
 	case DWARF_VALUE_MEMORY:
+	case DWARF_VALUE_MEMORY_STACK:
 	  write_memory (p->v.value, contents + offset, p->size);
 	  break;
 	default:
@@ -432,18 +437,20 @@ dwarf2_evaluate_loc_desc (struct symbol 
 	  break;
 
 	case DWARF_VALUE_MEMORY:
+	case DWARF_VALUE_MEMORY_STACK:
 	  {
 	    CORE_ADDR address = dwarf_expr_fetch (ctx, 0);
 
 	    retval = allocate_value (SYMBOL_TYPE (var));
 	    VALUE_LVAL (retval) = lval_memory;
 	    set_value_lazy (retval, 1);
-	    set_value_stack (retval, 1);
+	    if (ctx->location == DWARF_VALUE_MEMORY_STACK)
+	      set_value_stack (retval, 1);
 	    set_value_address (retval, address);
 	  }
 	  break;
 
-	case DWARF_VALUE_STACK:
+	case DWARF_VALUE_DWARF_STACK:
 	  {
 	    gdb_byte bytes[sizeof (ULONGEST)];
 	    ULONGEST value = (ULONGEST) dwarf_expr_fetch (ctx, 0);
@@ -485,10 +492,6 @@ dwarf2_evaluate_loc_desc (struct symbol 
 
   return retval;
 }
-
-
-
-
 \f
 /* Helper functions and baton for dwarf2_loc_desc_needs_frame.  */
 
--- ../../../dcache-for-stack2/src/gdb/dwarf2-frame.c	2009-09-10 11:23:40.000000000 -0700
+++ ./dwarf2-frame.c	2009-09-10 11:42:07.000000000 -0700
@@ -381,7 +381,8 @@ execute_stack_op (gdb_byte *exp, ULONGES
 
   if (ctx->location == DWARF_VALUE_REGISTER)
     result = read_reg (this_frame, result);
-  else if (ctx->location != DWARF_VALUE_MEMORY)
+  else if (ctx->location != DWARF_VALUE_MEMORY
+	   && ctx->location != DWARF_VALUE_MEMORY_STACK)
     {
       /* This is actually invalid DWARF, but if we ever do run across
 	 it somehow, we might as well support it.  So, instead, report

Re: [RFC] better dwarf checking for values on the stack

[Tom Tromey]

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

Doug> 2009-09-10  Doug Evans  <dje@google.com>
Doug> 	Add better checking for values on stack.
Doug> 	* dwarf2expr.h (dwarf_value_location): Rename DWARF_VALUE_STACK to
Doug> 	DWARF_VALUE_DWARF_STACK, all uses updated.
Doug> 	New enum DWARF_VALUE_MEMORY_STACK.
Doug> 	* dwarf2expr.c (execute_stack_op, case DW_OP_fbreg): Mark location
Doug> 	as DWARF_VALUE_MEMORY_STACK.
Doug> 	(execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
Doug> 	(execute_stack_op, cases DW_OP_deref, DW_OP_deref_size): Mark
Doug> 	location as DWARF_VALUE_MEMORY.
Doug> 	(execute_stack_op, case DW_OP_piece): Remove unused addr_or_regnum.

I think the tracking idea is sound, but I think this implementation has
a flaw.

It seems to me that the inferior-stack-ness of a value must be an
attribute carried alongside the value on the dwarf stack.

Here's a really bogus example that I hope still shows what I mean:

  DW_OP_call_frame_cfa
  DW_OP_deref
  DW_OP_call_frame_cfa
  DW_OP_drop

After the drop, ctx->location will be DWARF_VALUE_MEMORY_STACK.
However, this is incorrect.

Tom

Re: [RFC] better dwarf checking for values on the stack

[Doug Evans]

On Fri, Sep 11, 2009 at 11:48 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> 2009-09-10  Doug Evans  <dje@google.com>
> Doug>   Add better checking for values on stack.
> Doug>   * dwarf2expr.h (dwarf_value_location): Rename DWARF_VALUE_STACK to
> Doug>   DWARF_VALUE_DWARF_STACK, all uses updated.
> Doug>   New enum DWARF_VALUE_MEMORY_STACK.
> Doug>   * dwarf2expr.c (execute_stack_op, case DW_OP_fbreg): Mark location
> Doug>   as DWARF_VALUE_MEMORY_STACK.
> Doug>   (execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
> Doug>   (execute_stack_op, cases DW_OP_deref, DW_OP_deref_size): Mark
> Doug>   location as DWARF_VALUE_MEMORY.
> Doug>   (execute_stack_op, case DW_OP_piece): Remove unused addr_or_regnum.
>
> I think the tracking idea is sound, but I think this implementation has
> a flaw.
>
> It seems to me that the inferior-stack-ness of a value must be an
> attribute carried alongside the value on the dwarf stack.
>
> Here's a really bogus example that I hope still shows what I mean:
>
>  DW_OP_call_frame_cfa
>  DW_OP_deref
>  DW_OP_call_frame_cfa
>  DW_OP_drop
>
> After the drop, ctx->location will be DWARF_VALUE_MEMORY_STACK.
> However, this is incorrect.
>
> Tom
>

Yeah, I was wondering about this.  [It's what prompted my question on IRC.]

Re: [RFC] better dwarf checking for values on the stack

[Doug Evans]

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

On Fri, Sep 11, 2009 at 2:19 PM, Doug Evans<dje@google.com> wrote:
>>Tom Tromey wrote:
>>[...]
>> It seems to me that the inferior-stack-ness of a value must be an
>> attribute carried alongside the value on the dwarf stack.
>>[...]
>
> Yeah, I was wondering about this.  [It's what prompted my question on IRC.]

How about this?

2009-09-11  Doug Evans  <dje@google.com>

        * dwarf2expr.h (dwarf_value_location): Add more comments describing
        enum values.
        (struct dwarf_stack_value): New struct.
        (struct dwarf_expr_context): Change type of `stack' from CORE_ADDR*
        to structd dwarf_stack_value*.
        (struct dwarf_expr_piece): Move `v.value' into its own struct, v.expr,
        all uses updated.
        Add v.expr.in_stack_memory.
        (dwarf_expr_push): Update declaration.
        (dwarf_expr_fetch_in_stack_memory): Declare.
        * dwarf2expr.c (dwarf_expr_grow_stack): Update calculation of
        size of stack value.
        (dwarf_expr_push): New arg in_stack_memory, all callers updated.
        (dwarf_expr_fetch_in_stack_memory): New function.
        (add_piece): Set in_stack_memory for non-literal values.
        (execute_stack_op, case DW_OP_fbreg): Call dwarf_expr_push ourselves,
        mark value as in stack memory.
        (execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
        (execute_stack_op, cases DW_OP_swap, DW_OP_rot): Update type of
        dwarf stack value.
        * dwarf2loc.c (read_pieced_value): Call read_stack for values known
        to be on the program's stack.
        (dwarf2_evaluate_loc_desc, case DWARF_VALUE_MEMORY): Call
        set_value_stack only for objects known to be in stack memory.
        * dwarf2-frame.c (execute_stack_op): New arg initial_in_stack_memory,
        all callers updated.

[-- Attachment #2: gdb-090911-dwarf-stack-value-2.patch.txt --]
[-- Type: text/plain, Size: 12662 bytes --]

2009-09-11  Doug Evans  <dje@google.com>

	* dwarf2expr.h (dwarf_value_location): Add more comments describing
	enum values.
	(struct dwarf_stack_value): New struct.
	(struct dwarf_expr_context): Change type of `stack' from CORE_ADDR*
	to structd dwarf_stack_value*.
	(struct dwarf_expr_piece): Move `v.value' into its own struct, v.expr,
	all uses updated.
	Add v.expr.in_stack_memory.
	(dwarf_expr_push): Update declaration.
	(dwarf_expr_fetch_in_stack_memory): Declare.
	* dwarf2expr.c (dwarf_expr_grow_stack): Update calculation of
	size of stack value.
	(dwarf_expr_push): New arg in_stack_memory, all callers updated.
	(dwarf_expr_fetch_in_stack_memory): New function.
	(add_piece): Set in_stack_memory for non-literal values.
	(execute_stack_op, case DW_OP_fbreg): Call dwarf_expr_push ourselves,
	mark value as in stack memory.
	(execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
	(execute_stack_op, cases DW_OP_swap, DW_OP_rot): Update type of
	dwarf stack value.
	* dwarf2loc.c (read_pieced_value): Call read_stack for values known
	to be on the program's stack.
	(dwarf2_evaluate_loc_desc, case DWARF_VALUE_MEMORY): Call
	set_value_stack only for objects known to be in stack memory.
	* dwarf2-frame.c (execute_stack_op): New arg initial_in_stack_memory,
	all callers updated.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.98
diff -u -p -r1.98 dwarf2-frame.c
--- dwarf2-frame.c	11 Sep 2009 18:38:39 -0000	1.98
+++ dwarf2-frame.c	11 Sep 2009 22:56:06 -0000
@@ -357,7 +357,8 @@ register %s (#%d) at %s"),
 
 static CORE_ADDR
 execute_stack_op (gdb_byte *exp, ULONGEST len, int addr_size,
-		  struct frame_info *this_frame, CORE_ADDR initial)
+		  struct frame_info *this_frame, CORE_ADDR initial,
+		  int initial_in_stack_memory)
 {
   struct dwarf_expr_context *ctx;
   CORE_ADDR result;
@@ -375,7 +376,7 @@ execute_stack_op (gdb_byte *exp, ULONGES
   ctx->get_frame_cfa = no_get_frame_cfa;
   ctx->get_tls_address = no_get_tls_address;
 
-  dwarf_expr_push (ctx, initial);
+  dwarf_expr_push (ctx, initial, initial_in_stack_memory);
   dwarf_expr_eval (ctx, exp, len);
   result = dwarf_expr_fetch (ctx, 0);
 
@@ -975,7 +976,7 @@ dwarf2_frame_cache (struct frame_info *t
     case CFA_EXP:
       cache->cfa =
 	execute_stack_op (fs->regs.cfa_exp, fs->regs.cfa_exp_len,
-			  cache->addr_size, this_frame, 0);
+			  cache->addr_size, this_frame, 0, 0);
       break;
 
     default:
@@ -1131,7 +1132,7 @@ dwarf2_frame_prev_register (struct frame
     case DWARF2_FRAME_REG_SAVED_EXP:
       addr = execute_stack_op (cache->reg[regnum].loc.exp,
 			       cache->reg[regnum].exp_len,
-			       cache->addr_size, this_frame, cache->cfa);
+			       cache->addr_size, this_frame, cache->cfa, 1);
       return frame_unwind_got_memory (this_frame, regnum, addr);
 
     case DWARF2_FRAME_REG_SAVED_VAL_OFFSET:
@@ -1141,7 +1142,7 @@ dwarf2_frame_prev_register (struct frame
     case DWARF2_FRAME_REG_SAVED_VAL_EXP:
       addr = execute_stack_op (cache->reg[regnum].loc.exp,
 			       cache->reg[regnum].exp_len,
-			       cache->addr_size, this_frame, cache->cfa);
+			       cache->addr_size, this_frame, cache->cfa, 1);
       return frame_unwind_got_constant (this_frame, regnum, addr);
 
     case DWARF2_FRAME_REG_UNSPECIFIED:
Index: dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.36
diff -u -p -r1.36 dwarf2expr.c
--- dwarf2expr.c	11 Sep 2009 18:38:39 -0000	1.36
+++ dwarf2expr.c	11 Sep 2009 22:56:06 -0000
@@ -87,7 +87,7 @@ dwarf_expr_grow_stack (struct dwarf_expr
     {
       size_t newlen = ctx->stack_len + need + 10;
       ctx->stack = xrealloc (ctx->stack,
-			     newlen * sizeof (CORE_ADDR));
+			     newlen * sizeof (struct dwarf_stack_value));
       ctx->stack_allocated = newlen;
     }
 }
@@ -95,10 +95,15 @@ dwarf_expr_grow_stack (struct dwarf_expr
 /* Push VALUE onto CTX's stack.  */
 
 void
-dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value)
+dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value,
+		 int in_stack_memory)
 {
+  struct dwarf_stack_value *v;
+
   dwarf_expr_grow_stack (ctx, 1);
-  ctx->stack[ctx->stack_len++] = value;
+  v = &ctx->stack[ctx->stack_len++];
+  v->value = value;
+  v->in_stack_memory = in_stack_memory;
 }
 
 /* Pop the top item off of CTX's stack.  */
@@ -119,7 +124,19 @@ dwarf_expr_fetch (struct dwarf_expr_cont
   if (ctx->stack_len <= n)
      error (_("Asked for position %d of stack, stack only has %d elements on it."),
 	    n, ctx->stack_len);
-  return ctx->stack[ctx->stack_len - (1 + n)];
+  return ctx->stack[ctx->stack_len - (1 + n)].value;
+
+}
+
+/* Retrieve the in_stack_memory flag of the N'th item on CTX's stack.  */
+
+int
+dwarf_expr_fetch_in_stack_memory (struct dwarf_expr_context *ctx, int n)
+{
+  if (ctx->stack_len <= n)
+     error (_("Asked for position %d of stack, stack only has %d elements on it."),
+	    n, ctx->stack_len);
+  return ctx->stack[ctx->stack_len - (1 + n)].in_stack_memory;
 
 }
 
@@ -148,7 +165,10 @@ add_piece (struct dwarf_expr_context *ct
       p->v.literal.length = ctx->len;
     }
   else
-    p->v.value = dwarf_expr_fetch (ctx, 0);
+    {
+      p->v.expr.value = dwarf_expr_fetch (ctx, 0);
+      p->v.expr.in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0);
+    }
 }
 
 /* Evaluate the expression at ADDR (LEN bytes long) using the context
@@ -559,8 +579,10 @@ execute_stack_op (struct dwarf_expr_cont
 	    result = result + offset;
 	    ctx->stack_len = before_stack_len;
 	    ctx->location = DWARF_VALUE_MEMORY;
+	    dwarf_expr_push (ctx, result, 1);
 	  }
-	  break;
+	  goto no_push; /* already pushed */
+
 	case DW_OP_dup:
 	  result = dwarf_expr_fetch (ctx, 0);
 	  break;
@@ -576,7 +598,7 @@ execute_stack_op (struct dwarf_expr_cont
 	  
 	case DW_OP_swap:
 	  {
-	    CORE_ADDR t1, t2;
+	    struct dwarf_stack_value t1, t2;
 
 	    if (ctx->stack_len < 2)
 	       error (_("Not enough elements for DW_OP_swap. Need 2, have %d."),
@@ -594,7 +616,7 @@ execute_stack_op (struct dwarf_expr_cont
 
 	case DW_OP_rot:
 	  {
-	    CORE_ADDR t1, t2, t3;
+	    struct dwarf_stack_value t1, t2, t3;
 
 	    if (ctx->stack_len < 3)
 	       error (_("Not enough elements for DW_OP_rot. Need 3, have %d."),
@@ -758,7 +782,8 @@ execute_stack_op (struct dwarf_expr_cont
 
 	case DW_OP_call_frame_cfa:
 	  result = (ctx->get_frame_cfa) (ctx->baton);
-	  break;
+	  dwarf_expr_push (ctx, result, 1);
+	  goto no_push; /* already pushed */
 
 	case DW_OP_GNU_push_tls_address:
 	  /* Variable is at a constant offset in the thread-local
@@ -820,8 +844,9 @@ execute_stack_op (struct dwarf_expr_cont
 	  error (_("Unhandled dwarf expression opcode 0x%x"), op);
 	}
 
-      /* Most things push a result value.  */
-      dwarf_expr_push (ctx, result);
+      /* Most things push a result value (that doesn't live on the
+	 program's stack).  */	 
+      dwarf_expr_push (ctx, result, 0);
     no_push:;
     }
 
Index: dwarf2expr.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
retrieving revision 1.18
diff -u -p -r1.18 dwarf2expr.h
--- dwarf2expr.h	11 Sep 2009 18:38:39 -0000	1.18
+++ dwarf2expr.h	11 Sep 2009 22:56:06 -0000
@@ -26,22 +26,38 @@
 /* The location of a value.  */
 enum dwarf_value_location
 {
-  /* The piece is in memory.  */
+  /* The piece is in memory.
+     The value on the dwarf stack is its address.  */
   DWARF_VALUE_MEMORY,
-  /* The piece is in a register.  */
+
+  /* The piece is in a register.
+     The value on the dwarf stack is the register number.  */
   DWARF_VALUE_REGISTER,
-  /* The piece is on the stack.  */
+
+  /* The piece is on the dwarf stack.  */
   DWARF_VALUE_STACK,
+
   /* The piece is a literal.  */
   DWARF_VALUE_LITERAL
 };
 
+/* The dwarf expression stack.  */
+
+struct dwarf_stack_value
+{
+  CORE_ADDR value;
+
+  /* Non-zero if the piece is in memory and is known to be
+     on the program's stack.  */
+  int in_stack_memory;
+};
+
 /* The expression evaluator works with a dwarf_expr_context, describing
    its current state and its callbacks.  */
 struct dwarf_expr_context
 {
   /* The stack of values, allocated with xmalloc.  */
-  CORE_ADDR *stack;
+  struct dwarf_stack_value *stack;
 
   /* The number of values currently pushed on the stack, and the
      number of elements allocated to the stack.  */
@@ -111,7 +127,7 @@ struct dwarf_expr_context
      Each time DW_OP_piece is executed, we add a new element to the
      end of this array, recording the current top of the stack, the
      current location, and the size given as the operand to
-     DW_OP_piece.  We then pop the top value from the stack, rest the
+     DW_OP_piece.  We then pop the top value from the stack, reset the
      location, and resume evaluation.
 
      The Dwarf spec doesn't say whether DW_OP_piece pops the top value
@@ -140,8 +156,14 @@ struct dwarf_expr_piece
 
   union
   {
-    /* This piece's address or register number.  */
-    CORE_ADDR value;
+    struct
+    {
+      /* This piece's address or register number.  */
+      CORE_ADDR value;
+      /* Non-zero if the piece is known to be in memory and on
+	 the program's stack.  */
+      int in_stack_memory;
+    } expr;
 
     struct
     {
@@ -162,11 +184,13 @@ void free_dwarf_expr_context (struct dwa
 struct cleanup *
     make_cleanup_free_dwarf_expr_context (struct dwarf_expr_context *ctx);
 
-void dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value);
+void dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value,
+		      int in_stack_memory);
 void dwarf_expr_pop (struct dwarf_expr_context *ctx);
 void dwarf_expr_eval (struct dwarf_expr_context *ctx, unsigned char *addr,
 		      size_t len);
 CORE_ADDR dwarf_expr_fetch (struct dwarf_expr_context *ctx, int n);
+int dwarf_expr_fetch_in_stack_memory (struct dwarf_expr_context *ctx, int n);
 
 
 gdb_byte *read_uleb128 (gdb_byte *buf, gdb_byte *buf_end, ULONGEST * r);
Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.67
diff -u -p -r1.67 dwarf2loc.c
--- dwarf2loc.c	11 Sep 2009 18:38:39 -0000	1.67
+++ dwarf2loc.c	11 Sep 2009 22:56:06 -0000
@@ -265,14 +265,17 @@ read_pieced_value (struct value *v)
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    bfd_byte regval[MAX_REGISTER_SIZE];
 	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch,
-							   p->v.value);
+							   p->v.expr.value);
 	    get_frame_register (frame, gdb_regnum, regval);
 	    memcpy (contents + offset, regval, p->size);
 	  }
 	  break;
 
 	case DWARF_VALUE_MEMORY:
-	  read_memory (p->v.value, contents + offset, p->size);
+	  if (p->v.expr.in_stack_memory)
+	    read_stack (p->v.expr.value, contents + offset, p->size);
+	  else
+	    read_memory (p->v.expr.value, contents + offset, p->size);
 	  break;
 
 	case DWARF_VALUE_STACK:
@@ -282,7 +285,7 @@ read_pieced_value (struct value *v)
 	    int addr_size = gdbarch_addr_bit (c->arch) / 8;
 	    store_unsigned_integer (bytes, addr_size,
 				    gdbarch_byte_order (c->arch),
-				    p->v.value);
+				    p->v.expr.value);
 	    n = p->size;
 	    if (n > addr_size)
 	      n = addr_size;
@@ -330,12 +333,12 @@ write_pieced_value (struct value *to, st
 	case DWARF_VALUE_REGISTER:
 	  {
 	    struct gdbarch *arch = get_frame_arch (frame);
-	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.value);
+	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.expr.value);
 	    put_frame_register (frame, gdb_regnum, contents + offset);
 	  }
 	  break;
 	case DWARF_VALUE_MEMORY:
-	  write_memory (p->v.value, contents + offset, p->size);
+	  write_memory (p->v.expr.value, contents + offset, p->size);
 	  break;
 	default:
 	  set_value_optimized_out (to, 1);
@@ -434,11 +437,13 @@ dwarf2_evaluate_loc_desc (struct symbol 
 	case DWARF_VALUE_MEMORY:
 	  {
 	    CORE_ADDR address = dwarf_expr_fetch (ctx, 0);
+	    int in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0);
 
 	    retval = allocate_value (SYMBOL_TYPE (var));
 	    VALUE_LVAL (retval) = lval_memory;
 	    set_value_lazy (retval, 1);
-	    set_value_stack (retval, 1);
+	    if (in_stack_memory)
+	      set_value_stack (retval, 1);
 	    set_value_address (retval, address);
 	  }
 	  break;
@@ -485,10 +490,6 @@ dwarf2_evaluate_loc_desc (struct symbol 
 
   return retval;
 }
-
-
-
-
 \f
 /* Helper functions and baton for dwarf2_loc_desc_needs_frame.  */
 

Re: [RFC] better dwarf checking for values on the stack

[Doug Evans]

On Fri, Sep 11, 2009 at 4:23 PM, Doug Evans<dje@google.com> wrote:
> On Fri, Sep 11, 2009 at 2:19 PM, Doug Evans<dje@google.com> wrote:
>>>Tom Tromey wrote:
>>>[...]
>>> It seems to me that the inferior-stack-ness of a value must be an
>>> attribute carried alongside the value on the dwarf stack.
>>>[...]
>>
>> Yeah, I was wondering about this.  [It's what prompted my question on IRC.]
>
> How about this?
>
> 2009-09-11  Doug Evans  <dje@google.com>
>
>        * dwarf2expr.h (dwarf_value_location): Add more comments describing
>        enum values.
>        (struct dwarf_stack_value): New struct.
>        (struct dwarf_expr_context): Change type of `stack' from CORE_ADDR*
>        to structd dwarf_stack_value*.
>        (struct dwarf_expr_piece): Move `v.value' into its own struct, v.expr,
>        all uses updated.
>        Add v.expr.in_stack_memory.
>        (dwarf_expr_push): Update declaration.
>        (dwarf_expr_fetch_in_stack_memory): Declare.
>        * dwarf2expr.c (dwarf_expr_grow_stack): Update calculation of
>        size of stack value.
>        (dwarf_expr_push): New arg in_stack_memory, all callers updated.
>        (dwarf_expr_fetch_in_stack_memory): New function.
>        (add_piece): Set in_stack_memory for non-literal values.
>        (execute_stack_op, case DW_OP_fbreg): Call dwarf_expr_push ourselves,
>        mark value as in stack memory.
>        (execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
>        (execute_stack_op, cases DW_OP_swap, DW_OP_rot): Update type of
>        dwarf stack value.
>        * dwarf2loc.c (read_pieced_value): Call read_stack for values known
>        to be on the program's stack.
>        (dwarf2_evaluate_loc_desc, case DWARF_VALUE_MEMORY): Call
>        set_value_stack only for objects known to be in stack memory.
>        * dwarf2-frame.c (execute_stack_op): New arg initial_in_stack_memory,
>        all callers updated.
>

For completeness' sake ...

Yeah, I don't propagate in_stack_memory for DW_OP_dup, etc. etc.
If ya like the basic idea, I'll add in all the other details.

Re: [RFC] better dwarf checking for values on the stack

[Tom Tromey]

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

Tom> It seems to me that the inferior-stack-ness of a value must be an
Tom> attribute carried alongside the value on the dwarf stack.

Doug> How about this?

Yeah, I think this is the way to go.  Thanks.

Tom

Re: [RFC] better dwarf checking for values on the stack

[Doug Evans]

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

On Mon, Sep 14, 2009 at 10:53 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Tom> It seems to me that the inferior-stack-ness of a value must be an
> Tom> attribute carried alongside the value on the dwarf stack.
>
> Doug> How about this?
>
> Yeah, I think this is the way to go.  Thanks.

Thanks.

I'll check this in in a few days if there are no further issues.

2009-09-14  Doug Evans  <dje@google.com>

	* dwarf2expr.h (dwarf_value_location): Add more comments describing
	enum values.
	(struct dwarf_stack_value): New struct.
	(struct dwarf_expr_context): Change type of `stack' from CORE_ADDR*
	to struct dwarf_stack_value*.
	(struct dwarf_expr_piece): Move `v.value' into its own struct, v.expr,
	all uses updated.  Add v.expr.in_stack_memory.
	(dwarf_expr_push): Update declaration.
	(dwarf_expr_fetch_in_stack_memory): Declare.
	* dwarf2expr.c (dwarf_expr_grow_stack): Update calculation of
	size of stack value.
	(dwarf_expr_push): New arg in_stack_memory, all callers updated.
	(dwarf_expr_fetch_in_stack_memory): New function.
	(add_piece): Set in_stack_memory for non-literal values.
	(execute_stack_op): Allow ops to specify where the value is on the
	program's stack.
	(execute_stack_op, case DW_OP_fbreg): Mark value as in stack memory.
	(execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
	(execute_stack_op, case DW_OP_dup): Copy in_stack_memory flag.
	(execute_stack_op, cases DW_OP_pick, DW_OP_over): Ditto.
	(execute_stack_op, cases DW_OP_swap, DW_OP_rot): Update type of
	dwarf stack value.
	* dwarf2loc.c (read_pieced_value): Call read_stack for values known
	to be on the program's stack.
	(dwarf2_evaluate_loc_desc, case DWARF_VALUE_MEMORY): Call
	set_value_stack only for objects known to be in stack memory.
	* dwarf2-frame.c (execute_stack_op): New arg initial_in_stack_memory,
	all callers updated.

[-- Attachment #2: gdb-090914-dwarf-stack-value-3.patch.txt --]
[-- Type: text/plain, Size: 13791 bytes --]

2009-09-14  Doug Evans  <dje@google.com>

	* dwarf2expr.h (dwarf_value_location): Add more comments describing
	enum values.
	(struct dwarf_stack_value): New struct.
	(struct dwarf_expr_context): Change type of `stack' from CORE_ADDR*
	to struct dwarf_stack_value*.
	(struct dwarf_expr_piece): Move `v.value' into its own struct, v.expr,
	all uses updated.  Add v.expr.in_stack_memory.
	(dwarf_expr_push): Update declaration.
	(dwarf_expr_fetch_in_stack_memory): Declare.
	* dwarf2expr.c (dwarf_expr_grow_stack): Update calculation of
	size of stack value.
	(dwarf_expr_push): New arg in_stack_memory, all callers updated.
	(dwarf_expr_fetch_in_stack_memory): New function.
	(add_piece): Set in_stack_memory for non-literal values.
	(execute_stack_op): Allow ops to specify where the value is on the
	program's stack.
	(execute_stack_op, case DW_OP_fbreg): Mark value as in stack memory.
	(execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
	(execute_stack_op, case DW_OP_dup): Copy in_stack_memory flag.
	(execute_stack_op, cases DW_OP_pick, DW_OP_over): Ditto.
	(execute_stack_op, cases DW_OP_swap, DW_OP_rot): Update type of
	dwarf stack value.
	* dwarf2loc.c (read_pieced_value): Call read_stack for values known
	to be on the program's stack.
	(dwarf2_evaluate_loc_desc, case DWARF_VALUE_MEMORY): Call
	set_value_stack only for objects known to be in stack memory.
	* dwarf2-frame.c (execute_stack_op): New arg initial_in_stack_memory,
	all callers updated.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.98
diff -u -p -r1.98 dwarf2-frame.c
--- dwarf2-frame.c	11 Sep 2009 18:38:39 -0000	1.98
+++ dwarf2-frame.c	14 Sep 2009 22:43:28 -0000
@@ -357,7 +357,8 @@ register %s (#%d) at %s"),
 
 static CORE_ADDR
 execute_stack_op (gdb_byte *exp, ULONGEST len, int addr_size,
-		  struct frame_info *this_frame, CORE_ADDR initial)
+		  struct frame_info *this_frame, CORE_ADDR initial,
+		  int initial_in_stack_memory)
 {
   struct dwarf_expr_context *ctx;
   CORE_ADDR result;
@@ -375,7 +376,7 @@ execute_stack_op (gdb_byte *exp, ULONGES
   ctx->get_frame_cfa = no_get_frame_cfa;
   ctx->get_tls_address = no_get_tls_address;
 
-  dwarf_expr_push (ctx, initial);
+  dwarf_expr_push (ctx, initial, initial_in_stack_memory);
   dwarf_expr_eval (ctx, exp, len);
   result = dwarf_expr_fetch (ctx, 0);
 
@@ -975,7 +976,7 @@ dwarf2_frame_cache (struct frame_info *t
     case CFA_EXP:
       cache->cfa =
 	execute_stack_op (fs->regs.cfa_exp, fs->regs.cfa_exp_len,
-			  cache->addr_size, this_frame, 0);
+			  cache->addr_size, this_frame, 0, 0);
       break;
 
     default:
@@ -1131,7 +1132,7 @@ dwarf2_frame_prev_register (struct frame
     case DWARF2_FRAME_REG_SAVED_EXP:
       addr = execute_stack_op (cache->reg[regnum].loc.exp,
 			       cache->reg[regnum].exp_len,
-			       cache->addr_size, this_frame, cache->cfa);
+			       cache->addr_size, this_frame, cache->cfa, 1);
       return frame_unwind_got_memory (this_frame, regnum, addr);
 
     case DWARF2_FRAME_REG_SAVED_VAL_OFFSET:
@@ -1141,7 +1142,7 @@ dwarf2_frame_prev_register (struct frame
     case DWARF2_FRAME_REG_SAVED_VAL_EXP:
       addr = execute_stack_op (cache->reg[regnum].loc.exp,
 			       cache->reg[regnum].exp_len,
-			       cache->addr_size, this_frame, cache->cfa);
+			       cache->addr_size, this_frame, cache->cfa, 1);
       return frame_unwind_got_constant (this_frame, regnum, addr);
 
     case DWARF2_FRAME_REG_UNSPECIFIED:
Index: dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.37
diff -u -p -r1.37 dwarf2expr.c
--- dwarf2expr.c	11 Sep 2009 23:11:34 -0000	1.37
+++ dwarf2expr.c	14 Sep 2009 22:43:28 -0000
@@ -87,7 +87,7 @@ dwarf_expr_grow_stack (struct dwarf_expr
     {
       size_t newlen = ctx->stack_len + need + 10;
       ctx->stack = xrealloc (ctx->stack,
-			     newlen * sizeof (CORE_ADDR));
+			     newlen * sizeof (struct dwarf_stack_value));
       ctx->stack_allocated = newlen;
     }
 }
@@ -95,10 +95,15 @@ dwarf_expr_grow_stack (struct dwarf_expr
 /* Push VALUE onto CTX's stack.  */
 
 void
-dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value)
+dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value,
+		 int in_stack_memory)
 {
+  struct dwarf_stack_value *v;
+
   dwarf_expr_grow_stack (ctx, 1);
-  ctx->stack[ctx->stack_len++] = value;
+  v = &ctx->stack[ctx->stack_len++];
+  v->value = value;
+  v->in_stack_memory = in_stack_memory;
 }
 
 /* Pop the top item off of CTX's stack.  */
@@ -119,7 +124,19 @@ dwarf_expr_fetch (struct dwarf_expr_cont
   if (ctx->stack_len <= n)
      error (_("Asked for position %d of stack, stack only has %d elements on it."),
 	    n, ctx->stack_len);
-  return ctx->stack[ctx->stack_len - (1 + n)];
+  return ctx->stack[ctx->stack_len - (1 + n)].value;
+
+}
+
+/* Retrieve the in_stack_memory flag of the N'th item on CTX's stack.  */
+
+int
+dwarf_expr_fetch_in_stack_memory (struct dwarf_expr_context *ctx, int n)
+{
+  if (ctx->stack_len <= n)
+     error (_("Asked for position %d of stack, stack only has %d elements on it."),
+	    n, ctx->stack_len);
+  return ctx->stack[ctx->stack_len - (1 + n)].in_stack_memory;
 
 }
 
@@ -148,7 +165,10 @@ add_piece (struct dwarf_expr_context *ct
       p->v.literal.length = ctx->len;
     }
   else
-    p->v.value = dwarf_expr_fetch (ctx, 0);
+    {
+      p->v.expr.value = dwarf_expr_fetch (ctx, 0);
+      p->v.expr.in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0);
+    }
 }
 
 /* Evaluate the expression at ADDR (LEN bytes long) using the context
@@ -329,6 +349,13 @@ execute_stack_op (struct dwarf_expr_cont
     {
       enum dwarf_location_atom op = *op_ptr++;
       CORE_ADDR result;
+      /* Assume the value is not in stack memory.
+	 Code that knows otherwise sets this to 1.
+	 Some arithmetic on stack addresses can probably be assumed to still
+	 be a stack address, but we skip this complication for now.
+	 This is just an optimization, so it's always ok to punt
+	 and leave this as 0.  */
+      int in_stack_memory = 0;
       ULONGEST uoffset, reg;
       LONGEST offset;
 
@@ -557,12 +584,15 @@ execute_stack_op (struct dwarf_expr_cont
 	    if (ctx->location == DWARF_VALUE_REGISTER)
 	      result = (ctx->read_reg) (ctx->baton, result);
 	    result = result + offset;
+	    in_stack_memory = 1;
 	    ctx->stack_len = before_stack_len;
 	    ctx->location = DWARF_VALUE_MEMORY;
 	  }
 	  break;
+
 	case DW_OP_dup:
 	  result = dwarf_expr_fetch (ctx, 0);
+	  in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0);
 	  break;
 
 	case DW_OP_drop:
@@ -572,11 +602,12 @@ execute_stack_op (struct dwarf_expr_cont
 	case DW_OP_pick:
 	  offset = *op_ptr++;
 	  result = dwarf_expr_fetch (ctx, offset);
+	  in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, offset);
 	  break;
 	  
 	case DW_OP_swap:
 	  {
-	    CORE_ADDR t1, t2;
+	    struct dwarf_stack_value t1, t2;
 
 	    if (ctx->stack_len < 2)
 	       error (_("Not enough elements for DW_OP_swap. Need 2, have %d."),
@@ -590,11 +621,12 @@ execute_stack_op (struct dwarf_expr_cont
 
 	case DW_OP_over:
 	  result = dwarf_expr_fetch (ctx, 1);
+	  in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 1);
 	  break;
 
 	case DW_OP_rot:
 	  {
-	    CORE_ADDR t1, t2, t3;
+	    struct dwarf_stack_value t1, t2, t3;
 
 	    if (ctx->stack_len < 3)
 	       error (_("Not enough elements for DW_OP_rot. Need 3, have %d."),
@@ -758,6 +790,7 @@ execute_stack_op (struct dwarf_expr_cont
 
 	case DW_OP_call_frame_cfa:
 	  result = (ctx->get_frame_cfa) (ctx->baton);
+	  in_stack_memory = 1;
 	  break;
 
 	case DW_OP_GNU_push_tls_address:
@@ -820,7 +853,7 @@ execute_stack_op (struct dwarf_expr_cont
 	}
 
       /* Most things push a result value.  */
-      dwarf_expr_push (ctx, result);
+      dwarf_expr_push (ctx, result, in_stack_memory);
     no_push:;
     }
 
Index: dwarf2expr.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
retrieving revision 1.18
diff -u -p -r1.18 dwarf2expr.h
--- dwarf2expr.h	11 Sep 2009 18:38:39 -0000	1.18
+++ dwarf2expr.h	14 Sep 2009 22:43:28 -0000
@@ -26,22 +26,41 @@
 /* The location of a value.  */
 enum dwarf_value_location
 {
-  /* The piece is in memory.  */
+  /* The piece is in memory.
+     The value on the dwarf stack is its address.  */
   DWARF_VALUE_MEMORY,
-  /* The piece is in a register.  */
+
+  /* The piece is in a register.
+     The value on the dwarf stack is the register number.  */
   DWARF_VALUE_REGISTER,
-  /* The piece is on the stack.  */
+
+  /* The piece is on the dwarf stack.  */
   DWARF_VALUE_STACK,
+
   /* The piece is a literal.  */
   DWARF_VALUE_LITERAL
 };
 
+/* The dwarf expression stack.  */
+
+struct dwarf_stack_value
+{
+  CORE_ADDR value;
+
+  /* Non-zero if the piece is in memory and is known to be
+     on the program's stack.  It is always ok to set this to zero.
+     This is used, for example, to optimize memory access from the target.
+     It can vastly speed up backtraces on long latency connections when
+     "set stack-cache on".  */
+  int in_stack_memory;
+};
+
 /* The expression evaluator works with a dwarf_expr_context, describing
    its current state and its callbacks.  */
 struct dwarf_expr_context
 {
   /* The stack of values, allocated with xmalloc.  */
-  CORE_ADDR *stack;
+  struct dwarf_stack_value *stack;
 
   /* The number of values currently pushed on the stack, and the
      number of elements allocated to the stack.  */
@@ -111,7 +130,7 @@ struct dwarf_expr_context
      Each time DW_OP_piece is executed, we add a new element to the
      end of this array, recording the current top of the stack, the
      current location, and the size given as the operand to
-     DW_OP_piece.  We then pop the top value from the stack, rest the
+     DW_OP_piece.  We then pop the top value from the stack, reset the
      location, and resume evaluation.
 
      The Dwarf spec doesn't say whether DW_OP_piece pops the top value
@@ -140,8 +159,14 @@ struct dwarf_expr_piece
 
   union
   {
-    /* This piece's address or register number.  */
-    CORE_ADDR value;
+    struct
+    {
+      /* This piece's address or register number.  */
+      CORE_ADDR value;
+      /* Non-zero if the piece is known to be in memory and on
+	 the program's stack.  */
+      int in_stack_memory;
+    } expr;
 
     struct
     {
@@ -162,11 +187,13 @@ void free_dwarf_expr_context (struct dwa
 struct cleanup *
     make_cleanup_free_dwarf_expr_context (struct dwarf_expr_context *ctx);
 
-void dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value);
+void dwarf_expr_push (struct dwarf_expr_context *ctx, CORE_ADDR value,
+		      int in_stack_memory);
 void dwarf_expr_pop (struct dwarf_expr_context *ctx);
 void dwarf_expr_eval (struct dwarf_expr_context *ctx, unsigned char *addr,
 		      size_t len);
 CORE_ADDR dwarf_expr_fetch (struct dwarf_expr_context *ctx, int n);
+int dwarf_expr_fetch_in_stack_memory (struct dwarf_expr_context *ctx, int n);
 
 
 gdb_byte *read_uleb128 (gdb_byte *buf, gdb_byte *buf_end, ULONGEST * r);
Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.67
diff -u -p -r1.67 dwarf2loc.c
--- dwarf2loc.c	11 Sep 2009 18:38:39 -0000	1.67
+++ dwarf2loc.c	14 Sep 2009 22:43:28 -0000
@@ -265,14 +265,17 @@ read_pieced_value (struct value *v)
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    bfd_byte regval[MAX_REGISTER_SIZE];
 	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch,
-							   p->v.value);
+							   p->v.expr.value);
 	    get_frame_register (frame, gdb_regnum, regval);
 	    memcpy (contents + offset, regval, p->size);
 	  }
 	  break;
 
 	case DWARF_VALUE_MEMORY:
-	  read_memory (p->v.value, contents + offset, p->size);
+	  if (p->v.expr.in_stack_memory)
+	    read_stack (p->v.expr.value, contents + offset, p->size);
+	  else
+	    read_memory (p->v.expr.value, contents + offset, p->size);
 	  break;
 
 	case DWARF_VALUE_STACK:
@@ -282,7 +285,7 @@ read_pieced_value (struct value *v)
 	    int addr_size = gdbarch_addr_bit (c->arch) / 8;
 	    store_unsigned_integer (bytes, addr_size,
 				    gdbarch_byte_order (c->arch),
-				    p->v.value);
+				    p->v.expr.value);
 	    n = p->size;
 	    if (n > addr_size)
 	      n = addr_size;
@@ -330,12 +333,12 @@ write_pieced_value (struct value *to, st
 	case DWARF_VALUE_REGISTER:
 	  {
 	    struct gdbarch *arch = get_frame_arch (frame);
-	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.value);
+	    int gdb_regnum = gdbarch_dwarf2_reg_to_regnum (arch, p->v.expr.value);
 	    put_frame_register (frame, gdb_regnum, contents + offset);
 	  }
 	  break;
 	case DWARF_VALUE_MEMORY:
-	  write_memory (p->v.value, contents + offset, p->size);
+	  write_memory (p->v.expr.value, contents + offset, p->size);
 	  break;
 	default:
 	  set_value_optimized_out (to, 1);
@@ -434,11 +437,13 @@ dwarf2_evaluate_loc_desc (struct symbol 
 	case DWARF_VALUE_MEMORY:
 	  {
 	    CORE_ADDR address = dwarf_expr_fetch (ctx, 0);
+	    int in_stack_memory = dwarf_expr_fetch_in_stack_memory (ctx, 0);
 
 	    retval = allocate_value (SYMBOL_TYPE (var));
 	    VALUE_LVAL (retval) = lval_memory;
 	    set_value_lazy (retval, 1);
-	    set_value_stack (retval, 1);
+	    if (in_stack_memory)
+	      set_value_stack (retval, 1);
 	    set_value_address (retval, address);
 	  }
 	  break;
@@ -485,10 +490,6 @@ dwarf2_evaluate_loc_desc (struct symbol 
 
   return retval;
 }
-
-
-
-
 \f
 /* Helper functions and baton for dwarf2_loc_desc_needs_frame.  */
 

Re: [RFC] better dwarf checking for values on the stack

[Tom Tromey]

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

Tom> Yeah, I think this is the way to go.  Thanks.

Doug> Thanks.
Doug> I'll check this in in a few days if there are no further issues.

It looks good to me.

Tom

Re: [RFC] better dwarf checking for values on the stack

[Joel Brobecker]

> Doug> I'll check this in in a few days if there are no further issues.
[...]
> It looks good to me.

Since this is release-critical, and Tom reviewed it, would you mind
committing the change today? I'm hoping to be able to create the
branch tomorrow, and it'd be nice to have all blocking issues
resolved sometime today.

Thanks!
-- 
Joel

Re: [RFC] better dwarf checking for values on the stack

[Doug Evans]

On Tue, Sep 15, 2009 at 9:10 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>Tom wrote:
>> Doug> I'll check this in in a few days if there are no further issues.
> [...]
>> It looks good to me.

Thanks!

> Since this is release-critical, and Tom reviewed it, would you mind
> committing the change today? I'm hoping to be able to create the
> branch tomorrow, and it'd be nice to have all blocking issues
> resolved sometime today.
>
> Thanks!

Done.

Re: [RFC] better dwarf checking for values on the stack

[Cary Coutant]

>        * dwarf2expr.c (dwarf_expr_grow_stack): Update calculation of
>        size of stack value.
>        (dwarf_expr_push): New arg in_stack_memory, all callers updated.
>        (dwarf_expr_fetch_in_stack_memory): New function.
>        (add_piece): Set in_stack_memory for non-literal values.
>        (execute_stack_op): Allow ops to specify where the value is on the
>        program's stack.
>        (execute_stack_op, case DW_OP_fbreg): Mark value as in stack memory.
>        (execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
>        (execute_stack_op, case DW_OP_dup): Copy in_stack_memory flag.
>        (execute_stack_op, cases DW_OP_pick, DW_OP_over): Ditto.
>        (execute_stack_op, cases DW_OP_swap, DW_OP_rot): Update type of
>        dwarf stack value.

It seems to me that if you're going to go this route (rather than the
heuristic approach of your first patch), you need to do some type
algebra here. You've got three types of things on the expression
stack; let's call them M (generic addresses, probably not on the
memory stack), S (addresses of things on the memory stack), and K
(unitless constants). There are combining rules for these types; for
example:

  K <anyop> K -> K
  M +/- K -> M
  K +/- M -> M
  M - M -> K
  S +/- K -> S
  K +/- S -> S
  S - S -> K

There are combinations that don't make sense, but aren't technically
illegal in the DWARF spec, so these will need to be handled
conservatively; for example:

  M + M -> M
  M * M -> M
  M + S -> M

You could have an expression like S - S + M [ = (S - S) + M = K + M ->
M ] -- admittedly unlikely, but the same can be said for Tom's example
that fooled the heuristic approach -- which should yield a memory
address, but from what I can tell will end up claiming it's a stack
address.

-cary

Re: [RFC] better dwarf checking for values on the stack

[Doug Evans]

On Tue, Sep 15, 2009 at 1:20 PM, Cary Coutant <ccoutant@google.com> wrote:
>>        * dwarf2expr.c (dwarf_expr_grow_stack): Update calculation of
>>        size of stack value.
>>        (dwarf_expr_push): New arg in_stack_memory, all callers updated.
>>        (dwarf_expr_fetch_in_stack_memory): New function.
>>        (add_piece): Set in_stack_memory for non-literal values.
>>        (execute_stack_op): Allow ops to specify where the value is on the
>>        program's stack.
>>        (execute_stack_op, case DW_OP_fbreg): Mark value as in stack memory.
>>        (execute_stack_op, case DW_OP_call_frame_cfa): Ditto.
>>        (execute_stack_op, case DW_OP_dup): Copy in_stack_memory flag.
>>        (execute_stack_op, cases DW_OP_pick, DW_OP_over): Ditto.
>>        (execute_stack_op, cases DW_OP_swap, DW_OP_rot): Update type of
>>        dwarf stack value.
>
> It seems to me that if you're going to go this route (rather than the
> heuristic approach of your first patch), you need to do some type
> algebra here. You've got three types of things on the expression
> stack; let's call them M (generic addresses, probably not on the
> memory stack), S (addresses of things on the memory stack), and K
> (unitless constants). There are combining rules for these types; for
> example:
>
>  K <anyop> K -> K
>  M +/- K -> M
>  K +/- M -> M
>  M - M -> K
>  S +/- K -> S
>  K +/- S -> S
>  S - S -> K
>
> There are combinations that don't make sense, but aren't technically
> illegal in the DWARF spec, so these will need to be handled
> conservatively; for example:
>
>  M + M -> M
>  M * M -> M
>  M + S -> M
>
> You could have an expression like S - S + M [ = (S - S) + M = K + M ->
> M ] -- admittedly unlikely, but the same can be said for Tom's example
> that fooled the heuristic approach -- which should yield a memory
> address, but from what I can tell will end up claiming it's a stack
> address.

Yeah, if we wanted to catch more cases of what's on the stack than
what's currently there does, then we need to appropriately handle the
math.  I explicitly left that for another day.

dwarf2expr.c:

      /* Assume the value is not in stack memory.
         Code that knows otherwise sets this to 1.
         Some arithmetic on stack addresses can probably be assumed to
still
         be a stack address, but we skip this complication for now.
         This is just an optimization, so it's always ok to punt
         and leave this as 0.  */
      int in_stack_memory = 0;

Sound ok?

Re: [RFC] better dwarf checking for values on the stack

[Cary Coutant]

> Yeah, if we wanted to catch more cases of what's on the stack than
> what's currently there does, then we need to appropriately handle the
> math.  I explicitly left that for another day.
>
> dwarf2expr.c:
>
>      /* Assume the value is not in stack memory.
>         Code that knows otherwise sets this to 1.
>         Some arithmetic on stack addresses can probably be assumed to
> still
>         be a stack address, but we skip this complication for now.
>         This is just an optimization, so it's always ok to punt
>         and leave this as 0.  */
>      int in_stack_memory = 0;
>
> Sound ok?

Sure.

-cary