public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v4 05/13] vla: update type from newly created value
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (3 preceding siblings ...)
  2013-12-17 12:18 ` [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
@ 2013-12-17 12:18 ` Sanimir Agovic
  2013-12-18  3:44   ` Joel Brobecker
  2013-12-17 12:18 ` [PATCH v4 07/13] vla: support for DW_AT_count Sanimir Agovic
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:18 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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: I8a0ebcbd84d8ff8db2eb24960f099d131816cb18
---
 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 786ca7a..6710d7c 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 4d56318..eb740d6 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)))
@@ -1617,6 +1619,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 12726a1..7e6e0e7 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3362,6 +3362,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.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 06/13] vla: print "variable length" for unresolved dynamic bounds
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
  2013-12-17 12:18 ` [PATCH v4 04/13] vla: enable sizeof operator for indirection Sanimir Agovic
  2013-12-17 12:18 ` [PATCH v4 02/13] type: add c99 variable length array support Sanimir Agovic
@ 2013-12-17 12:18 ` Sanimir Agovic
  2013-12-17 12:18 ` [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:18 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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 not yet resolved high bound. If unresolved, print
	"variable length" string to the console instead of random
	length.

Change-Id: Ic6a5fc08c8651ef18760bdacacc492ed44fe4af4
---
 gdb/c-typeprint.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 2757337..739d770 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -689,7 +689,11 @@ 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)) == PROP_LOCEXPR
+	    || TYPE_HIGH_BOUND_KIND (TYPE_INDEX_TYPE (type)) == PROP_LOCLIST)
+	  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.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 07/13] vla: support for DW_AT_count
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (4 preceding siblings ...)
  2013-12-17 12:18 ` [PATCH v4 05/13] vla: update type from newly created value Sanimir Agovic
@ 2013-12-17 12:18 ` Sanimir Agovic
  2013-12-17 12:19 ` [PATCH v4 10/13] test: multi-dimensional c99 vla Sanimir Agovic
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:18 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

This patch adds support for DW_AT_count as requested in the code review:

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

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

	* dwarf2read.c (read_subrange_type): If DW_AT_count is dynamic
	create the following upper bound dwarf expression 'low + count + 1'
	by hand.

Change-Id: I0f70edd22f2f6d8d7aa32f245e45617a7dc1b24a
---
 gdb/dwarf2read.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0df385a..10f4e23 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14431,17 +14431,49 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_upper_bound, cu);
   if (!attr_to_dynamic_prop (attr, die, cu, &high))
     {
+      struct dynamic_prop count;
+
       attr = dwarf2_attr (die, DW_AT_count, cu);
-      if (attr)
+      if (attr_to_dynamic_prop (attr, die, cu, &count))
 	{
-	  int count = dwarf2_get_attr_constant_value (attr, 1);
-	  high.data.const_val = low.data.const_val + count - 1;
+	  if (count.kind == PROP_LOCEXPR)
+	    {
+	      enum bfd_endian byte_order;
+	      struct dwarf2_property_baton *baton = count.data.baton;
+	      const struct dwarf_block blk = {baton->locexpr.size,
+					      baton->locexpr.data};
+	      /* Allocate a static buffer to hold the following expression:
+		 DW_OP_const8u(1) + CORE_ADDR(8) + DW_OP_plus(1)
+		 + DW_OP_const1s(1), char(1) + DW_OP_minus(1) == 13 bytes.  */
+	      gdb_byte ops[13];
+
+	      ops[0] = DW_OP_const8u;
+	      byte_order = gdbarch_byte_order (get_objfile_arch (cu->objfile));
+	      store_unsigned_integer (ops + 1, 8, byte_order,
+				      low.data.const_val);
+
+	      ops[9] = DW_OP_plus;
+	      ops[10] = DW_OP_const1u;
+	      ops[11] = 1;
+	      ops[12] = DW_OP_minus;
+
+	      baton
+		= obstack_alloc (&cu->objfile->objfile_obstack,
+				 sizeof (struct dwarf2_property_baton));
+	      /* This should yield the following expression:
+		 high = low + count - 1.
+	         Whereas low is a dwarf expression itself.  */
+	      baton->locexpr = block_to_locexpr_baton (&blk, cu, ops,
+						       sizeof (ops));
+	      high.data.baton = baton;
+	      high.kind = PROP_LOCEXPR;
+	    }
+	  else if (count.kind == PROP_CONST)
+	    high.data.const_val = low.data.const_val + count.data.const_val - 1;
 	}
       else
-	{
-	  /* Unspecified array length.  */
-	  high.data.const_val = low.data.const_val - 1;
-	}
+	/* Unspecified array length.  */
+	high.data.const_val = low.data.const_val - 1;
     }
 
   /* Dwarf-2 specifications explicitly allows to create subrange types
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (2 preceding siblings ...)
  2013-12-17 12:18 ` [PATCH v4 06/13] vla: print "variable length" for unresolved dynamic bounds Sanimir Agovic
@ 2013-12-17 12:18 ` Sanimir Agovic
  2013-12-18  3:24   ` Joel Brobecker
  2013-12-17 12:18 ` [PATCH v4 05/13] vla: update type from newly created value Sanimir Agovic
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:18 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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 BOUND_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 bound_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 bound_prop and pass
	them to create_range_type_1.
	* gdbtypes.h (struct bound_prop): New struct.
	(create_range_type_1): New function prototype.
	(struct range_bounds): Use struct bound_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 BOUND_UNDEFINED.

Change-Id: I06fbeb877255a541a770c897e4b53d65f21dbbef
---
 gdb/dwarf2read.c | 43 +++++++++++++++++++++++++------------------
 gdb/gdbtypes.c   | 51 +++++++++++++++++++++++++++++++++++++--------------
 gdb/gdbtypes.h   | 53 +++++++++++++++++++++++++++++++++++++++--------------
 gdb/parse.c      |  3 ++-
 4 files changed, 103 insertions(+), 47 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1c7dfc5..6bdd45a 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 dynamic_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 = PROP_CONST;
+  high.kind = PROP_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 == PROP_CONST
+      && !TYPE_UNSIGNED (base_type) && (low.data.const_val & negative_mask))
+    low.data.const_val |= negative_mask;
+  if (high.kind == PROP_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) = PROP_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) = PROP_UNDEFINED;
 
   name = dwarf2_name (die, cu);
   if (name)
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 2470304..1934d9b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -798,19 +798,13 @@ allocate_stub_method (struct type *type)
   return mtype;
 }
 
-/* Create a range type using either a blank type supplied in
-   RESULT_TYPE, or creating a new type, inheriting the objfile from
-   INDEX_TYPE.
-
-   Indices will be of type INDEX_TYPE, and will range from LOW_BOUND
-   to HIGH_BOUND, inclusive.
-
-   FIXME: Maybe we should check the TYPE_CODE of RESULT_TYPE to make
-   sure it is TYPE_CODE_UNDEF before we bash it into a range type?  */
+/* 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 (struct type *result_type, struct type *index_type,
-		   LONGEST low_bound, LONGEST high_bound)
+create_range_type_1 (struct type *result_type, struct type *index_type,
+		     const struct dynamic_prop *low_bound,
+		     const struct dynamic_prop *high_bound)
 {
   if (result_type == NULL)
     result_type = alloc_type_copy (index_type);
@@ -820,17 +814,46 @@ create_range_type (struct type *result_type, struct type *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;
+  TYPE_RANGE_DATA (result_type)->low = *low_bound;
+  TYPE_RANGE_DATA (result_type)->high = *high_bound;
 
-  if (low_bound >= 0)
+  if (low_bound->kind == PROP_CONST && low_bound->data.const_val >= 0)
     TYPE_UNSIGNED (result_type) = 1;
 
   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.
+
+   Indices will be of type INDEX_TYPE, and will range from LOW_BOUND
+   to HIGH_BOUND, inclusive.
+
+   FIXME: Maybe we should check the TYPE_CODE of RESULT_TYPE to make
+   sure it is TYPE_CODE_UNDEF before we bash it into a range type?  */
+
+struct type *
+create_range_type (struct type *result_type, struct type *index_type,
+		   LONGEST low_bound, LONGEST high_bound)
+{
+  struct dynamic_prop low, high;
+
+  low.kind = PROP_CONST;
+  low.data.const_val = low_bound;
+
+  high.kind = PROP_CONST;
+  high.data.const_val = high_bound;
+
+  result_type = create_range_type_1 (result_type, index_type,
+				     &low, &high);
+
+  return result_type;
+}
+
 /* 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.  */
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c7bef5f..6d50e7f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -365,6 +365,28 @@ enum type_instance_flag_value
 #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
 				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 
+/* Used to store a dynamic property.  */
+
+struct dynamic_prop
+{
+  /* Determine which field of the union dynamic_prop.data is used.  */
+  enum
+  {
+    PROP_UNDEFINED,
+    PROP_CONST,
+    PROP_LOCEXPR,
+    PROP_LOCLIST
+  } kind;
+
+  /* Storage for dynamic or static value.  */
+  union data
+  {
+    LONGEST const_val;
+    void *baton;
+  } data;
+};
+
+
 /* Determine which field of the union main_type.fields[x].loc is used.  */
 
 enum field_loc_kind
@@ -589,19 +611,11 @@ struct main_type
     {
       /* Low bound of range.  */
 
-      LONGEST low;
+      struct dynamic_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 dynamic_prop high;
     } *bounds;
 
   } flds_bnds;
@@ -1066,12 +1080,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 == PROP_UNDEFINED)
 #define TYPE_HIGH_BOUND_UNDEFINED(range_type) \
-   TYPE_RANGE_DATA(range_type)->high_undefined
+  (TYPE_RANGE_DATA(range_type)->high.kind == PROP_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 +1546,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 dynamic_prop *,
+					 const struct dynamic_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..b1671f3 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))
+	    = PROP_UNDEFINED;
 	break;
       case tp_function:
 	/* FIXME-type-allocation: need a way to free this type when we are
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 02/13] type: add c99 variable length array support
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
  2013-12-17 12:18 ` [PATCH v4 04/13] vla: enable sizeof operator for indirection Sanimir Agovic
@ 2013-12-17 12:18 ` Sanimir Agovic
  2014-01-15 21:07   ` Tom Tromey
  2013-12-17 12:18 ` [PATCH v4 06/13] vla: print "variable length" for unresolved dynamic bounds Sanimir Agovic
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:18 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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.
	(dwarf2_evaluate_property): New function.
	* dwarf2loc.h (dwarf2_evaluate_property): New function prototype.
	* dwarf2read.c (block_to_locexprbaton): New function.
	(attr_to_dynamic_prop): New function.
	(read_subrange_type): Use attr_to_dynamic_prop to read high bound
	attribute.
	* gdbtypes.c: Include dwarf2loc.h.
	(is_dynamic_type): New function.
	(resolve_dynamic_type): New function.
	(resolve_dynamic_bounds): New function.
	(get_type_length): New function.
	(check_typedef): Use get_type_length to compute type length.
	* gdbtypes.h (TYPE_HIGH_BOUND_KIND): New macro.
	(TYPE_LOW_BOUND_KIND): New macro.
	(is_dynamic_type): New function prototype.
	* value.c (value_from_contents_and_address): Call resolve_dynamic_type
	to resolve dynamic properties of the type. Update comment.
	* valops.c (get_value_at, value_at, value_at_lazy): Update comment.

Change-Id: I59036232c0b93e79427bf34041b3bbf8992e731a
---
 gdb/dwarf2loc.c  | 105 ++++++++++++++++++++++
 gdb/dwarf2loc.h  |  26 ++++++
 gdb/dwarf2read.c | 132 +++++++++++++++++++++------
 gdb/gdbtypes.c   | 265 ++++++++++++++++++++++++++++++++++++++++++++-----------
 gdb/gdbtypes.h   |  10 +++
 gdb/valops.c     |  12 ++-
 gdb/value.c      |  17 ++--
 7 files changed, 479 insertions(+), 88 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 2b1f323..6a36474 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2431,6 +2431,111 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
   return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu, 0);
 }
 
+/* Evaluates a dwarf expression and stores the result in VAL, expecting
+   that the dwarf expression only produces a single CORE_ADDR.  ADDR is a
+   context (location of a variable) and might be needed to evaluate the
+   location expression.
+   Returns 1 on success, 0 otherwise.   */
+
+static 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;
+}
+
+/* See dwarf2loc.h.  */
+
+int
+dwarf2_evaluate_property (const struct dynamic_prop *prop, CORE_ADDR address,
+			  CORE_ADDR *value)
+{
+  if (prop == NULL)
+    return 0;
+
+  switch (prop->kind)
+    {
+    case PROP_LOCEXPR:
+      {
+	const struct dwarf2_property_baton *baton = prop->data.baton;
+
+	return dwarf2_locexpr_baton_eval (&baton->locexpr, address, value);
+      }
+    case PROP_LOCLIST:
+      {
+	struct dwarf2_property_baton *baton = prop->data.baton;
+	struct frame_info *frame = get_selected_frame (NULL);
+	CORE_ADDR pc = get_frame_address_in_block (frame);
+	const gdb_byte *data;
+	struct value *val;
+	size_t size;
+
+	data = dwarf2_find_location_expression (&baton->loc.list, &size, pc);
+	if (data != NULL)
+	  {
+	    val = dwarf2_evaluate_loc_desc (baton->loc.type, frame, data,
+					    size, baton->loc.list.per_cu);
+	    if (!value_optimized_out (val))
+	      *value = value_as_long (value_ind (val));
+	  }
+	return 1;
+      }
+    case PROP_CONST:
+      *value = prop->data.const_val;
+      return 1;
+    }
+
+  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..2155fe1 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);
 
+/* Converts a dynamic property into a static one.  ADDR is the address of
+   the object currently being evaluated and might be nedded.
+   Returns 1 if PROP could be converted and the static value is passed back
+   into VALUE, otherwise returns 0.  */
+int dwarf2_evaluate_property (const struct dynamic_prop *prop,
+			      CORE_ADDR addr, CORE_ADDR *value);
+
 CORE_ADDR dwarf2_read_addr_index (struct dwarf2_per_cu_data *per_cu,
 				  unsigned int addr_index);
 
@@ -135,6 +142,25 @@ struct dwarf2_loclist_baton
   unsigned char from_dwo;
 };
 
+/* A dynamic property is either expressed as a single location expression
+   or a location list in the context of TYPE.  */
+
+struct dwarf2_property_baton
+{
+  union
+  {
+    /* Location expression.  */
+    struct dwarf2_locexpr_baton locexpr;
+
+    /* Location list to be evaluated in the context of TYPE.  */
+    struct
+    {
+      struct dwarf2_loclist_baton list;
+      struct type *type;
+    } loc;
+  };
+};
+
 extern const struct symbol_computed_ops dwarf2_locexpr_funcs;
 extern const struct symbol_computed_ops dwarf2_loclist_funcs;
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6bdd45a..0df385a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14252,6 +14252,110 @@ 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_locexpr_baton (const struct dwarf_block *blk, struct dwarf2_cu *cu,
+			const gdb_byte *additional_data, int extra_size)
+{
+  struct dwarf2_locexpr_baton baton;
+
+  gdb_assert (blk != NULL);
+
+  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 (&cu->objfile->objfile_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 bound_prop.
+   Returns 1 if ATTR could be resolved into PROP, 0 otherwise.  */
+
+static int
+attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
+		      struct dwarf2_cu *cu, struct dynamic_prop *prop)
+{
+  struct dwarf2_property_baton *baton;
+  struct obstack *obstack = &cu->objfile->objfile_obstack;
+
+  if (attr == NULL || prop == NULL)
+    return 0;
+
+  if (attr_form_is_block (attr))
+    {
+      baton = obstack_alloc (obstack, sizeof (*baton));
+      baton->locexpr = block_to_locexpr_baton (DW_BLOCK (attr), cu, NULL, 0);
+      prop->data.baton = baton;
+      prop->kind = PROP_LOCEXPR;
+      gdb_assert (prop->data.baton != NULL);
+    }
+  else if (attr_form_is_ref (attr))
+    {
+      struct dwarf2_cu *target_cu = cu;
+      struct die_info *target_die;
+      struct attribute *target_attr;
+
+      target_die = follow_die_ref (die, attr, &target_cu);
+      target_attr = dwarf2_attr (target_die, DW_AT_location, target_cu);
+      if (target_attr == NULL)
+	return 0;
+
+      if (attr_form_is_section_offset (target_attr))
+	{
+	  baton = obstack_alloc (obstack, sizeof (*baton));
+	  baton->loc.type = die_type (target_die, target_cu);;
+	  fill_in_loclist_baton (cu, &baton->loc.list, target_attr);
+	  prop->data.baton = baton;
+	  prop->kind = PROP_LOCLIST;
+	  gdb_assert (prop->data.baton != NULL);
+	}
+      else if (attr_form_is_block (target_attr))
+	{
+	  const gdb_byte ops[] = {DW_OP_deref};
+
+	  baton = obstack_alloc (obstack, sizeof (*baton));
+	  baton->locexpr = block_to_locexpr_baton (DW_BLOCK (target_attr),
+						   cu, ops, sizeof (ops));
+	  prop->data.baton = baton;
+	  prop->kind = PROP_LOCEXPR;
+	  gdb_assert (prop->data.baton != NULL);
+	}
+    }
+  else if (attr_form_is_constant (attr))
+    {
+      prop->data.const_val = dwarf2_get_attr_constant_value (attr, 0);
+      prop->kind = PROP_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 +14429,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_dynamic_prop (attr, die, cu, &high))
     {
       attr = dwarf2_attr (die, DW_AT_count, cu);
       if (attr)
@@ -14409,12 +14493,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) = PROP_UNDEFINED;
-
   /* Ada expects an empty array on no boundary attributes.  */
   if (attr == NULL && cu->language != language_ada)
     TYPE_HIGH_BOUND_KIND (range_type) = PROP_UNDEFINED;
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 1934d9b..ef3def1 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -854,6 +854,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 == PROP_CONST
+	  && bounds->high.kind == PROP_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.  */
@@ -983,24 +994,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));
@@ -1531,7 +1546,182 @@ 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;
+}
+
+/* See gdbtypes.h.  */
+
+int
+is_dynamic_type (const struct type *type)
+{
+  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.
+   ADDRESS might be needed to resolve the subrange bounds, it is the location
+   of the associated array.  */
+
+static struct type *
+resolve_dynamic_bounds (struct type *type, CORE_ADDR addr)
+{
+  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;
+
+  if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF
+      || TYPE_CODE (type) == TYPE_CODE_PTR)
+    {
+      struct type *copy = copy_type (type);
+
+      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.kind = PROP_UNDEFINED;
+      low_bound.data.const_val = 0;
+    }
+
+  prop = &TYPE_RANGE_DATA (range_type)->high;
+  if (dwarf2_evaluate_property (prop, addr, &value))
+    {
+      high_bound.kind = PROP_CONST;
+      high_bound.data.const_val = value;
+    }
+  else
+    {
+      high_bound.kind = PROP_UNDEFINED;
+      high_bound.data.const_val = 0;
+    }
+
+  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);
+
+  return array_type;
+}
+
+/* See gdbtypes.h  */
+
+struct type *
+resolve_dynamic_type (struct type *type, CORE_ADDR addr)
+{
+  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;
+
+  resolved_type = resolve_dynamic_bounds (type, addr);
+  resolved_type->length = get_type_length (check_typedef (resolved_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.
@@ -1707,44 +1897,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 6d50e7f..ff520038 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1,3 +1,4 @@
+
 /* Internal type definitions for GDB.
 
    Copyright (C) 1992-2013 Free Software Foundation, Inc.
@@ -1570,6 +1571,15 @@ extern struct type *lookup_unsigned_typename (const struct language_defn *,
 extern struct type *lookup_signed_typename (const struct language_defn *,
 					    struct gdbarch *, const char *);
 
+/* Resolves all dynamic values of a type e.g. array bounds to static values.
+   ADDR specifies the location of the variable the type is bound to.
+   If TYPE has no dynamic values returns TYPE otherwise a new type with static
+   values is returned.  */
+extern struct type *resolve_dynamic_type (struct type *type, CORE_ADDR addr);
+
+/* Predicates if the type has dynamic values, which are not resolved yet.  */
+extern int is_dynamic_type (const struct type *type);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
diff --git a/gdb/valops.c b/gdb/valops.c
index d43c758..4d56318 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -902,7 +902,9 @@ value_one (struct type *type)
   return val;
 }
 
-/* Helper function for value_at, value_at_lazy, and value_at_lazy_stack.  */
+/* Helper function for value_at, value_at_lazy, and value_at_lazy_stack.
+   The type of the created value may differ from the passed type TYPE.
+   Make sure to retrieve values new type after this call.  */
 
 static struct value *
 get_value_at (struct type *type, CORE_ADDR addr, int lazy)
@@ -927,7 +929,9 @@ get_value_at (struct type *type, CORE_ADDR addr, int lazy)
    value_at_lazy instead.  value_at_lazy simply records the address of
    the data and sets the lazy-evaluation-required flag.  The lazy flag
    is tested in the value_contents macro, which is used if and when
-   the contents are actually required.
+   the contents are actually required.  The type of the created value
+   may differ from the passed type TYPE.  Make sure to retrieve values
+   new type after this call.
 
    Note: value_at does *NOT* handle embedded offsets; perform such
    adjustments before or after calling it.  */
@@ -938,7 +942,9 @@ value_at (struct type *type, CORE_ADDR addr)
   return get_value_at (type, addr, 0);
 }
 
-/* Return a lazy value with type TYPE located at ADDR (cf. value_at).  */
+/* Return a lazy value with type TYPE located at ADDR (cf. value_at).
+   The type of the created value may differ from the passed type TYPE.
+   Make sure to retrieve values new type after this call.  */
 
 struct value *
 value_at_lazy (struct type *type, CORE_ADDR addr)
diff --git a/gdb/value.c b/gdb/value.c
index a64e7e1..12726a1 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3178,32 +3178,37 @@ value_from_ulongest (struct type *type, ULONGEST num)
 
 
 /* Create a value representing a pointer of type TYPE to the address
-   ADDR.  */
+   ADDR.  The type of the created value may differ from the passed
+   type TYPE.  Make sure to retrieve values new type after this call.  */
 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;
 }
 
 
 /* Create a value of type TYPE whose contents come from VALADDR, if it
    is non-null, and whose memory address (in the inferior) is
-   ADDRESS.  */
+   ADDRESS.  The type of the created value may differ from the passed
+   type TYPE.  Make sure to retrieve values new type after this call.  */
 
 struct value *
 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.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 00/13] C99 variable length array support
@ 2013-12-17 12:18 Sanimir Agovic
  2013-12-17 12:18 ` [PATCH v4 04/13] vla: enable sizeof operator for indirection Sanimir Agovic
                   ` (14 more replies)
  0 siblings, 15 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:18 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

Hello,

this patch series (v3) 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. If 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 v4
=============
 - Documented side effect of value_at, value_at_lazy, value_from_pointer,
 value_from_contents_and_address
 - Removed usage of copy_type_recursive and instead use copy_type selective

Changes in v3
=============
 - Add line breaks to long test expressions.
 - Add side effect check for sizeof/ptype/whatis to vla-datatypes.exp.
 - Add test file for subranges with present DW_AT_count attribute.
 - Add introduction comments to all new functions.
 - Add patch which deals with value contents with constant byte-sequence.
   It will be squashed once the series is approved.
 - Deal with location lists in case we reference a die in DW_AT_location.
   This fixes the crash Pedro was mentioning in:
   https://sourceware.org/ml/gdb-patches/2013-11/msg00646.html
 - Renamed struct dwarf2_bound to dynamic_prop and remove all references
   to dwarf2 entities in gdbtypes.[c,h] introduced by this patch series.
 - Addressed various coding style issues.
 - Fixed typo in commit log.

Changes in v2
=============
 - 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 (13):
  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 "variable length" for unresolved dynamic bounds
  vla: support for DW_AT_count
  vla: resolve dynamic bounds if value contents is a constant
    byte-sequence
  test: cover subranges with present DW_AT_count attribute
  test: multi-dimensional c99 vla.
  test: evaluate pointers to C99 vla correctly.
  test: basic c99 vla tests for C primitives
  test: add mi vla test

 gdb/ada-lang.c                           |  13 +-
 gdb/c-typeprint.c                        |   6 +-
 gdb/cp-valprint.c                        |   2 +
 gdb/d-valprint.c                         |   1 +
 gdb/dwarf2loc.c                          | 105 ++++++++++
 gdb/dwarf2loc.h                          |  26 +++
 gdb/dwarf2read.c                         | 211 ++++++++++++++++-----
 gdb/eval.c                               |  10 +-
 gdb/findvar.c                            |   3 +
 gdb/gdbtypes.c                           | 316 ++++++++++++++++++++++++-------
 gdb/gdbtypes.h                           |  63 ++++--
 gdb/jv-valprint.c                        |   1 +
 gdb/parse.c                              |   3 +-
 gdb/testsuite/gdb.base/vla-datatypes.c   |  86 +++++++++
 gdb/testsuite/gdb.base/vla-datatypes.exp | 138 ++++++++++++++
 gdb/testsuite/gdb.base/vla-multi.c       |  55 ++++++
 gdb/testsuite/gdb.base/vla-multi.exp     |  48 +++++
 gdb/testsuite/gdb.base/vla-ptr.c         |  63 ++++++
 gdb/testsuite/gdb.base/vla-ptr.exp       |  52 +++++
 gdb/testsuite/gdb.dwarf2/count.exp       | 106 +++++++++++
 gdb/testsuite/gdb.mi/mi-vla-c99.exp      |  82 ++++++++
 gdb/testsuite/gdb.mi/vla.c               |  35 ++++
 gdb/valops.c                             |  15 +-
 gdb/value.c                              |  18 +-
 24 files changed, 1317 insertions(+), 141 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.dwarf2/count.exp
 create mode 100644 gdb/testsuite/gdb.mi/mi-vla-c99.exp
 create mode 100644 gdb/testsuite/gdb.mi/vla.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 04/13] vla: enable sizeof operator for indirection
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
@ 2013-12-17 12:18 ` Sanimir Agovic
  2014-01-15 21:28   ` Tom Tromey
  2013-12-17 12:18 ` [PATCH v4 02/13] type: add c99 variable length array support Sanimir Agovic
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:18 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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 sizeof (int) * 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: I3dbe85ebbc87ca93115a1952316b6436f750a16d
---
 gdb/eval.c | 2 ++
 1 file changed, 2 insertions(+)

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 03/13] vla: enable sizeof operator to work with variable length arrays
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (6 preceding siblings ...)
  2013-12-17 12:19 ` [PATCH v4 10/13] test: multi-dimensional c99 vla Sanimir Agovic
@ 2013-12-17 12:19 ` Sanimir Agovic
  2014-01-15 21:24   ` Tom Tromey
  2013-12-17 12:19 ` [PATCH v4 13/13] test: add mi vla test Sanimir Agovic
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:19 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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.

Change-Id: I79656d4ec5bd5d728748a1059ac414a5440eb659
---
 gdb/eval.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

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));
 
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 09/13] test: cover subranges with present DW_AT_count attribute
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (9 preceding siblings ...)
  2013-12-17 12:19 ` [PATCH v4 08/13] vla: resolve dynamic bounds if value contents is a constant byte-sequence Sanimir Agovic
@ 2013-12-17 12:19 ` Sanimir Agovic
  2013-12-17 12:19 ` [PATCH v4 12/13] test: basic c99 vla tests for C primitives Sanimir Agovic
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:19 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

The dwarf attribute DW_AT_count specifies the elements of a subrange.
This test covers subranges with present count but absent upper bound
attribute, both with static and dynamic attribute values.

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

testsuite:
	* gdb.dwarf2/count.exp: New test.

Change-Id: I0a537c23bc7d5e0bffe58d14d0125dc7cd753f48
---
 gdb/testsuite/gdb.dwarf2/count.exp | 106 +++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/count.exp

diff --git a/gdb/testsuite/gdb.dwarf2/count.exp b/gdb/testsuite/gdb.dwarf2/count.exp
new file mode 100644
index 0000000..80744b1
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/count.exp
@@ -0,0 +1,106 @@
+# 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/>.
+
+# Tests to cover DW_AT_count attribute in subranges.
+
+load_lib dwarf.exp
+
+# Only run on targets which support dwarf and gas.
+if { ![dwarf2_support] } {
+    return 0
+}
+
+standard_testfile main.c count.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {} {
+	compile_unit {{language @DW_LANG_C99}} {
+	    declare_labels char_label array_label static_array_label
+	    declare_labels var_label
+
+	    char_label: base_type {
+		{name char}
+		{encoding @DW_ATE_signed}
+		{byte_size 1 DW_FORM_sdata}
+	    }
+
+	    array_label: array_type {
+		{type :$char_label}
+	    } {
+		subrange_type {
+		    {count {DW_OP_lit5} SPECIAL_expr}
+		    {type :$char_label}
+		}
+	    }
+
+	    static_array_label: array_type {
+		{type :$char_label}
+	    } {
+		subrange_type {
+		    {count 5 DW_FORM_sdata}
+		    {type :$char_label}
+		}
+	    }
+
+	    DW_TAG_variable {
+		{name array}
+		{type :$array_label}
+		{const_value hello DW_FORM_block1}
+	    }
+
+	    DW_TAG_variable {
+		{name static_array}
+		{type :$static_array_label}
+		{const_value world DW_FORM_block1}
+	    }
+	}
+    }
+}
+
+if { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
+	  object {nodebug}] != "" } {
+    return -1
+}
+
+if { [gdb_compile $asm_file ${binfile}2.o object {nodebug}] != "" } {
+    return -1
+}
+
+if { [gdb_compile [list ${binfile}1.o ${binfile}2.o] \
+	  "${binfile}" executable {}] != "" } {
+    return -1
+}
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -readnow"]
+clean_restart ${testfile}
+set GDBFLAGS $saved_gdbflags
+
+if ![runto_main] {
+    perror "couldn't run to main"
+    return -1
+}
+
+gdb_test "ptype array" "type = char \\\[5\\\]" "ptype array"
+gdb_test "whatis array" "type = char \\\[5\\\]" "whatis array"
+gdb_test "print array" "\"hello\"" "print array"
+gdb_test "print sizeof array" "5" "print sizeof array"
+
+gdb_test "ptype static_array" "type = char \\\[5\\\]" "ptype static_array"
+gdb_test "whatis static_array" "type = char \\\[5\\\]" "whatis static_array"
+gdb_test "print static_array" "\"world\"" "print static_array"
+gdb_test "print sizeof static_array" "5" "print sizeof static_array"
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 13/13] test: add mi vla test
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (7 preceding siblings ...)
  2013-12-17 12:19 ` [PATCH v4 03/13] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
@ 2013-12-17 12:19 ` Sanimir Agovic
  2013-12-17 12:19 ` [PATCH v4 08/13] vla: resolve dynamic bounds if value contents is a constant byte-sequence Sanimir Agovic
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:19 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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: If86f3a770c9c1d6b415057a40f51ba2a6f20ba0e
---
 gdb/testsuite/gdb.mi/mi-vla-c99.exp | 82 +++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/vla.c          | 35 ++++++++++++++++
 2 files changed, 117 insertions(+)
 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..9d0c6cc
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-vla-c99.exp
@@ -0,0 +1,82 @@
+# 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.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 10/13] test: multi-dimensional c99 vla.
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (5 preceding siblings ...)
  2013-12-17 12:18 ` [PATCH v4 07/13] vla: support for DW_AT_count Sanimir Agovic
@ 2013-12-17 12:19 ` Sanimir Agovic
  2013-12-17 12:19 ` [PATCH v4 03/13] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:19 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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: If9cf7517301e0558aced1358723e1b62f6df0c97
---
 gdb/testsuite/gdb.base/vla-multi.c   | 55 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/vla-multi.exp | 48 +++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)
 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..53f626f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-multi.exp
@@ -0,0 +1,48 @@
+# 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.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 08/13] vla: resolve dynamic bounds if value contents is a constant byte-sequence
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (8 preceding siblings ...)
  2013-12-17 12:19 ` [PATCH v4 13/13] test: add mi vla test Sanimir Agovic
@ 2013-12-17 12:19 ` Sanimir Agovic
  2013-12-17 12:19 ` [PATCH v4 09/13] test: cover subranges with present DW_AT_count attribute Sanimir Agovic
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:19 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

A variable location might be a constant value and therefore no inferior memory
access is needed to read the content. In this case try to resolve the type
bounds.

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

	* findvar.c (default_read_var_value): Resolve dynamic bounds if location
	points to a constant blob.

Change-Id: I73c31b64976b7d2c603c68d4584909502c8c5928
---
 gdb/findvar.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/findvar.c b/gdb/findvar.c
index ec6afd6..a543dc4 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -468,6 +468,9 @@ default_read_var_value (struct symbol *var, struct frame_info *frame)
       return v;
 
     case LOC_CONST_BYTES:
+      if (is_dynamic_type (type))
+	/* Value is a constant byte-sequence and needs no memory access.  */
+	type = resolve_dynamic_type (type, /* Unused address.  */ 0);
       v = allocate_value (type);
       memcpy (value_contents_raw (v), SYMBOL_VALUE_BYTES (var),
 	      TYPE_LENGTH (type));
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 12/13] test: basic c99 vla tests for C primitives
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (10 preceding siblings ...)
  2013-12-17 12:19 ` [PATCH v4 09/13] test: cover subranges with present DW_AT_count attribute Sanimir Agovic
@ 2013-12-17 12:19 ` Sanimir Agovic
  2014-01-15 21:39   ` Tom Tromey
  2013-12-17 12:19 ` [PATCH v4 11/13] test: evaluate pointers to C99 vla correctly Sanimir Agovic
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:19 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

2013-10-18  Keven Boell  <keven.boell@intel.com>
            Sanimir Agovic  <sanimir.agovic@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: I7e9ac796ac7e9afa983d60e6d4b506017e4afe54
---
 gdb/testsuite/gdb.base/vla-datatypes.c   |  86 +++++++++++++++++++
 gdb/testsuite/gdb.base/vla-datatypes.exp | 138 +++++++++++++++++++++++++++++++
 2 files changed, 224 insertions(+)
 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..c251709
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-datatypes.exp
@@ -0,0 +1,138 @@
+# 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 the 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"
+
+# Check side effects for sizeof argument.
+set sizeof_int [get_sizeof "int" 4]
+gdb_test_no_output  "set variable int_vla\[0\] = 42" \
+                    "set variable int_vla\[0\] = 42"
+
+gdb_test "print sizeof (++int_vla\[0\])" "\\$\\d+ = ${sizeof_int}" \
+         "print sizeof (++int_vla\[0\])"
+gdb_test "print int_vla\[0\]" "\\$\\d+ = 42" \
+         "print int_vla\[0\] - sizeof no side effects"
+
+gdb_test "ptype ++int_vla\[0\]" "type = int" "ptype ++int_vla\[0\]"
+gdb_test "print int_vla\[0\]" "\\$\\d+ = 42" \
+         "print int_vla\[0\] - ptype no side effects"
+
+gdb_test "whatis ++int_vla\[0\]" "type = int" "whatis ++int_vla\[0\]"
+gdb_test "print int_vla\[0\]" "\\$\\d+ = 42" \
+         "print int_vla\[0\] - whatis no side effects"
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 11/13] test: evaluate pointers to C99 vla correctly.
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (11 preceding siblings ...)
  2013-12-17 12:19 ` [PATCH v4 12/13] test: basic c99 vla tests for C primitives Sanimir Agovic
@ 2013-12-17 12:19 ` Sanimir Agovic
  2013-12-18  3:01 ` [PATCH v4 00/13] C99 variable length array support Joel Brobecker
  2014-01-15 21:41 ` Tom Tromey
  14 siblings, 0 replies; 36+ messages in thread
From: Sanimir Agovic @ 2013-12-17 12:19 UTC (permalink / raw)
  To: tromey, palves, xdje42; +Cc: gdb-patches, keven.boell

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: Iea007544b3e95f9da4ca746d142e7be0350cf87b
---
 gdb/testsuite/gdb.base/vla-ptr.c   | 63 ++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/vla-ptr.exp | 52 +++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)
 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..a7785b0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-ptr.exp
@@ -0,0 +1,52 @@
+# 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.8.3.1

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 00/13] C99 variable length array support
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (12 preceding siblings ...)
  2013-12-17 12:19 ` [PATCH v4 11/13] test: evaluate pointers to C99 vla correctly Sanimir Agovic
@ 2013-12-18  3:01 ` Joel Brobecker
  2014-01-15 21:41 ` Tom Tromey
  14 siblings, 0 replies; 36+ messages in thread
From: Joel Brobecker @ 2013-12-18  3:01 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

> this patch series (v3) 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

I haven't had time so far to say how excited I am to see this patch
series. I think this is going to be extremely useful, and not just
for C. I can see Fortran being mentioned, but I think Pascal and Ada
also will be able to benefit from it.

So Thank You!

-- 
Joel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
  2013-12-17 12:18 ` [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
@ 2013-12-18  3:24   ` Joel Brobecker
  2013-12-18 15:59     ` Agovic, Sanimir
  0 siblings, 1 reply; 36+ messages in thread
From: Joel Brobecker @ 2013-12-18  3:24 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: tromey, palves, xdje42, gdb-patches, keven.boell

> 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 BOUND_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 bound_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 bound_prop and pass
> 	them to create_range_type_1.
> 	* gdbtypes.h (struct bound_prop): New struct.
> 	(create_range_type_1): New function prototype.
> 	(struct range_bounds): Use struct bound_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 BOUND_UNDEFINED.

Just a suggestion, which you may choose to ignore.

I think that the _1 suffix is usually used when the function performs
the private portion of a more public routine.  But in this case,
create_range_type_1 is meant to be a public routine, and the _1
suffix is not very explicit.  IMO, what would be ideal would be to
rename the current create_range_type into "create_static_range_type",
and then make create_range_type_1 the new create_range_type. I checked
the GDB tree, and there aren't that many calls to update. If people
prefer, I can even take care of that myself once the patche series
has gone in. Otherwise, another compromise solution is to rename
create_range_type_1 to create_range_type_full (for instance).

> +/* Used to store a dynamic property.  */
> +
> +struct dynamic_prop
> +{
> +  /* Determine which field of the union dynamic_prop.data is used.  */
> +  enum
> +  {
> +    PROP_UNDEFINED,
> +    PROP_CONST,
> +    PROP_LOCEXPR,
> +    PROP_LOCLIST
> +  } kind;
> +
> +  /* Storage for dynamic or static value.  */
> +  union data
> +  {
> +    LONGEST const_val;
> +    void *baton;
> +  } data;

Would you mind documenting each enumeration and union field?

> +#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 == PROP_UNDEFINED)
>  #define TYPE_HIGH_BOUND_UNDEFINED(range_type) \
> -   TYPE_RANGE_DATA(range_type)->high_undefined
> +  (TYPE_RANGE_DATA(range_type)->high.kind == PROP_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

For the record, I considered the idea of adding asserts in there,
in order to get an internal error instead of an odd bug when accessing
the wrong field. But this requires us making these macros read-only
accessors, rather than read-write. A quick experiment showed that
some units are using them to write some fields, and so we would need
to audit that first. It's a desirable change on its own, IMO, regardless
of whether we thinking adding the assert is desirable or not, but I don't
want to put the burden on this patch series, which seems already quite
sizeable on its own already.

-- 
Joel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 05/13] vla: update type from newly created value
  2013-12-17 12:18 ` [PATCH v4 05/13] vla: update type from newly created value Sanimir Agovic
@ 2013-12-18  3:44   ` Joel Brobecker
  0 siblings, 0 replies; 36+ messages in thread
From: Joel Brobecker @ 2013-12-18  3:44 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: tromey, palves, xdje42, gdb-patches, keven.boell

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

This patch makes me a little nervous, but unfortunately, the only option
I have to help with that is going to be a little labor-intensive, so
may not be practical. I'll leave it to you and the maintainers who have
been reviewing your patches so far.

I see that the type re-fetch was not added systematically, but only in
some of the locations where value_at/value_at_lazy/
value_from_contents_and_address are being called?  Was it in order
to fix some regressions revealed by the testsuite?  If that's the case,
I think this patch should be merged with the patch that causes the
regressions, just to make sure that each patch individually causes
no regression. This also helps when using the bisect feature of git.

It looks like it's very easy to miss a case where we should re-fetch
the type, which is what makes me slightly nervous. It's also putting
the onus on the user to remember that value_[...] may return a value
whose type is different from the one he has. So, now the labor-intensive
suggestion: How about adding a type parameter to those 3 functions,
to force the user to think about it and give him a chance to DTRT from
the start? This parameter would be allowed to be NULL in the cases were
we know we don't need to re-fecth. Another advantage I see from
this approach is that vendor code-bases would stop building as soon as
your change is imported, directing their attention to this change and
the implicit question, for each location, that needs to be answered.

I did a quick grep, and counted about 70 locations that would need
to be adjusted if we were to go that route.

-- 
Joel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
  2013-12-18  3:24   ` Joel Brobecker
@ 2013-12-18 15:59     ` Agovic, Sanimir
  2014-01-15 21:39       ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Agovic, Sanimir @ 2013-12-18 15:59 UTC (permalink / raw)
  To: 'Joel Brobecker'
  Cc: tromey, palves, xdje42, gdb-patches, Boell, Keven

Thanks for your review.

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Joel Brobecker
> Sent: Wednesday, December 18, 2013 04:24 AM
> To: Agovic, Sanimir
> Cc: tromey@redhat.com; palves@redhat.com; xdje42@gmail.com; gdb-patches@sourceware.org;
> Boell, Keven
> Subject: Re: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
> 
> > 	* dwarf2read.c (read_subrange_type): Use struct bound_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 bound_prop and pass
> > 	them to create_range_type_1.
> > 	* gdbtypes.h (struct bound_prop): New struct.
> > 	(create_range_type_1): New function prototype.
> > 	(struct range_bounds): Use struct bound_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 BOUND_UNDEFINED.
> 
> Just a suggestion, which you may choose to ignore.
> 
> I think that the _1 suffix is usually used when the function performs
> the private portion of a more public routine.  But in this case,
> create_range_type_1 is meant to be a public routine, and the _1
> suffix is not very explicit.  IMO, what would be ideal would be to
> rename the current create_range_type into "create_static_range_type",
> and then make create_range_type_1 the new create_range_type. I checked
> the GDB tree, and there aren't that many calls to update. If people
> prefer, I can even take care of that myself once the patche series
> has gone in. Otherwise, another compromise solution is to rename
> create_range_type_1 to create_range_type_full (for instance).
> 
Sounds good to me. I will prepend a patch doing the 
create_range_type -> create_static_range_type thingy and use create_range_type
in this patch instead of create_range_type_1.

> > +/* Used to store a dynamic property.  */
> > +
> > +struct dynamic_prop
> > +{
> > +  /* Determine which field of the union dynamic_prop.data is used.  */
> > +  enum
> > +  {
> > +    PROP_UNDEFINED,
> > +    PROP_CONST,
> > +    PROP_LOCEXPR,
> > +    PROP_LOCLIST
> > +  } kind;
> > +
> > +  /* Storage for dynamic or static value.  */
> > +  union data
> > +  {
> > +    LONGEST const_val;
> > +    void *baton;
> > +  } data;
> 
> Would you mind documenting each enumeration and union field?
> 
Definitely, wired that I missed it. Thanks.

> > +#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 == PROP_UNDEFINED)
> >  #define TYPE_HIGH_BOUND_UNDEFINED(range_type) \
> > -   TYPE_RANGE_DATA(range_type)->high_undefined
> > +  (TYPE_RANGE_DATA(range_type)->high.kind == PROP_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
> 
> For the record, I considered the idea of adding asserts in there,
> in order to get an internal error instead of an odd bug when accessing
> the wrong field.
>
Indeed, I have spent some time debugging just to figure out I passed the
wrong "type" to the macros.

> But this requires us making these macros read-only
> accessors, rather than read-write. A quick experiment showed that
> some units are using them to write some fields, and so we would need
> to audit that first. It's a desirable change on its own, IMO, regardless
> of whether we thinking adding the assert is desirable or not, but I don't
> want to put the burden on this patch series, which seems already quite
> sizeable on its own already.
> 
I agree here as well but I`d like to have this kind of refactoring's separated
from the patch series. Once the initial vla support is in I can have a closer look
at these macros.

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 02/13] type: add c99 variable length array support
  2013-12-17 12:18 ` [PATCH v4 02/13] type: add c99 variable length array support Sanimir Agovic
@ 2014-01-15 21:07   ` Tom Tromey
  2014-01-16 17:01     ` Agovic, Sanimir
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2014-01-15 21:07 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: palves, xdje42, gdb-patches, keven.boell

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

Sanimir> +int
Sanimir> +dwarf2_evaluate_property (const struct dynamic_prop *prop, CORE_ADDR address,
Sanimir> +			  CORE_ADDR *value)
Sanimir> +{
[...]
Sanimir> +	    if (!value_optimized_out (val))
Sanimir> +	      *value = value_as_long (value_ind (val));
Sanimir> +	  }
Sanimir> +	return 1;

This particular branch can return 1 but not set *value.
That seems wrong.

Sanimir> +      else if (attr_form_is_block (target_attr))
Sanimir> +	{
Sanimir> +	  const gdb_byte ops[] = {DW_OP_deref};
Sanimir> +
Sanimir> +	  baton = obstack_alloc (obstack, sizeof (*baton));
Sanimir> +	  baton->locexpr = block_to_locexpr_baton (DW_BLOCK (target_attr),
Sanimir> +						   cu, ops, sizeof (ops));

I noted in an earlier review that I think this approach will not work.
Not sure if I'm misunderstanding something; but I dug up the old thread
and you didn't respond to this bit:

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

The rest of this seemed fine to me.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 03/13] vla: enable sizeof operator to work with variable length arrays
  2013-12-17 12:19 ` [PATCH v4 03/13] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
@ 2014-01-15 21:24   ` Tom Tromey
  0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2014-01-15 21:24 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: palves, xdje42, gdb-patches, keven.boell

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

Sanimir> 2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
Sanimir>             Keven Boell  <keven.boell@intel.com>
Sanimir> 	* eval.c (evaluate_subexp_for_sizeof): If the type passed to sizeof is
Sanimir> 	dynamic  evaluate the argument to compute the length.

I think I finally understood this one.
Looks good, thanks.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 04/13] vla: enable sizeof operator for indirection
  2013-12-17 12:18 ` [PATCH v4 04/13] vla: enable sizeof operator for indirection Sanimir Agovic
@ 2014-01-15 21:28   ` Tom Tromey
  2014-01-16 17:02     ` Agovic, Sanimir
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2014-01-15 21:28 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: palves, xdje42, gdb-patches, keven.boell

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

Sanimir> 2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
Sanimir>             Keven Boell  <keven.boell@intel.com>
Sanimir> 	* eval.c (evaluate_subexp_for_sizeof): Create a indirect value and
Sanimir> 	retrieve the dynamic type size.

I neglected to mention it in the previous patch, but when changing one
case in a function like this, that consists of little but a big switch,
it's normal to mention the case, like:

	* eval.c (evaluate_subexp_for_sizeof) <UNOP_IND>: ...

The patch looks good.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
  2013-12-18 15:59     ` Agovic, Sanimir
@ 2014-01-15 21:39       ` Tom Tromey
  2014-01-16  2:45         ` Joel Brobecker
  2014-01-16 17:03         ` Agovic, Sanimir
  0 siblings, 2 replies; 36+ messages in thread
From: Tom Tromey @ 2014-01-15 21:39 UTC (permalink / raw)
  To: Agovic, Sanimir
  Cc: 'Joel Brobecker', palves, xdje42, gdb-patches, Boell, Keven

Joel> I think that the _1 suffix is usually used when the function performs
Joel> the private portion of a more public routine.  But in this case,
Joel> create_range_type_1 is meant to be a public routine, and the _1
Joel> suffix is not very explicit.  IMO, what would be ideal would be to
Joel> rename the current create_range_type into "create_static_range_type",
Joel> and then make create_range_type_1 the new create_range_type. I checked
Joel> the GDB tree, and there aren't that many calls to update. If people
Joel> prefer, I can even take care of that myself once the patche series
Joel> has gone in. Otherwise, another compromise solution is to rename
Joel> create_range_type_1 to create_range_type_full (for instance).

Sanimir> Sounds good to me. I will prepend a patch doing the
Sanimir> create_range_type -> create_static_range_type thingy and use
Sanimir> create_range_type in this patch instead of create_range_type_1.

I guess this is one of the remaining blockers for this series now.
Though since Joel was agreeable I think it would be fine if you'd prefer
to do it as a follow-up.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
  2013-12-17 12:19 ` [PATCH v4 12/13] test: basic c99 vla tests for C primitives Sanimir Agovic
@ 2014-01-15 21:39   ` Tom Tromey
  2014-01-16 17:02     ` Agovic, Sanimir
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2014-01-15 21:39 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: palves, xdje42, gdb-patches, keven.boell

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

Sanimir> +gdb_test "print sizeof (++int_vla\[0\])" "\\$\\d+ = ${sizeof_int}" \
Sanimir> +         "print sizeof (++int_vla\[0\])"
Sanimir> +gdb_test "print int_vla\[0\]" "\\$\\d+ = 42" \
Sanimir> +         "print int_vla\[0\] - sizeof no side effects"

I didn't see any tests for the case where there should be a side effect.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 00/13] C99 variable length array support
  2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
                   ` (13 preceding siblings ...)
  2013-12-18  3:01 ` [PATCH v4 00/13] C99 variable length array support Joel Brobecker
@ 2014-01-15 21:41 ` Tom Tromey
  2014-01-16 17:05   ` Agovic, Sanimir
  14 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2014-01-15 21:41 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: palves, xdje42, gdb-patches, keven.boell

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

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

Thanks.

I went through the series again and sent my notes.
I think the ones I did not reply to this time were ok according to my
reviews of v2, though I found it a little hard to keep track this time.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
  2014-01-15 21:39       ` Tom Tromey
@ 2014-01-16  2:45         ` Joel Brobecker
  2014-01-16 17:03         ` Agovic, Sanimir
  1 sibling, 0 replies; 36+ messages in thread
From: Joel Brobecker @ 2014-01-16  2:45 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Agovic, Sanimir, palves, xdje42, gdb-patches, Boell, Keven

> Sanimir> Sounds good to me. I will prepend a patch doing the
> Sanimir> create_range_type -> create_static_range_type thingy and use
> Sanimir> create_range_type in this patch instead of create_range_type_1.
> 
> I guess this is one of the remaining blockers for this series now.
> Though since Joel was agreeable I think it would be fine if you'd prefer
> to do it as a follow-up.

... and I am still happy lending a hand for that particular task!
Just let me know. :)

-- 
Joel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 02/13] type: add c99 variable length array support
  2014-01-15 21:07   ` Tom Tromey
@ 2014-01-16 17:01     ` Agovic, Sanimir
  0 siblings, 0 replies; 36+ messages in thread
From: Agovic, Sanimir @ 2014-01-16 17:01 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: palves, xdje42, gdb-patches, Boell, Keven

Thanks for your review.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, January 15, 2014 10:07 PM
> To: Agovic, Sanimir
> Cc: palves@redhat.com; xdje42@gmail.com; gdb-patches@sourceware.org; Boell, Keven
> Subject: Re: [PATCH v4 02/13] type: add c99 variable length array support
> 
> >>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> +int
> Sanimir> +dwarf2_evaluate_property (const struct dynamic_prop *prop, CORE_ADDR address,
> Sanimir> +			  CORE_ADDR *value)
> Sanimir> +{
> [...]
> Sanimir> +	    if (!value_optimized_out (val))
> Sanimir> +	      *value = value_as_long (value_ind (val));
> Sanimir> +	  }
> Sanimir> +	return 1;
> 
> This particular branch can return 1 but not set *value.
> That seems wrong.
> 
Fixed.

> Sanimir> +      else if (attr_form_is_block (target_attr))
> Sanimir> +	{
> Sanimir> +	  const gdb_byte ops[] = {DW_OP_deref};
> Sanimir> +
> Sanimir> +	  baton = obstack_alloc (obstack, sizeof (*baton));
> Sanimir> +	  baton->locexpr = block_to_locexpr_baton (DW_BLOCK (target_attr),
> Sanimir> +						   cu, ops, sizeof (ops));
> 
> I noted in an earlier review that I think this approach will not work.
> Not sure if I'm misunderstanding something; but I dug up the old thread
> and you didn't respond to this bit:
> 
>     https://sourceware.org/ml/gdb-patches/2013-11/msg00785.html
> 
Thanks, I got it now. I`m working on it.

 -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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
  2014-01-15 21:39   ` Tom Tromey
@ 2014-01-16 17:02     ` Agovic, Sanimir
  2014-01-16 17:33       ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Agovic, Sanimir @ 2014-01-16 17:02 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: palves, xdje42, gdb-patches, Boell, Keven

Thanks for your review.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, January 15, 2014 10:39 PM
> To: Agovic, Sanimir
> Cc: palves@redhat.com; xdje42@gmail.com; gdb-patches@sourceware.org; Boell, Keven
> Subject: Re: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
> 
> >>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> +gdb_test "print sizeof (++int_vla\[0\])" "\\$\\d+ = ${sizeof_int}" \
> Sanimir> +         "print sizeof (++int_vla\[0\])"
> Sanimir> +gdb_test "print int_vla\[0\]" "\\$\\d+ = 42" \
> Sanimir> +         "print int_vla\[0\] - sizeof no side effects"
> 
> I didn't see any tests for the case where there should be a side effect.
> 
This line from above:
 
  gdb_test "print sizeof (++int_vla\[0\])" [...]

Is used to ensure that no side effect happen to arguments passed to sizeof.

The test is trying to express the following: 

  int i = 42; sizeof(++i); assert (i == 42)

Is it OK?

 -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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 04/13] vla: enable sizeof operator for indirection
  2014-01-15 21:28   ` Tom Tromey
@ 2014-01-16 17:02     ` Agovic, Sanimir
  0 siblings, 0 replies; 36+ messages in thread
From: Agovic, Sanimir @ 2014-01-16 17:02 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: palves, xdje42, gdb-patches, Boell, Keven

Thanks for you review.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, January 15, 2014 10:28 PM
> To: Agovic, Sanimir
> Cc: palves@redhat.com; xdje42@gmail.com; gdb-patches@sourceware.org; Boell, Keven
> Subject: Re: [PATCH v4 04/13] vla: enable sizeof operator for indirection
> 
> >>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> 2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
> Sanimir>             Keven Boell  <keven.boell@intel.com>
> Sanimir> 	* eval.c (evaluate_subexp_for_sizeof): Create a indirect value and
> Sanimir> 	retrieve the dynamic type size.
> 
> I neglected to mention it in the previous patch, but when changing one
> case in a function like this, that consists of little but a big switch,
> it's normal to mention the case, like:
> 
> 	* eval.c (evaluate_subexp_for_sizeof) <UNOP_IND>: ...
> 
Will add the mentioned switch case, did not know. Thanks again.

 -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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
  2014-01-15 21:39       ` Tom Tromey
  2014-01-16  2:45         ` Joel Brobecker
@ 2014-01-16 17:03         ` Agovic, Sanimir
  2014-01-16 17:39           ` Tom Tromey
  1 sibling, 1 reply; 36+ messages in thread
From: Agovic, Sanimir @ 2014-01-16 17:03 UTC (permalink / raw)
  To: 'Tom Tromey', 'Joel Brobecker'
  Cc: palves, xdje42, gdb-patches, Boell, Keven

Thanks for your review.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, January 15, 2014 10:39 PM
> To: Agovic, Sanimir
> Cc: 'Joel Brobecker'; palves@redhat.com; xdje42@gmail.com; gdb-patches@sourceware.org;
> Boell, Keven
> Subject: Re: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
> 
> Joel> I think that the _1 suffix is usually used when the function performs
> Joel> the private portion of a more public routine.  But in this case,
> Joel> create_range_type_1 is meant to be a public routine, and the _1
> Joel> suffix is not very explicit.  IMO, what would be ideal would be to
> Joel> rename the current create_range_type into "create_static_range_type",
> Joel> and then make create_range_type_1 the new create_range_type. I checked
> Joel> the GDB tree, and there aren't that many calls to update. If people
> Joel> prefer, I can even take care of that myself once the patche series
> Joel> has gone in. Otherwise, another compromise solution is to rename
> Joel> create_range_type_1 to create_range_type_full (for instance).
> 
> Sanimir> Sounds good to me. I will prepend a patch doing the
> Sanimir> create_range_type -> create_static_range_type thingy and use
> Sanimir> create_range_type in this patch instead of create_range_type_1.
> 
> I guess this is one of the remaining blockers for this series now.
> Though since Joel was agreeable I think it would be fine if you'd prefer
> to do it as a follow-up.
> 
I had already prepared a patch doing the necessary renaming upfront. If it is OK
to you and/or Joel I like to keep it as the first patch in my series.

 -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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 00/13] C99 variable length array support
  2014-01-15 21:41 ` Tom Tromey
@ 2014-01-16 17:05   ` Agovic, Sanimir
  2014-01-16 22:11     ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Agovic, Sanimir @ 2014-01-16 17:05 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: palves, xdje42, gdb-patches, Boell, Keven

Thanks for your review.


> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, January 15, 2014 10:41 PM
> To: Agovic, Sanimir
> Cc: palves@redhat.com; xdje42@gmail.com; gdb-patches@sourceware.org; Boell, Keven
> Subject: Re: [PATCH v4 00/13] C99 variable length array support
> 
> >>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> this patch series (v3) add C99 variable length support to gdb.
> 
> Thanks.
> 
> I went through the series again and sent my notes.
> I think the ones I did not reply to this time were ok according to my
> reviews of v2, though I found it a little hard to keep track this time.
> 
Incorporating feedback should be more timely from now on. Besides that if
you or other reviewers of the patch series have suggestions how I can help
you in keeping track of changes, please tell me.

 -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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
  2014-01-16 17:02     ` Agovic, Sanimir
@ 2014-01-16 17:33       ` Tom Tromey
  2014-01-17 13:36         ` Agovic, Sanimir
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2014-01-16 17:33 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: palves, xdje42, gdb-patches, Boell, Keven

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

Tom> I didn't see any tests for the case where there should be a side effect.

Sanimir> This line from above:
Sanimir>   gdb_test "print sizeof (++int_vla\[0\])" [...]
Sanimir> Is used to ensure that no side effect happen to arguments
Sanimir> passed to sizeof.
Sanimir> The test is trying to express the following: 
Sanimir>   int i = 42; sizeof(++i); assert (i == 42)
Sanimir> Is it OK?

Yeah, that's good -- but I think there are also cases where a side
effect is expected, and it would be good to have a test for that as
well.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses
  2014-01-16 17:03         ` Agovic, Sanimir
@ 2014-01-16 17:39           ` Tom Tromey
  0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2014-01-16 17:39 UTC (permalink / raw)
  To: Agovic, Sanimir
  Cc: 'Joel Brobecker', palves, xdje42, gdb-patches, Boell, Keven

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

Sanimir> I had already prepared a patch doing the necessary renaming
Sanimir> upfront. If it is OK to you and/or Joel I like to keep it as
Sanimir> the first patch in my series.

Totally fine.  Thank you.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 00/13] C99 variable length array support
  2014-01-16 17:05   ` Agovic, Sanimir
@ 2014-01-16 22:11     ` Tom Tromey
  0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2014-01-16 22:11 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: palves, xdje42, gdb-patches, Boell, Keven

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

Sanimir> Incorporating feedback should be more timely from now
Sanimir> on. Besides that if you or other reviewers of the patch series
Sanimir> have suggestions how I can help you in keeping track of
Sanimir> changes, please tell me.

I suppose we could adopt the Reviewed-by convention.
It's fine by me to keep doing it manually though.
I'll keep better notes or something.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
  2014-01-16 17:33       ` Tom Tromey
@ 2014-01-17 13:36         ` Agovic, Sanimir
  2014-01-20  5:47           ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Agovic, Sanimir @ 2014-01-17 13:36 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: palves, xdje42, gdb-patches, Boell, Keven

> >>>>> "Sanimir" == Agovic, Sanimir <sanimir.agovic@intel.com> writes:
> 
> Tom> I didn't see any tests for the case where there should be a side effect.
> 
> Sanimir> This line from above:
> Sanimir>   gdb_test "print sizeof (++int_vla\[0\])" [...]
> Sanimir> Is used to ensure that no side effect happen to arguments
> Sanimir> passed to sizeof.
> Sanimir> The test is trying to express the following:
> Sanimir>   int i = 42; sizeof(++i); assert (i == 42)
> Sanimir> Is it OK?
> 
> Yeah, that's good -- but I think there are also cases where a side
> effect is expected, and it would be good to have a test for that as
> well.
> 
Can you please guide me and point out some cases? I will add them to
the test. Thanks.

 -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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
  2014-01-17 13:36         ` Agovic, Sanimir
@ 2014-01-20  5:47           ` Tom Tromey
  2014-01-20  9:32             ` Agovic, Sanimir
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2014-01-20  5:47 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: palves, xdje42, gdb-patches, Boell, Keven

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

Sanimir> Can you please guide me and point out some cases? I will add
Sanimir> them to the test. Thanks.

There are some good ones in the GCC test suite.
For example, gcc/testsuite/gcc.dg/vla-4.c; vla-15.c has one additional
test.

Tom

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
  2014-01-20  5:47           ` Tom Tromey
@ 2014-01-20  9:32             ` Agovic, Sanimir
  0 siblings, 0 replies; 36+ messages in thread
From: Agovic, Sanimir @ 2014-01-20  9:32 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: palves, xdje42, gdb-patches, Boell, Keven

Good hint! Did not looked into gcc-vla tests, thanks.

 -Sanimir

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Monday, January 20, 2014 06:47 AM
> To: Agovic, Sanimir
> Cc: palves@redhat.com; xdje42@gmail.com; gdb-patches@sourceware.org; Boell, Keven
> Subject: Re: [PATCH v4 12/13] test: basic c99 vla tests for C primitives
> 
> >>>>> "Sanimir" == Agovic, Sanimir <sanimir.agovic@intel.com> writes:
> 
> Sanimir> Can you please guide me and point out some cases? I will add
> Sanimir> them to the test. Thanks.
> 
> There are some good ones in the GCC test suite.
> For example, gcc/testsuite/gcc.dg/vla-4.c; vla-15.c has one additional
> test.
> 
> Tom
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

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2014-01-20  9:32 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-17 12:18 [PATCH v4 00/13] C99 variable length array support Sanimir Agovic
2013-12-17 12:18 ` [PATCH v4 04/13] vla: enable sizeof operator for indirection Sanimir Agovic
2014-01-15 21:28   ` Tom Tromey
2014-01-16 17:02     ` Agovic, Sanimir
2013-12-17 12:18 ` [PATCH v4 02/13] type: add c99 variable length array support Sanimir Agovic
2014-01-15 21:07   ` Tom Tromey
2014-01-16 17:01     ` Agovic, Sanimir
2013-12-17 12:18 ` [PATCH v4 06/13] vla: print "variable length" for unresolved dynamic bounds Sanimir Agovic
2013-12-17 12:18 ` [PATCH v4 01/13] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
2013-12-18  3:24   ` Joel Brobecker
2013-12-18 15:59     ` Agovic, Sanimir
2014-01-15 21:39       ` Tom Tromey
2014-01-16  2:45         ` Joel Brobecker
2014-01-16 17:03         ` Agovic, Sanimir
2014-01-16 17:39           ` Tom Tromey
2013-12-17 12:18 ` [PATCH v4 05/13] vla: update type from newly created value Sanimir Agovic
2013-12-18  3:44   ` Joel Brobecker
2013-12-17 12:18 ` [PATCH v4 07/13] vla: support for DW_AT_count Sanimir Agovic
2013-12-17 12:19 ` [PATCH v4 10/13] test: multi-dimensional c99 vla Sanimir Agovic
2013-12-17 12:19 ` [PATCH v4 03/13] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
2014-01-15 21:24   ` Tom Tromey
2013-12-17 12:19 ` [PATCH v4 13/13] test: add mi vla test Sanimir Agovic
2013-12-17 12:19 ` [PATCH v4 08/13] vla: resolve dynamic bounds if value contents is a constant byte-sequence Sanimir Agovic
2013-12-17 12:19 ` [PATCH v4 09/13] test: cover subranges with present DW_AT_count attribute Sanimir Agovic
2013-12-17 12:19 ` [PATCH v4 12/13] test: basic c99 vla tests for C primitives Sanimir Agovic
2014-01-15 21:39   ` Tom Tromey
2014-01-16 17:02     ` Agovic, Sanimir
2014-01-16 17:33       ` Tom Tromey
2014-01-17 13:36         ` Agovic, Sanimir
2014-01-20  5:47           ` Tom Tromey
2014-01-20  9:32             ` Agovic, Sanimir
2013-12-17 12:19 ` [PATCH v4 11/13] test: evaluate pointers to C99 vla correctly Sanimir Agovic
2013-12-18  3:01 ` [PATCH v4 00/13] C99 variable length array support Joel Brobecker
2014-01-15 21:41 ` Tom Tromey
2014-01-16 17:05   ` Agovic, Sanimir
2014-01-16 22:11     ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).