[binutils-gdb] Prefer existing data when evaluating DWARF expression

[binutils-gdb] Prefer existing data when evaluating DWARF expression

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b249d2c2c01775fb015b38b272389b8693e414f6

commit b249d2c2c01775fb015b38b272389b8693e414f6
Author: Tom Tromey <tromey@adacore.com>
Date:   Fri Apr 24 13:40:31 2020 -0600

    Prefer existing data when evaluating DWARF expression
    
    When evaluating a DWARF expression, the dynamic type resolution code
    will pass in a buffer of bytes via the property_addr_info.  However,
    the DWARF expression evaluator will then proceed to read memory from
    the inferior, even when the request could be filled from this buffer.
    
    This, in turn, is a problem in some cases; and specifically when
    trying to handle the Ada scenario of extracting a variable-length
    value from a packed array.  Here, the ordinary DWARF expression cannot
    be directly evaluated, because the data may appear at some arbitrary
    bit offset.  So, it is unpacked into a staging area and then the
    expression is evaluated -- using an address of 0.
    
    This patch fixes the problem by arranging for the DWARF evaluator, in
    this case, to prefer passed-in memory when possible.  The type of the
    buffer in the property_addr_info is changed to an array_view so that
    bounds checking can be done.
    
    gdb/ChangeLog
    2020-04-24  Tom Tromey  <tromey@adacore.com>
    
            * ada-lang.c (ada_discrete_type_high_bound, ada_discrete_type_low)
            (ada_value_primitive_packed_val): Update.
            * ada-valprint.c (ada_value_print_1): Update.
            * dwarf2/loc.c (evaluate_for_locexpr_baton): New struct.
            (dwarf2_locexpr_baton_eval): Take a property_addr_info rather than
            just an address.  Use evaluate_for_locexpr_baton.
            (dwarf2_evaluate_property): Update.
            * dwarf2/loc.h (struct property_addr_info) <valaddr>: Now an
            array_view.
            * findvar.c (default_read_var_value): Update.
            * gdbtypes.c (compute_variant_fields_inner)
            (resolve_dynamic_type_internal): Update.
            (resolve_dynamic_type): Change type of valaddr parameter.
            * gdbtypes.h (resolve_dynamic_type): Update.
            * valarith.c (value_subscripted_rvalue): Update.
            * value.c (value_from_contents_and_address): Update.

Diff:
---
 gdb/ChangeLog      | 19 ++++++++++++++
 gdb/ada-lang.c     |  6 ++---
 gdb/ada-valprint.c |  4 ++-
 gdb/dwarf2/loc.c   | 73 +++++++++++++++++++++++++++++++++++++++++++-----------
 gdb/dwarf2/loc.h   |  2 +-
 gdb/findvar.c      |  6 ++---
 gdb/gdbtypes.c     | 15 ++++++-----
 gdb/gdbtypes.h     |  6 ++---
 gdb/valarith.c     |  2 +-
 gdb/value.c        |  5 +++-
 10 files changed, 104 insertions(+), 34 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5db26a60947..305b133b4fb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2020-04-24  Tom Tromey  <tromey@adacore.com>
+
+	* ada-lang.c (ada_discrete_type_high_bound, ada_discrete_type_low)
+	(ada_value_primitive_packed_val): Update.
+	* ada-valprint.c (ada_value_print_1): Update.
+	* dwarf2/loc.c (evaluate_for_locexpr_baton): New struct.
+	(dwarf2_locexpr_baton_eval): Take a property_addr_info rather than
+	just an address.  Use evaluate_for_locexpr_baton.
+	(dwarf2_evaluate_property): Update.
+	* dwarf2/loc.h (struct property_addr_info) <valaddr>: Now an
+	array_view.
+	* findvar.c (default_read_var_value): Update.
+	* gdbtypes.c (compute_variant_fields_inner)
+	(resolve_dynamic_type_internal): Update.
+	(resolve_dynamic_type): Change type of valaddr parameter.
+	* gdbtypes.h (resolve_dynamic_type): Update.
+	* valarith.c (value_subscripted_rvalue): Update.
+	* value.c (value_from_contents_and_address): Update.
+
 2020-04-24  Tom Tromey  <tromey@adacore.com>
 
 	* dwarf2/loc.c (dwarf2_locexpr_baton_eval): Add
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 49f2280198e..bfbc69084ec 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -752,7 +752,7 @@ min_of_type (struct type *t)
 LONGEST
 ada_discrete_type_high_bound (struct type *type)
 {
-  type = resolve_dynamic_type (type, NULL, 0);
+  type = resolve_dynamic_type (type, {}, 0);
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_RANGE:
@@ -773,7 +773,7 @@ ada_discrete_type_high_bound (struct type *type)
 LONGEST
 ada_discrete_type_low_bound (struct type *type)
 {
-  type = resolve_dynamic_type (type, NULL, 0);
+  type = resolve_dynamic_type (type, {}, 0);
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_RANGE:
@@ -2508,7 +2508,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
 			        staging.data (), staging.size (),
 				is_big_endian, has_negatives (type),
 				is_scalar);
-      type = resolve_dynamic_type (type, staging.data (), 0);
+      type = resolve_dynamic_type (type, staging, 0);
       if (TYPE_LENGTH (type) < (bit_size + HOST_CHAR_BIT - 1) / HOST_CHAR_BIT)
 	{
 	  /* This happens when the length of the object is dynamic,
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 2768829cdb3..474b0799910 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -1057,7 +1057,9 @@ ada_value_print_1 (struct value *val, struct ui_file *stream, int recurse,
 
   const gdb_byte *valaddr = value_contents_for_printing (val);
   CORE_ADDR address = value_address (val);
-  type = ada_check_typedef (resolve_dynamic_type (type, valaddr, address));
+  gdb::array_view<const gdb_byte> view
+    = gdb::make_array_view (valaddr, TYPE_LENGTH (type));
+  type = ada_check_typedef (resolve_dynamic_type (type, view, address));
   if (type != saved_type)
     {
       val = value_copy (val);
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 8df655f660a..5b690ca9276 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2384,18 +2384,56 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
 					NULL, 0);
 }
 
-/* Evaluates a dwarf expression and stores the result in VAL, expecting
-   that the dwarf expression only produces a single CORE_ADDR.  FRAME is the
-   frame in which the expression is evaluated.  ADDR is a context (location of
-   a variable) and might be needed to evaluate the location expression.
-   PUSH_INITIAL_VALUE is true if ADDR should be pushed on the stack
-   before evaluating the expression;  this is required by certain
-   forms of DWARF expression.  Returns 1 on success, 0 otherwise.  */
+/* A specialization of dwarf_evaluate_loc_desc that is used by
+   dwarf2_locexpr_baton_eval.  This subclass exists to handle the case
+   where a caller of dwarf2_locexpr_baton_eval passes in some data,
+   but with the address being 0.  In this situation, we arrange for
+   memory reads to come from the passed-in buffer.  */
+
+struct evaluate_for_locexpr_baton : public dwarf_evaluate_loc_desc
+{
+  /* The data that was passed in.  */
+  gdb::array_view<const gdb_byte> data_view;
+
+  CORE_ADDR get_object_address () override
+  {
+    if (data_view.data () == nullptr && obj_address == 0)
+      error (_("Location address is not set."));
+    return obj_address;
+  }
+
+  void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t len) override
+  {
+    if (len == 0)
+      return;
+
+    /* Prefer the passed-in memory, if it exists.  */
+    CORE_ADDR offset = addr - obj_address;
+    if (offset < data_view.size () && offset + len <= data_view.size ())
+      {
+	memcpy (buf, data_view.data (), len);
+	return;
+      }
+
+    read_memory (addr, buf, len);
+  }
+};
+
+/* Evaluates a dwarf expression and stores the result in VAL,
+   expecting that the dwarf expression only produces a single
+   CORE_ADDR.  FRAME is the frame in which the expression is
+   evaluated.  ADDR_STACK is a context (location of a variable) and
+   might be needed to evaluate the location expression.
+   PUSH_INITIAL_VALUE is true if the address (either from ADDR_STACK,
+   or the default of 0) should be pushed on the DWARF expression
+   evaluation stack before evaluating the expression; this is required
+   by certain forms of DWARF expression.  Returns 1 on success, 0
+   otherwise.  */
 
 static int
 dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
 			   struct frame_info *frame,
-			   CORE_ADDR addr,
+			   const struct property_addr_info *addr_stack,
 			   CORE_ADDR *valp,
 			   bool push_initial_value)
 {
@@ -2404,11 +2442,17 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   if (dlbaton == NULL || dlbaton->size == 0)
     return 0;
 
-  dwarf_evaluate_loc_desc ctx;
+  evaluate_for_locexpr_baton ctx;
 
   ctx.frame = frame;
   ctx.per_cu = dlbaton->per_cu;
-  ctx.obj_address = addr;
+  if (addr_stack == nullptr)
+    ctx.obj_address = 0;
+  else
+    {
+      ctx.obj_address = addr_stack->addr;
+      ctx.data_view = addr_stack->valaddr;
+    }
 
   objfile = dlbaton->per_cu->objfile ();
 
@@ -2418,7 +2462,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   ctx.offset = dlbaton->per_cu->text_offset ();
 
   if (push_initial_value)
-    ctx.push_address (addr, false);
+    ctx.push_address (ctx.obj_address, false);
 
   try
     {
@@ -2485,8 +2529,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	  = (const struct dwarf2_property_baton *) prop->data.baton;
 	gdb_assert (baton->property_type != NULL);
 
-	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
-				       addr_stack ? addr_stack->addr : 0,
+	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame, addr_stack,
 				       value, push_initial_value))
 	  {
 	    if (baton->locexpr.is_reference)
@@ -2569,10 +2612,10 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	  }
 	if (pinfo == NULL)
 	  error (_("cannot find reference address for offset property"));
-	if (pinfo->valaddr != NULL)
+	if (pinfo->valaddr.data () != NULL)
 	  val = value_from_contents
 		  (baton->offset_info.type,
-		   pinfo->valaddr + baton->offset_info.offset);
+		   pinfo->valaddr.data () + baton->offset_info.offset);
 	else
 	  val = value_at (baton->offset_info.type,
 			  pinfo->addr + baton->offset_info.offset);
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 6ff9b79dc03..9815368d625 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -73,7 +73,7 @@ struct property_addr_info
   struct type *type;
 
   /* If not NULL, a buffer containing the object's value.  */
-  const gdb_byte *valaddr;
+  gdb::array_view<const gdb_byte> valaddr;
 
   /* The address of that object.  */
   CORE_ADDR addr;
diff --git a/gdb/findvar.c b/gdb/findvar.c
index a836c63dc5d..ac4f5c3997d 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -615,7 +615,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
       if (is_dynamic_type (type))
 	{
 	  /* Value is a constant byte-sequence and needs no memory access.  */
-	  type = resolve_dynamic_type (type, NULL, /* Unused address.  */ 0);
+	  type = resolve_dynamic_type (type, {}, /* Unused address.  */ 0);
 	}
       /* Put the constant back in target format. */
       v = allocate_value (type);
@@ -647,7 +647,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
       if (is_dynamic_type (type))
 	{
 	  /* Value is a constant byte-sequence and needs no memory access.  */
-	  type = resolve_dynamic_type (type, NULL, /* Unused address.  */ 0);
+	  type = resolve_dynamic_type (type, {}, /* Unused address.  */ 0);
 	}
       v = allocate_value (type);
       memcpy (value_contents_raw (v), SYMBOL_VALUE_BYTES (var),
@@ -788,7 +788,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
 
     case LOC_OPTIMIZED_OUT:
       if (is_dynamic_type (type))
-	type = resolve_dynamic_type (type, NULL, /* Unused address.  */ 0);
+	type = resolve_dynamic_type (type, {}, /* Unused address.  */ 0);
       return allocate_optimized_out_value (type);
 
     default:
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c2a1d221377..1bd9d8a55c7 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2298,8 +2298,9 @@ compute_variant_fields_inner (struct type *type,
 	error (_("Cannot determine struct field location"
 		 " (invalid location kind)"));
 
-      if (addr_stack->valaddr != NULL)
-	discr_value = unpack_field_as_long (type, addr_stack->valaddr, idx);
+      if (addr_stack->valaddr.data () != NULL)
+	discr_value = unpack_field_as_long (type, addr_stack->valaddr.data (),
+					    idx);
       else
 	{
 	  CORE_ADDR addr = (addr_stack->addr
@@ -2516,9 +2517,10 @@ resolve_dynamic_type_internal (struct type *type,
 	    struct property_addr_info pinfo;
 
 	    pinfo.type = check_typedef (TYPE_TARGET_TYPE (type));
-	    pinfo.valaddr = NULL;
-	    if (addr_stack->valaddr != NULL)
-	      pinfo.addr = extract_typed_address (addr_stack->valaddr, type);
+	    pinfo.valaddr = {};
+	    if (addr_stack->valaddr.data () != NULL)
+	      pinfo.addr = extract_typed_address (addr_stack->valaddr.data (),
+						  type);
 	    else
 	      pinfo.addr = read_memory_typed_address (addr_stack->addr, type);
 	    pinfo.next = addr_stack;
@@ -2566,7 +2568,8 @@ resolve_dynamic_type_internal (struct type *type,
 /* See gdbtypes.h  */
 
 struct type *
-resolve_dynamic_type (struct type *type, const gdb_byte *valaddr,
+resolve_dynamic_type (struct type *type,
+		      gdb::array_view<const gdb_byte> valaddr,
 		      CORE_ADDR addr)
 {
   struct property_addr_info pinfo
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 8b4da6e3f9f..f686e5407ba 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2140,9 +2140,9 @@ extern void get_signed_type_minmax (struct type *, LONGEST *, LONGEST *);
    ADDR specifies the location of the variable the type is bound to.
    If TYPE has no dynamic properties return TYPE; otherwise a new type with
    static properties is returned.  */
-extern struct type *resolve_dynamic_type (struct type *type,
-					  const gdb_byte *valaddr,
-					  CORE_ADDR addr);
+extern struct type *resolve_dynamic_type
+  (struct type *type, gdb::array_view<const gdb_byte> valaddr,
+   CORE_ADDR addr);
 
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 07cb5014bb2..504264b1d82 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -220,7 +220,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, LONGEST lowerbound
       CORE_ADDR address;
 
       address = value_address (array) + elt_offs;
-      elt_type = resolve_dynamic_type (elt_type, NULL, address);
+      elt_type = resolve_dynamic_type (elt_type, {}, address);
     }
 
   return value_from_component (array, elt_type, elt_offs);
diff --git a/gdb/value.c b/gdb/value.c
index 5ae0b32f4fc..7ea39af5551 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3477,7 +3477,10 @@ value_from_contents_and_address (struct type *type,
 				 const gdb_byte *valaddr,
 				 CORE_ADDR address)
 {
-  struct type *resolved_type = resolve_dynamic_type (type, valaddr, address);
+  gdb::array_view<const gdb_byte> view;
+  if (valaddr != nullptr)
+    view = gdb::make_array_view (valaddr, TYPE_LENGTH (type));
+  struct type *resolved_type = resolve_dynamic_type (type, view, address);
   struct type *resolved_type_no_typedef = check_typedef (resolved_type);
   struct value *v;