RE: [Patch v2 02/10] type: add c99 variable length array support

["Agovic, Sanimir" <sanimir dot agovic at intel dot com>]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Friday, November 22, 2013 09:37 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org; Boell, Keven
> Subject: Re: [Patch v2 02/10] type: add c99 variable length array support
> 
> 
> Sanimir> +struct type *
> Sanimir> +resolve_dynamic_type (struct type *type, CORE_ADDR address)
> Sanimir> +{
> [...]
> Sanimir> +  /* Make a deep copy of the type.  */
> Sanimir> +  copied_types = create_copied_types_hash (TYPE_OBJFILE (type));
> Sanimir> +  cleanup = make_cleanup_htab_delete (copied_types);
> Sanimir> +  resolved_type = copy_type_recursive
> Sanimir> +    (TYPE_OBJFILE (type), type, copied_types);
> 
> A couple problems here actually.
> 
> copy_type_recursive is actually not very general purpose.
> 
> It allocates temporary data on the objfile obstack, because it "knows"
> it is only called when the objfile is being destroyed and so some extra
> allocations there are unimportant.  (This is of course fixable.)
> 
> It doesn't copy everything -- it doesn't preserve some C++ data.
> 
> Also it deep copies the things it does copy, so including things like
> field names.
> 
> On the whole this seems pretty expensive.  Don't you just need to copy a
> top-level array type?  If so maybe copy_type would work better.
> 
Thanks for pointing out.

In the prototype below I omit any call to copy_type_recursive, it works by
selective re-constructing array and range types. As c99 vla are quite restrictive
this works, but languages with less restrictive vla implementation
(see Fortran vla branch) need to deep copy the type. Comments are welcome.

 -Sanimir

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 7951060..fdfd12f 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1635,52 +1635,74 @@ is_dynamic_type (const struct type *type)
    ADDRESS might be needed to resolve the subrange bounds, it is the location
    of the associated array.  */
 
-static void
-resolve_dynamic_bounds (struct type *type, CORE_ADDR address)
+static struct type *
+resolve_dynamic_bounds (struct type *type, CORE_ADDR addr)
 {
-  struct type *real_type;
-  const struct dynamic_prop *prop;
   CORE_ADDR value;
+  struct type *array_type;
+  struct type *range_type;
+  struct type *ary_dim;
+  const struct dynamic_prop *prop;
+  const struct dwarf2_locexpr_baton *baton;
+  struct dynamic_prop low_bound, high_bound;
 
-  gdb_assert (TYPE_CODE (type) != TYPE_CODE_TYPEDEF);
+  if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF
+      || TYPE_CODE (type) == TYPE_CODE_PTR)
+    {
+      struct type *copy = copy_type (type);
 
-  if (TYPE_CODE (type) == TYPE_CODE_PTR
-      || TYPE_CODE (type) == TYPE_CODE_REF)
-    resolve_dynamic_bounds (check_typedef (TYPE_TARGET_TYPE (type)), address);
+      TYPE_TARGET_TYPE (copy)
+	= resolve_dynamic_bounds (TYPE_TARGET_TYPE (copy), addr);
+
+      return copy;
+    }
+
+  gdb_assert (TYPE_CODE (type) == TYPE_CODE_ARRAY);
+
+  array_type = check_typedef (type);
+  range_type = check_typedef (TYPE_INDEX_TYPE (array_type));
+
+  prop = &TYPE_RANGE_DATA (range_type)->low;
+  if (dwarf2_evaluate_property (prop, addr, &value))
+    {
+      low_bound.kind = PROP_CONST;
+      low_bound.data.const_val = value;
+    }
   else
+    low_bound = *prop;
+
+  prop = &TYPE_RANGE_DATA (range_type)->high;
+  if (dwarf2_evaluate_property (prop, addr, &value))
     {
-      if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
-	{
-	  struct type *ary_dim = type;
-
-	  do {
-	    struct type *range_type = check_typedef (TYPE_INDEX_TYPE (ary_dim));
-
-	    prop = &TYPE_RANGE_DATA (range_type)->low;
-	    if (dwarf2_evaluate_property (prop, address, &value))
-	      {
-		TYPE_LOW_BOUND (range_type) = value;
-		TYPE_LOW_BOUND_KIND (range_type) = PROP_CONST;
-	      }
-
-	    prop = &TYPE_RANGE_DATA (range_type)->high;
-	    if (dwarf2_evaluate_property (prop, address, &value))
-	      {
-		TYPE_HIGH_BOUND (range_type) = value;
-		TYPE_HIGH_BOUND_KIND (range_type) = PROP_CONST;
-	      }
-
-	    ary_dim = check_typedef (TYPE_TARGET_TYPE (ary_dim));
-	  } while (ary_dim != NULL && TYPE_CODE (ary_dim) == TYPE_CODE_ARRAY);
-	}
+      high_bound.kind = PROP_CONST;
+      high_bound.data.const_val = value;
     }
+  else
+    high_bound = *prop;
+
+  ary_dim = check_typedef (TYPE_TARGET_TYPE (array_type));
+
+  if (ary_dim != NULL && TYPE_CODE (ary_dim) == TYPE_CODE_ARRAY)
+    array_type = resolve_dynamic_bounds (TYPE_TARGET_TYPE (type), addr);
+  else
+    array_type = TYPE_TARGET_TYPE (type);
+
+  range_type
+    = create_range_type_1 (NULL,
+			   TYPE_TARGET_TYPE (range_type),
+			   &low_bound, &high_bound);
+  array_type = create_array_type (copy_type (type),
+				  array_type,
+				  range_type);
 
   prop = TYPE_DATA_LOCATION (type);
-  if (dwarf2_evaluate_property (prop, address, &value))
+  if (dwarf2_evaluate_property (prop, addr, &value))
     {
-      TYPE_DATA_LOCATION_ADDR (type) = value;
-      TYPE_DATA_LOCATION_KIND (type) = PROP_CONST;
+      TYPE_DATA_LOCATION_ADDR (array_type) = value;
+      TYPE_DATA_LOCATION_KIND (array_type) = PROP_CONST;
     }
+
+  return array_type;
 }
 
 /* See gdbtypes.h  */
@@ -1699,17 +1721,8 @@ resolve_dynamic_type (struct type *type, CORE_ADDR addr)
   if (!is_dynamic_type (real_type))
     return type;
 
-  /* Make a deep copy of the type.  */
-  copied_types = create_copied_types_hash (TYPE_OBJFILE (type));
-  cleanup = make_cleanup_htab_delete (copied_types);
-  resolved_type = copy_type_recursive
-    (TYPE_OBJFILE (type), type, copied_types);
-  do_cleanups (cleanup);
-
-  real_type = check_typedef (resolved_type);
-  resolve_dynamic_bounds (real_type, addr);
-
-  resolved_type->length = get_type_length (real_type);
+  resolved_type = resolve_dynamic_bounds (type, addr);
+  resolved_type->length = get_type_length (check_typedef (resolved_type));
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052