[Patch v2 00/10] C99 variable length array support

[Patch v2 00/10] C99 variable length array support

[Sanimir Agovic]

Hello,

this patch series (v2) add C99 variable length support to gdb.

It allows the user to evaluate a vla like an ordinary static array e.g. print
its elements instead of printing the pointer to the array. In addition the size
of a vla can be retrieved with gdbs builtin sizeof operator.


    1| void foo (size_t n) {
    2|   int ary[n];
    3|   memset(ary, 0, sizeof(ary));
    4| }

    (gdb) print ary
    $1 = {0 <repeats 42 times>}

    (gdb) print sizeof ary
    $2 = 168

Some technical background
=========================

Dwarf allows certain attributes e.g upper/lower bound to be computed
dynamically. To support this feature with the current gdb type-system types
are "normalized". This means types with dynamic properties are converted
to types with static properties.

To convert a type with dynamic properties into one with static properties
access to inferior memory is needed. Therefore we hooked into the following
value constructors value_at/value_at_lazy/value_from_contents_and_address
as they require an inferior address in addition to a type to instantiate
a value. IIF the passed type has dynamic properties we resolve the bounds
and thus the type is modified, not in place but rather by a new copy.

Given the following code snippet:

  struct value *val = value_at (my_vla_type, at_address);

Before this was always true:
  TYPE_LENGTH (value_type (val)) == TYPE_LENGTH (my_vla_type)

This is not the case after applying this patch series. Type normalization
is done in the mentioned value constructors and might change the value type.

Some documentation, examples as well as a github branch with support for c99
and Fortran variable length arrays is availabel at  http://intel-gdb.github.io/

Changes in v1:
 - Removed patch 05/10: allow side effects for sizeof argument
 - New patch: support for DW_AT_count
 - Replaced undefined_low, high with DWARF_UNDEFINED
 - Add varobj testcase to mi test
 - Fixed commit log
 - Various small issues mentioned in patches 01/10 and 02/10

 -Sanimir & Keven

Sanimir Agovic (10):
  vla: introduce new bound type abstraction adapt uses
  type: add c99 variable length array support
  vla: enable sizeof operator to work with variable length arrays
  vla: enable sizeof operator for indirection
  vla: update type from newly created value
  vla: print "dynamic length" for unresolved dynamic bounds
  test: multi-dimensional c99 vla.
  test: evaluate pointers to C99 vla correctly.
  test: basic c99 vla tests
  test: add mi vla test

 gdb/ada-lang.c                           |   13 +-
 gdb/c-typeprint.c                        |    5 +-
 gdb/cp-valprint.c                        |    2 +
 gdb/d-valprint.c                         |    1 +
 gdb/dwarf2loc.c                          |   60 ++++++-
 gdb/dwarf2loc.h                          |    7 +
 gdb/dwarf2read.c                         |  152 ++++++++++----
 gdb/eval.c                               |   10 +-
 gdb/gdbtypes.c                           |  322 ++++++++++++++++++++++++------
 gdb/gdbtypes.h                           |   59 ++++--
 gdb/jv-valprint.c                        |    1 +
 gdb/parse.c                              |    3 +-
 gdb/testsuite/gdb.base/vla-datatypes.c   |   86 ++++++++
 gdb/testsuite/gdb.base/vla-datatypes.exp |   82 ++++++++
 gdb/testsuite/gdb.base/vla-multi.c       |   55 +++++
 gdb/testsuite/gdb.base/vla-multi.exp     |   44 ++++
 gdb/testsuite/gdb.base/vla-ptr.c         |   63 ++++++
 gdb/testsuite/gdb.base/vla-ptr.exp       |   49 +++++
 gdb/testsuite/gdb.mi/mi-vla-c99.exp      |   80 ++++++++
 gdb/testsuite/gdb.mi/vla.c               |   35 ++++
 gdb/valops.c                             |    3 +
 gdb/value.c                              |   11 +-
 22 files changed, 1013 insertions(+), 130 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/vla-datatypes.c
 create mode 100644 gdb/testsuite/gdb.base/vla-datatypes.exp
 create mode 100644 gdb/testsuite/gdb.base/vla-multi.c
 create mode 100644 gdb/testsuite/gdb.base/vla-multi.exp
 create mode 100644 gdb/testsuite/gdb.base/vla-ptr.c
 create mode 100644 gdb/testsuite/gdb.base/vla-ptr.exp
 create mode 100644 gdb/testsuite/gdb.mi/mi-vla-c99.exp
 create mode 100644 gdb/testsuite/gdb.mi/vla.c

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

[Sanimir Agovic]

The dwarf standard allow certain attributes to be expressed as dwarf
expressions rather than constants. For instance upper-/lowerbound attributes.
In case of a c99 variable length array the upperbound is a dynamic attribute.

With this change c99 vla behave the same as with static arrays.

1| void foo (size_t n) {
2|   int ary[n];
3|   memset(ary, 0, sizeof(ary));
4| }

(gdb) print ary
$1 = {0 <repeats 42 times>}

2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
            Keven Boell  <keven.boell@intel.com>

	* dwarf2loc.c (dwarf2_locexpr_baton_eval): New function.
	* dwarf2loc.h (dwarf2_locexpr_baton_eval): New function prototype.
	* dwarf2read.c (block_to_locexprbaton): New function.
	(attr_to_dwarf2_prop): New function.
	(read_subrange_type): Use attr_to_dwarf2_prop to read high bound
	attribute.
	* gdbtypes.c: Include dwarf2loc.h.
	(resolve_dynamic_prop): New function.
	(is_dynamic_type): New function.
	(resolve_dynamic_values_1): New function.
	(resolve_dynamic_type): New function.
	(get_type_length): New function.
	(check_typedef): Use get_type_length to compute type length.
	* gdbtypes.h (TYPE_HIGH_BOUND_KIND): Define.
	(TYPE_LOW_BOUND_KIND): Define.

Change-Id: I97b91b9ad0b8ac8d30294dae6bd0dd4c905713ce
---
 gdb/dwarf2loc.c  |   60 ++++++++++++-
 gdb/dwarf2loc.h  |    7 ++
 gdb/dwarf2read.c |  115 +++++++++++++++++------
 gdb/gdbtypes.c   |  276 +++++++++++++++++++++++++++++++++++++++++++----------
 gdb/gdbtypes.h   |    3 +
 gdb/value.c      |   10 +-
 6 files changed, 387 insertions(+), 84 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 2d15546..57a11aa 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1,4 +1,4 @@
-/* DWARF 2 location expression support for GDB.
+/* DWARF location expression support for GDB.
 
    Copyright (C) 2003-2013 Free Software Foundation, Inc.
 
@@ -2430,6 +2430,64 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
   return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu, 0);
 }
 
+/* See dwarf2loc.h.  */
+
+int
+dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
+			   CORE_ADDR addr, CORE_ADDR *valp)
+{
+  struct dwarf_expr_context *ctx;
+  struct dwarf_expr_baton baton;
+  struct objfile *objfile;
+  struct cleanup *cleanup;
+
+  if (dlbaton == NULL || dlbaton->size == 0)
+    return 0;
+
+  ctx = new_dwarf_expr_context ();
+  cleanup = make_cleanup_free_dwarf_expr_context (ctx);
+
+  baton.frame = get_selected_frame (NULL);
+  baton.per_cu = dlbaton->per_cu;
+
+  objfile = dwarf2_per_cu_objfile (dlbaton->per_cu);
+
+  ctx->gdbarch = get_objfile_arch (objfile);
+  ctx->addr_size = dwarf2_per_cu_addr_size (dlbaton->per_cu);
+  ctx->ref_addr_size = dwarf2_per_cu_ref_addr_size (dlbaton->per_cu);
+  ctx->offset = dwarf2_per_cu_text_offset (dlbaton->per_cu);
+  ctx->funcs = &dwarf_expr_ctx_funcs;
+  ctx->baton = &baton;
+
+  dwarf_expr_eval (ctx, dlbaton->data, dlbaton->size);
+
+  switch (ctx->location)
+    {
+    case DWARF_VALUE_REGISTER:
+    case DWARF_VALUE_MEMORY:
+    case DWARF_VALUE_STACK:
+      *valp = dwarf_expr_fetch_address (ctx, 0);
+      if (ctx->location == DWARF_VALUE_REGISTER)
+	*valp = dwarf_expr_read_addr_from_reg (&baton, *valp);
+      do_cleanups (cleanup);
+      return 1;
+    case DWARF_VALUE_LITERAL:
+      *valp = extract_signed_integer (ctx->data, ctx->len,
+				      gdbarch_byte_order (ctx->gdbarch));
+      do_cleanups (cleanup);
+      return 1;
+      /* Not supported dwarf values.  */
+    case DWARF_VALUE_OPTIMIZED_OUT:
+    case DWARF_VALUE_IMPLICIT_POINTER:
+      break;
+    }
+
+  do_cleanups (cleanup);
+
+  return 0;
+}
+
+
 \f
 /* Helper functions and baton for dwarf2_loc_desc_needs_frame.  */
 
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index 9bc8ca5..83c8044 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -90,6 +90,13 @@ struct value *dwarf2_evaluate_loc_desc (struct type *type,
 					size_t size,
 					struct dwarf2_per_cu_data *per_cu);
 
+/* Evaluate a dwarf expression and stores the result in VAL, expecting
+   that the dwarf expression only produces a single CORE_ADDR.
+   Returns 1 on success, 0 otherwise.   */
+
+int dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
+			       CORE_ADDR addr, CORE_ADDR *value);
+
 CORE_ADDR dwarf2_read_addr_index (struct dwarf2_per_cu_data *per_cu,
 				  unsigned int addr_index);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index e3ec91f..7f31211 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14252,6 +14252,93 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   return set_die_type (die, type, cu);
 }
 
+/* Turn Dwarf block into location expression baton structure. Used to store
+   baton into "dynamic" types, e.g. VLA's.  Specifing ADDITIONAL_DATA and
+   EXTRA_SIZE allows to append additional opcodes to the dwarf expression.  */
+
+static struct dwarf2_locexpr_baton*
+block_to_locexprbaton (const struct dwarf_block *blk, struct dwarf2_cu *cu,
+		       const gdb_byte *additional_data, int extra_size)
+{
+  struct obstack *obstack = &cu->objfile->objfile_obstack;
+  struct dwarf2_locexpr_baton *baton;
+
+  gdb_assert (blk != NULL && obstack != NULL);
+
+  baton = obstack_alloc (obstack, sizeof (struct dwarf2_locexpr_baton));
+  baton->per_cu = cu->per_cu;
+  baton->size = blk->size + extra_size;
+
+  if (additional_data != NULL && extra_size > 0)
+    {
+      gdb_byte *data;
+
+      data = obstack_alloc (obstack, baton->size);
+      baton->data = data;
+
+      memcpy (data, blk->data, blk->size);
+      memcpy (data + blk->size, additional_data, extra_size);
+    }
+  else
+    /* Copy the data pointer as the block's lifetime is bound to its
+       object file. */
+    baton->data = blk->data;
+
+  gdb_assert (baton->data != NULL);
+
+  return baton;
+}
+
+/* Parse dwarf attribute if it's a block, reference or constant and put the
+   resulting value of the attribute into struct dwarf2_prop.
+   Returns 1 if ATTR could be resolved into PROP, 0 otherwise.  */
+
+static int
+attr_to_dwarf2_prop (const struct attribute *attr, struct die_info *die,
+		     struct dwarf2_cu *cu, struct dwarf2_prop *prop)
+{
+  if (die == NULL || attr == NULL || cu == NULL || prop == NULL)
+    return 0;
+
+  if (attr_form_is_block (attr))
+    {
+      prop->data.locexpr = block_to_locexprbaton (DW_BLOCK (attr), cu, NULL, 0);
+      prop->kind = DWARF_LOCEXPR;
+      gdb_assert (prop->data.locexpr != NULL);
+    }
+  else if (attr_form_is_ref (attr))
+    {
+      struct dwarf2_cu *target_cu = cu;
+      struct die_info *target_die;
+      struct attribute *target_attr;
+      const gdb_byte append_ops[] = { DW_OP_deref };
+
+      target_die = follow_die_ref (die, attr, &target_cu);
+      target_attr = dwarf2_attr (target_die, DW_AT_location, target_cu);
+      if (target_attr == NULL || !attr_form_is_block (target_attr))
+	return 0;
+
+      prop->data.locexpr =
+	block_to_locexprbaton (DW_BLOCK (target_attr), cu, append_ops,
+			       sizeof (append_ops) / sizeof (append_ops[0]));
+      prop->kind = DWARF_LOCEXPR;
+      gdb_assert (prop->data.locexpr != NULL);
+    }
+  else if (attr_form_is_constant (attr))
+    {
+      prop->data.const_val = dwarf2_get_attr_constant_value (attr, 0);
+      prop->kind = DWARF_CONST;
+    }
+  else
+    {
+      dwarf2_invalid_attrib_class_complaint (dwarf_form_name (attr->form),
+					    dwarf2_name (die, cu));
+      return 0;
+    }
+
+  return 1;
+}
+
 /* Read the given DW_AT_subrange DIE.  */
 
 static struct type *
@@ -14325,27 +14412,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 	       die->offset.sect_off, objfile_name (cu->objfile));
 
   attr = dwarf2_attr (die, DW_AT_upper_bound, cu);
-  if (attr)
-    {
-      if (attr_form_is_block (attr) || attr_form_is_ref (attr))
-        {
-          /* GCC encodes arrays with unspecified or dynamic length
-             with a DW_FORM_block1 attribute or a reference attribute.
-             FIXME: GDB does not yet know how to handle dynamic
-             arrays properly, treat them as arrays with unspecified
-             length for now.
-
-             FIXME: jimb/2003-09-22: GDB does not really know
-             how to handle arrays of unspecified length
-             either; we just represent them as zero-length
-             arrays.  Choose an appropriate upper bound given
-             the lower bound we've computed above.  */
-          high.data.const_val = low.data.const_val - 1;
-        }
-      else
-        high.data.const_val = dwarf2_get_attr_constant_value (attr, 1);
-    }
-  else
+  if (!attr_to_dwarf2_prop (attr, die, cu, &high))
     {
       attr = dwarf2_attr (die, DW_AT_count, cu);
       if (attr)
@@ -14409,12 +14476,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 
   range_type = create_range_type_1 (NULL, orig_base_type, &low, &high);
 
-  /* Mark arrays with dynamic length at least as an array of unspecified
-     length.  GDB could check the boundary but before it gets implemented at
-     least allow accessing the array elements.  */
-  if (attr && attr_form_is_block (attr))
-    TYPE_HIGH_BOUND_KIND (range_type) = DWARF_UNDEFINED;
-
   /* Ada expects an empty array on no boundary attributes.  */
   if (attr == NULL && cu->language != language_ada)
     TYPE_HIGH_BOUND_KIND (range_type) = DWARF_UNDEFINED;
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 612f74e..59246f1 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -38,6 +38,7 @@
 #include "hashtab.h"
 #include "exceptions.h"
 #include "cp-support.h"
+#include "dwarf2loc.h"
 
 /* Initialize BADNESS constants.  */
 
@@ -851,6 +852,17 @@ create_range_type (struct type *result_type, struct type *index_type,
   return result_type;
 }
 
+/* Predicate tests whether BOUNDS are static. Returns 1 if all bounds values
+   are static, otherwise returns 0.  */
+
+static int
+has_static_range (const struct range_bounds *bounds)
+{
+  return (bounds->low.kind == DWARF_CONST
+	  && bounds->high.kind == DWARF_CONST);
+}
+
+
 /* Set *LOWP and *HIGHP to the lower and upper bounds of discrete type
    TYPE.  Return 1 if type is a range type, 0 if it is discrete (and
    bounds will fit in LONGEST), or -1 otherwise.  */
@@ -980,24 +992,28 @@ create_array_type (struct type *result_type,
 		   struct type *element_type,
 		   struct type *range_type)
 {
-  LONGEST low_bound, high_bound;
-
   if (result_type == NULL)
     result_type = alloc_type_copy (range_type);
 
   TYPE_CODE (result_type) = TYPE_CODE_ARRAY;
   TYPE_TARGET_TYPE (result_type) = element_type;
-  if (get_discrete_bounds (range_type, &low_bound, &high_bound) < 0)
-    low_bound = high_bound = 0;
-  CHECK_TYPEDEF (element_type);
-  /* Be careful when setting the array length.  Ada arrays can be
-     empty arrays with the high_bound being smaller than the low_bound.
-     In such cases, the array length should be zero.  */
-  if (high_bound < low_bound)
-    TYPE_LENGTH (result_type) = 0;
-  else
-    TYPE_LENGTH (result_type) =
-      TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
+
+  if (has_static_range (TYPE_RANGE_DATA (range_type)))
+    {
+      LONGEST low_bound, high_bound;
+
+      if (get_discrete_bounds (range_type, &low_bound, &high_bound) < 0)
+	low_bound = high_bound = 0;
+      CHECK_TYPEDEF (element_type);
+      /* Be careful when setting the array length.  Ada arrays can be
+	 empty arrays with the high_bound being smaller than the low_bound.
+	 In such cases, the array length should be zero.  */
+      if (high_bound < low_bound)
+	TYPE_LENGTH (result_type) = 0;
+      else
+	TYPE_LENGTH (result_type) =
+	  TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
+    }
   TYPE_NFIELDS (result_type) = 1;
   TYPE_FIELDS (result_type) =
     (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
@@ -1528,7 +1544,192 @@ stub_noname_complaint (void)
   complaint (&symfile_complaints, _("stub type has NULL name"));
 }
 
-/* Find the real type of TYPE.  This function returns the real type,
+/* Calculates the size of a type. If TYPE has static bound values takes upper
+   and lower bound into account, otherwise only the TYPE length is returned.
+   TYPE is expected not to be a typedef.  */
+
+static ULONGEST
+get_type_length (const struct type *type)
+{
+  const struct type *range_type, *target_type;
+  ULONGEST len = TYPE_LENGTH (type);
+  LONGEST low_bound, high_bound;
+
+  gdb_assert (TYPE_CODE (type) != TYPE_CODE_TYPEDEF);
+
+  if (TYPE_CODE (type) != TYPE_CODE_ARRAY
+      && TYPE_CODE (type) != TYPE_CODE_STRING)
+    return len;
+
+  range_type = check_typedef (TYPE_INDEX_TYPE (type));
+
+  if (!has_static_range (TYPE_RANGE_DATA (range_type)))
+    return len;
+
+  target_type = check_typedef (TYPE_TARGET_TYPE (type));
+
+  /* Now recompute the length of the array type, based on its
+     number of elements and the target type's length.
+     Watch out for Ada null Ada arrays where the high bound
+     is smaller than the low bound.  */
+  low_bound = TYPE_LOW_BOUND (range_type);
+  high_bound = TYPE_HIGH_BOUND (range_type);
+
+  if (high_bound < low_bound)
+    len = 0;
+  else
+    {
+      /* For now, we conservatively take the array length to be 0
+         if its length exceeds UINT_MAX.  The code below assumes
+         that for x < 0, (ULONGEST) x == -x + ULONGEST_MAX + 1,
+         which is technically not guaranteed by C, but is usually true
+         (because it would be true if x were unsigned with its
+         high-order bit on).  It uses the fact that
+         high_bound-low_bound is always representable in
+         ULONGEST and that if high_bound-low_bound+1 overflows,
+         it overflows to 0.  We must change these tests if we
+         decide to increase the representation of TYPE_LENGTH
+         from unsigned int to ULONGEST.  */
+      ULONGEST ulow = low_bound, uhigh = high_bound;
+      ULONGEST tlen = get_type_length (target_type);
+
+      len = tlen * (uhigh - ulow + 1);
+      if (tlen == 0 || (len / tlen - 1 + ulow) != uhigh || len > UINT_MAX)
+        len = 0;
+    }
+
+  return len;
+}
+
+/* Converts a dynamic value into a static one. Returns 1 if PROP could be
+   converted and the static value is passed back into VALUE, otherwise
+   returns 0.  */
+
+static int
+resolve_dynamic_prop (const struct dwarf2_prop *prop, CORE_ADDR address,
+		      CORE_ADDR *value)
+{
+  if (prop == NULL)
+    return 0;
+
+  switch (prop->kind)
+    {
+    case DWARF_LOCEXPR:
+      {
+	const struct dwarf2_locexpr_baton *baton = prop->data.locexpr;
+
+	return dwarf2_locexpr_baton_eval (baton, address, value);
+      }
+    case DWARF_CONST:
+      break;
+    }
+
+  return 0;
+}
+
+/* Predicates if the type has dynamic values, which are not resolved yet.  */
+
+static int
+is_dynamic_type (const struct type *type)
+{
+  if (type == NULL)
+    return 0;
+
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
+      && TYPE_NFIELDS (type) == 1)
+    {
+      const struct type *range_type = TYPE_INDEX_TYPE (type);
+
+      if (!has_static_range (TYPE_RANGE_DATA (range_type)))
+	return 1;
+    }
+
+  if (TYPE_CODE (type) == TYPE_CODE_PTR
+      || TYPE_CODE (type) == TYPE_CODE_REF
+      || TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
+    return is_dynamic_type (TYPE_TARGET_TYPE (type));
+
+  return 0;
+}
+
+/* Resolves dynamic bound values of an array type to static ones.
+   TYPE is modified in place and is expected not to be a typedef.  */
+
+static void
+resolve_dynamic_bounds (struct type *type, CORE_ADDR address)
+{
+  struct type *real_type;
+  const struct dwarf2_prop *prop;
+  CORE_ADDR value;
+
+  gdb_assert (TYPE_CODE (type) != TYPE_CODE_TYPEDEF);
+
+  if (TYPE_CODE (type) == TYPE_CODE_PTR
+      || TYPE_CODE (type) == TYPE_CODE_REF)
+    resolve_dynamic_bounds (check_typedef (TYPE_TARGET_TYPE (type)), address);
+  else
+    {
+      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 (resolve_dynamic_prop (prop, address, &value))
+	      {
+		TYPE_LOW_BOUND (range_type) = value;
+		TYPE_LOW_BOUND_KIND (range_type) = DWARF_CONST;
+	      }
+
+	    prop = &TYPE_RANGE_DATA (range_type)->high;
+	    if (resolve_dynamic_prop (prop, address, &value))
+	      {
+		TYPE_HIGH_BOUND (range_type) = value;
+		TYPE_HIGH_BOUND_KIND (range_type) = DWARF_CONST;
+	      }
+
+	    ary_dim = check_typedef (TYPE_TARGET_TYPE (ary_dim));
+	  } while (ary_dim != NULL && TYPE_CODE (ary_dim) == TYPE_CODE_ARRAY);
+	}
+    }
+}
+
+/* Resolves all dynamic values of a type e.g. array bounds to static values.
+   If TYPE has no dynamic values returns TYPE otherwise a new type with static
+   values is returned.  */
+
+struct type *
+resolve_dynamic_type (struct type *type, CORE_ADDR address)
+{
+  struct type *real_type = check_typedef (type);
+  struct type *resolved_type;
+  struct cleanup *cleanup;
+  htab_t copied_types;
+
+  if (!TYPE_OBJFILE_OWNED (real_type))
+    return type;
+
+  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, address);
+
+  resolved_type->length = get_type_length (real_type);
+
+  return resolved_type;
+}
+
+/* find the real type of TYPE.  This function returns the real type,
    after removing all layers of typedefs, and completing opaque or stub
    types.  Completion changes the TYPE argument, but stripping of
    typedefs does not.
@@ -1704,44 +1905,15 @@ check_typedef (struct type *type)
 	  /* Nothing we can do.  */
 	}
       else if (TYPE_CODE (type) == TYPE_CODE_ARRAY
-	       && TYPE_NFIELDS (type) == 1
-	       && (TYPE_CODE (range_type = TYPE_INDEX_TYPE (type))
-		   == TYPE_CODE_RANGE))
-	{
-	  /* Now recompute the length of the array type, based on its
-	     number of elements and the target type's length.
-	     Watch out for Ada null Ada arrays where the high bound
-	     is smaller than the low bound.  */
-	  const LONGEST low_bound = TYPE_LOW_BOUND (range_type);
-	  const LONGEST high_bound = TYPE_HIGH_BOUND (range_type);
-	  ULONGEST len;
-
-	  if (high_bound < low_bound)
-	    len = 0;
-	  else
-	    {
-	      /* For now, we conservatively take the array length to be 0
-		 if its length exceeds UINT_MAX.  The code below assumes
-		 that for x < 0, (ULONGEST) x == -x + ULONGEST_MAX + 1,
-		 which is technically not guaranteed by C, but is usually true
-		 (because it would be true if x were unsigned with its
-		 high-order bit on).  It uses the fact that
-		 high_bound-low_bound is always representable in
-		 ULONGEST and that if high_bound-low_bound+1 overflows,
-		 it overflows to 0.  We must change these tests if we 
-		 decide to increase the representation of TYPE_LENGTH
-		 from unsigned int to ULONGEST.  */
-	      ULONGEST ulow = low_bound, uhigh = high_bound;
-	      ULONGEST tlen = TYPE_LENGTH (target_type);
-
-	      len = tlen * (uhigh - ulow + 1);
-	      if (tlen == 0 || (len / tlen - 1 + ulow) != uhigh 
-		  || len > UINT_MAX)
-		len = 0;
-	    }
-	  TYPE_LENGTH (type) = len;
-	  TYPE_TARGET_STUB (type) = 0;
-	}
+	       || TYPE_CODE (type) == TYPE_CODE_STRING)
+        {
+          range_type = TYPE_INDEX_TYPE (type);
+          if (has_static_range (TYPE_RANGE_DATA (range_type)))
+            {
+              TYPE_LENGTH (type) = get_type_length (type);
+              TYPE_TARGET_STUB (type) = 0;
+            }
+        }
       else if (TYPE_CODE (type) == TYPE_CODE_RANGE)
 	{
 	  TYPE_LENGTH (type) = TYPE_LENGTH (target_type);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index d01f75e..5500e94 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1094,6 +1094,7 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_LOW_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->low.kind
 
+
 /* Moto-specific stuff for FORTRAN arrays.  */
 
 #define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \
@@ -1571,6 +1572,8 @@ extern struct type *lookup_unsigned_typename (const struct language_defn *,
 extern struct type *lookup_signed_typename (const struct language_defn *,
 					    struct gdbarch *, const char *);
 
+extern struct type *resolve_dynamic_type (struct type *, CORE_ADDR);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
diff --git a/gdb/value.c b/gdb/value.c
index 8c263ea..bd4d6aa 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3182,9 +3182,10 @@ value_from_ulongest (struct type *type, ULONGEST num)
 struct value *
 value_from_pointer (struct type *type, CORE_ADDR addr)
 {
-  struct value *val = allocate_value (type);
+  struct type *resolved_type = resolve_dynamic_type (type, addr);
+  struct value *val = allocate_value (resolved_type);
 
-  store_typed_address (value_contents_raw (val), check_typedef (type), addr);
+  store_typed_address (value_contents_raw (val), check_typedef (resolved_type), addr);
   return val;
 }
 
@@ -3198,12 +3199,13 @@ value_from_contents_and_address (struct type *type,
 				 const gdb_byte *valaddr,
 				 CORE_ADDR address)
 {
+  struct type *resolved_type = resolve_dynamic_type (type, address);
   struct value *v;
 
   if (valaddr == NULL)
-    v = allocate_value_lazy (type);
+    v = allocate_value_lazy (resolved_type);
   else
-    v = value_from_contents (type, valaddr);
+    v = value_from_contents (resolved_type, valaddr);
   set_value_address (v, address);
   VALUE_LVAL (v) = lval_memory;
   return v;
-- 
1.7.0.7

[Patch v2 01/10] vla: introduce new bound type abstraction adapt uses

[Sanimir Agovic]

The rational behind this patch is to get started to implement the feature
described in dwarf4 standard (2.19) Static and Dynamic Values of Attributes.
It adds new DWARF2_PROP to store either a constant, exprloc, or reference to
describe an upper-/lower bound of a subrange. Other than that no new features
are introduce.

2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
            Keven Boell  <keven.boell@intel.com>

	* dwarf2read.c (read_subrange_type): Use struct dwarf2_prop for
	declaring high/low bounds and change uses accordingly. Call
	create_range_type_1 instead of create_range_type,
	* gdbtypes.c (create_range_type_1): New function.
	(create_range_type): Convert bounds into struct dwarf2_prop and pass
	them to create_range_type_1.
	* gdbtypes.h (struct dwarf2_prop): New struct.
	(create_range_type_1): New function prototype.
	(struct range_bounds): Use struct dwarf2_prop instead of LONGEST for
	high/low bounds. Remove low_undefined/high_undefined and adapt all uses.
	(TYPE_LOW_BOUND,TYPE_HIGH_BOUND): Adapt macros to refer to the static
	part of the bound.
	* parse.c (follow_types): Set high bound kind to DWARF_UNDEFINED.

Change-Id: I04ccd6b9bbf7519b99e814e9ee7119dbcd1f7baa
---
 gdb/dwarf2read.c |   43 +++++++++++++++++++++++++------------------
 gdb/gdbtypes.c   |   46 ++++++++++++++++++++++++++++++++++------------
 gdb/gdbtypes.h   |   54 ++++++++++++++++++++++++++++++++++++++++--------------
 gdb/parse.c      |    3 ++-
 4 files changed, 101 insertions(+), 45 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1c7dfc5..e3ec91f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14260,7 +14260,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *base_type, *orig_base_type;
   struct type *range_type;
   struct attribute *attr;
-  LONGEST low, high;
+  struct dwarf2_prop low, high;
   int low_default_is_valid;
   const char *name;
   LONGEST negative_mask;
@@ -14277,33 +14277,37 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   if (range_type)
     return range_type;
 
+  low.kind = DWARF_CONST;
+  high.kind = DWARF_CONST;
+  high.data.const_val = 0;
+
   /* Set LOW_DEFAULT_IS_VALID if current language and DWARF version allow
      omitting DW_AT_lower_bound.  */
   switch (cu->language)
     {
     case language_c:
     case language_cplus:
-      low = 0;
+      low.data.const_val = 0;
       low_default_is_valid = 1;
       break;
     case language_fortran:
-      low = 1;
+      low.data.const_val = 1;
       low_default_is_valid = 1;
       break;
     case language_d:
     case language_java:
     case language_objc:
-      low = 0;
+      low.data.const_val = 0;
       low_default_is_valid = (cu->header.version >= 4);
       break;
     case language_ada:
     case language_m2:
     case language_pascal:
-      low = 1;
+      low.data.const_val = 1;
       low_default_is_valid = (cu->header.version >= 4);
       break;
     default:
-      low = 0;
+      low.data.const_val = 0;
       low_default_is_valid = 0;
       break;
     }
@@ -14313,7 +14317,8 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
      but we don't know how to handle it.  */
   attr = dwarf2_attr (die, DW_AT_lower_bound, cu);
   if (attr)
-    low = dwarf2_get_attr_constant_value (attr, low);
+    low.data.const_val =
+      dwarf2_get_attr_constant_value (attr, low.data.const_val);
   else if (!low_default_is_valid)
     complaint (&symfile_complaints, _("Missing DW_AT_lower_bound "
 				      "- DIE at 0x%x [in module %s]"),
@@ -14335,10 +14340,10 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
              either; we just represent them as zero-length
              arrays.  Choose an appropriate upper bound given
              the lower bound we've computed above.  */
-          high = low - 1;
+          high.data.const_val = low.data.const_val - 1;
         }
       else
-        high = dwarf2_get_attr_constant_value (attr, 1);
+        high.data.const_val = dwarf2_get_attr_constant_value (attr, 1);
     }
   else
     {
@@ -14346,12 +14351,12 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
       if (attr)
 	{
 	  int count = dwarf2_get_attr_constant_value (attr, 1);
-	  high = low + count - 1;
+	  high.data.const_val = low.data.const_val + count - 1;
 	}
       else
 	{
 	  /* Unspecified array length.  */
-	  high = low - 1;
+	  high.data.const_val = low.data.const_val - 1;
 	}
     }
 
@@ -14395,22 +14400,24 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 
   negative_mask =
     (LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1);
-  if (!TYPE_UNSIGNED (base_type) && (low & negative_mask))
-    low |= negative_mask;
-  if (!TYPE_UNSIGNED (base_type) && (high & negative_mask))
-    high |= negative_mask;
+  if (low.kind == DWARF_CONST
+      && !TYPE_UNSIGNED (base_type) && (low.data.const_val & negative_mask))
+    low.data.const_val |= negative_mask;
+  if (high.kind == DWARF_CONST
+      && !TYPE_UNSIGNED (base_type) && (high.data.const_val & negative_mask))
+    high.data.const_val |= negative_mask;
 
-  range_type = create_range_type (NULL, orig_base_type, low, high);
+  range_type = create_range_type_1 (NULL, orig_base_type, &low, &high);
 
   /* Mark arrays with dynamic length at least as an array of unspecified
      length.  GDB could check the boundary but before it gets implemented at
      least allow accessing the array elements.  */
   if (attr && attr_form_is_block (attr))
-    TYPE_HIGH_BOUND_UNDEFINED (range_type) = 1;
+    TYPE_HIGH_BOUND_KIND (range_type) = DWARF_UNDEFINED;
 
   /* Ada expects an empty array on no boundary attributes.  */
   if (attr == NULL && cu->language != language_ada)
-    TYPE_HIGH_BOUND_UNDEFINED (range_type) = 1;
+    TYPE_HIGH_BOUND_KIND (range_type) = DWARF_UNDEFINED;
 
   name = dwarf2_name (die, cu);
   if (name)
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index e18a0d2..612f74e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -796,6 +796,30 @@ allocate_stub_method (struct type *type)
   return mtype;
 }
 
+/* Create a range type with a dynamic range from LOW_BOUND to
+   HIGH_BOUND, inclusive. See create_range_type for further details. */
+
+struct type *
+create_range_type_1 (struct type *result_type, struct type *index_type,
+		     const struct dwarf2_prop *low_bound,
+		     const struct dwarf2_prop *high_bound)
+{
+  if (result_type == NULL)
+    result_type = alloc_type_copy (index_type);
+  TYPE_CODE (result_type) = TYPE_CODE_RANGE;
+  TYPE_TARGET_TYPE (result_type) = index_type;
+  if (TYPE_STUB (index_type))
+    TYPE_TARGET_STUB (result_type) = 1;
+  else
+    TYPE_LENGTH (result_type) = TYPE_LENGTH (check_typedef (index_type));
+  TYPE_RANGE_DATA (result_type) = (struct range_bounds *)
+    TYPE_ZALLOC (result_type, sizeof (struct range_bounds));
+  TYPE_RANGE_DATA (result_type)->low = *low_bound;
+  TYPE_RANGE_DATA (result_type)->high = *high_bound;
+
+  return result_type;
+}
+
 /* Create a range type using either a blank type supplied in
    RESULT_TYPE, or creating a new type, inheriting the objfile from
    INDEX_TYPE.
@@ -810,18 +834,16 @@ struct type *
 create_range_type (struct type *result_type, struct type *index_type,
 		   LONGEST low_bound, LONGEST high_bound)
 {
-  if (result_type == NULL)
-    result_type = alloc_type_copy (index_type);
-  TYPE_CODE (result_type) = TYPE_CODE_RANGE;
-  TYPE_TARGET_TYPE (result_type) = index_type;
-  if (TYPE_STUB (index_type))
-    TYPE_TARGET_STUB (result_type) = 1;
-  else
-    TYPE_LENGTH (result_type) = TYPE_LENGTH (check_typedef (index_type));
-  TYPE_RANGE_DATA (result_type) = (struct range_bounds *)
-    TYPE_ZALLOC (result_type, sizeof (struct range_bounds));
-  TYPE_LOW_BOUND (result_type) = low_bound;
-  TYPE_HIGH_BOUND (result_type) = high_bound;
+  struct dwarf2_prop low, high;
+
+  low.kind = DWARF_CONST;
+  low.data.const_val = low_bound;
+
+  high.kind = DWARF_CONST;
+  high.data.const_val = high_bound;
+
+  result_type = create_range_type_1 (result_type, index_type,
+				     &low, &high);
 
   if (low_bound >= 0)
     TYPE_UNSIGNED (result_type) = 1;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index d7fdedf..d01f75e 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -365,6 +365,29 @@ enum type_instance_flag_value
 #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
 				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 
+/* Used to store bound information for a type.  */
+
+struct dwarf2_prop
+{
+  /* Determine which field of the union dwarf2_prop.data is used.  */
+  enum
+  {
+    DWARF_UNDEFINED,
+    DWARF_CONST,
+    DWARF_LOCEXPR
+  } kind;
+
+  /* Stores information as location expression, location list,
+     or constant value.  */
+  union data
+  {
+    LONGEST const_val;
+    struct dwarf2_locexpr_baton *locexpr;
+    struct dwarf2_loclist_baton *loclist;
+  } data;
+};
+
+
 /* Determine which field of the union main_type.fields[x].loc is used.  */
 
 enum field_loc_kind
@@ -589,19 +612,11 @@ struct main_type
     {
       /* Low bound of range.  */
 
-      LONGEST low;
+      struct dwarf2_prop low;
 
       /* High bound of range.  */
 
-      LONGEST high;
-
-      /* Flags indicating whether the values of low and high are
-         valid.  When true, the respective range value is
-         undefined.  Currently used only for FORTRAN arrays.  */
-           
-      char low_undefined;
-      char high_undefined;
-
+      struct dwarf2_prop high;
     } *bounds;
 
   } flds_bnds;
@@ -1066,12 +1081,18 @@ extern void allocate_gnat_aux_type (struct type *);
 
 #define TYPE_INDEX_TYPE(type) TYPE_FIELD_TYPE (type, 0)
 #define TYPE_RANGE_DATA(thistype) TYPE_MAIN_TYPE(thistype)->flds_bnds.bounds
-#define TYPE_LOW_BOUND(range_type) TYPE_RANGE_DATA(range_type)->low
-#define TYPE_HIGH_BOUND(range_type) TYPE_RANGE_DATA(range_type)->high
+#define TYPE_LOW_BOUND(range_type) \
+  TYPE_RANGE_DATA(range_type)->low.data.const_val
+#define TYPE_HIGH_BOUND(range_type) \
+  TYPE_RANGE_DATA(range_type)->high.data.const_val
 #define TYPE_LOW_BOUND_UNDEFINED(range_type) \
-   TYPE_RANGE_DATA(range_type)->low_undefined
+  (TYPE_RANGE_DATA(range_type)->low.kind == DWARF_UNDEFINED)
 #define TYPE_HIGH_BOUND_UNDEFINED(range_type) \
-   TYPE_RANGE_DATA(range_type)->high_undefined
+  (TYPE_RANGE_DATA(range_type)->high.kind == DWARF_UNDEFINED)
+#define TYPE_HIGH_BOUND_KIND(range_type) \
+  TYPE_RANGE_DATA(range_type)->high.kind
+#define TYPE_LOW_BOUND_KIND(range_type) \
+  TYPE_RANGE_DATA(range_type)->low.kind
 
 /* Moto-specific stuff for FORTRAN arrays.  */
 
@@ -1526,6 +1547,11 @@ extern struct type *lookup_function_type_with_arguments (struct type *,
 							 int,
 							 struct type **);
 
+extern struct type *create_range_type_1 (struct type *, struct type *,
+					 const struct dwarf2_prop *,
+					 const struct dwarf2_prop *);
+
+
 extern struct type *create_range_type (struct type *, struct type *, LONGEST,
 				       LONGEST);
 
diff --git a/gdb/parse.c b/gdb/parse.c
index 4b9ca5d..c5150a0 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -1710,7 +1710,8 @@ follow_types (struct type *follow_type)
 	  lookup_array_range_type (follow_type,
 				   0, array_size >= 0 ? array_size - 1 : 0);
 	if (array_size < 0)
-	  TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (follow_type) = 1;
+	  TYPE_HIGH_BOUND_KIND (TYPE_INDEX_TYPE (follow_type))
+	    = DWARF_UNDEFINED;
 	break;
       case tp_function:
 	/* FIXME-type-allocation: need a way to free this type when we are
-- 
1.7.0.7

[Patch v2 03/10] vla: enable sizeof operator to work with variable length arrays

[Sanimir Agovic]

In C99 the sizeof operator computes the size of a variable length array
at runtime (6.5.3.4 The sizeof operator). This patch reflects the semantic
change in the debugger.

We now are able to get the size of a vla:

1| void foo (size_t n) {
2|   int vla[n];
3| }

(gdb) p sizeof(vla)

yields N * sizeof(int).

2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
            Keven Boell  <keven.boell@intel.com>

	* eval.c (evaluate_subexp_for_sizeof): If the type passed to sizeof is
	dynamic	evaluate the argument to compute the length.
	* gdbtypes.c (is_dynamic_type): Make extern.
	* gdbtypes.c (is_dynamic_type): Make extern.

Change-Id: I490f695f7a164be5f1f824fe3ba33f805d1ab689
---
 gdb/eval.c     |    8 +++++++-
 gdb/gdbtypes.c |    2 +-
 gdb/gdbtypes.h |    2 ++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 9d81a92..a81e789 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -3041,8 +3041,14 @@ evaluate_subexp_for_sizeof (struct expression *exp, int *pos)
       return value_from_longest (size_type, (LONGEST) TYPE_LENGTH (type));
 
     case OP_VAR_VALUE:
-      (*pos) += 4;
       type = check_typedef (SYMBOL_TYPE (exp->elts[pc + 2].symbol));
+      if (is_dynamic_type (type))
+	{
+	  val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);
+	  type = value_type (val);
+	}
+      else
+	(*pos) += 4;
       return
 	value_from_longest (size_type, (LONGEST) TYPE_LENGTH (type));
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 59246f1..ac83ffe 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1629,7 +1629,7 @@ resolve_dynamic_prop (const struct dwarf2_prop *prop, CORE_ADDR address,
 
 /* Predicates if the type has dynamic values, which are not resolved yet.  */
 
-static int
+int
 is_dynamic_type (const struct type *type)
 {
   if (type == NULL)
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 5500e94..9d24d78 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1572,6 +1572,8 @@ extern struct type *lookup_unsigned_typename (const struct language_defn *,
 extern struct type *lookup_signed_typename (const struct language_defn *,
 					    struct gdbarch *, const char *);
 
+extern int is_dynamic_type (const struct type *type);
+
 extern struct type *resolve_dynamic_type (struct type *, CORE_ADDR);
 
 extern struct type *check_typedef (struct type *);
-- 
1.7.0.7

[Patch v2 05/10] vla: update type from newly created value

[Sanimir Agovic]

Constructing a value based on a type and address might change the type
of the newly constructed value. Thus re-fetch type via value_type to ensure
we have the correct type at hand.

2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
            Keven Boell  <keven.boell@intel.com>

	* ada-lang.c (ada_value_primitive_packed_val): Re-fetch type from value.
	(ada_template_to_fixed_record_type_1): Likewise.
	(ada_to_fixed_type_1): Likewise.
	* cp-valprint.c (cp_print_value_fields_rtti): Likewise.
	(cp_print_value): Likewise.
	* d-valprint.c (dynamic_array_type): Likewise.
	* jv-valprint.c (java_value_print): Likewise.
	* valops.c (value_ind): Likewise.
	* value.c (coerce_ref): Likewise.

Change-Id: Ice9ae63215f637c4b99babf0f108e7b5b58988cd
---
 gdb/ada-lang.c    |   13 +++++++++++--
 gdb/cp-valprint.c |    2 ++
 gdb/d-valprint.c  |    1 +
 gdb/jv-valprint.c |    1 +
 gdb/valops.c      |    3 +++
 gdb/value.c       |    1 +
 6 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 417232c..7e1706b 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -2300,6 +2300,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
   else if (VALUE_LVAL (obj) == lval_memory && value_lazy (obj))
     {
       v = value_at (type, value_address (obj));
+      type = value_type (v);
       bytes = (unsigned char *) alloca (len);
       read_memory (value_address (v) + offset, bytes, len);
     }
@@ -7657,6 +7658,7 @@ ada_template_to_fixed_record_type_1 (struct type *type,
 		 size first before creating the value.  */
 	      check_size (rtype);
 	      dval = value_from_contents_and_address (rtype, valaddr, address);
+	      rtype = value_type (dval);
 	    }
           else
             dval = dval0;
@@ -7759,7 +7761,10 @@ ada_template_to_fixed_record_type_1 (struct type *type,
       off = TYPE_FIELD_BITPOS (rtype, variant_field);
 
       if (dval0 == NULL)
-        dval = value_from_contents_and_address (rtype, valaddr, address);
+	{
+	  dval = value_from_contents_and_address (rtype, valaddr, address);
+	  rtype = value_type (dval);
+	}
       else
         dval = dval0;
 
@@ -7900,7 +7905,10 @@ to_record_with_fixed_variant_part (struct type *type, const gdb_byte *valaddr,
     return type;
 
   if (dval0 == NULL)
-    dval = value_from_contents_and_address (type, valaddr, address);
+    {
+      dval = value_from_contents_and_address (type, valaddr, address);
+      type = value_type (dval);
+    }
   else
     dval = dval0;
 
@@ -8198,6 +8206,7 @@ ada_to_fixed_type_1 (struct type *type, const gdb_byte *valaddr,
 	      value_from_contents_and_address (fixed_record_type,
 					       valaddr,
 					       address);
+            fixed_record_type = value_type (obj);
             if (real_type != NULL)
               return to_fixed_record_type
 		(real_type, NULL,
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index bcf54ff..b868d37 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -443,6 +443,7 @@ cp_print_value_fields_rtti (struct type *type,
       /* Ugh, we have to convert back to a value here.  */
       value = value_from_contents_and_address (type, valaddr + offset,
 					       address + offset);
+      type = value_type (value);
       /* We don't actually care about most of the result here -- just
 	 the type.  We already have the correct offset, due to how
 	 val_print was initially called.  */
@@ -545,6 +546,7 @@ cp_print_value (struct type *type, struct type *real_type,
 		  base_val = value_from_contents_and_address (baseclass,
 							      buf,
 							      address + boffset);
+		  baseclass = value_type (base_val);
 		  thisoffset = 0;
 		  boffset = 0;
 		  thistype = baseclass;
diff --git a/gdb/d-valprint.c b/gdb/d-valprint.c
index 6e9c28d..cca629a 100644
--- a/gdb/d-valprint.c
+++ b/gdb/d-valprint.c
@@ -59,6 +59,7 @@ dynamic_array_type (struct type *type, const gdb_byte *valaddr,
 
       true_type = lookup_array_range_type (true_type, 0, length - 1);
       ival = value_at (true_type, addr);
+      true_type = value_type (ival);
 
       d_val_print (true_type,
 		   value_contents_for_printing (ival),
diff --git a/gdb/jv-valprint.c b/gdb/jv-valprint.c
index f465ca0..808e01b 100644
--- a/gdb/jv-valprint.c
+++ b/gdb/jv-valprint.c
@@ -65,6 +65,7 @@ java_value_print (struct value *val, struct ui_file *stream,
 	  type = lookup_pointer_type (type);
 
 	  val = value_at (type, address);
+	  type = value_type (val);
 	}
     }
 
diff --git a/gdb/valops.c b/gdb/valops.c
index 4fc57ec..ec2f1f6 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -268,6 +268,7 @@ value_cast_structs (struct type *type, struct value *v2)
 	{
 	  v = value_full_object (v2, real_type, full, top, using_enc);
 	  v = value_at_lazy (real_type, value_address (v));
+	  real_type = value_type (v);
 
 	  /* We might be trying to cast to the outermost enclosing
 	     type, in which case search_struct_field won't work.  */
@@ -803,6 +804,7 @@ value_dynamic_cast (struct type *type, struct value *arg)
     return value_at_lazy (type, addr);
 
   tem = value_at (type, addr);
+  type = value_type (tem);
 
   /* The first dynamic check specified in 5.2.7.  */
   if (is_public_ancestor (arg_type, TYPE_TARGET_TYPE (resolved_type)))
@@ -1610,6 +1612,7 @@ value_ind (struct value *arg1)
 			      (value_as_address (arg1)
 			       - value_pointed_to_offset (arg1)));
 
+      enc_type = value_type (arg2);
       return readjust_indirect_value_type (arg2, enc_type, base_type, arg1);
     }
 
diff --git a/gdb/value.c b/gdb/value.c
index bd4d6aa..6e5af94 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3359,6 +3359,7 @@ coerce_ref (struct value *arg)
   retval = value_at_lazy (enc_type,
                           unpack_pointer (value_type (arg),
                                           value_contents (arg)));
+  enc_type = value_type (retval);
   return readjust_indirect_value_type (retval, enc_type,
                                        value_type_arg_tmp, arg);
 }
-- 
1.7.0.7

[Patch v2 04/10] vla: enable sizeof operator for indirection

[Sanimir Agovic]

This patch enables the sizeof operator for indirections:

1| void foo (size_t n) {
2|   int vla[n];
3|   int *vla_ptr = &vla;
4| }

(gdb) p sizeof(*vla_ptr)

yields N.

2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
            Keven Boell  <keven.boell@intel.com>

	* eval.c (evaluate_subexp_for_sizeof): Create a indirect value and
	retrieve the dynamic type size.

Change-Id: I0251a9bece82b221f936f369dcfe9195099b6ee2
---
 gdb/eval.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index a81e789..b3e45ca 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -3027,6 +3027,8 @@ evaluate_subexp_for_sizeof (struct expression *exp, int *pos)
 	  && TYPE_CODE (type) != TYPE_CODE_ARRAY)
 	error (_("Attempt to take contents of a non-pointer value."));
       type = check_typedef (TYPE_TARGET_TYPE (type));
+      if (is_dynamic_type (type))
+	type = value_type (value_ind (val));
       return value_from_longest (size_type, (LONGEST) TYPE_LENGTH (type));
 
     case UNOP_MEMVAL:
-- 
1.7.0.7

[Patch v2 08/10] test: evaluate pointers to C99 vla correctly.

[Sanimir Agovic]

2013-10-18  Keven Boell  <keven.boell@intel.com>

gdb/testsuite:
	* gdb.base/vla-ptr.c: New. Test source file
	for testing pointers to VLA's in C.
	* gdb.base/vla-ptr.exp: New. Tests ensure that
	the evaluation of pointers to VLA's work
	correctly in C.

Change-Id: I12695184edc0d9621d658615ea48d783649f813d
Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/testsuite/gdb.base/vla-ptr.c   |   63 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/vla-ptr.exp |   49 ++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/vla-ptr.c
 create mode 100644 gdb/testsuite/gdb.base/vla-ptr.exp

diff --git a/gdb/testsuite/gdb.base/vla-ptr.c b/gdb/testsuite/gdb.base/vla-ptr.c
new file mode 100644
index 0000000..85c41e0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-ptr.c
@@ -0,0 +1,63 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define SIZE 5
+
+void
+foo (int n, int vla_ptr[n])
+{
+  return;         /* foo_bp */
+}
+
+void
+bar (int *vla_ptr)
+{
+  return;         /* bar_bp */
+}
+
+void
+vla_func (int n)
+{
+  int vla[n];
+  int (*vla_ptr)[n];
+  typedef int typedef_vla[n];
+  typedef_vla td_vla;
+  int i;
+
+  for (i = 0; i < n; i++)
+    {
+      vla[i] = 2+i;
+      td_vla[i] = 4+i;
+    }
+
+  foo(n, vla);
+  bar(vla);
+
+  vla_ptr = &vla;
+
+  typedef_vla *td_ptr = &td_vla;  /* vla_ptr_assigned */
+
+  return;         /* typedef_ptr_assigned */
+}
+
+int
+main ()
+{
+  vla_func(SIZE);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/vla-ptr.exp b/gdb/testsuite/gdb.base/vla-ptr.exp
new file mode 100644
index 0000000..843cbd4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-ptr.exp
@@ -0,0 +1,49 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile ".c"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+set sizeof_int [get_sizeof "int" 4]
+
+# check that VLA passed to function (pointer) points to the first element
+gdb_breakpoint [gdb_get_line_number "foo_bp"]
+gdb_continue_to_breakpoint "foo_bp"
+gdb_test "print vla_ptr" "\\\(int \\\*\\\) $hex" "print vla_ptr (foo)"
+gdb_test "print *vla_ptr" "\\$\\d+ = 2" "print *vla_ptr (foo)"
+
+gdb_breakpoint [gdb_get_line_number "bar_bp"]
+gdb_continue_to_breakpoint "bar_bp"
+gdb_test "print vla_ptr" "\\\(int \\\*\\\) $hex" "print vla_ptr (bar)"
+gdb_test "print *vla_ptr" "\\$\\d+ = 2" "print *vla_ptr (bar)"
+
+gdb_breakpoint [gdb_get_line_number "vla_ptr_assigned"]
+gdb_continue_to_breakpoint "vla_ptr_assigned"
+gdb_test "print *vla_ptr" "\\$\\d+ = \\\{2, 3, 4, 5, 6\\\}" "print *vla_ptr (vla_func)"
+# calculate the overall size of the vla
+set sizeof_vla [ expr "5" * "$sizeof_int" ]
+gdb_test "print sizeof(*vla_ptr)" "\\$\\d+ = ${sizeof_vla}" "print sizeof(*vla_ptr) (vla_func)"
+
+gdb_breakpoint [gdb_get_line_number "typedef_ptr_assigned"]
+gdb_continue_to_breakpoint "typedef_ptr_assigned"
+gdb_test "print td_vla" "\\$\\d+ = \\\{4, 5, 6, 7, 8\\\}" "print td_vla"
+gdb_test "print *td_ptr" "\\$\\d+ = \\\{4, 5, 6, 7, 8\\\}" "print *td_ptr"
-- 
1.7.0.7

[Patch v2 06/10] vla: print "dynamic length" for unresolved dynamic bounds

[Sanimir Agovic]

1| void foo (size_t n) {
2|   int vla[n];
3| }

Given the following expression

  (gdb) ptype &vla

Gdb evaluates the expression with EVAL_AVOID_SIDE_EFFECTS and thus
does not resolve the bounds information and misinterprets the high
bound as a constant. The current output is:

  type = int (*)[1289346]

this patch deals with this case and prints:

  type = int (*)[variable length]

instead.

2013-08-30  Keven Boell  <keven.boell@intel.com>
            Sanimir Agovic  <sanimir.agovic@intel.com>

	* c-typeprint.c (c_type_print_varspec_suffix): Added
	check for non yet resolved high bound. If unresolved, print
	"dyanmic length" string to the console instead of random
	length.

Change-Id: I238616814d0baae07b68f80fd61e664b5fbfc4d0
Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/c-typeprint.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 2757337..a5f7905 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -689,7 +689,10 @@ c_type_print_varspec_suffix (struct type *type,
 
 	fprintf_filtered (stream, (is_vector ?
 				   " __attribute__ ((vector_size(" : "["));
-	if (get_array_bounds (type, &low_bound, &high_bound))
+	/* Bounds are not yet resolved, print a bounds placeholder instead.  */
+	if (TYPE_HIGH_BOUND_KIND (TYPE_INDEX_TYPE (type)) == DWARF_LOCEXPR)
+	  fprintf_filtered (stream, "variable length");
+	else if (get_array_bounds (type, &low_bound, &high_bound))
 	  fprintf_filtered (stream, "%s", 
 			    plongest (high_bound - low_bound + 1));
 	fprintf_filtered (stream, (is_vector ? ")))" : "]"));
-- 
1.7.0.7

[Patch v2 07/10] test: multi-dimensional c99 vla.

[Sanimir Agovic]

2013-10-18  Keven Boell  <keven.boell@intel.com>
            Sanimir Agovic  <sanimir.agovic@intel.com>

gdb/testsuite:
	* gdb.base/vla-multi.c: New. Test source file
	for testing multi-dimensional VLA's in C.
	* gdb.base/vla-multi.exp: New. Tests ensure
	that multi-dimensional VLA's can be evaluated
	correctly in C.

Change-Id: I2fd81dc113f20eafed363e7a5a0b0a5a6ce155bc
Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/testsuite/gdb.base/vla-multi.c   |   55 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/vla-multi.exp |   44 +++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/vla-multi.c
 create mode 100644 gdb/testsuite/gdb.base/vla-multi.exp

diff --git a/gdb/testsuite/gdb.base/vla-multi.c b/gdb/testsuite/gdb.base/vla-multi.c
new file mode 100644
index 0000000..47c753e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-multi.c
@@ -0,0 +1,55 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+void
+f1 (int n, int m, int vla_ptr[n][m])
+{
+  return;                                 /* f1_breakpoint */
+}
+
+void
+f2 (int m, int vla_ptr[][m])
+{
+  return;                                 /* f2_breakpoint */
+}
+
+void
+vla_mult (int n, int m)
+{
+  int vla[n][m];
+  int i, j;
+
+  for (i = 0; i < n; i++)
+    {
+      for (j = 0; j < m; j++)
+        {
+          vla[i][j] = i + j;
+        }
+    }
+
+  f1(n, m, vla);                          /* vla_filled */
+  f2(m, vla);
+
+  return;
+}
+
+int
+main()
+{
+  vla_mult(2, 2);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/vla-multi.exp b/gdb/testsuite/gdb.base/vla-multi.exp
new file mode 100644
index 0000000..1c004c0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-multi.exp
@@ -0,0 +1,44 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile ".c"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+set sizeof_int [get_sizeof "int" 4]
+
+gdb_breakpoint [gdb_get_line_number "vla_filled"]
+gdb_continue_to_breakpoint "vla_filled"
+gdb_test "print vla" "\\$\\d+ = \\\{\\\{0, 1\\\}, \\\{1, 2\\\}\\\}" "print vla"
+gdb_test "print vla\[0\]\[1\]" "\\$\\d+ = 1" "print vla\[0\]\[1\]"
+
+gdb_breakpoint [gdb_get_line_number "f1_breakpoint"]
+gdb_continue_to_breakpoint "f1_breakpoint"
+gdb_test "print *vla_ptr" "\\$\\d+ = \\\{0, 1\\\}" "print *vla_ptr (f1)"
+# calculate the overall size of the vla
+set sizeof_vla [ expr "2" * "$sizeof_int" ]
+gdb_test "print sizeof vla_ptr\[0\]" "\\$\\d+ = ${sizeof_vla}" "print sizeof vla_ptr\[0\]"
+gdb_test "ptype &vla_ptr" "type = int \\\(\\\*\\\*\\\)\\\[variable length\\\]" \
+    "ptype &vla_ptr"
+
+gdb_breakpoint [gdb_get_line_number "f2_breakpoint"]
+gdb_continue_to_breakpoint "f2_breakpoint"
+gdb_test "print *vla_ptr" "\\$\\d+ = \\\{0, 1\\\}" "print *vla_ptr (f2)"
-- 
1.7.0.7

[Patch v2 09/10] test: basic c99 vla tests

[Sanimir Agovic]

2013-10-18  Keven Boell  <keven.boell@intel.com>

gdb/testsuite:
	* gdb.base/vla-datatypes.c: New. Test source file
	for VLA datatype checks.
	* gdb.base/vla-datatypes.exp: New. Tests ensure that
	a VLA in C can be evaluated correctly with standard
	C types.

Change-Id: I0517c095522f242deeb00a0f1db31f730dbff228
Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/testsuite/gdb.base/vla-datatypes.c   |   86 ++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/vla-datatypes.exp |   82 ++++++++++++++++++++++++++++
 2 files changed, 168 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/vla-datatypes.c
 create mode 100644 gdb/testsuite/gdb.base/vla-datatypes.exp

diff --git a/gdb/testsuite/gdb.base/vla-datatypes.c b/gdb/testsuite/gdb.base/vla-datatypes.c
new file mode 100644
index 0000000..267d239
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-datatypes.c
@@ -0,0 +1,86 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stddef.h>
+#define SIZE 5
+
+struct foo
+{
+  int a;
+};
+
+typedef struct bar
+{
+  int x;
+  struct foo y;
+} BAR;
+
+void
+vla_factory (int n)
+{
+  int             int_vla[n];
+  unsigned int    unsigned_int_vla[n];
+  double          double_vla[n];
+  float           float_vla[n];
+  long            long_vla[n];
+  unsigned long   unsigned_long_vla[n];
+  char            char_vla[n];
+  short           short_vla[n];
+  unsigned short  unsigned_short_vla[n];
+  unsigned char   unsigned_char_vla[n];
+  struct foo      foo_vla[n];
+  BAR             bar_vla[n];
+  int i;
+
+  for (i = 0; i < n; i++)
+    {
+      int_vla[i] = i*2;
+      unsigned_int_vla[i] = i*2;
+      double_vla[i] = i/2.0;
+      float_vla[i] = i/2.0f;
+      long_vla[i] = i*2;
+      unsigned_long_vla[i] = i*2;
+      char_vla[i] = 'A';
+      short_vla[i] = i*2;
+      unsigned_short_vla[i] = i*2;
+      unsigned_char_vla[i] = 'A';
+      foo_vla[i].a = i*2;
+      bar_vla[i].x = i*2;
+      bar_vla[i].y.a = i*2;
+    }
+
+  size_t int_size        = sizeof(int_vla);     /* vlas_filled */
+  size_t uint_size       = sizeof(unsigned_int_vla);
+  size_t double_size     = sizeof(double_vla);
+  size_t float_size      = sizeof(float_vla);
+  size_t long_size       = sizeof(long_vla);
+  size_t char_size       = sizeof(char_vla);
+  size_t short_size      = sizeof(short_vla);
+  size_t ushort_size     = sizeof(unsigned_short_vla);
+  size_t uchar_size      = sizeof(unsigned_char_vla);
+  size_t foo_size        = sizeof(foo_vla);
+  size_t bar_size        = sizeof(bar_vla);
+
+  return;                                 /* break_end_of_vla_factory */
+}
+
+int
+main ()
+{
+  vla_factory(SIZE);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/vla-datatypes.exp b/gdb/testsuite/gdb.base/vla-datatypes.exp
new file mode 100644
index 0000000..304124e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-datatypes.exp
@@ -0,0 +1,82 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile ".c"
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "vlas_filled"]
+gdb_continue_to_breakpoint "vlas_filled"
+# check values of VLA's
+gdb_test "print int_vla" "\\$\\d+ = \\\{0, 2, 4, 6, 8\\\}" "print int_vla"
+gdb_test "print unsigned_int_vla" "\\$\\d+ = \\\{0, 2, 4, 6, 8\\\}" "print unsigned_int_vla"
+gdb_test "print double_vla" "\\$\\d+ = \\\{0, 0.5, 1, 1.5, 2\\\}" "print double_vla"
+gdb_test "print float_vla" "\\$\\d+ = \\\{0, 0.5, 1, 1.5, 2\\\}" "print float_vla"
+gdb_test "print long_vla" "\\$\\d+ = \\\{0, 2, 4, 6, 8\\\}" "print long_vla"
+gdb_test "print unsigned_long_vla" "\\$\\d+ = \\\{0, 2, 4, 6, 8\\\}" "print unsigned_long_vla"
+gdb_test "print char_vla" "\\$\\d+ = \"AAAAA\"" "print char_vla"
+gdb_test "print short_vla" "\\$\\d+ = \\\{0, 2, 4, 6, 8\\\}" "print short_vla"
+gdb_test "print unsigned_short_vla" "\\$\\d+ = \\\{0, 2, 4, 6, 8\\\}" "print unsigned_short_vla"
+gdb_test "print unsigned_char_vla" "\\$\\d+ = \"AAAAA\"" "print unsigned_char_vla"
+gdb_test "print foo_vla" "\\\{\\\{a = 0\\\}, \\\{a = 2\\\}, \\\{a = 4\\\}, \\\{a = 6\\\}, \\\{a = 8\\\}\\\}" "print foo_vla"
+gdb_test "print bar_vla" "\\\{\\\{x = 0, y = \\\{a = 0\\\}\\\}, \\\{x = 2, y = \\\{a = 2\\\}\\\}, \\\{x = 4, y = \\\{a = 4\\\}\\\}, \\\{x = 6, y = \\\{a = 6\\\}\\\}, \\\{x = 8, y = \\\{a = 8\\\}\\\}\\\}" "print bar_vla"
+
+# check whatis of VLA's
+gdb_test "whatis int_vla" "type = int \\\[5\\\]" "whatis int_vla"
+gdb_test "whatis unsigned_int_vla" "type = unsigned int \\\[5\\\]" "whatis unsigned_int_vla"
+gdb_test "whatis double_vla" "type = double \\\[5\\\]" "whatis double_vla"
+gdb_test "whatis float_vla" "type = float \\\[5\\\]" "whatis float_vla"
+gdb_test "whatis long_vla" "type = long( int)? \\\[5\\\]" "whatis long_vla"
+gdb_test "whatis unsigned_long_vla" "type = (long unsigned int|unsigned long) \\\[5\\\]" "whatis unsigned_long_vla"
+gdb_test "whatis char_vla" "type = char \\\[5\\\]" "whatis char_vla"
+gdb_test "whatis short_vla" "type = short( int)? \\\[5\\\]" "whatis short_vla"
+gdb_test "whatis unsigned_short_vla" "type = (short unsigned int|unsigned short) \\\[5\\\]" "whatis unsigned_short_vla"
+gdb_test "whatis unsigned_char_vla" "type = unsigned char \\\[5\\\]" "whatis unsigned_char_vla"
+gdb_test "whatis foo_vla" "type = struct foo \\\[5\\\]" "whatis foo_vla"
+gdb_test "whatis bar_vla" "type = BAR \\\[5\\\]" "whatis bar_vla"
+
+# check ptype of VLA's
+gdb_test "ptype int_vla" "type = int \\\[5\\\]" "ptype int_vla"
+gdb_test "ptype unsigned_int_vla" "type = unsigned int \\\[5\\\]" "ptype unsigned_int_vla"
+gdb_test "ptype double_vla" "type = double \\\[5\\\]" "ptype double_vla"
+gdb_test "ptype float_vla" "type = float \\\[5\\\]" "ptype float_vla"
+gdb_test "ptype long_vla" "type = long( int)? \\\[5\\\]" "ptype long_vla"
+gdb_test "ptype unsigned_long_vla" "type = unsigned long \\\[5\\\]" "ptype unsigned_long_vla"
+gdb_test "ptype char_vla" "type = char \\\[5\\\]" "ptype char_vla"
+gdb_test "ptype short_vla" "type = short( int)? \\\[5\\\]" "ptype short_vla"
+gdb_test "ptype unsigned_short_vla" "type = unsigned short \\\[5\\\]" "ptype unsigned_short_vla"
+gdb_test "ptype unsigned_char_vla" "type = unsigned char \\\[5\\\]" "ptype unsigned_char_vla"
+gdb_test "ptype foo_vla" "type = struct foo {\r\n\\s+int a;\r\n} \\\[5\\\]" "ptype foo_vla"
+gdb_test "ptype bar_vla" "type = struct bar {\r\n\\s+int x;\r\n\\s+struct foo y;\r\n} \\\[5\\\]" "ptype bar_vla"
+
+# check the size of the VLA's
+gdb_breakpoint [gdb_get_line_number "break_end_of_vla_factory"]
+gdb_continue_to_breakpoint "break_end_of_vla_factory"
+gdb_test "print int_size == sizeof(int_vla)" "\\$\\d+ = 1" "size of int_vla"
+gdb_test "print uint_size == sizeof(unsigned_int_vla)" "\\$\\d+ = 1" "size of unsigned_int_vla"
+gdb_test "print double_size == sizeof(double_vla)" "\\$\\d+ = 1" "size of double_vla"
+gdb_test "print float_size == sizeof(float_vla)" "\\$\\d+ = 1" "size of float_vla"
+gdb_test "print long_size == sizeof(long_vla)" "\\$\\d+ = 1" "size of long_vla"
+gdb_test "print char_size == sizeof(char_vla)" "\\$\\d+ = 1" "size of char_vla"
+gdb_test "print short_size == sizeof(short_vla)" "\\$\\d+ = 1" "size of short_vla"
+gdb_test "print ushort_size == sizeof(unsigned_short_vla)" "\\$\\d+ = 1" "size of unsigned_short_vla"
+gdb_test "print uchar_size == sizeof(unsigned_char_vla)" "\\$\\d+ = 1" "size of unsigned_char_vla"
+gdb_test "print foo_size == sizeof(foo_vla)" "\\$\\d+ = 1" "size of foo_vla"
+gdb_test "print bar_size == sizeof(bar_vla)" "\\$\\d+ = 1" "size of bar_vla"
-- 
1.7.0.7

[Patch v2 10/10] test: add mi vla test

[Sanimir Agovic]

2013-10-18  Keven Boell  <keven.boell@intel.com>
            Sanimir Agovic  <sanimir.agovic@intel.com>

testsuite/gdb.mi/

	* mi-vla-c99.exp: New file.
	* vla.c: New file.

Change-Id: I9827c8b80b3ba33e2ea7a376bc725154e4ece4c6
---
 gdb/testsuite/gdb.mi/mi-vla-c99.exp |   80 +++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/vla.c          |   35 +++++++++++++++
 2 files changed, 115 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-vla-c99.exp
 create mode 100644 gdb/testsuite/gdb.mi/vla.c

diff --git a/gdb/testsuite/gdb.mi/mi-vla-c99.exp b/gdb/testsuite/gdb.mi/mi-vla-c99.exp
new file mode 100644
index 0000000..4e28576
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-vla-c99.exp
@@ -0,0 +1,80 @@
+# Copyright 1999-2013 Free Software Foundation, Inc.
+
+# Contributed by Intel Corp. <keven.boell@intel.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Verify that, using the MI, we can evaluate a simple C Variable Length
+# Array (VLA).
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile vla.c
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-vla-basics.exp
+     return -1
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+set bp_lineno [gdb_get_line_number "vla-filled"]
+
+mi_create_breakpoint "-t vla.c:$bp_lineno" 1 "del" "func" ".*vla.c" $bp_lineno $hex \
+             "insert breakpoint at line $bp_lineno after vla is filled"
+mi_run_cmd
+mi_expect_stop "breakpoint-hit" "func" "\{name=\"n\",value=\"5\"\}" ".*vla.c" "$bp_lineno" \
+  { "" "disp=\"del\"" } "run to breakpoint at line $bp_lineno"
+
+mi_gdb_test "500-data-evaluate-expression vla" \
+    "500\\^done,value=\"\\{0, 1, 2, 3, 4\\}\"" "evaluate complete vla"
+
+mi_gdb_test "501-data-evaluate-expression vla\[0\]" \
+    "501\\^done,value=\"0\"" "evaluate vla\[0\]"
+
+mi_gdb_test "502-data-evaluate-expression vla\[2\]" \
+    "502\\^done,value=\"2\"" "evaluate vla\[2\]"
+
+mi_gdb_test "503-data-evaluate-expression vla\[4\]" \
+    "503\\^done,value=\"4\"" "evaluate vla\[4\]"
+
+mi_create_varobj_checked vla vla "int \\\[5\\\]" "create local variable vla"
+
+mi_gdb_test "504-var-info-type vla" \
+    "504\\^done,type=\"int \\\[5\\\]\"" \
+    "info type variable vla"
+
+mi_gdb_test "505-var-show-format vla" \
+    "505\\^done,format=\"natural\"" \
+    "show format variable vla"
+
+mi_gdb_test "506-var-evaluate-expression vla" \
+    "506\\^done,value=\"\\\[5\\\]\"" \
+    "eval variable vla"
+
+mi_list_array_varobj_children "vla" "5" "int" \
+    "get children of vla"
+
+
+
+mi_gdb_exit
+return 0
diff --git a/gdb/testsuite/gdb.mi/vla.c b/gdb/testsuite/gdb.mi/vla.c
new file mode 100644
index 0000000..9b33fc8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/vla.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Contributed by Intel Corp. <keven.boell@intel.com>
+
+   Copyright 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int func (int n)
+{
+  int vla[n], i;
+
+  for (i = 0; i < n; i++)
+    vla[i] = i;
+
+  return n;                 /* vla-filled */
+}
+
+int main ()
+{
+  func (5);
+
+  return 0;
+}
-- 
1.7.0.7

Re: [Patch v2 07/10] test: multi-dimensional c99 vla.

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> 2013-10-18  Keven Boell  <keven.boell@intel.com>
Sanimir>             Sanimir Agovic  <sanimir.agovic@intel.com>

Sanimir> gdb/testsuite:
Sanimir> 	* gdb.base/vla-multi.c: New. Test source file
Sanimir> 	for testing multi-dimensional VLA's in C.
Sanimir> 	* gdb.base/vla-multi.exp: New. Tests ensure
Sanimir> 	that multi-dimensional VLA's can be evaluated
Sanimir> 	correctly in C.

Sanimir> +gdb_test "print sizeof vla_ptr\[0\]" "\\$\\d+ = ${sizeof_vla}" "print sizeof vla_ptr\[0\]"

This line is too long, but otherwise this is ok.

Tom

Re: [Patch v2 09/10] test: basic c99 vla tests

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> +gdb_test "print unsigned_long_vla" "\\$\\d+ = \\\{0, 2, 4, 6, 8\\\}" "print unsigned_long_vla"

Lots of over-long lines in this file.
They are all simple to fix up though.

I read through the tests and didn't see a case of sizeof with a side
effect.

Tom

Re: [Patch v2 08/10] test: evaluate pointers to C99 vla correctly.

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> 2013-10-18  Keven Boell  <keven.boell@intel.com>
Sanimir> gdb/testsuite:
Sanimir> 	* gdb.base/vla-ptr.c: New. Test source file
Sanimir> 	for testing pointers to VLA's in C.
Sanimir> 	* gdb.base/vla-ptr.exp: New. Tests ensure that
Sanimir> 	the evaluation of pointers to VLA's work
Sanimir> 	correctly in C.

Sanimir> +# check that VLA passed to function (pointer) points to the first element

Normally comments should start with a capital letter and end with a
period.

Sanimir> +gdb_test "print *vla_ptr" "\\$\\d+ = \\\{2, 3, 4, 5, 6\\\}" "print *vla_ptr (vla_func)"

Some too-long lines in this file.

Everything important is ok though.

Tom

Re: [Patch v2 10/10] test: add mi vla test

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {

Line break.

Sanimir> +mi_create_breakpoint "-t vla.c:$bp_lineno" 1 "del" "func" ".*vla.c" $bp_lineno $hex \
Sanimir> +             "insert breakpoint at line $bp_lineno after vla is filled"

Ditto.

Sanimir> +mi_expect_stop "breakpoint-hit" "func" "\{name=\"n\",value=\"5\"\}" ".*vla.c" "$bp_lineno" \

Ditto.

Otherwise looks good, thanks.

Tom

Re: [Patch v2 01/10] vla: introduce new bound type abstraction adapt uses

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> The rational behind this patch is to get started to implement
Sanimir> the feature described in dwarf4 standard (2.19) Static and
Sanimir> Dynamic Values of Attributes.  It adds new DWARF2_PROP to store
Sanimir> either a constant, exprloc, or reference to describe an
Sanimir> upper-/lower bound of a subrange. Other than that no new
Sanimir> features are introduce.

Thanks.

I have one nit and one more serious question.

Sanimir> +    low.data.const_val =
Sanimir> +      dwarf2_get_attr_constant_value (attr, low.data.const_val);

Split before the "=", not after.

Sanimir> +struct type *
Sanimir> +create_range_type_1 (struct type *result_type, struct type *index_type,
Sanimir> +		     const struct dwarf2_prop *low_bound,
Sanimir> +		     const struct dwarf2_prop *high_bound)
[...]

Sanimir>  create_range_type (struct type *result_type, struct type *index_type,
Sanimir>  		   LONGEST low_bound, LONGEST high_bound)
Sanimir>  {
[...]
Sanimir> +  result_type = create_range_type_1 (result_type, index_type,
Sanimir> +				     &low, &high);
 
Sanimir>    if (low_bound >= 0)
Sanimir>      TYPE_UNSIGNED (result_type) = 1;

I wonder why the TYPE_UNSIGNED setting is in create_range_type but not
in create_range_type_1.  It seems like this would cause a change in
semantics in dwarf2read, which now calls the new function.

Tom

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

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> -/* DWARF 2 location expression support for GDB.
Sanimir> +/* DWARF location expression support for GDB.

While I agree with this change, it's really unrelated to the patch at
hand.

Sanimir> +static int
Sanimir> +attr_to_dwarf2_prop (const struct attribute *attr, struct die_info *die,
Sanimir> +		     struct dwarf2_cu *cu, struct dwarf2_prop *prop)
Sanimir> +{
Sanimir> +  if (die == NULL || attr == NULL || cu == NULL || prop == NULL)
Sanimir> +    return 0;

Can all of these cases actually occur?
I would guess that at least 'die' and 'cu' can never be NULL.

Sanimir> +  else if (attr_form_is_ref (attr))
Sanimir> +    {
Sanimir> +      struct dwarf2_cu *target_cu = cu;
Sanimir> +      struct die_info *target_die;
Sanimir> +      struct attribute *target_attr;
Sanimir> +      const gdb_byte append_ops[] = { DW_OP_deref };

I think this won't work in all cases.  For example, suppose the
reference is to a DIE that has a DWARF expression that evaluates to a
register.  In this case, you want the value of the register -- but this
will do something else.

I appreciate what the deref is trying to do here but I think a different
approach is probably needed.

Sanimir> +static int
Sanimir> +resolve_dynamic_prop (const struct dwarf2_prop *prop, CORE_ADDR address,
Sanimir> +		      CORE_ADDR *value)
Sanimir> +{
Sanimir> +  if (prop == NULL)
Sanimir> +    return 0;
Sanimir> +
Sanimir> +  switch (prop->kind)
Sanimir> +    {
Sanimir> +    case DWARF_LOCEXPR:
Sanimir> +      {
Sanimir> +	const struct dwarf2_locexpr_baton *baton = prop->data.locexpr;
Sanimir> +
Sanimir> +	return dwarf2_locexpr_baton_eval (baton, address, value);
Sanimir> +      }
Sanimir> +    case DWARF_CONST:
Sanimir> +      break;
Sanimir> +    }

This is a switch not covering all the cases.
I think it's fine to use a simple "if" instead.
Or, if you really want a switch, put the remaining constant in there too.

Sanimir> +static int
Sanimir> +is_dynamic_type (const struct type *type)
Sanimir> +{
Sanimir> +  if (type == NULL)
Sanimir> +    return 0;

Can this happen?

Sanimir> +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
Sanimir> +      && TYPE_NFIELDS (type) == 1)
Sanimir> +    {
Sanimir> +      const struct type *range_type = TYPE_INDEX_TYPE (type);

Probably should have a check_typedef.

Sanimir> +  if (TYPE_CODE (type) == TYPE_CODE_PTR
Sanimir> +      || TYPE_CODE (type) == TYPE_CODE_REF
Sanimir> +      || TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
Sanimir> +    return is_dynamic_type (TYPE_TARGET_TYPE (type));

check_typedef here too.

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.


Also I noticed that the intro comments for resolve_dynamic_prop,
resolve_dynamic_bounds, and resolve_dynamic_type do not document the
meaning of the ADDRESS parameter.

Sanimir> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
Sanimir> index d01f75e..5500e94 100644
Sanimir> --- a/gdb/gdbtypes.h
Sanimir> +++ b/gdb/gdbtypes.h
Sanimir> @@ -1094,6 +1094,7 @@ extern void allocate_gnat_aux_type (struct type *);
Sanimir>  #define TYPE_LOW_BOUND_KIND(range_type) \
Sanimir>    TYPE_RANGE_DATA(range_type)->low.kind
 
Sanimir> +
Sanimir>  /* Moto-specific stuff for FORTRAN arrays.  */

Random change.

Sanimir> +  store_typed_address (value_contents_raw (val), check_typedef (resolved_type), addr);

Line too long.

Tom

Re: [Patch v2 05/10] vla: update type from newly created value

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> Constructing a value based on a type and address might change
Sanimir> the type of the newly constructed value. Thus re-fetch type via
Sanimir> value_type to ensure we have the correct type at hand.

Sanimir> 2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
Sanimir>             Keven Boell  <keven.boell@intel.com>

Sanimir> 	* ada-lang.c (ada_value_primitive_packed_val): Re-fetch type from value.
Sanimir> 	(ada_template_to_fixed_record_type_1): Likewise.
Sanimir> 	(ada_to_fixed_type_1): Likewise.
Sanimir> 	* cp-valprint.c (cp_print_value_fields_rtti): Likewise.
Sanimir> 	(cp_print_value): Likewise.
Sanimir> 	* d-valprint.c (dynamic_array_type): Likewise.
Sanimir> 	* jv-valprint.c (java_value_print): Likewise.
Sanimir> 	* valops.c (value_ind): Likewise.
Sanimir> 	* value.c (coerce_ref): Likewise.

Looks good.  Thanks.

Tom

Re: [Patch v2 06/10] vla: print "dynamic length" for unresolved dynamic bounds

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> 2013-08-30  Keven Boell  <keven.boell@intel.com>
Sanimir>             Sanimir Agovic  <sanimir.agovic@intel.com>

Sanimir> 	* c-typeprint.c (c_type_print_varspec_suffix): Added
Sanimir> 	check for non yet resolved high bound. If unresolved, print

Typo, should be "not yet", or just "unresolved".

Sanimir> 	"dyanmic length" string to the console instead of random

It really prints "variable length" though.

The patch itself looks fine.

Tom

Re: [Patch v2 00/10] C99 variable length array support

[Tom Tromey]

>>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:

Sanimir> this patch series (v2) add C99 variable length support to gdb.

Thanks once again.

I read through all the patches and sent my comments.

Tom

Re: [Patch v2 01/10] vla: introduce new bound type abstraction adapt uses

[Doug Evans]

On Thu, Nov 21, 2013 at 6:30 AM, Sanimir Agovic
<sanimir.agovic@intel.com> wrote:
> The rational behind this patch is to get started to implement the feature
> described in dwarf4 standard (2.19) Static and Dynamic Values of Attributes.
> It adds new DWARF2_PROP to store either a constant, exprloc, or reference to
> describe an upper-/lower bound of a subrange. Other than that no new features
> are introduce.
>
> 2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
>             Keven Boell  <keven.boell@intel.com>
>
>         * dwarf2read.c (read_subrange_type): Use struct dwarf2_prop for
>         declaring high/low bounds and change uses accordingly. Call
>         create_range_type_1 instead of create_range_type,
>         * gdbtypes.c (create_range_type_1): New function.
>         (create_range_type): Convert bounds into struct dwarf2_prop and pass
>         them to create_range_type_1.
>         * gdbtypes.h (struct dwarf2_prop): New struct.
>         (create_range_type_1): New function prototype.
>         (struct range_bounds): Use struct dwarf2_prop instead of LONGEST for
>         high/low bounds. Remove low_undefined/high_undefined and adapt all uses.
>         (TYPE_LOW_BOUND,TYPE_HIGH_BOUND): Adapt macros to refer to the static
>         part of the bound.
>         * parse.c (follow_types): Set high bound kind to DWARF_UNDEFINED.

Do we know other debug formats don't support dynamic ranges?

There are various abstraction boundaries that gdb tries to maintain
between gdb proper and other pieces (like debug format readers).
We don't always maintain these boundaries (e.g., grep for existing
uses of the word "dwarf" in gdbtypes.h) but IWBN to at least have a
discussion every time such boundaries are violated.

The trivial way to fix this in cases such as these, is of course to,
basically, not use the word "dwarf" in anything that is used in gdb
proper (modulo the the very few cases where we call into dwarf*.c).
Whether it's sufficient and/or worth it in any particular situation is
of course *the* question.  But it doesn't feel right to proceed
without having it being asked and answered.

RE: [Patch v2 01/10] vla: introduce new bound type abstraction adapt uses

[Agovic, Sanimir]

Thanks for your review.

> There are various abstraction boundaries that gdb tries to maintain
> between gdb proper and other pieces (like debug format readers).
> We don't always maintain these boundaries (e.g., grep for existing
> uses of the word "dwarf" in gdbtypes.h) but IWBN to at least have a
> discussion every time such boundaries are violated.
> 
Agreed, the dwarf bits leak implementation details of the bounds
implementation. If you agree I will make this change below part of v3.
Of course the remaining files need to be adapted as well.

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 85d2768..da1dfe2 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -367,14 +367,14 @@ enum type_instance_flag_value

 /* Used to store bound information for a type.  */

-struct dwarf2_prop
+struct bound_prop
 {
   /* Determine which field of the union dwarf2_prop.data is used.  */
   enum
   {
-    DWARF_UNDEFINED,
-    DWARF_CONST,
-    DWARF_LOCEXPR
+    BOUND_UNDEFINED,
+    BOUND_CONST,
+    BOUND_LOCEXPR
   } kind;

   /* Stores information as location expression, location list,
@@ -382,8 +382,8 @@ struct dwarf2_prop
   union data
   {
     LONGEST const_val;
-    struct dwarf2_locexpr_baton *locexpr;
+    void *locexpr;
   } data;
 };

@@ -613,11 +613,11 @@ struct main_type
     {
       /* Low bound of range.  */

-      struct dwarf2_prop low;
+      struct bound_prop low;

       /* High bound of range.  */

-      struct dwarf2_prop high;
+      struct bound_prop high;
     } *bounds;

   } flds_bnds;
@@ -1097,9 +1097,9 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_HIGH_BOUND(range_type) \
   TYPE_RANGE_DATA(range_type)->high.data.const_val
 #define TYPE_LOW_BOUND_UNDEFINED(range_type) \
-  (TYPE_RANGE_DATA(range_type)->low.kind == DWARF_UNDEFINED)
+  (TYPE_RANGE_DATA(range_type)->low.kind == BOUND_UNDEFINED)
 #define TYPE_HIGH_BOUND_UNDEFINED(range_type) \
-  (TYPE_RANGE_DATA(range_type)->high.kind == DWARF_UNDEFINED)
+  (TYPE_RANGE_DATA(range_type)->high.kind == BOUND_UNDEFINED)
 #define TYPE_HIGH_BOUND_KIND(range_type) \
   TYPE_RANGE_DATA(range_type)->high.kind
 #define TYPE_LOW_BOUND_KIND(range_type) \
@@ -1567,8 +1567,8 @@ extern struct type *lookup_function_type_with_arguments (struct type *,
                                                         struct type **);

 extern struct type *create_range_type_1 (struct type *, struct type *,
-                                        const struct dwarf2_prop *,
-                                        const struct dwarf2_prop *);
+                                        const struct bound_prop *,
+                                        const struct bound_prop *);


 -Sanimir

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

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

[Agovic, Sanimir]

Thanks for your review.

> Sanimir> +  else if (attr_form_is_ref (attr))
> Sanimir> +    {
> Sanimir> +      struct dwarf2_cu *target_cu = cu;
> Sanimir> +      struct die_info *target_die;
> Sanimir> +      struct attribute *target_attr;
> Sanimir> +      const gdb_byte append_ops[] = { DW_OP_deref };
> 
> I think this won't work in all cases.  For example, suppose the
> reference is to a DIE that has a DWARF expression that evaluates to a
> register.  In this case, you want the value of the register -- but this
> will do something else.
> 
> I appreciate what the deref is trying to do here but I think a different
> approach is probably needed.
> 
> Sanimir> +static int
> Sanimir> +resolve_dynamic_prop (const struct dwarf2_prop *prop, CORE_ADDR address,
> Sanimir> +		      CORE_ADDR *value)
> Sanimir> +{
> Sanimir> +  if (prop == NULL)
> Sanimir> +    return 0;
> Sanimir> +
> Sanimir> +  switch (prop->kind)
> Sanimir> +    {
> Sanimir> +    case DWARF_LOCEXPR:
> Sanimir> +      {
> Sanimir> +	const struct dwarf2_locexpr_baton *baton = prop->data.locexpr;
> Sanimir> +
> Sanimir> +	return dwarf2_locexpr_baton_eval (baton, address, value);
> Sanimir> +      }
> Sanimir> +    case DWARF_CONST:
> Sanimir> +      break;
> Sanimir> +    }
> 
> This is a switch not covering all the cases.
> I think it's fine to use a simple "if" instead.
> Or, if you really want a switch, put the remaining constant in there too.
> 
Pedro run into the above issue(s) as well:

 https://sourceware.org/ml/gdb-patches/2013-11/msg00646.html

I will address this in a separate email/patch. Instead of manually
retrieving DW_AT_location I will re-use the exiting functions dealing with
dwarf locations. This should fix both of your mentioned issues.

> 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.
> 
We need to copy TYPE_CODE_ARRAY, including TYPE_CODE_RANGE. TYPE_CODE_TYPEDEF
needs to be copied as well. Otherwise we modify the bounds in-place. Let me
try to re-create an array with create_array_type/create_range_type
instead to avoid the expensive deep copy.

Below you will find the obvious fixes.

 -Sanimir

> -----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" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> -/* DWARF 2 location expression support for GDB.
> Sanimir> +/* DWARF location expression support for GDB.
> 
> While I agree with this change, it's really unrelated to the patch at
> hand.
> 
Will undo.

> Sanimir> +static int
> Sanimir> +attr_to_dwarf2_prop (const struct attribute *attr, struct die_info *die,
> Sanimir> +		     struct dwarf2_cu *cu, struct dwarf2_prop *prop)
> Sanimir> +{
> Sanimir> +  if (die == NULL || attr == NULL || cu == NULL || prop == NULL)
> Sanimir> +    return 0;
> 
> Can all of these cases actually occur?
> I would guess that at least 'die' and 'cu' can never be NULL.
> 
Left attr/prop check in place, removed die/cu null_ptr checks.

> Sanimir> +static int
> Sanimir> +is_dynamic_type (const struct type *type)
> Sanimir> +{
> Sanimir> +  if (type == NULL)
> Sanimir> +    return 0;
> 
> Can this happen?
> 
Removed null_ptr check.

> Sanimir> +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
> Sanimir> +      && TYPE_NFIELDS (type) == 1)
> Sanimir> +    {
> Sanimir> +      const struct type *range_type = TYPE_INDEX_TYPE (type);
> 
> Probably should have a check_typedef.
> 
> Sanimir> +  if (TYPE_CODE (type) == TYPE_CODE_PTR
> Sanimir> +      || TYPE_CODE (type) == TYPE_CODE_REF
> Sanimir> +      || TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
> Sanimir> +    return is_dynamic_type (TYPE_TARGET_TYPE (type));
> 
> check_typedef here too.
> 
Added both check_typedefs.

> Also I noticed that the intro comments for resolve_dynamic_prop,
> resolve_dynamic_bounds, and resolve_dynamic_type do not document the
> meaning of the ADDRESS parameter.
> 
Documentation added.

> Sanimir> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> Sanimir> index d01f75e..5500e94 100644
> Sanimir> --- a/gdb/gdbtypes.h
> Sanimir> +++ b/gdb/gdbtypes.h
> Sanimir> @@ -1094,6 +1094,7 @@ extern void allocate_gnat_aux_type (struct type *);
> Sanimir>  #define TYPE_LOW_BOUND_KIND(range_type) \
> Sanimir>    TYPE_RANGE_DATA(range_type)->low.kind
> 
> Sanimir> +
> Sanimir>  /* Moto-specific stuff for FORTRAN arrays.  */
> 
> Random change.
> 
Done.

> Sanimir> +  store_typed_address (value_contents_raw (val), check_typedef (resolved_type),
> addr);
> 
> Line too long.
> 
Done.

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

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

[Agovic, Sanimir]

> -----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

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

[Tom Tromey]

>>>>> "Sanimir" == Agovic, Sanimir <sanimir.agovic@intel.com> writes:

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

For the Fortran changes you may want to introduce a new variant of
copy_type_recursive that just copies the bits you need, and not all the
extraneous stuff like strings.

Tom

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

[Agovic, Sanimir]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Tom Tromey
> Sent: Monday, December 16, 2013 11:14 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" == Agovic, Sanimir <sanimir.agovic@intel.com> writes:
> 
> Sanimir> In the prototype below I omit any call to copy_type_recursive,
> Sanimir> it works by selective re-constructing array and range types. As
> Sanimir> c99 vla are quite restrictive this works, but languages with
> Sanimir> less restrictive vla implementation (see Fortran vla branch)
> Sanimir> need to deep copy the type. Comments are welcome.
> 
> For the Fortran changes you may want to introduce a new variant of
> copy_type_recursive that just copies the bits you need, and not all the
> extraneous stuff like strings.
> 
Agreed, thanks Tom. The patch will be merged into my upcoming v4.

 -Sanimir
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