gdb: fix value_subscript when array upper bound is not known

Since commit 7c6f27129631 ("gdb: make get_discrete_bounds check for
non-constant range bounds"), subscripting  flexible array member fails:

    struct no_size
    {
      int n;
      int items[];
    };

    (gdb) p *ns
    $1 = {n = 3, items = 0x5555555592a4}
    (gdb) p ns->items[0]
    Cannot access memory at address 0xfffe555b733a0164
    (gdb) p *((int *) 0x5555555592a4)
    $2 = 101  <--- we would expect that
    (gdb) p &ns->items[0]
    $3 = (int *) 0xfffe5559ee829a24  <--- wrong address

Since the flexible array member (items) has an unspecified size, the array type
created for it in the DWARF doesn't have dimensions (this is with gcc 9.3.0,
Ubuntu 20.04):

    0x000000a4:   DW_TAG_array_type
                    DW_AT_type [DW_FORM_ref4]       (0x00000038 "int")
                    DW_AT_sibling [DW_FORM_ref4]    (0x000000b3)

    0x000000ad:     DW_TAG_subrange_type
                      DW_AT_type [DW_FORM_ref4]     (0x00000031 "long unsigned int")

This causes GDB to create a range type (TYPE_CODE_RANGE) with a defined
constant low bound (dynamic_prop with kind PROP_CONST) and an undefined
high bound (dynamic_prop with kind PROP_UNDEFINED).

value_subscript gets both bounds of that range using
get_discrete_bounds.  Before commit 7c6f27129631, get_discrete_bounds
didn't check the kind of the dynamic_props and would just blindly read
them as if they were PROP_CONST.  It would return 0 for the high bound,
because we zero-initialize the range_bounds structure.  And it didn't
really matter in this case, because the returned high bound wasn't used
in the end.

Commit 7c6f27129631 changed get_discrete_bounds to return a failure if
either the low or high bound is not a constant, to make sure we don't
read a dynamic prop that isn't a PROP_CONST as a PROP_CONST.  This
change made get_discrete_bounds start to return a failure for that
range, and as a result would not set *lowp and *highp.  And since
value_subscript doesn't check get_discrete_bounds' return value, it just
carries on an uses an uninitialized value for the low bound.  If
value_subscript did check the return value of get_discrete_bounds, we
would get an error message instead of a bogus value.  But it would still
be a bug, as we wouldn't be able to print the flexible array member's
elements.

Looking at value_subscript, we see that the low bound is always needed,
but the high bound is only needed if !c_style.  So, change
value_subscript to use get_discrete_low_bound and
get_discrete_high_bound separately.  This fixes the case described
above, where the low bound is known but the high bound isn't (and is not
needed).  This restores the original behavior without accessing a
dynamic_prop in a wrong way.

A test is added.  In addition to the case described above, a case with
an array member of size 0 is added, which is a GNU C extension that
existed before flexible array members were introduced.  That case
currently fails when compiled with gcc <= 8.  gcc <= 8 produces DWARF
similar to the one shown above, while gcc 9 adds a DW_AT_count of 0 in
there, which makes the high bound known.  A case where an array member
of size 0 is the only member of the struct is also added, as that was
how PR 28675 was originally reported, and it's an interesting corner
case that I think could trigger other funny bugs.

Question about the implementation: in value_subscript, I made it such
that if the low or high bound is unknown, we fall back to zero.  That
effectively makes it the same as it was before 7c6f27129631.  But should
we instead error() out?

gdb/ChangeLog:

	PR 26875, PR 26901
	* gdbtypes.c (get_discrete_low_bound): Make non-static.
	(get_discrete_high_bound): Make non-static.
	* gdbtypes.h (get_discrete_low_bound): New declaration.
	(get_discrete_high_bound): New declaration.
	* valarith.c (value_subscript): Only fetch high bound if
	necessary.

gdb/testsuite/ChangeLog:

	PR 26875, PR 26901
	* gdb.base/flexible-array-member.c: New test.
	* gdb.base/flexible-array-member.exp: New test.

Change-Id: I832056f80e6c56f621f398b4780d55a3a1e299d7

7 files changed, 179 insertions, 11 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3b022c1..0e01628 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2020-12-09  Simon Marchi  <simon.marchi@efficios.com>
 
+	PR 26875, PR 26901
+	* gdbtypes.c (get_discrete_low_bound): Make non-static.
+	(get_discrete_high_bound): Make non-static.
+	* gdbtypes.h (get_discrete_low_bound): New declaration.
+	(get_discrete_high_bound): New declaration.
+	* valarith.c (value_subscript): Only fetch high bound if
+	necessary.
+
+2020-12-09  Simon Marchi  <simon.marchi@efficios.com>
+
 	* gdbtypes.c (get_discrete_bounds): Implement with
 	get_discrete_low_bound and get_discrete_high_bound.
 	(get_discrete_low_bound): New.
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 367ca5f..9a17008 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1036,9 +1036,9 @@ has_static_range (const struct range_bounds *bounds)
 	  && bounds->stride.kind () == PROP_CONST);
 }
 
-/* If TYPE's low bound is a known constant, return it, else return nullopt.  */
+/* See gdbtypes.h.  */
 
-static gdb::optional<LONGEST>
+gdb::optional<LONGEST>
 get_discrete_low_bound (struct type *type)
 {
   type = check_typedef (type);
@@ -1107,9 +1107,9 @@ get_discrete_low_bound (struct type *type)
     }
 }
 
-/* If TYPE's high bound is a known constant, return it, else return nullopt.  */
+/* See gdbtypes.h.  */
 
-static gdb::optional<LONGEST>
+gdb::optional<LONGEST>
 get_discrete_high_bound (struct type *type)
 {
   type = check_typedef (type);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 19d0740..02fd8bc 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2474,6 +2474,14 @@ extern int get_vptr_fieldno (struct type *, struct type **);
 extern bool get_discrete_bounds (struct type *type, LONGEST *lowp,
 				 LONGEST *highp);
 
+/* If TYPE's low bound is a known constant, return it, else return nullopt.  */
+
+extern gdb::optional<LONGEST> get_discrete_low_bound (struct type *type);
+
+/* If TYPE's high bound is a known constant, return it, else return nullopt.  */
+
+extern gdb::optional<LONGEST> get_discrete_high_bound (struct type *type);
+
 /* Assuming TYPE is a simple, non-empty array type, compute its upper
    and lower bound.  Save the low bound into LOW_BOUND if not NULL.
    Save the high bound into HIGH_BOUND if not NULL.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index dcbb9b2..09cd099 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2020-12-09  Simon Marchi  <simon.marchi@efficios.com>
+
+	PR 26875, PR 26901
+	* gdb.base/flexible-array-member.c: New test.
+	* gdb.base/flexible-array-member.exp: New test.
+
 2020-12-08  Tom de Vries  <tdevries@suse.de>
 
 	* gdb.arch/amd64-gs_base.exp: Undo commit 67748e0f66, reimplement
diff --git a/gdb/testsuite/gdb.base/flexible-array-member.c b/gdb/testsuite/gdb.base/flexible-array-member.c
new file mode 100644
index 0000000..1d8bb06
--- /dev/null
+++ b/gdb/testsuite/gdb.base/flexible-array-member.c
@@ -0,0 +1,70 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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 <stdlib.h>
+
+struct no_size
+{
+  int n;
+  int items[];
+};
+
+struct zero_size
+{
+  int n;
+  int items[0];
+};
+
+struct zero_size_only
+{
+  int items[0];
+};
+
+struct no_size *ns;
+struct zero_size *zs;
+struct zero_size_only *zso;
+
+static void
+break_here (void)
+{
+}
+
+int
+main (void)
+{
+  ns = (struct no_size *) malloc (sizeof (*ns) + 3 * sizeof (int));
+  zs = (struct zero_size *) malloc (sizeof (*zs) + 3 * sizeof (int));
+  zso = (struct zero_size_only *) malloc (sizeof (*zso) + 3 * sizeof (int));
+
+  ns->n = 3;
+  ns->items[0] = 101;
+  ns->items[1] = 102;
+  ns->items[2] = 103;
+
+  zs->n = 3;
+  zs->items[0] = 201;
+  zs->items[1] = 202;
+  zs->items[2] = 203;
+
+  zso->items[0] = 301;
+  zso->items[1] = 302;
+  zso->items[2] = 303;
+
+  break_here ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/flexible-array-member.exp b/gdb/testsuite/gdb.base/flexible-array-member.exp
new file mode 100644
index 0000000..973a248
--- /dev/null
+++ b/gdb/testsuite/gdb.base/flexible-array-member.exp
@@ -0,0 +1,66 @@
+# Copyright 2020 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/>.
+
+# Test printing and subscripting flexible array members.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" \
+	${testfile} ${srcfile}] } {
+    return
+}
+
+if { ![runto break_here] } {
+    untested "could not run to break_here"
+    return
+}
+
+# The various cases are:
+#
+#  - ns: flexible array member with no size
+#  - zs: flexible array member with size 0 (GNU C extension that predates the
+#    standardization of the feature, but widely supported)
+#  - zso: zero-size only, a corner case where the array is the sole member of
+#    the structure
+
+# Print the whole structure.
+
+gdb_test "print *ns" " = {n = 3, items = $hex}"
+gdb_test "print *zs" " = {n = 3, items = $hex}"
+gdb_test "print *zso" " = {items = $hex}"
+
+# Print all items.
+
+gdb_test "print ns->items\[0\]" " = 101"
+gdb_test "print ns->items\[1\]" " = 102"
+gdb_test "print ns->items\[2\]" " = 103"
+
+gdb_test "print zs->items\[0\]" " = 201"
+gdb_test "print zs->items\[1\]" " = 202"
+gdb_test "print zs->items\[2\]" " = 203"
+
+gdb_test "print zso->items\[0\]" " = 301"
+gdb_test "print zso->items\[1\]" " = 302"
+gdb_test "print zso->items\[2\]" " = 303"
+
+# Check taking the address of array elements (how PR 28675 was originally
+# reported).
+
+gdb_test "print ns->items == &ns->items\[0\]" " = 1"
+gdb_test "print ns->items + 1 == &ns->items\[1\]" " = 1"
+gdb_test "print zs->items == &zs->items\[0\]" " = 1"
+gdb_test "print zs->items + 1 == &zs->items\[1\]" " = 1"
+gdb_test "print zso->items == &zso->items\[0\]" " = 1"
+gdb_test "print zso->items + 1 == &zso->items\[1\]" " = 1"
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 077bcb4..37988f1 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -150,25 +150,33 @@ value_subscript (struct value *array, LONGEST index)
       || tarray->code () == TYPE_CODE_STRING)
     {
       struct type *range_type = tarray->index_type ();
-      LONGEST lowerbound, upperbound;
+      gdb::optional<LONGEST> lowerbound = get_discrete_low_bound (range_type);
+      if (!lowerbound.has_value ())
+	lowerbound = 0;
 
-      get_discrete_bounds (range_type, &lowerbound, &upperbound);
       if (VALUE_LVAL (array) != lval_memory)
-	return value_subscripted_rvalue (array, index, lowerbound);
+	return value_subscripted_rvalue (array, index, *lowerbound);
 
       if (!c_style)
 	{
-	  if (index >= lowerbound && index <= upperbound)
-	    return value_subscripted_rvalue (array, index, lowerbound);
+	  gdb::optional<LONGEST> upperbound
+	    = get_discrete_high_bound (range_type);
+
+	  if (!upperbound.has_value ())
+	    upperbound = 0;
+
+	  if (index >= *lowerbound && index <= *upperbound)
+	    return value_subscripted_rvalue (array, index, *lowerbound);
+
 	  /* Emit warning unless we have an array of unknown size.
 	     An array of unknown size has lowerbound 0 and upperbound -1.  */
-	  if (upperbound > -1)
+	  if (*upperbound > -1)
 	    warning (_("array or string index out of range"));
 	  /* fall doing C stuff */
 	  c_style = true;
 	}
 
-      index -= lowerbound;
+      index -= *lowerbound;
       array = value_coerce_array (array);
     }