Make DWARF evaluator return a single struct value
authorZoran Zaric <zoran.zaric@amd.com>
Tue, 15 Sep 2020 15:52:11 +0000 (16:52 +0100)
committerZoran Zaric <zoran.zaric@amd.com>
Thu, 5 Aug 2021 15:40:47 +0000 (16:40 +0100)
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
gdb/dwarf2/expr.h
gdb/dwarf2/frame.c
gdb/dwarf2/loc.c
gdb/dwarf2/loc.h

index d0a74f1a586a3502271103ba21d3d04082940d45..5b4b734ccd2daf9d4392de6e86413146ce39df86 100644 (file)
@@ -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
index c9ba2a6325d127edd2f9b2bd677e77c673b05ff0..4b94951c3a5f0c3996d27f20e499aca065bcf744 100644 (file)
@@ -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);
-  bool fetch_in_stack_memory (int n);
+
+  /* Fetch the result of the expression evaluation in a form of
+     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 */
index c1219d997ec75f74964f3c5875e67c07d814c811..c773034c3057513973b3d12ec6b31073e1b4b9b9 100644 (file)
@@ -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)
-    result = ctx.fetch_address (0);
-  else if (ctx.location == DWARF_VALUE_REGISTER)
-    result = read_addr_from_reg (this_frame, value_as_long (ctx.fetch (0)));
+  CORE_ADDR result;
+  struct value *result_val = ctx.fetch_result ();
+
+  if (VALUE_LVAL (result_val) == lval_memory)
+    result = value_address (result_val);
   else
-    {
-      /* 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"));
-    }
+    result = value_as_address (result_val);
 
   return result;
 }
index 4e1316dcbd4afaa932ffe8172513c59826317e27..894cda238a98bd794b54cb8493b8f01001af48f2 100644 (file)
@@ -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)
-    {
-      struct piece_closure *c;
-      ULONGEST bit_size = 0;
-
-      for (dwarf_expr_piece &piece : ctx.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 ();
-
-      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);
+  /* 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
+     RETVAL from the value garbage collection, so we need to make
+     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 ();
 
-  return retval;
+  return value_copy (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;
+
+  if (VALUE_LVAL (result) == lval_memory)
+    *valp = value_address (result);
+  else
     {
-    case DWARF_VALUE_STACK:
-      *is_reference = false;
-      /* FALLTHROUGH */
-
-    case DWARF_VALUE_REGISTER:
-    case DWARF_VALUE_MEMORY:
-      *valp = ctx.fetch_address (0);
-      if (ctx.location == DWARF_VALUE_REGISTER)
-       *valp = read_addr_from_reg (frame, *valp);
-      return 1;
-    case DWARF_VALUE_LITERAL:
-      *valp = extract_signed_integer (ctx.data, ctx.len,
-                                     gdbarch_byte_order (ctx.gdbarch));
-      return 1;
-      /* Unsupported dwarf values.  */
-    case DWARF_VALUE_OPTIMIZED_OUT:
-    case DWARF_VALUE_IMPLICIT_POINTER:
-      break;
+      if (VALUE_LVAL (result) == not_lval)
+       *is_reference = false;
+
+      *valp = value_as_address (result);
     }
 
-  return 0;
+  return 1;
 }
 
 /* See dwarf2loc.h.  */
index 33a5e8adc109309b3fe45d54ad9d73ad68404a0d..5d964b07a5035143fc8278d66a90a675a552360e 100644 (file)
@@ -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