Make DWARF evaluator return a single struct value

The patch is addressing the issue of class users writing and reading
the internal data of the dwarf_expr_context class.

At this point, all conditions are met for the DWARF evaluator to return
an evaluation result in a form of a single struct value object.

gdb/ChangeLog:

	* dwarf2/expr.c (pieced_value_funcs): Chenge to static
	function.
	(allocate_piece_closure): Change to static function.
	(dwarf_expr_context::fetch_result): New function.
	* dwarf2/expr.h (struct piece_closure): Remove declaration.
	(struct dwarf_expr_context): fetch_result new declaration.
	fetch, fetch_address and fetch_in_stack_memory members move
	to private.
	(allocate_piece_closure): Remove.
	* dwarf2/frame.c (execute_stack_op): Change to use
	fetch_result.
	* dwarf2/loc.c (dwarf2_evaluate_loc_desc_full): Change to use
	fetch_result.
	(dwarf2_locexpr_baton_eval): Change to use fetch_result.
        * dwarf2/loc.h (invalid_synthetic_pointer): Expose function.

gdb/dwarf2/expr.c

@@ -117,9 +117,10 @@ struct piece_closure
   struct frame_id frame_id;
 };
 
-/* See expr.h.  */
+/* Allocate a closure for a value formed from separately-described
+   PIECES.  */
 
-piece_closure *
+static piece_closure *
 allocate_piece_closure (dwarf2_per_cu_data *per_cu,
 			dwarf2_per_objfile *per_objfile,
 			std::vector<dwarf_expr_piece> &&pieces,
@@ -612,7 +613,7 @@ free_pieced_value_closure (value *v)
 }
 
 /* Functions for accessing a variable described by DW_OP_piece.  */
-const struct lval_funcs pieced_value_funcs = {
+static const struct lval_funcs pieced_value_funcs = {
   read_pieced_value,
   write_pieced_value,
   indirect_pieced_value,
@@ -879,6 +880,158 @@ dwarf_expr_context::push_dwarf_reg_entry_value (call_site_parameter_kind kind,
   this->eval (data_src, size);
 }
 
+/* See expr.h.  */
+
+value *
+dwarf_expr_context::fetch_result (struct type *type, struct type *subobj_type,
+				  LONGEST subobj_offset)
+{
+  value *retval = nullptr;
+
+  if (type == nullptr)
+    type = address_type ();
+
+  if (subobj_type == nullptr)
+    subobj_type = type;
+
+  if (this->pieces.size () > 0)
+    {
+      ULONGEST bit_size = 0;
+
+      for (dwarf_expr_piece &piece : this->pieces)
+	bit_size += piece.size;
+      /* Complain if the expression is larger than the size of the
+	 outer type.  */
+      if (bit_size > 8 * TYPE_LENGTH (type))
+	invalid_synthetic_pointer ();
+
+      piece_closure *c
+	= allocate_piece_closure (this->per_cu, this->per_objfile,
+				  std::move (this->pieces), this->frame);
+      retval = allocate_computed_value (subobj_type,
+					&pieced_value_funcs, c);
+      set_value_offset (retval, subobj_offset);
+    }
+  else
+    {
+      switch (this->location)
+	{
+	case DWARF_VALUE_REGISTER:
+	  {
+	    int dwarf_regnum
+	      = longest_to_int (value_as_long (this->fetch (0)));
+	    int gdb_regnum = dwarf_reg_to_regnum_or_error (this->gdbarch,
+							   dwarf_regnum);
+
+	    if (subobj_offset != 0)
+	      error (_("cannot use offset on synthetic pointer to register"));
+
+	    gdb_assert (this->frame != NULL);
+
+	    retval = value_from_register (subobj_type, gdb_regnum,
+					  this->frame);
+	    if (value_optimized_out (retval))
+	      {
+		/* This means the register has undefined value / was
+		   not saved.  As we're computing the location of some
+		   variable etc. in the program, not a value for
+		   inspecting a register ($pc, $sp, etc.), return a
+		   generic optimized out value instead, so that we show
+		   <optimized out> instead of <not saved>.  */
+		value *tmp = allocate_value (subobj_type);
+		value_contents_copy (tmp, 0, retval, 0,
+				     TYPE_LENGTH (subobj_type));
+		retval = tmp;
+	      }
+	  }
+	  break;
+
+	case DWARF_VALUE_MEMORY:
+	  {
+	    struct type *ptr_type;
+	    CORE_ADDR address = this->fetch_address (0);
+	    bool in_stack_memory = this->fetch_in_stack_memory (0);
+
+	    /* DW_OP_deref_size (and possibly other operations too) may
+	       create a pointer instead of an address.  Ideally, the
+	       pointer to address conversion would be performed as part
+	       of those operations, but the type of the object to
+	       which the address refers is not known at the time of
+	       the operation.  Therefore, we do the conversion here
+	       since the type is readily available.  */
+
+	    switch (subobj_type->code ())
+	      {
+		case TYPE_CODE_FUNC:
+		case TYPE_CODE_METHOD:
+		  ptr_type = builtin_type (this->gdbarch)->builtin_func_ptr;
+		  break;
+		default:
+		  ptr_type = builtin_type (this->gdbarch)->builtin_data_ptr;
+		  break;
+	      }
+	    address = value_as_address (value_from_pointer (ptr_type, address));
+
+	    retval = value_at_lazy (subobj_type,
+				    address + subobj_offset);
+	    if (in_stack_memory)
+	      set_value_stack (retval, 1);
+	  }
+	  break;
+
+	case DWARF_VALUE_STACK:
+	  {
+	    value *val = this->fetch (0);
+	    size_t n = TYPE_LENGTH (value_type (val));
+	    size_t len = TYPE_LENGTH (subobj_type);
+	    size_t max = TYPE_LENGTH (type);
+
+	    if (subobj_offset + len > max)
+	      invalid_synthetic_pointer ();
+
+	    retval = allocate_value (subobj_type);
+
+	    /* The given offset is relative to the actual object.  */
+	    if (gdbarch_byte_order (this->gdbarch) == BFD_ENDIAN_BIG)
+	      subobj_offset += n - max;
+
+	    memcpy (value_contents_raw (retval),
+		    value_contents_all (val) + subobj_offset, len);
+	  }
+	  break;
+
+	case DWARF_VALUE_LITERAL:
+	  {
+	    size_t n = TYPE_LENGTH (subobj_type);
+
+	    if (subobj_offset + n > this->len)
+	      invalid_synthetic_pointer ();
+
+	    retval = allocate_value (subobj_type);
+	    bfd_byte *contents = value_contents_raw (retval);
+	    memcpy (contents, this->data + subobj_offset, n);
+	  }
+	  break;
+
+	case DWARF_VALUE_OPTIMIZED_OUT:
+	  retval = allocate_optimized_out_value (subobj_type);
+	  break;
+
+	  /* DWARF_VALUE_IMPLICIT_POINTER was converted to a pieced
+	     operation by execute_stack_op.  */
+	case DWARF_VALUE_IMPLICIT_POINTER:
+	  /* DWARF_VALUE_OPTIMIZED_OUT can't occur in this context --
+	     it can only be encountered when making a piece.  */
+	default:
+	  internal_error (__FILE__, __LINE__, _("invalid location type"));
+	}
+    }
+
+  set_value_initialized (retval, this->initialized);
+
+  return retval;
+}
+
 /* Require that TYPE be an integral type; throw an exception if not.  */
 
 static void

gdb/dwarf2/expr.h

@@ -26,7 +26,6 @@
 #include "gdbtypes.h"
 
 struct dwarf2_per_objfile;
-struct piece_closure;
 
 /* The location of a value.  */
 enum dwarf_value_location
@@ -125,9 +124,13 @@ struct dwarf_expr_context
 
   void push_address (CORE_ADDR value, bool in_stack_memory);
   void eval (const gdb_byte *addr, size_t len);
-  struct value *fetch (int n);
+
-  CORE_ADDR fetch_address (int n);
+  /* Fetch the result of the expression evaluation in a form of
-  bool fetch_in_stack_memory (int n);
+     a struct value, where TYPE, SUBOBJ_TYPE and SUBOBJ_OFFSET
+     describe the source level representation of that result.  */
+  value *fetch_result (struct type *type = nullptr,
+		       struct type *subobj_type = nullptr,
+		       LONGEST subobj_offset = 0);
 
   /* The stack of values.  */
   std::vector<dwarf_stack_value> stack;
@@ -203,6 +206,9 @@ private:
   void add_piece (ULONGEST size, ULONGEST offset);
   void execute_stack_op (const gdb_byte *op_ptr, const gdb_byte *op_end);
   void pop ();
+  struct value *fetch (int n);
+  CORE_ADDR fetch_address (int n);
+  bool fetch_in_stack_memory (int n);
 
   /* Return the location expression for the frame base attribute, in
      START and LENGTH.  The result must be live until the current
@@ -301,13 +307,4 @@ extern const gdb_byte *safe_read_sleb128 (const gdb_byte *buf,
 extern const gdb_byte *safe_skip_leb128 (const gdb_byte *buf,
 					 const gdb_byte *buf_end);
 
-extern const struct lval_funcs pieced_value_funcs;
-
-/* Allocate a closure for a value formed from separately-described
-   PIECES.  */
-
-piece_closure *allocate_piece_closure
-  (dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile,
-   std::vector<dwarf_expr_piece> &&pieces, frame_info *frame);
-
 #endif /* dwarf2expr.h */

gdb/dwarf2/frame.c

@@ -229,8 +229,6 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
 		  struct frame_info *this_frame, CORE_ADDR initial,
 		  int initial_in_stack_memory, dwarf2_per_objfile *per_objfile)
 {
-  CORE_ADDR result;
-
   dwarf_expr_context ctx (per_objfile);
   scoped_value_mark free_values;
 
@@ -241,18 +239,13 @@ execute_stack_op (const gdb_byte *exp, ULONGEST len, int addr_size,
   ctx.push_address (initial, initial_in_stack_memory);
   ctx.eval (exp, len);
 
-  if (ctx.location == DWARF_VALUE_MEMORY)
+  CORE_ADDR result;
-    result = ctx.fetch_address (0);
+  struct value *result_val = ctx.fetch_result ();
-  else if (ctx.location == DWARF_VALUE_REGISTER)
+
-    result = read_addr_from_reg (this_frame, value_as_long (ctx.fetch (0)));
+  if (VALUE_LVAL (result_val) == lval_memory)
+    result = value_address (result_val);
   else
-    {
+    result = value_as_address (result_val);
-      /* This is actually invalid DWARF, but if we ever do run across
-	 it somehow, we might as well support it.  So, instead, report
-	 it as unimplemented.  */
-      error (_("\
-Not implemented: computing unwound register using explicit value operator"));
-    }
 
   return result;
 }

gdb/dwarf2/loc.c

@@ -92,7 +92,7 @@ enum debug_loc_kind
 /* Helper function which throws an error if a synthetic pointer is
    invalid.  */
 
-static void
+void
 invalid_synthetic_pointer (void)
 {
   error (_("access outside bounds of object "
@@ -1487,6 +1487,8 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
   try
     {
       ctx.eval (data, size);
+      retval = ctx.fetch_result (type, subobj_type,
+				 subobj_byte_offset);
     }
   catch (const gdb_exception_error &ex)
     {
@@ -1509,155 +1511,15 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	throw;
     }
 
-  if (ctx.pieces.size () > 0)
+  /* We need to clean up all the values that are not needed any more.
-    {
+     The problem with a value_ref_ptr class is that it disconnects the
-      struct piece_closure *c;
+     RETVAL from the value garbage collection, so we need to make
-      ULONGEST bit_size = 0;
+     a copy of that value on the stack to keep everything consistent.
+     The value_ref_ptr will clean up after itself at the end of this block.  */
+  value_ref_ptr value_holder = value_ref_ptr::new_reference (retval);
+  free_values.free_to_mark ();
 
-      for (dwarf_expr_piece &piece : ctx.pieces)
+  return value_copy (retval);
-	bit_size += piece.size;
-      /* Complain if the expression is larger than the size of the
-	 outer type.  */
-      if (bit_size > 8 * TYPE_LENGTH (type))
-	invalid_synthetic_pointer ();
-
-      c = allocate_piece_closure (per_cu, per_objfile, std::move (ctx.pieces),
-				  frame);
-      /* We must clean up the value chain after creating the piece
-	 closure but before allocating the result.  */
-      free_values.free_to_mark ();
-      retval = allocate_computed_value (subobj_type,
-					&pieced_value_funcs, c);
-      set_value_offset (retval, subobj_byte_offset);
-    }
-  else
-    {
-      switch (ctx.location)
-	{
-	case DWARF_VALUE_REGISTER:
-	  {
-	    struct gdbarch *arch = get_frame_arch (frame);
-	    int dwarf_regnum
-	      = longest_to_int (value_as_long (ctx.fetch (0)));
-	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, dwarf_regnum);
-
-	    if (subobj_byte_offset != 0)
-	      error (_("cannot use offset on synthetic pointer to register"));
-	    free_values.free_to_mark ();
-	    retval = value_from_register (subobj_type, gdb_regnum, frame);
-	    if (value_optimized_out (retval))
-	      {
-		struct value *tmp;
-
-		/* This means the register has undefined value / was
-		   not saved.  As we're computing the location of some
-		   variable etc. in the program, not a value for
-		   inspecting a register ($pc, $sp, etc.), return a
-		   generic optimized out value instead, so that we show
-		   <optimized out> instead of <not saved>.  */
-		tmp = allocate_value (subobj_type);
-		value_contents_copy (tmp, 0, retval, 0,
-				     TYPE_LENGTH (subobj_type));
-		retval = tmp;
-	      }
-	  }
-	  break;
-
-	case DWARF_VALUE_MEMORY:
-	  {
-	    struct type *ptr_type;
-	    CORE_ADDR address = ctx.fetch_address (0);
-	    bool in_stack_memory = ctx.fetch_in_stack_memory (0);
-
-	    /* DW_OP_deref_size (and possibly other operations too) may
-	       create a pointer instead of an address.  Ideally, the
-	       pointer to address conversion would be performed as part
-	       of those operations, but the type of the object to
-	       which the address refers is not known at the time of
-	       the operation.  Therefore, we do the conversion here
-	       since the type is readily available.  */
-
-	    switch (subobj_type->code ())
-	      {
-		case TYPE_CODE_FUNC:
-		case TYPE_CODE_METHOD:
-		  ptr_type = builtin_type (ctx.gdbarch)->builtin_func_ptr;
-		  break;
-		default:
-		  ptr_type = builtin_type (ctx.gdbarch)->builtin_data_ptr;
-		  break;
-	      }
-	    address = value_as_address (value_from_pointer (ptr_type, address));
-
-	    free_values.free_to_mark ();
-	    retval = value_at_lazy (subobj_type,
-				    address + subobj_byte_offset);
-	    if (in_stack_memory)
-	      set_value_stack (retval, 1);
-	  }
-	  break;
-
-	case DWARF_VALUE_STACK:
-	  {
-	    struct value *value = ctx.fetch (0);
-	    size_t n = TYPE_LENGTH (value_type (value));
-	    size_t len = TYPE_LENGTH (subobj_type);
-	    size_t max = TYPE_LENGTH (type);
-	    gdbarch *objfile_gdbarch = per_objfile->objfile->arch ();
-
-	    if (subobj_byte_offset + len > max)
-	      invalid_synthetic_pointer ();
-
-	    /* Preserve VALUE because we are going to free values back
-	       to the mark, but we still need the value contents
-	       below.  */
-	    value_ref_ptr value_holder = value_ref_ptr::new_reference (value);
-	    free_values.free_to_mark ();
-
-	    retval = allocate_value (subobj_type);
-
-	    /* The given offset is relative to the actual object.  */
-	    if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
-	      subobj_byte_offset += n - max;
-
-	    memcpy (value_contents_raw (retval),
-		    value_contents_all (value) + subobj_byte_offset, len);
-	  }
-	  break;
-
-	case DWARF_VALUE_LITERAL:
-	  {
-	    bfd_byte *contents;
-	    size_t n = TYPE_LENGTH (subobj_type);
-
-	    if (subobj_byte_offset + n > ctx.len)
-	      invalid_synthetic_pointer ();
-
-	    free_values.free_to_mark ();
-	    retval = allocate_value (subobj_type);
-	    contents = value_contents_raw (retval);
-	    memcpy (contents, ctx.data + subobj_byte_offset, n);
-	  }
-	  break;
-
-	case DWARF_VALUE_OPTIMIZED_OUT:
-	  free_values.free_to_mark ();
-	  retval = allocate_optimized_out_value (subobj_type);
-	  break;
-
-	  /* DWARF_VALUE_IMPLICIT_POINTER was converted to a pieced
-	     operation by execute_stack_op.  */
-	case DWARF_VALUE_IMPLICIT_POINTER:
-	  /* DWARF_VALUE_OPTIMIZED_OUT can't occur in this context --
-	     it can only be encountered when making a piece.  */
-	default:
-	  internal_error (__FILE__, __LINE__, _("invalid location type"));
-	}
-    }
-
-  set_value_initialized (retval, ctx.initialized);
-
-  return retval;
 }
 
 /* The exported interface to dwarf2_evaluate_loc_desc_full; it always
@@ -1698,6 +1560,9 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   dwarf2_per_objfile *per_objfile = dlbaton->per_objfile;
   dwarf_expr_context ctx (per_objfile);
 
+  struct value *result;
+  scoped_value_mark free_values;
+
   ctx.frame = frame;
   ctx.per_cu = dlbaton->per_cu;
   if (addr_stack != nullptr)
@@ -1715,6 +1580,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   try
     {
       ctx.eval (dlbaton->data, dlbaton->size);
+      result = ctx.fetch_result ();
     }
   catch (const gdb_exception_error &ex)
     {
@@ -1732,29 +1598,20 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
 	throw;
     }
 
-  switch (ctx.location)
+  if (value_optimized_out (result))
-    {
+    return 0;
-    case DWARF_VALUE_STACK:
-      *is_reference = false;
-      /* FALLTHROUGH */
 
-    case DWARF_VALUE_REGISTER:
+  if (VALUE_LVAL (result) == lval_memory)
-    case DWARF_VALUE_MEMORY:
+    *valp = value_address (result);
-      *valp = ctx.fetch_address (0);
+  else
-      if (ctx.location == DWARF_VALUE_REGISTER)
+    {
-	*valp = read_addr_from_reg (frame, *valp);
+      if (VALUE_LVAL (result) == not_lval)
-      return 1;
+	*is_reference = false;
-    case DWARF_VALUE_LITERAL:
+
-      *valp = extract_signed_integer (ctx.data, ctx.len,
+      *valp = value_as_address (result);
-				      gdbarch_byte_order (ctx.gdbarch));
-      return 1;
-      /* Unsupported dwarf values.  */
-    case DWARF_VALUE_OPTIMIZED_OUT:
-    case DWARF_VALUE_IMPLICIT_POINTER:
-      break;
     }
 
-  return 0;
+  return 1;
 }
 
 /* See dwarf2loc.h.  */

gdb/dwarf2/loc.h

@@ -280,6 +280,11 @@ extern int dwarf_reg_to_regnum (struct gdbarch *arch, int dwarf_reg);
 extern int dwarf_reg_to_regnum_or_error (struct gdbarch *arch,
 					 ULONGEST dwarf_reg);
 
+/* Helper function which throws an error if a synthetic pointer is
+   invalid.  */
+
+extern void invalid_synthetic_pointer ();
+
 /* Fetch the value pointed to by a synthetic pointer.  */
 
 extern struct value *indirect_synthetic_pointer