public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
  2013-10-21 14:40 ` [PATCH 07/10] test: evaluate pointers to C99 vla correctly Sanimir Agovic
  2013-10-21 14:40 ` [PATCH 02/10] type: add c99 variable length array support Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-10-24 19:55   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 03/10] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

C99 requires the sizeof operator to be evaluated at runtime to compute
the size of the vla, reflect this behavior in gdb.

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

(gdb) p sizeof(vla[0])

yields N.

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

	* eval.c (evaluate_subexp_for_sizeof): Allow side effects for
	argument to sizeof.

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

diff --git a/gdb/eval.c b/gdb/eval.c
index ab3cb95..91cc299 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -3055,7 +3055,7 @@ evaluate_subexp_for_sizeof (struct expression *exp, int *pos)
 	value_from_longest (size_type, (LONGEST) TYPE_LENGTH (type));
 
     default:
-      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_AVOID_SIDE_EFFECTS);
+      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);
       return value_from_longest (size_type,
 				 (LONGEST) TYPE_LENGTH (value_type (val)));
     }
-- 
1.7.0.7

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

* [PATCH 08/10] test: multi-dimensional c99 vla.
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (6 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 06/10] vla: update type from newly created value Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 21:19   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 01/10] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

From: Keven Boell <keven.boell@intel.com>

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

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

Change-Id: I2fd81dc113f20eafed363e7a5a0b0a5a6ce155bc
Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/testsuite/gdb.base/vla-multi.c   |   55 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/vla-multi.exp |   38 +++++++++++++++++++++++
 2 files changed, 93 insertions(+), 0 deletions(-)
 create mode 100755 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 100755
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..fa2fe18
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-multi.exp
@@ -0,0 +1,38 @@
+# 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 "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)"
+gdb_test "print sizeof vla_ptr\[0\]" "\\$\\d+ = 8" "print sizeof vla_ptr\[0\]"
+
+gdb_breakpoint [gdb_get_line_number "f2_breakpoint"]
+gdb_continue_to_breakpoint "f2_breakpoint"
+gdb_test "print *vla_ptr" "\\$\\d+ = \\\{0, 1\\\}" "print *vla_ptr (f2)"
-- 
1.7.0.7

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

* [PATCH 01/10] vla: introduce new bound type abstraction adapt uses
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (7 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 08/10] test: multi-dimensional c99 vla Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 19:00   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 04/10] vla: enable sizeof operator for indirection Sanimir Agovic
  2013-11-21 18:52 ` C99 variable length array support Pedro Alves
  10 siblings, 1 reply; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

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

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

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

Change-Id: I04ccd6b9bbf7519b99e814e9ee7119dbcd1f7baa
---
 gdb/dwarf2read.c |   40 +++++++++++++++++++++++-----------------
 gdb/gdbtypes.c   |   43 +++++++++++++++++++++++++++++++------------
 gdb/gdbtypes.h   |   38 ++++++++++++++++++++++++++++++++++----
 3 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 3974d0b..36ed9bd 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14142,7 +14142,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *base_type, *orig_base_type;
   struct type *range_type;
   struct attribute *attr;
-  LONGEST low, high;
+  struct dwarf2_prop low, high;
   int low_default_is_valid;
   const char *name;
   LONGEST negative_mask;
@@ -14159,33 +14159,37 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   if (range_type)
     return range_type;
 
+  low.kind = DWARF_CONST;
+  high.kind = DWARF_CONST;
+  high.data.const_val = 0;
+
   /* Set LOW_DEFAULT_IS_VALID if current language and DWARF version allow
      omitting DW_AT_lower_bound.  */
   switch (cu->language)
     {
     case language_c:
     case language_cplus:
-      low = 0;
+      low.data.const_val = 0;
       low_default_is_valid = 1;
       break;
     case language_fortran:
-      low = 1;
+      low.data.const_val = 1;
       low_default_is_valid = 1;
       break;
     case language_d:
     case language_java:
     case language_objc:
-      low = 0;
+      low.data.const_val = 0;
       low_default_is_valid = (cu->header.version >= 4);
       break;
     case language_ada:
     case language_m2:
     case language_pascal:
-      low = 1;
+      low.data.const_val = 1;
       low_default_is_valid = (cu->header.version >= 4);
       break;
     default:
-      low = 0;
+      low.data.const_val = 0;
       low_default_is_valid = 0;
       break;
     }
@@ -14195,7 +14199,7 @@ 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]"),
@@ -14217,10 +14221,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
     {
@@ -14228,12 +14232,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;
 	}
     }
 
@@ -14277,12 +14281,14 @@ 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;
-
-  range_type = create_range_type (NULL, orig_base_type, low, high);
+  if (low.kind == DWARF_CONST
+      && !TYPE_UNSIGNED (base_type) && (low.data.const_val & negative_mask))
+    low.data.const_val |= negative_mask;
+  if (high.kind == DWARF_CONST
+      && !TYPE_UNSIGNED (base_type) && (high.data.const_val & negative_mask))
+    high.data.const_val |= negative_mask;
+
+  range_type = create_range_type_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
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9069a11..3cfcf62 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -796,6 +796,27 @@ allocate_stub_method (struct type *type)
   return mtype;
 }
 
+struct type *
+create_range_type_1 (struct type *result_type, struct type *index_type,
+		     const struct dwarf2_prop *low_bound,
+		     const struct dwarf2_prop *high_bound)
+{
+  if (result_type == NULL)
+    result_type = alloc_type_copy (index_type);
+  TYPE_CODE (result_type) = TYPE_CODE_RANGE;
+  TYPE_TARGET_TYPE (result_type) = index_type;
+  if (TYPE_STUB (index_type))
+    TYPE_TARGET_STUB (result_type) = 1;
+  else
+    TYPE_LENGTH (result_type) = TYPE_LENGTH (check_typedef (index_type));
+  TYPE_RANGE_DATA (result_type) = (struct range_bounds *)
+    TYPE_ZALLOC (result_type, sizeof (struct range_bounds));
+  TYPE_RANGE_DATA (result_type)->low = *low_bound;
+  TYPE_RANGE_DATA (result_type)->high = *high_bound;
+
+  return result_type;
+}
+
 /* Create a range type using either a blank type supplied in
    RESULT_TYPE, or creating a new type, inheriting the objfile from
    INDEX_TYPE.
@@ -810,18 +831,16 @@ struct type *
 create_range_type (struct type *result_type, struct type *index_type,
 		   LONGEST low_bound, LONGEST high_bound)
 {
-  if (result_type == NULL)
-    result_type = alloc_type_copy (index_type);
-  TYPE_CODE (result_type) = TYPE_CODE_RANGE;
-  TYPE_TARGET_TYPE (result_type) = index_type;
-  if (TYPE_STUB (index_type))
-    TYPE_TARGET_STUB (result_type) = 1;
-  else
-    TYPE_LENGTH (result_type) = TYPE_LENGTH (check_typedef (index_type));
-  TYPE_RANGE_DATA (result_type) = (struct range_bounds *)
-    TYPE_ZALLOC (result_type, sizeof (struct range_bounds));
-  TYPE_LOW_BOUND (result_type) = low_bound;
-  TYPE_HIGH_BOUND (result_type) = high_bound;
+  struct dwarf2_prop low, high;
+
+  low.kind = DWARF_CONST;
+  low.data.const_val = low_bound;
+
+  high.kind = DWARF_CONST;
+  high.data.const_val = high_bound;
+
+  result_type = create_range_type_1 (result_type, index_type,
+				     &low, &high);
 
   if (low_bound >= 0)
     TYPE_UNSIGNED (result_type) = 1;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index d7fdedf..f563a1c 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -365,6 +365,29 @@ enum type_instance_flag_value
 #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
 				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 
+/* Used to store bound information for a type.  */
+
+struct dwarf2_prop
+{
+  /* Determine which field of the union dwarf2_prop.data is used.  */
+  enum
+  {
+    DWARF_CONST,
+    DWARF_LOCEXPR,
+    DWARF_LOCLIST
+  } kind;
+
+  /* Stores information as location expression, location list,
+     or constant value.  */
+  union data
+  {
+    LONGEST const_val;
+    struct dwarf2_locexpr_baton *locexpr;
+    struct dwarf2_loclist_baton *loclist;
+  } data;
+};
+
+
 /* Determine which field of the union main_type.fields[x].loc is used.  */
 
 enum field_loc_kind
@@ -589,11 +612,11 @@ struct main_type
     {
       /* Low bound of range.  */
 
-      LONGEST low;
+      struct dwarf2_prop low;
 
       /* High bound of range.  */
 
-      LONGEST high;
+      struct dwarf2_prop high;
 
       /* Flags indicating whether the values of low and high are
          valid.  When true, the respective range value is
@@ -1066,8 +1089,10 @@ 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
 #define TYPE_HIGH_BOUND_UNDEFINED(range_type) \
@@ -1526,6 +1551,11 @@ extern struct type *lookup_function_type_with_arguments (struct type *,
 							 int,
 							 struct type **);
 
+extern struct type *create_range_type_1 (struct type *, struct type *,
+					 const struct dwarf2_prop *,
+					 const struct dwarf2_prop *);
+
+
 extern struct type *create_range_type (struct type *, struct type *, LONGEST,
 				       LONGEST);
 
-- 
1.7.0.7

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

* [PATCH 09/10] test: basic c99 vla tests
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (4 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 10/10] test: add mi vla test Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 21:23   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 06/10] vla: update type from newly created value Sanimir Agovic
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

From: Keven Boell <keven.boell@intel.com>

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

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

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

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

* [PATCH 07/10] test: evaluate pointers to C99 vla correctly.
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 20:57   ` Tom Tromey
  2013-11-07 21:14   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 02/10] type: add c99 variable length array support Sanimir Agovic
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

From: Keven Boell <keven.boell@intel.com>

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

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

Change-Id: I12695184edc0d9621d658615ea48d783649f813d
Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/testsuite/gdb.base/vla-ptr.c   |   63 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/vla-ptr.exp |   45 +++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 0 deletions(-)
 create mode 100755 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 100755
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..54c753f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/vla-ptr.exp
@@ -0,0 +1,45 @@
+# 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
+}
+
+# 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)"
+gdb_test "print sizeof(*vla_ptr)" "\\$\\d+ = 20" "print sizeof(*vla_ptr) (vla_func)"
+
+gdb_breakpoint [gdb_get_line_number "typedef_ptr_assigned"]
+gdb_continue_to_breakpoint "typedef_ptr_assigned"
+gdb_test "print td_vla" "\\$\\d+ = \\\{4, 5, 6, 7, 8\\\}" "print td_vla"
+gdb_test "print *td_ptr" "\\$\\d+ = \\\{4, 5, 6, 7, 8\\\}" "print *td_ptr"
-- 
1.7.0.7

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

* [PATCH 03/10] vla: enable sizeof operator to work with variable length arrays
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (2 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 05/10] vla: allow side effects for sizeof argument Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 19:10   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 10/10] test: add mi vla test Sanimir Agovic
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

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.

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

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

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

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

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

* [PATCH 06/10] vla: update type from newly created value
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (5 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 09/10] test: basic c99 vla tests Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 20:56   ` Tom Tromey
  2013-11-20 11:02   ` Pedro Alves
  2013-10-21 14:40 ` [PATCH 08/10] test: multi-dimensional c99 vla Sanimir Agovic
                   ` (3 subsequent siblings)
  10 siblings, 2 replies; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

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>

	* valops.c (value_ind): Re-fetch type from value.
	* value.c (coerce_ref): Re-fetch type from value.

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

diff --git a/gdb/valops.c b/gdb/valops.c
index 15fd7c3..6706d9f 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1610,6 +1610,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 b9266db..ce778ea 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3360,6 +3360,7 @@ coerce_ref (struct value *arg)
   retval = value_at_lazy (enc_type,
                           unpack_pointer (value_type (arg),
                                           value_contents (arg)));
+  enc_type = value_type (retval);
   return readjust_indirect_value_type (retval, enc_type,
                                        value_type_arg_tmp, arg);
 }
-- 
1.7.0.7

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

* [PATCH 02/10] type: add c99 variable length array support
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
  2013-10-21 14:40 ` [PATCH 07/10] test: evaluate pointers to C99 vla correctly Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 19:02   ` Tom Tromey
  2013-11-07 19:10   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 05/10] vla: allow side effects for sizeof argument Sanimir Agovic
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

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

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

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

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

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

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

Change-Id: I97b91b9ad0b8ac8d30294dae6bd0dd4c905713ce
---
 gdb/dwarf2loc.c  |   45 +++++++++
 gdb/dwarf2loc.h  |    7 ++
 gdb/dwarf2read.c |  131 +++++++++++++++++++++------
 gdb/gdbtypes.c   |  272 +++++++++++++++++++++++++++++++++++++++++++----------
 gdb/gdbtypes.h   |    7 ++
 gdb/value.c      |   10 +-
 6 files changed, 389 insertions(+), 83 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 8242dca..289cf78 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2416,6 +2416,51 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
   return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu, 0);
 }
 
+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->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:
+      *valp = dwarf_expr_read_reg (&baton, dwarf_expr_fetch_address (ctx, 0));
+      break;
+    case DWARF_VALUE_MEMORY:
+      *valp = dwarf_expr_fetch_address (ctx, 0);
+      break;
+    }
+
+  do_cleanups (cleanup);
+
+  return 1;
+}
+
+
 \f
 /* Helper functions and baton for dwarf2_loc_desc_needs_frame.  */
 
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index 9bc8ca5..83c8044 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -90,6 +90,13 @@ struct value *dwarf2_evaluate_loc_desc (struct type *type,
 					size_t size,
 					struct dwarf2_per_cu_data *per_cu);
 
+/* Evaluate a dwarf expression and stores the result in VAL, expecting
+   that the dwarf expression only produces a single CORE_ADDR.
+   Returns 1 on success, 0 otherwise.   */
+
+int dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
+			       CORE_ADDR addr, CORE_ADDR *value);
+
 CORE_ADDR dwarf2_read_addr_index (struct dwarf2_per_cu_data *per_cu,
 				  unsigned int addr_index);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 36ed9bd..f552df2 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1848,6 +1848,17 @@ static void free_dwo_file_cleanup (void *);
 static void process_cu_includes (void);
 
 static void check_producer (struct dwarf2_cu *cu);
+
+static struct dwarf2_locexpr_baton* attr_to_locexprbaton
+(const struct attribute *, struct dwarf2_cu *);
+
+static struct dwarf2_locexpr_baton* attr_to_locexprbaton_1
+(const struct attribute *, struct dwarf2_cu *, const gdb_byte *additional_data,
+ int extra_size);
+
+static int attr_to_dwarf2_prop
+(struct die_info *, const struct attribute *, struct dwarf2_cu *,
+ struct dwarf2_prop *);
 \f
 /* Various complaints about symbol reading that don't abort the process.  */
 
@@ -14206,27 +14217,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
 	       die->offset.sect_off, objfile_name (cu->objfile));
 
   attr = dwarf2_attr (die, DW_AT_upper_bound, cu);
-  if (attr)
-    {
-      if (attr_form_is_block (attr) || attr_form_is_ref (attr))
-        {
-          /* GCC encodes arrays with unspecified or dynamic length
-             with a DW_FORM_block1 attribute or a reference attribute.
-             FIXME: GDB does not yet know how to handle dynamic
-             arrays properly, treat them as arrays with unspecified
-             length for now.
-
-             FIXME: jimb/2003-09-22: GDB does not really know
-             how to handle arrays of unspecified length
-             either; we just represent them as zero-length
-             arrays.  Choose an appropriate upper bound given
-             the lower bound we've computed above.  */
-          high.data.const_val = low.data.const_val - 1;
-        }
-      else
-        high.data.const_val = dwarf2_get_attr_constant_value (attr, 1);
-    }
-  else
+  if (!attr_to_dwarf2_prop (die, attr, cu, &high))
     {
       attr = dwarf2_attr (die, DW_AT_count, cu);
       if (attr)
@@ -14290,12 +14281,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_UNDEFINED (range_type) = 1;
-
   /* Ada expects an empty array on no boundary attributes.  */
   if (attr == NULL && cu->language != language_ada)
     TYPE_HIGH_BOUND_UNDEFINED (range_type) = 1;
@@ -22301,6 +22286,98 @@ save_gdb_index_command (char *arg, int from_tty)
   }
 }
 
+/* Turn Dwarf block into location expression baton structure. Used to store
+   baton into "dynamic" types, e.g. VLA's. */
+
+static struct dwarf2_locexpr_baton*
+attr_to_locexprbaton (const struct attribute *attribute, struct dwarf2_cu *cu)
+{
+  return attr_to_locexprbaton_1 (attribute, cu, NULL, 0);
+}
+
+static struct dwarf2_locexpr_baton*
+attr_to_locexprbaton_1 (const struct attribute *attribute, struct dwarf2_cu *cu,
+			const gdb_byte *additional_data, int extra_size)
+{
+  struct objfile *objfile = dwarf2_per_objfile->objfile;
+  struct dwarf2_locexpr_baton *baton;
+
+  gdb_assert (attribute != NULL && cu != NULL &&
+	      attr_form_is_block (attribute));
+
+  baton = obstack_alloc (&objfile->objfile_obstack,
+			 sizeof (struct dwarf2_locexpr_baton));
+  baton->per_cu = cu->per_cu;
+  baton->size = DW_BLOCK (attribute)->size + extra_size;
+
+  if (additional_data != NULL && extra_size > 0)
+    {
+      gdb_byte *data;
+
+      data = obstack_alloc (&objfile->objfile_obstack, baton->size);
+      baton->data = data;
+
+      memcpy (data, DW_BLOCK (attribute)->data, DW_BLOCK (attribute)->size);
+      memcpy (data, additional_data, extra_size);
+    }
+  else
+    /* Copy the data pointer as the blocks lifetime is
+       bound to its object file. */
+    baton->data = DW_BLOCK (attribute)->data;
+
+  gdb_assert(baton->data != NULL);
+
+  return baton;
+}
+
+/* Parse dwarf attribute if it's a block, reference or constant and put the
+   resulting value of the attribute into struct dwarf2_prop. */
+
+static int
+attr_to_dwarf2_prop (struct die_info *die, const struct attribute *attr,
+		     struct dwarf2_cu *cu,
+		     struct dwarf2_prop *prop)
+{
+  if (die == NULL || attr == NULL || cu == NULL || prop == NULL)
+    return 0;
+
+  if (attr_form_is_block (attr))
+    {
+      prop->data.locexpr = attr_to_locexprbaton (attr, cu);
+      prop->kind = DWARF_LOCEXPR;
+      gdb_assert (prop->data.locexpr != NULL);
+    }
+  else if (attr_form_is_ref (attr))
+    {
+      struct dwarf2_cu *target_cu = cu;
+      struct die_info *target_die;
+      struct attribute *target_attr;
+      const gdb_byte append_ops[] = { DW_OP_deref };
+
+      target_die = follow_die_ref (die, attr, &target_cu);
+      target_attr = dwarf2_attr (target_die, DW_AT_location, target_cu);
+
+      prop->data.locexpr =
+	attr_to_locexprbaton_1 (target_attr, cu, append_ops,
+				sizeof (append_ops) / sizeof (append_ops[0]));
+      prop->kind = DWARF_LOCEXPR;
+      gdb_assert (prop->data.locexpr != NULL);
+    }
+  else if (attr_form_is_constant (attr))
+    {
+      prop->data.const_val = dwarf2_get_attr_constant_value (attr, 0);
+      prop->kind = DWARF_CONST;
+    }
+  else
+    {
+      dwarf2_invalid_attrib_class_complaint(dwarf_form_name (attr->form),
+					    dwarf2_name (die, cu));
+      return 0;
+    }
+
+  return 1;
+}
+
 \f
 
 int dwarf2_always_disassemble;
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 3cfcf62..c0f375d 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -38,6 +38,7 @@
 #include "hashtab.h"
 #include "exceptions.h"
 #include "cp-support.h"
+#include "dwarf2loc.h"
 
 /* Initialize BADNESS constants.  */
 
@@ -848,6 +849,17 @@ create_range_type (struct type *result_type, struct type *index_type,
   return result_type;
 }
 
+/* Predicate tests whether BOUNDS are static. Returns 1 if all bounds values
+   are static, otherwise returns 0.  */
+
+static int
+has_static_range (const struct range_bounds *bounds)
+{
+  return bounds->low.kind == DWARF_CONST
+    && bounds->high.kind == DWARF_CONST;
+}
+
+
 /* Set *LOWP and *HIGHP to the lower and upper bounds of discrete type
    TYPE.  Return 1 if type is a range type, 0 if it is discrete (and
    bounds will fit in LONGEST), or -1 otherwise.  */
@@ -977,24 +989,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));
@@ -1525,7 +1541,188 @@ 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 given the upper and lower bound of a dynamic
+   type.  */
+
+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;
+
+  if (TYPE_CODE (type) != TYPE_CODE_ARRAY
+      && TYPE_CODE (type) != TYPE_CODE_STRING)
+    return len;
+
+  range_type = TYPE_INDEX_TYPE (type);
+
+  if (!has_static_range (TYPE_RANGE_DATA (range_type)))
+    return len;
+
+  target_type = check_typedef (TYPE_TARGET_TYPE (type));
+
+  /* Now recompute the length of the array type, based on its
+     number of elements and the target type's length.
+     Watch out for Ada null Ada arrays where the high bound
+     is smaller than the low bound.  */
+  low_bound = TYPE_LOW_BOUND (range_type);
+  high_bound = TYPE_HIGH_BOUND (range_type);
+
+  if (high_bound < low_bound)
+    len = 0;
+  else
+    {
+      /* For now, we conservatively take the array length to be 0
+         if its length exceeds UINT_MAX.  The code below assumes
+         that for x < 0, (ULONGEST) x == -x + ULONGEST_MAX + 1,
+         which is technically not guaranteed by C, but is usually true
+         (because it would be true if x were unsigned with its
+         high-order bit on).  It uses the fact that
+         high_bound-low_bound is always representable in
+         ULONGEST and that if high_bound-low_bound+1 overflows,
+         it overflows to 0.  We must change these tests if we
+         decide to increase the representation of TYPE_LENGTH
+         from unsigned int to ULONGEST.  */
+      ULONGEST ulow = low_bound, uhigh = high_bound;
+      ULONGEST tlen = get_type_length (target_type);
+
+      len = tlen * (uhigh - ulow + 1);
+      if (tlen == 0 || (len / tlen - 1 + ulow) != uhigh || len > UINT_MAX)
+        len = 0;
+    }
+
+  return len;
+}
+
+/* Converts a dynamic value into a static one. Returns 1 if PROP could be
+   converted and the static value is passed back into VALUE, otherwise
+   returns 0.  */
+
+static int
+resolve_dynamic_prop (const struct dwarf2_prop *prop, CORE_ADDR address,
+		      CORE_ADDR *value)
+{
+  if (prop == NULL)
+    return 0;
+
+  switch (prop->kind)
+    {
+    case DWARF_LOCEXPR:
+      {
+	const struct dwarf2_locexpr_baton *baton = prop->data.locexpr;
+
+	return dwarf2_locexpr_baton_eval (baton, address, value);
+      }
+    case DWARF_LOCLIST:
+    case DWARF_CONST:
+      break;
+    }
+
+  return 0;
+}
+
+/* Predicates if the type has dynamic values, which are not resolved yet.  */
+
+static int
+is_dynamic_type (const struct type *type)
+{
+  if (type == NULL)
+    return 0;
+
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
+      && TYPE_NFIELDS (type) == 1)
+    {
+      const struct type *range_type = TYPE_INDEX_TYPE (type);
+
+      if (!has_static_range (TYPE_RANGE_DATA (range_type)))
+	return 1;
+    }
+
+  if (TYPE_CODE (type) == TYPE_CODE_PTR
+      || TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
+    return is_dynamic_type (TYPE_TARGET_TYPE (type));
+
+  return 0;
+}
+
+static void
+resolve_dynamic_bounds (struct type *type, CORE_ADDR address)
+{
+  struct type *real_type;
+  const struct dwarf2_prop *prop;
+  CORE_ADDR value;
+
+  if (TYPE_CODE (type) == TYPE_CODE_PTR
+      || TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
+    resolve_dynamic_bounds (TYPE_TARGET_TYPE (type), address);
+  else
+    {
+      real_type = check_typedef (type);
+
+      if (TYPE_CODE (real_type) == TYPE_CODE_ARRAY)
+	{
+	  struct type *ary_dim = real_type;
+
+	  do {
+	    struct type *range_type = TYPE_INDEX_TYPE (ary_dim);
+
+	    prop = &TYPE_RANGE_DATA (range_type)->low;
+	    if (resolve_dynamic_prop (prop, address, &value))
+	      {
+		TYPE_LOW_BOUND (range_type) = value;
+		TYPE_LOW_BOUND_KIND (range_type) = DWARF_CONST;
+	      }
+
+	    prop = &TYPE_RANGE_DATA (range_type)->high;
+	    if (resolve_dynamic_prop (prop, address, &value))
+	      {
+		TYPE_HIGH_BOUND (range_type) = value;
+		TYPE_HIGH_BOUND_KIND (range_type) = DWARF_CONST;
+	      }
+
+	    ary_dim = TYPE_TARGET_TYPE (ary_dim);
+	  } while (ary_dim != NULL && TYPE_CODE (ary_dim) == TYPE_CODE_ARRAY);
+	}
+    }
+}
+
+/* Resolves all dynamic values of a type e.g. array bounds to static values.
+   If TYPE has no dynamic values returns TYPE otherwise a new type with static
+   values is returned.  */
+
+struct type *
+resolve_dynamic_type (struct type *type, CORE_ADDR address)
+{
+  const struct type *ty = check_typedef (type);
+  struct type *resolved_type, *real_type;
+  struct obstack obstack;
+  struct cleanup *cleanup;
+  const struct dwarf2_prop *prop;
+  htab_t copied_types;
+  CORE_ADDR value;
+
+  if (!TYPE_OBJFILE_OWNED (ty))
+    return type;
+
+  if (!is_dynamic_type (ty))
+    return type;
+
+  /* Make a deep copy of the type.  */
+  copied_types = create_copied_types_hash (TYPE_OBJFILE (type));
+  cleanup = make_cleanup_htab_delete (copied_types);
+  resolved_type = copy_type_recursive
+    (TYPE_OBJFILE (type), type, copied_types);
+  do_cleanups (cleanup);
+
+  resolve_dynamic_bounds (resolved_type, address);
+
+  resolved_type->length = get_type_length (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.
@@ -1701,44 +1898,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 f563a1c..96e0ed7 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1097,6 +1097,11 @@ extern void allocate_gnat_aux_type (struct type *);
    TYPE_RANGE_DATA(range_type)->low_undefined
 #define TYPE_HIGH_BOUND_UNDEFINED(range_type) \
    TYPE_RANGE_DATA(range_type)->high_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.  */
 
@@ -1575,6 +1580,8 @@ extern struct type *lookup_unsigned_typename (const struct language_defn *,
 extern struct type *lookup_signed_typename (const struct language_defn *,
 					    struct gdbarch *, const char *);
 
+extern struct type *resolve_dynamic_type (struct type *, CORE_ADDR);
+
 extern struct type *check_typedef (struct type *);
 
 #define CHECK_TYPEDEF(TYPE)			\
diff --git a/gdb/value.c b/gdb/value.c
index d96d285..b9266db 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3183,9 +3183,10 @@ value_from_ulongest (struct type *type, ULONGEST num)
 struct value *
 value_from_pointer (struct type *type, CORE_ADDR addr)
 {
-  struct value *val = allocate_value (type);
+  struct type *resolved_type = resolve_dynamic_type (type, addr);
+  struct value *val = allocate_value (resolved_type);
 
-  store_typed_address (value_contents_raw (val), check_typedef (type), addr);
+  store_typed_address (value_contents_raw (val), check_typedef (resolved_type), addr);
   return val;
 }
 
@@ -3199,12 +3200,13 @@ value_from_contents_and_address (struct type *type,
 				 const gdb_byte *valaddr,
 				 CORE_ADDR address)
 {
+  struct type *resolved_type = resolve_dynamic_type (type, address);
   struct value *v;
 
   if (valaddr == NULL)
-    v = allocate_value_lazy (type);
+    v = allocate_value_lazy (resolved_type);
   else
-    v = value_from_contents (type, valaddr);
+    v = value_from_contents (resolved_type, valaddr);
   set_value_address (v, address);
   VALUE_LVAL (v) = lval_memory;
   return v;
-- 
1.7.0.7

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

* C99 variable length array support
@ 2013-10-21 14:40 Sanimir Agovic
  2013-10-21 14:40 ` [PATCH 07/10] test: evaluate pointers to C99 vla correctly Sanimir Agovic
                   ` (10 more replies)
  0 siblings, 11 replies; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

Hello,

this patch series 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 = 42

 -Sanimir & Keven

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

* [PATCH 10/10] test: add mi vla test
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (3 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 03/10] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 21:31   ` Tom Tromey
  2013-10-21 14:40 ` [PATCH 09/10] test: basic c99 vla tests Sanimir Agovic
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

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

testsuite/gdb.mi/

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

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

diff --git a/gdb/testsuite/gdb.mi/mi-vla-c99.exp b/gdb/testsuite/gdb.mi/mi-vla-c99.exp
new file mode 100644
index 0000000..7c74a41
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-vla-c99.exp
@@ -0,0 +1,61 @@
+# 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_gdb_exit
+return 0
diff --git a/gdb/testsuite/gdb.mi/vla.c b/gdb/testsuite/gdb.mi/vla.c
new file mode 100644
index 0000000..9b33fc8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/vla.c
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Contributed by Intel Corp. <keven.boell@intel.com>
+
+   Copyright 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int func (int n)
+{
+  int vla[n], i;
+
+  for (i = 0; i < n; i++)
+    vla[i] = i;
+
+  return n;                 /* vla-filled */
+}
+
+int main ()
+{
+  func (5);
+
+  return 0;
+}
-- 
1.7.0.7

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

* [PATCH 04/10] vla: enable sizeof operator for indirection
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (8 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 01/10] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
@ 2013-10-21 14:40 ` Sanimir Agovic
  2013-11-07 19:57   ` Tom Tromey
  2013-11-21 18:52 ` C99 variable length array support Pedro Alves
  10 siblings, 1 reply; 48+ messages in thread
From: Sanimir Agovic @ 2013-10-21 14:40 UTC (permalink / raw)
  To: gdb-patches

This patch enables the sizeof operator for indirections:

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

(gdb) p sizeof(*vla_ptr)

yields N.

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

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

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

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

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

* Re: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-10-21 14:40 ` [PATCH 05/10] vla: allow side effects for sizeof argument Sanimir Agovic
@ 2013-10-24 19:55   ` Tom Tromey
  2013-10-25  8:13     ` Agovic, Sanimir
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Tromey @ 2013-10-24 19:55 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

Sanimir> C99 requires the sizeof operator to be evaluated at runtime to compute
Sanimir> the size of the vla, reflect this behavior in gdb.

Sanimir> -      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_AVOID_SIDE_EFFECTS);
Sanimir> +      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);

I think this will do the wrong thing for cases like (assuming "int x"):

    (gdb) print sizeof (x++)

I think some other approach will be needed for this patch.

Tom

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

* RE: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-10-24 19:55   ` Tom Tromey
@ 2013-10-25  8:13     ` Agovic, Sanimir
  2013-10-28 21:00       ` Tom Tromey
  0 siblings, 1 reply; 48+ messages in thread
From: Agovic, Sanimir @ 2013-10-25  8:13 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: gdb-patches

Thanks for your review.

I avoid to re-send the complete patch series, thus see my 
alternative proposal below. Once I receive more feedback
I will re-publish my patch series including this fix if 
applicable.

Meanwhile you can track our latest development efforts on
http://intel-gdb.github.io/ branch vla-c99.

 -Sanimir

    vla: allow side effects for subscripts in the sizeof operator

    C99 requires the sizeof operator to be evaluated at runtime to retrieve
    the size of the vla, reflect this behavior in gdb.

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

    (gdb) p sizeof(vla[0])

    yields N.

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

        * eval.c (evaluate_subexp_for_sizeof): Allow side effects for
        subscripts in the sizeof operator.

    Change-Id: I945e8a259e850cc0470faa742a0d2191122ef37b

diff --git a/gdb/eval.c b/gdb/eval.c
index ab3cb95..255ea09 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -3054,6 +3054,13 @@ evaluate_subexp_for_sizeof (struct expression *exp, int *pos)
       return
        value_from_longest (size_type, (LONGEST) TYPE_LENGTH (type));

+    case BINOP_SUBSCRIPT:
+      /* In case of a variable length array we need to evaluate the subscript
+        with side effects to correcly infere the size.  */
+      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);
+      return value_from_longest (size_type,
+                                (LONGEST) TYPE_LENGTH (value_type (val)));
+
     default:
       val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_AVOID_SIDE_EFFECTS);
       return value_from_longest (size_type,

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Thursday, October 24, 2013 09:55 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 05/10] vla: allow side effects for sizeof argument
> 
> >>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> C99 requires the sizeof operator to be evaluated at runtime to compute
> Sanimir> the size of the vla, reflect this behavior in gdb.
> 
> Sanimir> -      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_AVOID_SIDE_EFFECTS);
> Sanimir> +      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);
> 
> I think this will do the wrong thing for cases like (assuming "int x"):
> 
>     (gdb) print sizeof (x++)
> 
> I think some other approach will be needed for this patch.
> 
> 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] 48+ messages in thread

* Re: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-10-25  8:13     ` Agovic, Sanimir
@ 2013-10-28 21:00       ` Tom Tromey
  2013-11-18  9:37         ` Agovic, Sanimir
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Tromey @ 2013-10-28 21:00 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: gdb-patches

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

Sanimir> I avoid to re-send the complete patch series, thus see my 
Sanimir> alternative proposal below. Once I receive more feedback
Sanimir> I will re-publish my patch series including this fix if 
Sanimir> applicable.

I don't see how this version of the patch solves what I thought the
problem was.

I'm sure it is just my misunderstanding though.

Sanimir> Meanwhile you can track our latest development efforts on
Sanimir> http://intel-gdb.github.io/ branch vla-c99.

Is that all rebased to binutils-gdb.git now?
If so I will start fetching from it again.

Sanimir> +    case BINOP_SUBSCRIPT:
Sanimir> +      /* In case of a variable length array we need to evaluate the subscript
Sanimir> +        with side effects to correcly infere the size.  */
Sanimir> +      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);
Sanimir> +      return value_from_longest (size_type,
Sanimir> +                                (LONGEST) TYPE_LENGTH (value_type (val)));

This makes it seem like the problem case is "sizeof (vla[index])".
But I would have thought it was "sizeof (vla)".

In any case I think this patch still has the issue, just at one remove.
Like:

    (gdb) print sizeof (array[x++])

For an ordinary array this should not modify x.

Tom

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

* Re: [PATCH 01/10] vla: introduce new bound type abstraction adapt uses
  2013-10-21 14:40 ` [PATCH 01/10] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
@ 2013-11-07 19:00   ` Tom Tromey
  2013-11-18 11:15     ` Agovic, Sanimir
  0 siblings, 1 reply; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 19:00 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

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

Thanks very much for working on this.

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

This line is too long and should be wrapped.

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

This needs an introductory comment.

Sanimir> +/* Used to store bound information for a type.  */
Sanimir> +
Sanimir> +struct dwarf2_prop
Sanimir> +{
Sanimir> +  /* Determine which field of the union dwarf2_prop.data is used.  */
Sanimir> +  enum
Sanimir> +  {
Sanimir> +    DWARF_CONST,
Sanimir> +    DWARF_LOCEXPR,
Sanimir> +    DWARF_LOCLIST
Sanimir> +  } kind;
Sanimir> +
Sanimir> +  /* Stores information as location expression, location list,
Sanimir> +     or constant value.  */
Sanimir> +  union data
Sanimir> +  {
Sanimir> +    LONGEST const_val;
Sanimir> +    struct dwarf2_locexpr_baton *locexpr;
Sanimir> +    struct dwarf2_loclist_baton *loclist;
Sanimir> +  } data;
Sanimir> +};

Sanimir> @@ -589,11 +612,11 @@ struct main_type
Sanimir>      {
Sanimir>        /* Low bound of range.  */
 
Sanimir> -      LONGEST low;
Sanimir> +      struct dwarf2_prop low;
 
Sanimir>        /* High bound of range.  */
 
Sanimir> -      LONGEST high;
Sanimir> +      struct dwarf2_prop high;
 

Just after this hunk of "struct range_bounds" is this code:

      /* 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;

It seems to me that it would be cleanest to make this a new value of the
enum you introduced, like "DWARF_UNDEFINED", and update a few macros to
follow.  What do you think?

Tom

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

* Re: [PATCH 02/10] type: add c99 variable length array support
  2013-10-21 14:40 ` [PATCH 02/10] type: add c99 variable length array support Sanimir Agovic
@ 2013-11-07 19:02   ` Tom Tromey
  2013-11-19 15:31     ` Agovic, Sanimir
  2013-11-07 19:10   ` Tom Tromey
  1 sibling, 1 reply; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 19:02 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

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

 
Sanimir> +int
Sanimir> +dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
Sanimir> +			   CORE_ADDR addr, CORE_ADDR *valp)

Need an introductory comment.  It can just say "See dwarf2loc.h.", since
you put the real comment there.

Sanimir> +  switch (ctx->location)
Sanimir> +    {
Sanimir> +    case DWARF_VALUE_REGISTER:
Sanimir> +      *valp = dwarf_expr_read_reg (&baton, dwarf_expr_fetch_address (ctx, 0));
Sanimir> +      break;
Sanimir> +    case DWARF_VALUE_MEMORY:
Sanimir> +      *valp = dwarf_expr_fetch_address (ctx, 0);
Sanimir> +      break;
Sanimir> +    }

It seems that something should be done for other DWARF_VALUE_* results
here.

Sanimir> +static struct dwarf2_locexpr_baton* attr_to_locexprbaton
Sanimir> +(const struct attribute *, struct dwarf2_cu *);
Sanimir> +
Sanimir> +static struct dwarf2_locexpr_baton* attr_to_locexprbaton_1
Sanimir> +(const struct attribute *, struct dwarf2_cu *, const gdb_byte *additional_data,
Sanimir> + int extra_size);
Sanimir> +
Sanimir> +static int attr_to_dwarf2_prop
Sanimir> +(struct die_info *, const struct attribute *, struct dwarf2_cu *,
Sanimir> + struct dwarf2_prop *);

In cases like this we usually indent the subsequent lines a bit, like:

    static int attr_to_dwarf2_prop
        (struct die_info *, const struct attribute *, struct dwarf2_cu *,
         struct dwarf2_prop *);

However in this case I think it may be preferable to rearrange the
functions so that forward declarations are not needed.  What do you
think?

Sanimir> +static struct dwarf2_locexpr_baton*
Sanimir> +attr_to_locexprbaton (const struct attribute *attribute, struct dwarf2_cu *cu)
Sanimir> +{
Sanimir> +  return attr_to_locexprbaton_1 (attribute, cu, NULL, 0);
Sanimir> +}

If there is just a single caller (there is in this patch, but I haven't
read all the patches yet), I would remove this function and just update
the caller.

Sanimir> +static struct dwarf2_locexpr_baton*
Sanimir> +attr_to_locexprbaton_1 (const struct attribute *attribute, struct dwarf2_cu *cu,
Sanimir> +			const gdb_byte *additional_data, int extra_size)

Needs an introductory comment.

Sanimir> +    /* Copy the data pointer as the blocks lifetime is

Missing apostrophe: "block's".

Sanimir> +  gdb_assert(baton->data != NULL);

Space before open paren.

Sanimir> +/* Parse dwarf attribute if it's a block, reference or constant and put the
Sanimir> +   resulting value of the attribute into struct dwarf2_prop. */
Sanimir> +
Sanimir> +static int
Sanimir> +attr_to_dwarf2_prop (struct die_info *die, const struct attribute *attr,
Sanimir> +		     struct dwarf2_cu *cu,
Sanimir> +		     struct dwarf2_prop *prop)

I think it would be good if the introductory comment describe the return
value.

Sanimir> +  else if (attr_form_is_ref (attr))
Sanimir> +    {
Sanimir> +      struct dwarf2_cu *target_cu = cu;
Sanimir> +      struct die_info *target_die;
Sanimir> +      struct attribute *target_attr;
Sanimir> +      const gdb_byte append_ops[] = { DW_OP_deref };
Sanimir> +
Sanimir> +      target_die = follow_die_ref (die, attr, &target_cu);
Sanimir> +      target_attr = dwarf2_attr (target_die, DW_AT_location, target_cu);
Sanimir> +
Sanimir> +      prop->data.locexpr =
Sanimir> +	attr_to_locexprbaton_1 (target_attr, cu, append_ops,
Sanimir> +				sizeof (append_ops) / sizeof (append_ops[0]));
Sanimir> +      prop->kind = DWARF_LOCEXPR;
Sanimir> +      gdb_assert (prop->data.locexpr != NULL);

I don't understand this hunk.  Could you say why it is needed?

I wonder if this series also needs to handle DW_AT_count.
Maybe no compiler generates it.

Sanimir> +      dwarf2_invalid_attrib_class_complaint(dwarf_form_name (attr->form),
Sanimir> +					    dwarf2_name (die, cu));

Missing space before a paren.

Sanimir> +static int
Sanimir> +has_static_range (const struct range_bounds *bounds)
Sanimir> +{
Sanimir> +  return bounds->low.kind == DWARF_CONST
Sanimir> +    && bounds->high.kind == DWARF_CONST;
Sanimir> +}

THis needs parens around the argument to "return" and then an
indentation fix on the second line.

Sanimir> +/* Calculates the size of a type given the upper and lower bound of a dynamic
Sanimir> +   type.  */
Sanimir> +
Sanimir> +static ULONGEST
Sanimir> +get_type_length (const struct type *type)
Sanimir> +{
Sanimir> +  const struct type *range_type, *target_type;
Sanimir> +  ULONGEST len = TYPE_LENGTH (type);
Sanimir> +  LONGEST low_bound, high_bound;
Sanimir> +
Sanimir> +  if (TYPE_CODE (type) != TYPE_CODE_ARRAY
Sanimir> +      && TYPE_CODE (type) != TYPE_CODE_STRING)
Sanimir> +    return len;
Sanimir> +
Sanimir> +  range_type = TYPE_INDEX_TYPE (type);
Sanimir> +
Sanimir> +  if (!has_static_range (TYPE_RANGE_DATA (range_type)))
Sanimir> +    return len;

This seems like it doesn't follow what the introductory comment says it
does.

Sanimir> +
Sanimir> +static void
Sanimir> +resolve_dynamic_bounds (struct type *type, CORE_ADDR address)

Introductory comment.

Sanimir> +	  do {
Sanimir> +	    struct type *range_type = TYPE_INDEX_TYPE (ary_dim);

It's hard to know but perhaps a check_typedef is required here.

Sanimir> +	    ary_dim = TYPE_TARGET_TYPE (ary_dim);

Here too.

Sanimir> +struct type *
Sanimir> +resolve_dynamic_type (struct type *type, CORE_ADDR address)
Sanimir> +{
[...]
Sanimir> +  if (!TYPE_OBJFILE_OWNED (ty))
Sanimir> +    return type;

This seems like a bit of a wart, though I am not sure whether the
situation can actually arise.


One thing I didn't see in here is error-checking of whether resolution
makes sense.

E.g., suppose I print the value of a pointer-to-VLA.  Then I move to
some other frame and "print *$".

In this situation the bounds have not been resolved -- but applying the
DWARF expression in the currently-selected frame will silently do the
wrong thing.

Tom

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

* Re: [PATCH 02/10] type: add c99 variable length array support
  2013-10-21 14:40 ` [PATCH 02/10] type: add c99 variable length array support Sanimir Agovic
  2013-11-07 19:02   ` Tom Tromey
@ 2013-11-07 19:10   ` Tom Tromey
  1 sibling, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 19:10 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

Sanimir> +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
Sanimir> +      && TYPE_NFIELDS (type) == 1)
Sanimir> +    {
Sanimir> +      const struct type *range_type = TYPE_INDEX_TYPE (type);
Sanimir> +
Sanimir> +      if (!has_static_range (TYPE_RANGE_DATA (range_type)))
Sanimir> +	return 1;
Sanimir> +    }
Sanimir> +
Sanimir> +  if (TYPE_CODE (type) == TYPE_CODE_PTR
Sanimir> +      || TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
Sanimir> +    return is_dynamic_type (TYPE_TARGET_TYPE (type));

I didn't notice the first time, but I think some check_typedef calls are
needed here as well.

Tom

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

* Re: [PATCH 03/10] vla: enable sizeof operator to work with variable length arrays
  2013-10-21 14:40 ` [PATCH 03/10] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
@ 2013-11-07 19:10   ` Tom Tromey
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 19:10 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

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

Sanimir> We now are able to get the size of a vla:
Sanimir> 1| void foo (size_t n) {
Sanimir> 2|   int vla[n];
Sanimir> 3| }
Sanimir> (gdb) p sizeof(vla)
Sanimir> yields N.

It should yield N * sizeof(int), but I imagine that was just a minor
oversight in your email, as the patch looks correct to me.

Tom

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

* Re: [PATCH 04/10] vla: enable sizeof operator for indirection
  2013-10-21 14:40 ` [PATCH 04/10] vla: enable sizeof operator for indirection Sanimir Agovic
@ 2013-11-07 19:57   ` Tom Tromey
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 19:57 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

This one looks good, assuming that the error-checking mentioned in an
earlier review is done elsewhere.

Tom

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

* Re: [PATCH 06/10] vla: update type from newly created value
  2013-10-21 14:40 ` [PATCH 06/10] vla: update type from newly created value Sanimir Agovic
@ 2013-11-07 20:56   ` Tom Tromey
  2013-11-20  7:56     ` Agovic, Sanimir
  2013-11-20 11:02   ` Pedro Alves
  1 sibling, 1 reply; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 20:56 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

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

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

Sanimir> 	* valops.c (value_ind): Re-fetch type from value.
Sanimir> 	* value.c (coerce_ref): Re-fetch type from value.

This is fine by itself; however, I wonder whether there are other calls
that require fixing.  Did you check through the rest of the code?

Tom

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

* Re: [PATCH 07/10] test: evaluate pointers to C99 vla correctly.
  2013-10-21 14:40 ` [PATCH 07/10] test: evaluate pointers to C99 vla correctly Sanimir Agovic
@ 2013-11-07 20:57   ` Tom Tromey
  2013-11-08  6:37     ` Tom Tromey
  2013-11-07 21:14   ` Tom Tromey
  1 sibling, 1 reply; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 20:57 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

Sanimir> +void
Sanimir> +foo (int n, int vla_ptr[n])
Sanimir> +{
Sanimir> +  return;         /* foo_bp */
Sanimir> +}

Sanimir> +gdb_test "print vla_ptr" "\\\(int \\\*\\\) $hex" "print vla_ptr (foo)"
Sanimir> +gdb_test "print *vla_ptr" "\\$\\d+ = 2" "print *vla_ptr (foo)"

This seems odd to me.

I suppose right now gcc claims that 'vla_ptr' has type 'int *'.
But I don't see why that is necessarily so.  Are compilers required to
declare this parameter as an "int *" and not "int[n]"?

That is, this test seems dependent on a compiler quirk.

The second test could maybe be rephrased as "print vla_ptr\[0\]" and
have a well-defined meaning across different styles of DWARF.

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

Printing plain old "vla" would not be amiss.

Perhaps also reasonable would be a "VLA of typedefs" test like:

   typedef int something;
   something svla[n];
   ...

Tom

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

* Re: [PATCH 07/10] test: evaluate pointers to C99 vla correctly.
  2013-10-21 14:40 ` [PATCH 07/10] test: evaluate pointers to C99 vla correctly Sanimir Agovic
  2013-11-07 20:57   ` Tom Tromey
@ 2013-11-07 21:14   ` Tom Tromey
  1 sibling, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 21:14 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

Sanimir> +gdb_test "print sizeof(*vla_ptr)" "\\$\\d+ = 20" "print sizeof(*vla_ptr) (vla_func)"
Sanimir> +

Also, this depends on sizeof(int).

Tom

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

* Re: [PATCH 08/10] test: multi-dimensional c99 vla.
  2013-10-21 14:40 ` [PATCH 08/10] test: multi-dimensional c99 vla Sanimir Agovic
@ 2013-11-07 21:19   ` Tom Tromey
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 21:19 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

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

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

Looks ok to me.

Tom

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

* Re: [PATCH 09/10] test: basic c99 vla tests
  2013-10-21 14:40 ` [PATCH 09/10] test: basic c99 vla tests Sanimir Agovic
@ 2013-11-07 21:23   ` Tom Tromey
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 21:23 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

Sanimir> 2013-10-18  Keven Boell  <keven.boell@intel.com>
Sanimir> 	* gdb.base/vla-datatypes.c: New. Test source file
Sanimir> 	for VLA datatype checks.
Sanimir> 	* gdb.base/vla-datatypes.exp: New. Tests ensure that
Sanimir> 	a VLA in C can be evaluated correctly with standard
Sanimir> 	C types.

Looks good.

In an earlier note I mentioned having a test for VLA-of-typedef, but I
see there is one in here, so you needn't bother updating the other one.

thanks,
Tom

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

* Re: [PATCH 10/10] test: add mi vla test
  2013-10-21 14:40 ` [PATCH 10/10] test: add mi vla test Sanimir Agovic
@ 2013-11-07 21:31   ` Tom Tromey
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-07 21:31 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

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

Sanimir> 2013-10-18  Keven Boell  <keven.boell@intel.com>
Sanimir> testsuite/gdb.mi/

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

This looks good.

I think it would be worthwhile to have a varobj test as well.

Tom

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

* Re: [PATCH 07/10] test: evaluate pointers to C99 vla correctly.
  2013-11-07 20:57   ` Tom Tromey
@ 2013-11-08  6:37     ` Tom Tromey
  0 siblings, 0 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-08  6:37 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

Tom> I suppose right now gcc claims that 'vla_ptr' has type 'int *'.
Tom> But I don't see why that is necessarily so.  Are compilers required to
Tom> declare this parameter as an "int *" and not "int[n]"?

Ok, I read up on array-like declarators and I see that this actually is
how C is defined, even for VLAs.  Given that I think this patch is fine.

Tom

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

* RE: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-10-28 21:00       ` Tom Tromey
@ 2013-11-18  9:37         ` Agovic, Sanimir
  2013-11-18 15:56           ` Tom Tromey
  2013-11-19 17:08           ` Pedro Alves
  0 siblings, 2 replies; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-18  9:37 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: gdb-patches

Thanks for your review.

It turns out this patch is not needed anymore as the bounds are resolved 
before the sizeof function is called. So in v2 this patch will be dropped
from the series.

>     (gdb) print sizeof (array[x++])
> 
> For an ordinary array this should not modify x.
Indeed, that is the behavior we have now in HEAD@vla-c99.

> Sanimir> Meanwhile you can track our latest development efforts on
> Sanimir> http://intel-gdb.github.io/ branch vla-c99.
> 
> Is that all rebased to binutils-gdb.git now?
> If so I will start fetching from it again.
Meanwhile the above repository got re-setup to mirror binutils-gdb.git.

 -Sanimir

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Tom Tromey
> Sent: Monday, October 28, 2013 10:00 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 05/10] vla: allow side effects for sizeof argument
> 
> >>>>> "Sanimir" == Agovic, Sanimir <sanimir.agovic@intel.com> writes:
> 
> Sanimir> I avoid to re-send the complete patch series, thus see my
> Sanimir> alternative proposal below. Once I receive more feedback
> Sanimir> I will re-publish my patch series including this fix if
> Sanimir> applicable.
> 
> I don't see how this version of the patch solves what I thought the
> problem was.
> 
> I'm sure it is just my misunderstanding though.
> 
> Sanimir> Meanwhile you can track our latest development efforts on
> Sanimir> http://intel-gdb.github.io/ branch vla-c99.
> 
> Is that all rebased to binutils-gdb.git now?
> If so I will start fetching from it again.
> 
> Sanimir> +    case BINOP_SUBSCRIPT:
> Sanimir> +      /* In case of a variable length array we need to evaluate the subscript
> Sanimir> +        with side effects to correcly infere the size.  */
> Sanimir> +      val = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);
> Sanimir> +      return value_from_longest (size_type,
> Sanimir> +                                (LONGEST) TYPE_LENGTH (value_type (val)));
> 
> This makes it seem like the problem case is "sizeof (vla[index])".
> But I would have thought it was "sizeof (vla)".
> 
> In any case I think this patch still has the issue, just at one remove.
> Like:
> 
>     (gdb) print sizeof (array[x++])
> 
> For an ordinary array this should not modify x.
> 
> 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] 48+ messages in thread

* RE: [PATCH 01/10] vla: introduce new bound type abstraction adapt uses
  2013-11-07 19:00   ` Tom Tromey
@ 2013-11-18 11:15     ` Agovic, Sanimir
  0 siblings, 0 replies; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-18 11:15 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: gdb-patches

Thanks for your review.

All your comments being addressed and part of v2, see below.

 -Sanimir

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Thursday, November 07, 2013 07:02 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 01/10] vla: introduce new bound type abstraction adapt uses
> 
> Sanimir> +    low.data.const_val = dwarf2_get_attr_constant_value (attr,
> low.data.const_val);
> 
> This line is too long and should be wrapped.
>
Done

> 
> Sanimir> +struct type *
> Sanimir> +create_range_type_1 (struct type *result_type, struct type *index_type,
> Sanimir> +		     const struct dwarf2_prop *low_bound,
> Sanimir> +		     const struct dwarf2_prop *high_bound)
> Sanimir> +{
> 
> This needs an introductory comment.
> 
Done. 

> Sanimir> +/* Used to store bound information for a type.  */
> Sanimir> +
> Sanimir> +struct dwarf2_prop
> Sanimir> +{
> Sanimir> +  /* Determine which field of the union dwarf2_prop.data is used.  */
> Sanimir> +  enum
> Sanimir> +  {
> Sanimir> +    DWARF_CONST,
> Sanimir> +    DWARF_LOCEXPR,
> Sanimir> +    DWARF_LOCLIST
> Sanimir> +  } kind;
> Sanimir> +
> Sanimir> +  /* Stores information as location expression, location list,
> Sanimir> +     or constant value.  */
> Sanimir> +  union data
> Sanimir> +  {
> Sanimir> +    LONGEST const_val;
> Sanimir> +    struct dwarf2_locexpr_baton *locexpr;
> Sanimir> +    struct dwarf2_loclist_baton *loclist;
> Sanimir> +  } data;
> Sanimir> +};
> 
> Sanimir> @@ -589,11 +612,11 @@ struct main_type
> Sanimir>      {
> Sanimir>        /* Low bound of range.  */
> 
> Sanimir> -      LONGEST low;
> Sanimir> +      struct dwarf2_prop low;
> 
> Sanimir>        /* High bound of range.  */
> 
> Sanimir> -      LONGEST high;
> Sanimir> +      struct dwarf2_prop high;
> 
> 
> Just after this hunk of "struct range_bounds" is this code:
> 
>       /* 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;
> 
> It seems to me that it would be cleanest to make this a new value of the
> enum you introduced, like "DWARF_UNDEFINED", and update a few macros to
> follow.  What do you think?
> 
Agreed. Added DWARF_UNDEFINED and updated macro & uses.

 -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] 48+ messages in thread

* Re: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-11-18  9:37         ` Agovic, Sanimir
@ 2013-11-18 15:56           ` Tom Tromey
  2013-11-20 12:18             ` Agovic, Sanimir
  2013-11-19 17:08           ` Pedro Alves
  1 sibling, 1 reply; 48+ messages in thread
From: Tom Tromey @ 2013-11-18 15:56 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: gdb-patches

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

Sanimir> It turns out this patch is not needed anymore as the bounds are
Sanimir> resolved before the sizeof function is called. So in v2 this
Sanimir> patch will be dropped from the series.

Tom> (gdb) print sizeof (array[x++])
Tom> For an ordinary array this should not modify x.

Sanimir> Indeed, that is the behavior we have now in HEAD@vla-c99.

I thought sizeof with a vla as an argument had to evaluate its argument.
So it seems that the above may yield incorrect semantics in some other
test case.

Tom

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

* RE: [PATCH 02/10] type: add c99 variable length array support
  2013-11-07 19:02   ` Tom Tromey
@ 2013-11-19 15:31     ` Agovic, Sanimir
  2013-11-22 20:00       ` Tom Tromey
  0 siblings, 1 reply; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-19 15:31 UTC (permalink / raw)
  To: Tom Tromey (tromey@redhat.com); +Cc: gdb-patches

Thanks for your review.

> Sanimir> +  switch (ctx->location)
> Sanimir> +    {
> Sanimir> +    case DWARF_VALUE_REGISTER:
> Sanimir> +      *valp = dwarf_expr_read_reg (&baton, dwarf_expr_fetch_address (ctx, 0));
> Sanimir> +      break;
> Sanimir> +    case DWARF_VALUE_MEMORY:
> Sanimir> +      *valp = dwarf_expr_fetch_address (ctx, 0);
> Sanimir> +      break;
> Sanimir> +    }
> 
> It seems that something should be done for other DWARF_VALUE_* results
> here.
>
I will try, please have a closer look at this hunk in the upcoming v2.

> Sanimir> +struct type *
> Sanimir> +resolve_dynamic_type (struct type *type, CORE_ADDR address)
> Sanimir> +{
> [...]
> Sanimir> +  if (!TYPE_OBJFILE_OWNED (ty))
> Sanimir> +    return type;
> 
> This seems like a bit of a wart, though I am not sure whether the
> situation can actually arise.
> 
> 
> One thing I didn't see in here is error-checking of whether resolution
> makes sense.
> 
> E.g., suppose I print the value of a pointer-to-VLA.  Then I move to
> some other frame and "print *$".
> 
1| void foo (size_t n) {
2|   int vla[n];
3| }

(gdb) print &vla
$1 = (int (*)[42]) 0xffffffffffff
(gdb) up
(gdb) print *$
$2 = {0 <repeats 42 times>}
(gdb) down
(gdb) print *$
$3 = {0 <repeats 42 times>}

> In this situation the bounds have not been resolved -- but applying the
> DWARF expression in the currently-selected frame will silently do the
> wrong thing.
>
The bounds will be resolved in this case and therefore it is save to change
the frame+re-evaluate. If you have a different example in mind please tell
me and I see how to deal with it.

> I wonder if this series also needs to handle DW_AT_count.
> Maybe no compiler generates it.
> 
I will add a separate patch which implements DW_AT_count, I could not test
it as no compiler emits this attribute so far.


Below you will find the trivial reviews.

 -Sanimir

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Thursday, November 07, 2013 08:00 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 02/10] type: add c99 variable length array support
> 
> >>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> The dwarf standard allow certain attributes to be expressed as
> Sanimir> dwarf expressions rather than constants. For instance
> Sanimir> upper-/lowerbound attributes.  In case of a c99 variable length
> Sanimir> array the upperbound is a dynamic attribute.
> 
> 
> Sanimir> +int
> Sanimir> +dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
> Sanimir> +			   CORE_ADDR addr, CORE_ADDR *valp)
> 
> Need an introductory comment.  It can just say "See dwarf2loc.h.", since
> you put the real comment there.
> 
Done.

> Sanimir> +static struct dwarf2_locexpr_baton* attr_to_locexprbaton
> Sanimir> +(const struct attribute *, struct dwarf2_cu *);
> Sanimir> +
> Sanimir> +static struct dwarf2_locexpr_baton* attr_to_locexprbaton_1
> Sanimir> +(const struct attribute *, struct dwarf2_cu *, const gdb_byte *additional_data,
> Sanimir> + int extra_size);
> Sanimir> +
> Sanimir> +static int attr_to_dwarf2_prop
> Sanimir> +(struct die_info *, const struct attribute *, struct dwarf2_cu *,
> Sanimir> + struct dwarf2_prop *);
> 
> In cases like this we usually indent the subsequent lines a bit, like:
> 
>     static int attr_to_dwarf2_prop
>         (struct die_info *, const struct attribute *, struct dwarf2_cu *,
>          struct dwarf2_prop *);
> 
> However in this case I think it may be preferable to rearrange the
> functions so that forward declarations are not needed.  What do you
> think?
> 
Done. Moved functions and dropped forward declaration.

> Sanimir> +static struct dwarf2_locexpr_baton*
> Sanimir> +attr_to_locexprbaton (const struct attribute *attribute, struct dwarf2_cu *cu)
> Sanimir> +{
> Sanimir> +  return attr_to_locexprbaton_1 (attribute, cu, NULL, 0);
> Sanimir> +}
> 
> If there is just a single caller (there is in this patch, but I haven't
> read all the patches yet), I would remove this function and just update
> the caller.
> 
Done. Folded function attr_to_locexprbaton/_1 into attr_to_locexprbaton.

> Sanimir> +static struct dwarf2_locexpr_baton*
> Sanimir> +attr_to_locexprbaton_1 (const struct attribute *attribute, struct dwarf2_cu *cu,
> Sanimir> +			const gdb_byte *additional_data, int extra_size)
> 
> Needs an introductory comment.
> 
Done.

> Sanimir> +    /* Copy the data pointer as the blocks lifetime is
> 
> Missing apostrophe: "block's".
> 
Done.

> Sanimir> +  gdb_assert(baton->data != NULL);
> 
> Space before open paren.
> 
Done.

> Sanimir> +/* Parse dwarf attribute if it's a block, reference or constant and put the
> Sanimir> +   resulting value of the attribute into struct dwarf2_prop. */
> Sanimir> +
> Sanimir> +static int
> Sanimir> +attr_to_dwarf2_prop (struct die_info *die, const struct attribute *attr,
> Sanimir> +		     struct dwarf2_cu *cu,
> Sanimir> +		     struct dwarf2_prop *prop)
> 
> I think it would be good if the introductory comment describe the return
> value.
> 
Done.

> Sanimir> +  else if (attr_form_is_ref (attr))
> Sanimir> +    {
> Sanimir> +      struct dwarf2_cu *target_cu = cu;
> Sanimir> +      struct die_info *target_die;
> Sanimir> +      struct attribute *target_attr;
> Sanimir> +      const gdb_byte append_ops[] = { DW_OP_deref };
> Sanimir> +
> Sanimir> +      target_die = follow_die_ref (die, attr, &target_cu);
> Sanimir> +      target_attr = dwarf2_attr (target_die, DW_AT_location, target_cu);
> Sanimir> +
> Sanimir> +      prop->data.locexpr =
> Sanimir> +	attr_to_locexprbaton_1 (target_attr, cu, append_ops,
> Sanimir> +				sizeof (append_ops) / sizeof (append_ops[0]));
> Sanimir> +      prop->kind = DWARF_LOCEXPR;
> Sanimir> +      gdb_assert (prop->data.locexpr != NULL);
> 
> I don't understand this hunk.  Could you say why it is needed?
> 
DW_AT_upper_bound may reference a die e.g. artificial variable, in this case we
need to read DW_AT_location of the target die. Some compilers emit this form to
implement vla. 

> Sanimir> +      dwarf2_invalid_attrib_class_complaint(dwarf_form_name (attr->form),
> Sanimir> +					    dwarf2_name (die, cu));
> 
> Missing space before a paren.
> 
Done.

> Sanimir> +static int
> Sanimir> +has_static_range (const struct range_bounds *bounds)
> Sanimir> +{
> Sanimir> +  return bounds->low.kind == DWARF_CONST
> Sanimir> +    && bounds->high.kind == DWARF_CONST;
> Sanimir> +}
> 
> THis needs parens around the argument to "return" and then an
> indentation fix on the second line.
> 
Done.

> Sanimir> +/* Calculates the size of a type given the upper and lower bound of a dynamic
> Sanimir> +   type.  */
> Sanimir> +
> Sanimir> +static ULONGEST
> Sanimir> +get_type_length (const struct type *type)
> Sanimir> +{
> Sanimir> +  const struct type *range_type, *target_type;
> Sanimir> +  ULONGEST len = TYPE_LENGTH (type);
> Sanimir> +  LONGEST low_bound, high_bound;
> Sanimir> +
> Sanimir> +  if (TYPE_CODE (type) != TYPE_CODE_ARRAY
> Sanimir> +      && TYPE_CODE (type) != TYPE_CODE_STRING)
> Sanimir> +    return len;
> Sanimir> +
> Sanimir> +  range_type = TYPE_INDEX_TYPE (type);
> Sanimir> +
> Sanimir> +  if (!has_static_range (TYPE_RANGE_DATA (range_type)))
> Sanimir> +    return len;
> 
> This seems like it doesn't follow what the introductory comment says it
> does.
> 
Fixed document.

> Sanimir> +
> Sanimir> +static void
> Sanimir> +resolve_dynamic_bounds (struct type *type, CORE_ADDR address)
> 
> Introductory comment.
> 
Done.

> Sanimir> +	  do {
> Sanimir> +	    struct type *range_type = TYPE_INDEX_TYPE (ary_dim);
> 
> It's hard to know but perhaps a check_typedef is required here.
> 
> Sanimir> +	    ary_dim = TYPE_TARGET_TYPE (ary_dim);
> 
> Here too.
> 
Done.




Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

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

* Re: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-11-18  9:37         ` Agovic, Sanimir
  2013-11-18 15:56           ` Tom Tromey
@ 2013-11-19 17:08           ` Pedro Alves
  2013-11-20 12:47             ` Agovic, Sanimir
  1 sibling, 1 reply; 48+ messages in thread
From: Pedro Alves @ 2013-11-19 17:08 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: 'Tom Tromey', gdb-patches

On 11/18/2013 08:39 AM, Agovic, Sanimir wrote:
> 
> It turns out this patch is not needed anymore as the bounds are resolved 
> before the sizeof function is called. So in v2 this patch will be dropped
> from the series.
> 
>> >     (gdb) print sizeof (array[x++])
>> > 
>> > For an ordinary array this should not modify x.
> Indeed, that is the behavior we have now in HEAD@vla-c99.

Sounds like something that the testsuite should have caught.
I guess we don't have such a test?

-- 
Pedro Alves

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

* RE: [PATCH 06/10] vla: update type from newly created value
  2013-11-07 20:56   ` Tom Tromey
@ 2013-11-20  7:56     ` Agovic, Sanimir
  0 siblings, 0 replies; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-20  7:56 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: gdb-patches

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Thursday, November 07, 2013 09:43 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 06/10] vla: update type from newly created value
> 
> >>>>> "Sanimir" == Sanimir Agovic <sanimir.agovic@intel.com> writes:
> 
> Sanimir> Constructing a value based on a type and address might change
> Sanimir> the type of the newly constructed value. Thus re-fetch type via
> Sanimir> value_type to ensure we have the correct type at hand.
> 
> Sanimir> 2013-10-18  Sanimir Agovic  <sanimir.agovic@intel.com>
> Sanimir>             Keven Boell  <keven.boell@intel.com>
> 
> Sanimir> 	* valops.c (value_ind): Re-fetch type from value.
> Sanimir> 	* value.c (coerce_ref): Re-fetch type from value.
> 
> This is fine by itself; however, I wonder whether there are other calls
> that require fixing.  Did you check through the rest of the code?
> 
My approach to find similar occurrences:
1) Grep sources for value_at / value_at_lazy / value_from_contents_and_address
2) Follow control flow within a function, if the type passed to the above
   constructors is used after constructing a value I re-fetched the type.

Running the testsuite before/after this patch series showed no regressions.
I used Jans diffgdb. gjc, gcc/++, pascal, and ada was used on Fedora20. 

I run into some ada fails with and without c99 patches. Not sure if this
is known or expected:
FAIL: gdb.ada/interface.exp: print s
FAIL: gdb.ada/iwide.exp: print My_Drawable
FAIL: gdb.ada/iwide.exp: print s_access.all
FAIL: gdb.ada/iwide.exp: print sp_access.all
FAIL: gdb.ada/iwide.exp: print d_access.all
FAIL: gdb.ada/iwide.exp: print dp_access.all
FAIL: gdb.ada/ptype_tagged_param.exp: ptype s
FAIL: gdb.ada/tagged.exp: ptype obj
FAIL: gdb.ada/tagged.exp: print obj

Below you will find all hits. Let me know if it is OK.

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 9ff3ab9..26d0c24 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -2299,6 +2299,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);
     }
@@ -7656,6 +7657,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;
@@ -7758,7 +7760,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;

@@ -7899,7 +7904,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;

@@ -8197,6 +8205,8 @@ 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 1d7147c..72c8d41 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -446,6 +446,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.  */
@@ -548,6 +549,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 cb89a85..27901a3 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 6706d9f..b539610 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)))

 -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] 48+ messages in thread

* Re: [PATCH 06/10] vla: update type from newly created value
  2013-10-21 14:40 ` [PATCH 06/10] vla: update type from newly created value Sanimir Agovic
  2013-11-07 20:56   ` Tom Tromey
@ 2013-11-20 11:02   ` Pedro Alves
  2013-11-20 13:08     ` Agovic, Sanimir
  2013-11-23 19:27     ` Doug Evans
  1 sibling, 2 replies; 48+ messages in thread
From: Pedro Alves @ 2013-11-20 11:02 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

On 10/21/2013 03:40 PM, Sanimir Agovic wrote:
> Constructing a value based on a type and address might change the type
> of the newly constructed value. 

OOC (and for the archives), why's that?  Where does that occur?

> Thus re-fetch type via value_type to ensure
> we have the correct type at hand.

-- 
Pedro Alves

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

* RE: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-11-18 15:56           ` Tom Tromey
@ 2013-11-20 12:18             ` Agovic, Sanimir
  0 siblings, 0 replies; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-20 12:18 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: gdb-patches

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Monday, November 18, 2013 04:53 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 05/10] vla: allow side effects for sizeof argument
> 
> >>>>> "Sanimir" == Agovic, Sanimir <sanimir.agovic@intel.com> writes:
> 
> Sanimir> It turns out this patch is not needed anymore as the bounds are
> Sanimir> resolved before the sizeof function is called. So in v2 this
> Sanimir> patch will be dropped from the series.
> 
> Tom> (gdb) print sizeof (array[x++])
> Tom> For an ordinary array this should not modify x.
> 
> Sanimir> Indeed, that is the behavior we have now in HEAD@vla-c99.
> 
> I thought sizeof with a vla as an argument had to evaluate its argument.
> So it seems that the above may yield incorrect semantics in some other
> test case.
> 
The patch was needed for a now discontinued vla prototype, meanwhile we
moved the logic to the following value constructors 
value_at/value_at_lazy/value_from_contents_and_address which is easier to
maintain and is mostly transparent to existing code.

In this particular case we will end up in dispatching 'array' in the switch
case 'OP_VAR_VALUE', using value_of_variable which will end up calling a
value constructor with an address, therefore we resolved correctly the
bounds and the patch is not needed anymore.

 -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] 48+ messages in thread

* RE: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-11-19 17:08           ` Pedro Alves
@ 2013-11-20 12:47             ` Agovic, Sanimir
  2013-11-20 13:24               ` Pedro Alves
  0 siblings, 1 reply; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-20 12:47 UTC (permalink / raw)
  To: 'Pedro Alves'; +Cc: 'Tom Tromey', gdb-patches

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, November 19, 2013 06:06 PM
> To: Agovic, Sanimir
> Cc: 'Tom Tromey'; gdb-patches@sourceware.org
> Subject: Re: [PATCH 05/10] vla: allow side effects for sizeof argument
> 
> On 11/18/2013 08:39 AM, Agovic, Sanimir wrote:
> >
> > It turns out this patch is not needed anymore as the bounds are resolved
> > before the sizeof function is called. So in v2 this patch will be dropped
> > from the series.
> >
> >> >     (gdb) print sizeof (array[x++])
> >> >
> >> > For an ordinary array this should not modify x.
> > Indeed, that is the behavior we have now in HEAD@vla-c99.
> 
> Sounds like something that the testsuite should have caught.
> I guess we don't have such a test?
> 
No yet, see below.

test: test eval routines with EVAL_AVOID_SIDE_EFFECTS flag set

Ensure that certain commands (e.g. whatis/ptype) and sizeof intrinsic
have no side effects (variables cannot be altered).

2013-11-20  Sanimir Agovic  <sanimir.agovic@intel.com>

testsuite/
	* gdb.base/eval-avoid-side-effects.exp: New test.

diff --git a/gdb/testsuite/gdb.base/eval-avoid-side-effects.exp b/gdb/testsuite/gdb.base/eval-avoid-side-effects.exp
new file mode 100644
index 0000000..da1e36f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/eval-avoid-side-effects.exp
@@ -0,0 +1,40 @@
+# 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 evaluate_subexp and others with EVAL_AVOID_SIDE_EFFECTS
+# flag set.
+
+standard_testfile int-type.c
+
+if { [prepare_for_testing ${testfile}.exp $testfile $srcfile] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+set sizeof_int [get_sizeof "int" 4]
+
+gdb_test_no_output "set variable x=42" "set variable x=42"
+
+gdb_test "print sizeof ++x" "= ${sizeof_int}" "test sizeof side effects"
+gdb_test "print x" "= 42" "sizeof has no side effects"
+
+gdb_test "ptype ++x" "= int" "test ptype side effects"
+gdb_test "print x" "= 42" "ptype has no side effects"
+
+gdb_test "whatis ++x" "= int" "test whatis side effects"
+gdb_test "print x" "= 42" "whatis has no side effects"
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] 48+ messages in thread

* RE: [PATCH 06/10] vla: update type from newly created value
  2013-11-20 11:02   ` Pedro Alves
@ 2013-11-20 13:08     ` Agovic, Sanimir
  2013-11-21 18:50       ` Pedro Alves
  2013-11-23 19:27     ` Doug Evans
  1 sibling, 1 reply; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-20 13:08 UTC (permalink / raw)
  To: 'Pedro Alves'; +Cc: gdb-patches

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, November 20, 2013 11:30 AM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 06/10] vla: update type from newly created value
> 
> On 10/21/2013 03:40 PM, Sanimir Agovic wrote:
> > Constructing a value based on a type and address might change the type
> > of the newly constructed value.
> 
> OOC (and for the archives), why's that?  Where does that occur?
> 
Since dwarf3 certain attributes e.g upper/lower bound may be computed
dynamically. To support such attributes with the current gdb type-system
we require such types to be "normalized". This means types with dynamic
properties are converted to types with static properties.
To successful convert a type with dynamic properties into one with static
properties access to inferior memory is needed. Therefore we hooked into
the following value constructors
value_at/value_at_lazy/value_from_contents_and_address
as they require an inferior address in addition to a type to instantiate
a value. IIF the passed type has dynamic properties we resolve the bounds
and thus change the type.

Given the following statement:

  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 (vla-c00) patch series. Type 
normalization is done in the mentioned value constructors and might 
change the value type, therefore we need to re-fetch the type from
the constructed value.

I hope this makes sense.

> > Thus re-fetch type via value_type to ensure
> > we have the correct type at hand.
> 
> --
> Pedro Alves

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] 48+ messages in thread

* Re: [PATCH 05/10] vla: allow side effects for sizeof argument
  2013-11-20 12:47             ` Agovic, Sanimir
@ 2013-11-20 13:24               ` Pedro Alves
  0 siblings, 0 replies; 48+ messages in thread
From: Pedro Alves @ 2013-11-20 13:24 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: 'Tom Tromey', gdb-patches

On 11/20/2013 12:17 PM, Agovic, Sanimir wrote:

>> Sounds like something that the testsuite should have caught.
>> I guess we don't have such a test?
>>
> No yet, see below.
> 
> test: test eval routines with EVAL_AVOID_SIDE_EFFECTS flag set
> 
> Ensure that certain commands (e.g. whatis/ptype) and sizeof intrinsic
> have no side effects (variables cannot be altered).
> 
> 2013-11-20  Sanimir Agovic  <sanimir.agovic@intel.com>
> 
> testsuite/
> 	* gdb.base/eval-avoid-side-effects.exp: New test.

Excellent!  Thanks, this is OK.

-- 
Pedro Alves

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

* Re: [PATCH 06/10] vla: update type from newly created value
  2013-11-20 13:08     ` Agovic, Sanimir
@ 2013-11-21 18:50       ` Pedro Alves
  0 siblings, 0 replies; 48+ messages in thread
From: Pedro Alves @ 2013-11-21 18:50 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: gdb-patches

On 11/20/2013 12:46 PM, Agovic, Sanimir wrote:

> I hope this makes sense.

Thanks, it does.

-- 
Pedro Alves

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

* Re: C99 variable length array support
  2013-10-21 14:40 C99 variable length array support Sanimir Agovic
                   ` (9 preceding siblings ...)
  2013-10-21 14:40 ` [PATCH 04/10] vla: enable sizeof operator for indirection Sanimir Agovic
@ 2013-11-21 18:52 ` Pedro Alves
  2013-11-21 19:01   ` Pedro Alves
  10 siblings, 1 reply; 48+ messages in thread
From: Pedro Alves @ 2013-11-21 18:52 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

Hi!

I had this series applied locally (on top of 2acaf6f),
and now noticed that running "$ ./gdb ./gdb", and then "start" triggers an
assertion:

../../src/gdb/dwarf2read.c:22435: internal-error: attr_to_locexprbaton_1: Assertion `attribute != NULL && cu != NULL && attr_form_is_block (attribute)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.

#0  internal_error (file=0x8d3105 "../../src/gdb/dwarf2read.c", line=22435, string=0x8d2a40 "%s: Assertion `%s' failed.") at ../../src/gdb/utils.c:829
#1  0x000000000066f0d6 in attr_to_locexprbaton_1 (attribute=0x1838158, cu=0xfd6d20, additional_data=0x7fffffffb827 "\006 ", <incomplete sequence \375>, extra_size=1)
    at ../../src/gdb/dwarf2read.c:22434
#2  0x000000000066f54a in attr_to_dwarf2_prop (die=0x1848130, attr=0x1848168, cu=0xfd6d20, prop=0x7fffffffb870) at ../../src/gdb/dwarf2read.c:22490
#3  0x000000000065db20 in read_subrange_type (die=0x1848130, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:14338
#4  0x0000000000665cf6 in read_type_die_1 (die=0x1848130, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:18151
#5  0x0000000000665b3d in read_type_die (die=0x1848130, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:18093
#6  0x000000000065b563 in read_array_type (die=0x18480e0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:13256
#7  0x0000000000665bde in read_type_die_1 (die=0x18480e0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:18121
#8  0x0000000000665b3d in read_type_die (die=0x18480e0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:18093
#9  0x0000000000665ac4 in lookup_die_type (die=0x1837360, attr=0x1837398, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:18065
#10 0x0000000000665554 in die_type (die=0x1837360, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:17915
#11 0x000000000066402b in new_symbol_full (die=0x1837360, type=0x0, cu=0xfd6d20, space=0x0) at ../../src/gdb/dwarf2read.c:17390
#12 0x0000000000664b2d in new_symbol (die=0x1837360, type=0x0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:17723
#13 0x000000000064e3d9 in process_die (die=0x1837360, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:8099
#14 0x000000000065551c in read_lexical_block_scope (die=0x1837260, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:11158
#15 0x000000000064e213 in process_die (die=0x1837260, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:8033
#16 0x000000000065551c in read_lexical_block_scope (die=0x1833c40, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:11158
#17 0x000000000064e213 in process_die (die=0x1833c40, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:8033
#18 0x0000000000654f70 in read_func_scope (die=0x1833b50, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:11053
#19 0x000000000064e1fb in process_die (die=0x1833b50, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:8028
#20 0x0000000000654f70 in read_func_scope (die=0x1832be0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:11053
#21 0x000000000064e1fb in process_die (die=0x1832be0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:8028
#22 0x000000000064fafd in read_file_scope (die=0x17d56d0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:8903
#23 0x000000000064e1cb in process_die (die=0x17d56d0, cu=0xfd6d20) at ../../src/gdb/dwarf2read.c:8021
#24 0x000000000064daf4 in process_full_comp_unit (per_cu=0x179c400, pretend_language=language_minimal) at ../../src/gdb/dwarf2read.c:7822
#25 0x000000000064ca6a in process_queue () at ../../src/gdb/dwarf2read.c:7362
#26 0x0000000000641dca in dw2_do_instantiate_symtab (per_cu=0x179c400) at ../../src/gdb/dwarf2read.c:2625
#27 0x000000000064cd29 in psymtab_to_symtab_1 (pst=0x179f690) at ../../src/gdb/dwarf2read.c:7451
#28 0x000000000064c755 in dwarf2_read_symtab (self=0x179f690, objfile=0x1793af0) at ../../src/gdb/dwarf2read.c:7231
#29 0x00000000005af163 in psymtab_to_symtab (objfile=0x1793af0, pst=0x179f690) at ../../src/gdb/psymtab.c:779
#30 0x00000000005ae403 in find_pc_sect_symtab_from_partial (objfile=0x1793af0, msymbol=0x178d360, pc=215771788970, section=0x0, warn_if_readin=1)
    at ../../src/gdb/psymtab.c:391
#31 0x00000000005a7fff in find_pc_sect_symtab (pc=215771788970, section=0x0) at ../../src/gdb/symtab.c:2191
#32 0x00000000005a8054 in find_pc_symtab (pc=215771788970) at ../../src/gdb/symtab.c:2208
#33 0x00000000006fa506 in select_frame (fi=0xd9e670) at ../../src/gdb/frame.c:1519
#34 0x00000000006fa433 in get_selected_frame (message=0x0) at ../../src/gdb/frame.c:1457
#35 0x0000000000584bd8 in evaluate_subexp_standard (expect_type=0xf46980, exp=0xed0920, pos=0x7fffffffce6c, noside=EVAL_NORMAL) at ../../src/gdb/eval.c:835
#36 0x00000000006a9b8f in evaluate_subexp_c (expect_type=0xf46980, exp=0xed0920, pos=0x7fffffffce6c, noside=EVAL_NORMAL) at ../../src/gdb/c-lang.c:701
#37 0x00000000005830df in evaluate_subexp (expect_type=0xf46980, exp=0xed0920, pos=0x7fffffffce6c, noside=EVAL_NORMAL) at ../../src/gdb/eval.c:71
#38 0x000000000058a3bf in evaluate_subexp_standard (expect_type=0xf46980, exp=0xed0920, pos=0x7fffffffce6c, noside=EVAL_NORMAL) at ../../src/gdb/eval.c:2571
#39 0x00000000005266a4 in stap_evaluate_probe_argument (probe_generic=0x1774a10, n=1) at ../../src/gdb/stap-probe.c:1110
#40 0x0000000000524050 in elf_evaluate_probe_argument (probe=0x1774a10, n=1) at ../../src/gdb/elfread.c:1532
#41 0x000000000072865d in evaluate_probe_argument (probe=0x1774a10, n=1) at ../../src/gdb/probe.c:662
#42 0x0000000000484717 in svr4_handle_solib_event () at ../../src/gdb/solib-svr4.c:1819
#43 0x000000000070b994 in handle_solib_event () at ../../src/gdb/solib.c:1249
#44 0x0000000000557a73 in bpstat_stop_status (aspace=0xe03f20, bp_addr=215771788970, ptid=..., ws=0x7fffffffd2e0) at ../../src/gdb/breakpoint.c:5338
#45 0x00000000005d452f in handle_signal_stop (ecs=0x7fffffffd2c0) at ../../src/gdb/infrun.c:4234
#46 0x00000000005d30ec in handle_inferior_event (ecs=0x7fffffffd2c0) at ../../src/gdb/infrun.c:3721
#47 0x00000000005d1103 in wait_for_inferior () at ../../src/gdb/infrun.c:2757
#48 0x00000000005d04b7 in proceed (addr=215771780400, siggnal=GDB_SIGNAL_0, step=0) at ../../src/gdb/infrun.c:2330
#49 0x00000000005c84c0 in run_command_1 (args=0x0, from_tty=1, tbreak_at_main=1) at ../../src/gdb/infcmd.c:611
#50 0x00000000005c855d in start_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:644
#51 0x00000000004dd637 in do_cfunc (c=0xd7a920, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:107
#52 0x00000000004e06cc in cmd_func (cmd=0xd7a920, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:1882
#53 0x00000000006edfb9 in execute_command (p=0xcbf6c5 "", from_tty=1) at ../../src/gdb/top.c:467
#54 0x00000000005f2807 in command_handler (command=0xcbf6c0 "") at ../../src/gdb/event-top.c:435
#55 0x00000000005f2dc6 in command_line_handler (rl=0x162a3b0 "main") at ../../src/gdb/event-top.c:632
#56 0x0000000000747ac2 in rl_callback_read_char () at ../../src/readline/callback.c:220
#57 0x00000000005f2329 in rl_callback_read_char_wrapper (client_data=0x0) at ../../src/gdb/event-top.c:164
#58 0x00000000005f271e in stdin_event_handler (error=0, client_data=0x0) at ../../src/gdb/event-top.c:375
#59 0x00000000005f12ae in handle_file_event (data=...) at ../../src/gdb/event-loop.c:768
#60 0x00000000005f0757 in process_event () at ../../src/gdb/event-loop.c:342
#61 0x00000000005f07f9 in gdb_do_one_event () at ../../src/gdb/event-loop.c:394
#62 0x00000000005f086f in start_event_loop () at ../../src/gdb/event-loop.c:431
#63 0x00000000005f235b in cli_command_loop (data=0x0) at ../../src/gdb/event-top.c:179
#64 0x00000000005e8e87 in current_interp_command_loop () at ../../src/gdb/interps.c:327
#65 0x00000000005e98b3 in captured_command_loop (data=0x0) at ../../src/gdb/main.c:267
#66 0x00000000005e72aa in catch_errors (func=0x5e9898 <captured_command_loop>, func_args=0x0, errstring=0x8afe14 "", mask=RETURN_MASK_ALL)
    at ../../src/gdb/exceptions.c:524
#67 0x00000000005eacd5 in captured_main (data=0x7fffffffda20) at ../../src/gdb/main.c:1067
#68 0x00000000005e72aa in catch_errors (func=0x5e9b4f <captured_main>, func_args=0x7fffffffda20, errstring=0x8afe14 "", mask=RETURN_MASK_ALL)
    at ../../src/gdb/exceptions.c:524
#69 0x00000000005ead0b in gdb_main (args=0x7fffffffda20) at ../../src/gdb/main.c:1076
#70 0x000000000045b8da in main (argc=2, argv=0x7fffffffdb28) at ../../src/gdb/gdb.c:34
(top-gdb)

I haven't tried the v2 series.

-- 
Pedro Alves

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

* Re: C99 variable length array support
  2013-11-21 18:52 ` C99 variable length array support Pedro Alves
@ 2013-11-21 19:01   ` Pedro Alves
  2013-11-21 19:10     ` Pedro Alves
  0 siblings, 1 reply; 48+ messages in thread
From: Pedro Alves @ 2013-11-21 19:01 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

On 11/21/2013 06:50 PM, Pedro Alves wrote:
> Hi!
> 
> I had this series applied locally (on top of 2acaf6f),
> and now noticed that running "$ ./gdb ./gdb", and then "start" triggers an
> assertion:
> 
> ../../src/gdb/dwarf2read.c:22435: internal-error: attr_to_locexprbaton_1: Assertion `attribute != NULL && cu != NULL && attr_form_is_block (attribute)' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> 

BTW, that was:

x86_64 Fedora 17, gcc version 4.7.2 20120921 (Red Hat 4.7.2-2) (GCC)

-- 
Pedro Alves

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

* Re: C99 variable length array support
  2013-11-21 19:01   ` Pedro Alves
@ 2013-11-21 19:10     ` Pedro Alves
  2013-11-22 10:53       ` Agovic, Sanimir
  0 siblings, 1 reply; 48+ messages in thread
From: Pedro Alves @ 2013-11-21 19:10 UTC (permalink / raw)
  To: Sanimir Agovic; +Cc: gdb-patches

FYI,

With v2 I get no assertion.  Seems to have been fixed somehow.

Thanks,
-- 
Pedro Alves

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

* RE: C99 variable length array support
  2013-11-21 19:10     ` Pedro Alves
@ 2013-11-22 10:53       ` Agovic, Sanimir
  2013-11-22 12:35         ` Pedro Alves
  0 siblings, 1 reply; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-22 10:53 UTC (permalink / raw)
  To: 'Pedro Alves'; +Cc: gdb-patches

Could you point me to the location of the assertion or a reproducer? Thanks.

 -Sanimir

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Thursday, November 21, 2013 08:02 PM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: C99 variable length array support
> 
> FYI,
> 
> With v2 I get no assertion.  Seems to have been fixed somehow.
> 
> Thanks,
> --
> Pedro Alves

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] 48+ messages in thread

* Re: C99 variable length array support
  2013-11-22 10:53       ` Agovic, Sanimir
@ 2013-11-22 12:35         ` Pedro Alves
  2013-11-22 17:06           ` Agovic, Sanimir
  0 siblings, 1 reply; 48+ messages in thread
From: Pedro Alves @ 2013-11-22 12:35 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: gdb-patches

On 11/22/2013 08:41 AM, Agovic, Sanimir wrote:
> Could you point me to the location of the assertion or a reproducer? Thanks.

The location was here, with a backtrace:

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

I've sent you the GDB binary off list.

Thanks,
-- 
Pedro Alves

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

* RE: C99 variable length array support
  2013-11-22 12:35         ` Pedro Alves
@ 2013-11-22 17:06           ` Agovic, Sanimir
  0 siblings, 0 replies; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-22 17:06 UTC (permalink / raw)
  To: 'Pedro Alves'; +Cc: gdb-patches

Thank you very much Pedro! This should help me to fix the issue.

 -Sanimir

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, November 22, 2013 11:53 AM
> To: Agovic, Sanimir
> Cc: gdb-patches@sourceware.org
> Subject: Re: C99 variable length array support
> 
> On 11/22/2013 08:41 AM, Agovic, Sanimir wrote:
> > Could you point me to the location of the assertion or a reproducer? Thanks.
> 
> The location was here, with a backtrace:
> 
>  https://sourceware.org/ml/gdb-patches/2013-11/msg00646.html
> 
> I've sent you the GDB binary off list.
> 
> Thanks,
> --
> Pedro Alves

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] 48+ messages in thread

* Re: [PATCH 02/10] type: add c99 variable length array support
  2013-11-19 15:31     ` Agovic, Sanimir
@ 2013-11-22 20:00       ` Tom Tromey
  2013-11-27 17:08         ` Agovic, Sanimir
       [not found]         ` <0377C58828D86C4588AEEC42FC3B85A7176BC3DE@IRSMSX105.ger.corp.intel.com>
  0 siblings, 2 replies; 48+ messages in thread
From: Tom Tromey @ 2013-11-22 20:00 UTC (permalink / raw)
  To: Agovic, Sanimir; +Cc: gdb-patches

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

>> I wonder if this series also needs to handle DW_AT_count.
>> Maybe no compiler generates it.

Sanimir> I will add a separate patch which implements DW_AT_count, I
Sanimir> could not test it as no compiler emits this attribute so far.

FWIW, it can be tested via the DWARF assembler.

However it is fine by me if you want to leave it out.

Tom

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

* Re: [PATCH 06/10] vla: update type from newly created value
  2013-11-20 11:02   ` Pedro Alves
  2013-11-20 13:08     ` Agovic, Sanimir
@ 2013-11-23 19:27     ` Doug Evans
  1 sibling, 0 replies; 48+ messages in thread
From: Doug Evans @ 2013-11-23 19:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Sanimir Agovic, gdb-patches

On Wed, Nov 20, 2013 at 2:29 AM, Pedro Alves <palves@redhat.com> wrote:
> On 10/21/2013 03:40 PM, Sanimir Agovic wrote:
>> Constructing a value based on a type and address might change the type
>> of the newly constructed value.
>
> OOC (and for the archives), why's that?  Where does that occur?
>
>> Thus re-fetch type via value_type to ensure
>> we have the correct type at hand.

Not just for the archives.

I didn't find it in the patch series, but I could have missed it of course.

I think a VERY LOUD comment needs to be added to these routines to
tell the reader "HEADS UP! If you do <this> after calling me, then you
MUST also do <this>."
[or words to that effect]

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

* RE: [PATCH 02/10] type: add c99 variable length array support
  2013-11-22 20:00       ` Tom Tromey
@ 2013-11-27 17:08         ` Agovic, Sanimir
       [not found]         ` <0377C58828D86C4588AEEC42FC3B85A7176BC3DE@IRSMSX105.ger.corp.intel.com>
  1 sibling, 0 replies; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-27 17:08 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: gdb-patches

> Sanimir> I will add a separate patch which implements DW_AT_count, I
> Sanimir> could not test it as no compiler emits this attribute so far.
> 
> FWIW, it can be tested via the DWARF assembler.
> 
> However it is fine by me if you want to leave it out.
>
Thanks for your review, below you will find a testcase and a fix for a
bug the test revealed. The latter patch will be squashed in v3.

commit 5351ec3e002726893c42c6cf210c8b697a8e51d0
Author: Sanimir Agovic <sanimir.agovic@intel.com>
Date:   Tue Nov 26 14:47:16 2013 +0000

    test: cover subranges with present DW_AT_count attribute
    
    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: I58868d4f82691ec3be06b55face759ebc72c930c

diff --git a/gdb/testsuite/gdb.dwarf2/count.exp b/gdb/testsuite/gdb.dwarf2/count.exp
new file mode 100644
index 0000000..a524e89
--- /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"




commit 7d44299555837bac03f2230129bc5d88ede6a307
Author: Sanimir Agovic <sanimir.agovic@intel.com>
Date:   Tue Nov 26 14:35:43 2013 +0000

    vla: resolve dynamic bounds if value contents is a constant byte-sequence

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

diff --git a/gdb/findvar.c b/gdb/findvar.c
index ec6afd6..c1302d8 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));
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] 48+ messages in thread

* RE: [PATCH 02/10] type: add c99 variable length array support
       [not found]         ` <0377C58828D86C4588AEEC42FC3B85A7176BC3DE@IRSMSX105.ger.corp.intel.com>
@ 2013-11-27 17:15           ` Agovic, Sanimir
  0 siblings, 0 replies; 48+ messages in thread
From: Agovic, Sanimir @ 2013-11-27 17:15 UTC (permalink / raw)
  To: 'Tom Tromey'; +Cc: gdb-patches

> +gdb_test "ptype array" "type = char \\\[5]\\\" "ptype array"
>
This should be rather:
+gdb_test "ptype array" "type = char \\\[5\\\]" "ptype array"

Escaped the quote instead of closing bracket.

 -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] 48+ messages in thread

end of thread, other threads:[~2013-11-27 15:58 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-21 14:40 C99 variable length array support Sanimir Agovic
2013-10-21 14:40 ` [PATCH 07/10] test: evaluate pointers to C99 vla correctly Sanimir Agovic
2013-11-07 20:57   ` Tom Tromey
2013-11-08  6:37     ` Tom Tromey
2013-11-07 21:14   ` Tom Tromey
2013-10-21 14:40 ` [PATCH 02/10] type: add c99 variable length array support Sanimir Agovic
2013-11-07 19:02   ` Tom Tromey
2013-11-19 15:31     ` Agovic, Sanimir
2013-11-22 20:00       ` Tom Tromey
2013-11-27 17:08         ` Agovic, Sanimir
     [not found]         ` <0377C58828D86C4588AEEC42FC3B85A7176BC3DE@IRSMSX105.ger.corp.intel.com>
2013-11-27 17:15           ` Agovic, Sanimir
2013-11-07 19:10   ` Tom Tromey
2013-10-21 14:40 ` [PATCH 05/10] vla: allow side effects for sizeof argument Sanimir Agovic
2013-10-24 19:55   ` Tom Tromey
2013-10-25  8:13     ` Agovic, Sanimir
2013-10-28 21:00       ` Tom Tromey
2013-11-18  9:37         ` Agovic, Sanimir
2013-11-18 15:56           ` Tom Tromey
2013-11-20 12:18             ` Agovic, Sanimir
2013-11-19 17:08           ` Pedro Alves
2013-11-20 12:47             ` Agovic, Sanimir
2013-11-20 13:24               ` Pedro Alves
2013-10-21 14:40 ` [PATCH 03/10] vla: enable sizeof operator to work with variable length arrays Sanimir Agovic
2013-11-07 19:10   ` Tom Tromey
2013-10-21 14:40 ` [PATCH 10/10] test: add mi vla test Sanimir Agovic
2013-11-07 21:31   ` Tom Tromey
2013-10-21 14:40 ` [PATCH 09/10] test: basic c99 vla tests Sanimir Agovic
2013-11-07 21:23   ` Tom Tromey
2013-10-21 14:40 ` [PATCH 06/10] vla: update type from newly created value Sanimir Agovic
2013-11-07 20:56   ` Tom Tromey
2013-11-20  7:56     ` Agovic, Sanimir
2013-11-20 11:02   ` Pedro Alves
2013-11-20 13:08     ` Agovic, Sanimir
2013-11-21 18:50       ` Pedro Alves
2013-11-23 19:27     ` Doug Evans
2013-10-21 14:40 ` [PATCH 08/10] test: multi-dimensional c99 vla Sanimir Agovic
2013-11-07 21:19   ` Tom Tromey
2013-10-21 14:40 ` [PATCH 01/10] vla: introduce new bound type abstraction adapt uses Sanimir Agovic
2013-11-07 19:00   ` Tom Tromey
2013-11-18 11:15     ` Agovic, Sanimir
2013-10-21 14:40 ` [PATCH 04/10] vla: enable sizeof operator for indirection Sanimir Agovic
2013-11-07 19:57   ` Tom Tromey
2013-11-21 18:52 ` C99 variable length array support Pedro Alves
2013-11-21 19:01   ` Pedro Alves
2013-11-21 19:10     ` Pedro Alves
2013-11-22 10:53       ` Agovic, Sanimir
2013-11-22 12:35         ` Pedro Alves
2013-11-22 17:06           ` Agovic, Sanimir

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