[PATCH 0/2] Accessing array elements of optimised out VLA

[PATCH 0/2] Accessing array elements of optimised out VLA

[Andrew Burgess]

I wondered what would happen if you asked GDB for an element from a
VLA that was optimised out.  Turns out the answer is that GDB reads
some random memory from GDB's own address space and gives you that..

...not doing that seemed better.

The first patch is a clean up to merge together 3 almost identical
test scripts.

Thanks,
Andrew

--

Andrew Burgess (2):
  gdb: Merge similar tests into a single test script
  gdb: Check element of optimised out vla exists

 gdb/ChangeLog                                      |  6 +++
 gdb/testsuite/ChangeLog                            | 11 ++++
 .../gdb.base/vla-optimized-out-o3-strict.exp       | 40 ---------------
 gdb/testsuite/gdb.base/vla-optimized-out-o3.exp    | 36 -------------
 gdb/testsuite/gdb.base/vla-optimized-out.exp       | 60 +++++++++++++++++++---
 gdb/valarith.c                                     |  7 ++-
 6 files changed, 74 insertions(+), 86 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp
 delete mode 100644 gdb/testsuite/gdb.base/vla-optimized-out-o3.exp

-- 
2.14.4

[PATCH 2/2] gdb: Check element of optimised out vla exists

[Andrew Burgess]

The new tests cover accessing an element that should exist in the VLA
(except it has been optimised out), accessing just beyond the VLA, and
accessing far beyond the VLA.

The third test is included as this might cause GDB to crash without
the fix (though undefined behaviour, so, you never know).

I did have a patch that added an assertion and caught the rogue
access, however, it turns out the assertion triggers from our
extension language pretty-printer code <sigh> so I've had to drop the
assertion for now while I work on fixing that.

Thanks,
Andrew

---

If a vla is not in memory, and the upper bound is not defined, then we
can't know that an array element exists or not, and we should not try
to access the array element.  One case where this happens is for
arrays that have been optimised out, the array will then have
VALUE_LVAL of not_lval, and an undefined upper bound, if we then try
to access an element of this array we will index into random GDB
memory.

An argument could be made that even for arrays that are in inferior
memory, if the upper bound is not defined then we should not try to
access the array element, however, in some of the Fortran tests, it
seems as though we do rely indexing from a base address into an array
which has no bounds defined.  In this case GDBs standard protection
for detecting unreadable target memory prevents bad thing happening.

gdb/ChangeLog:

	* valarith.c (value_subscripted_rvalue): If an array is not in
	memory, and we don't know the upper bound, then we can't know that
	the requested element exists or not.

gdb/testsuite/ChangeLog:

	* gdb.base/vla-optimized-out.exp: Add new test.
---
 gdb/ChangeLog                                |  6 ++++++
 gdb/testsuite/ChangeLog                      |  4 ++++
 gdb/testsuite/gdb.base/vla-optimized-out.exp | 22 ++++++++++++++++++++++
 gdb/valarith.c                               |  7 +++++--
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/vla-optimized-out.exp b/gdb/testsuite/gdb.base/vla-optimized-out.exp
index fa521765969..ed9f269b5f3 100644
--- a/gdb/testsuite/gdb.base/vla-optimized-out.exp
+++ b/gdb/testsuite/gdb.base/vla-optimized-out.exp
@@ -37,6 +37,28 @@ proc vla_optimized_out {compile_flags exe_suffix sizeof_result} {
     gdb_test "p sizeof (a)" \
 	" = $sizeof_result" \
 	"printed size of optimized out vla"
+
+    # At lower optimisation levels, the upper bound of the array is
+    # still defined, it's just the loctaion that tells GDB the array
+    # is optimised out.  In that case, when we access an element that
+    # is within the bounds of the array an answer of '<optimized out>'
+    # is reasonable.
+    #
+    # At higher optimisation levels, the array bounds themselves have
+    # been removed.  As such GDB can't be expected to know if the
+    # array contains _any_ elements at all.  It seems reasonable in
+    # that case to reply with 'no such vector element'.
+    gdb_test "p a\[0\]" \
+	"(= <optimized out>|no such vector element)" \
+	"print out of range element of vla (0)"
+
+    gdb_test "p a\[6\]" \
+	"no such vector element" \
+	"print out of range element of vla (6)"
+
+    gdb_test "p a\[0xffffffff\]" \
+	"no such vector element" \
+	"print out of range element of vla (0xffffffff)"
 }
 
 foreach test_settings [list \
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 01ca50d3d21..807cdd5dbd4 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -189,8 +189,11 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
   ULONGEST elt_size = type_length_units (elt_type);
   ULONGEST elt_offs = elt_size * (index - lowerbound);
 
-  if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
-			     && elt_offs >= type_length_units (array_type)))
+  if (index < lowerbound
+      || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
+          && elt_offs >= type_length_units (array_type))
+      || (VALUE_LVAL (array) != lval_memory
+          && TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)))
     {
       if (type_not_associated (array_type))
         error (_("no such vector element (vector not associated)"));
-- 
2.14.4

Re: [PATCH 2/2] gdb: Check element of optimised out vla exists

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The third test is included as this might cause GDB to crash without
Andrew> the fix (though undefined behaviour, so, you never know).

This will be nice if we enable -fsanitize=undefined, because in that
case it would cause a test failure.

Andrew> An argument could be made that even for arrays that are in inferior
Andrew> memory, if the upper bound is not defined then we should not try to
Andrew> access the array element

In this case, it seems to me that it's fine for gdb to try to respect
the user's request.  After all, although gdb doesn't know the array
bound, the user might.

Andrew> gdb/ChangeLog:

Andrew> 	* valarith.c (value_subscripted_rvalue): If an array is not in
Andrew> 	memory, and we don't know the upper bound, then we can't know that
Andrew> 	the requested element exists or not.

Andrew> gdb/testsuite/ChangeLog:

Andrew> 	* gdb.base/vla-optimized-out.exp: Add new test.

Thank you.  This is ok.

Tom

[PATCH 1/2] gdb: Merge similar tests into a single test script

[Andrew Burgess]

The three test scripts:

  gdb/testsuite/gdb.base/vla-optimized-out.exp
  gdb/testsuite/gdb.base/vla-optimized-out-o3.exp
  gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp

are all pretty similar, with differences in the compile flags used,
and some of the expected results.

Instead of maintaining 3 files, merge them into a single test script,
and use parameters to control the test behaviour.

gdb/testsuite/ChangeLog:

	* gdb.base/vla-optimized-out-o3.exp: Delete.
	* gdb.base/vla-optimized-out-o3-strict.exp: Delete.
	* gdb.base/vla-optimized-out.exp: Extend to cover all of the
	deleted tests.
---
 gdb/testsuite/ChangeLog                            |  7 ++++
 .../gdb.base/vla-optimized-out-o3-strict.exp       | 40 ----------------------
 gdb/testsuite/gdb.base/vla-optimized-out-o3.exp    | 36 -------------------
 gdb/testsuite/gdb.base/vla-optimized-out.exp       | 38 +++++++++++++++-----
 4 files changed, 37 insertions(+), 84 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp
 delete mode 100644 gdb/testsuite/gdb.base/vla-optimized-out-o3.exp

diff --git a/gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp b/gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp
deleted file mode 100644
index 81ada875b37..00000000000
--- a/gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp
+++ /dev/null
@@ -1,40 +0,0 @@
-# Copyright 2018 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/>.
-
-# Check whether we can determine the size of an optimized-out vla.
-
-standard_testfile
-
-if { [prepare_for_testing "failed to prepare" $testfile vla-optimized-out.c \
-	  {debug optimize=-O3 additional_flags=-gstrict-dwarf}] } {
-    return -1
-}
-
-proc vla_optimized_out { } {
-    if ![runto f1] {
-	fail "can't run to f1"
-	return
-    }
-
-    gdb_test "p a" \
-	{ = <optimized out>} \
-	"printed optimized out vla"
-
-    gdb_test "p sizeof (a)" \
-	{ = <optimized out>} \
-	"printed optimized out size of optimized out vla"
-}
-
-vla_optimized_out
diff --git a/gdb/testsuite/gdb.base/vla-optimized-out-o3.exp b/gdb/testsuite/gdb.base/vla-optimized-out-o3.exp
deleted file mode 100644
index 60707e7aff8..00000000000
--- a/gdb/testsuite/gdb.base/vla-optimized-out-o3.exp
+++ /dev/null
@@ -1,36 +0,0 @@
-# Copyright 2018 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/>.
-
-# Check whether we can print an optimized-out vla.
-
-standard_testfile
-
-if { [prepare_for_testing "failed to prepare" $testfile "vla-optimized-out.c" \
-	  {debug optimize=-O3}] } {
-    return -1
-}
-
-proc vla_optimized_out { } {
-    if ![runto f1] {
-	fail "can't run to f1"
-	return
-    }
-
-    gdb_test "p a" \
-	{ = <optimized out>} \
-	"printed optimized out vla"
-}
-
-vla_optimized_out
diff --git a/gdb/testsuite/gdb.base/vla-optimized-out.exp b/gdb/testsuite/gdb.base/vla-optimized-out.exp
index b27569ed1e1..fa521765969 100644
--- a/gdb/testsuite/gdb.base/vla-optimized-out.exp
+++ b/gdb/testsuite/gdb.base/vla-optimized-out.exp
@@ -17,24 +17,46 @@
 
 standard_testfile
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
-	  {debug optimize=-O1 additional_flags=-DNOCLONE}] } {
-    return -1
-}
+proc vla_optimized_out {compile_flags exe_suffix sizeof_result} {
+    global testfile srcfile
+
+    if { [prepare_for_testing "failed to prepare" "$testfile-$exe_suffix" $srcfile \
+	      $compile_flags] } {
+	return -1
+    }
 
-proc vla_optimized_out { } {
     if ![runto f1] {
 	fail "can't run to f1"
 	return
     }
 
     gdb_test "p a" \
-	{ = <optimized out>} \
+	" = <optimized out>" \
 	"printed optimized out vla"
 
     gdb_test "p sizeof (a)" \
-	{ = 6} \
+	" = $sizeof_result" \
 	"printed size of optimized out vla"
 }
 
-vla_optimized_out
+foreach test_settings [list \
+			   [list "o1" \
+				{debug optimize=-O1 \
+				     additional_flags=-DNOCLONE} \
+				"6"] \
+			   [list "o3" \
+				{debug optimize=-O3} \
+				"<optimized out>"]\
+			   [list "o3_strict" \
+				{debug optimize=-O3 \
+				     additional_flags=-gstrict-dwarf} \
+				"<optimized out>"]] {
+    set test_prefix [lindex $test_settings 0]
+    set compile_flags [lindex $test_settings 1]
+    set sizeof_result [lindex $test_settings 2]
+
+    with_test_prefix $test_prefix {
+	vla_optimized_out $compile_flags $test_prefix $sizeof_result
+    }
+}
+
-- 
2.14.4

Re: [PATCH 1/2] gdb: Merge similar tests into a single test script

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> +foreach test_settings [list \
Andrew> +			   [list "o1" \
Andrew> +				{debug optimize=-O1 \
Andrew> +				     additional_flags=-DNOCLONE} \
Andrew> +				"6"] \
Andrew> +			   [list "o3" \
Andrew> +				{debug optimize=-O3} \
Andrew> +				"<optimized out>"]\
Andrew> +			   [list "o3_strict" \
Andrew> +				{debug optimize=-O3 \
Andrew> +				     additional_flags=-gstrict-dwarf} \
Andrew> +				"<optimized out>"]] {
Andrew> +    set test_prefix [lindex $test_settings 0]
Andrew> +    set compile_flags [lindex $test_settings 1]
Andrew> +    set sizeof_result [lindex $test_settings 2]

For constant lists you can just use {} rather than [list], like

 {{o1 {debug optimize=-O1...} 6}
  {o3 ... }}

Also, if you flatten the list, you can use a destructuring foreach here,
like:

  foreach {test_prefix compile_flags sizeof_result} {o1 {debug optimize=-O1 ...} ...}

Or, alternatively, use lassign to set the values.

Tom

Re: [PATCH 1/2] gdb: Merge similar tests into a single test script

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2018-08-02 12:50:08 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> +foreach test_settings [list \
> Andrew> +			   [list "o1" \
> Andrew> +				{debug optimize=-O1 \
> Andrew> +				     additional_flags=-DNOCLONE} \
> Andrew> +				"6"] \
> Andrew> +			   [list "o3" \
> Andrew> +				{debug optimize=-O3} \
> Andrew> +				"<optimized out>"]\
> Andrew> +			   [list "o3_strict" \
> Andrew> +				{debug optimize=-O3 \
> Andrew> +				     additional_flags=-gstrict-dwarf} \
> Andrew> +				"<optimized out>"]] {
> Andrew> +    set test_prefix [lindex $test_settings 0]
> Andrew> +    set compile_flags [lindex $test_settings 1]
> Andrew> +    set sizeof_result [lindex $test_settings 2]
> 
> For constant lists you can just use {} rather than [list], like
> 
>  {{o1 {debug optimize=-O1...} 6}
>   {o3 ... }}
> 
> Also, if you flatten the list, you can use a destructuring foreach here,
> like:
> 
>   foreach {test_prefix compile_flags sizeof_result} {o1 {debug optimize=-O1 ...} ...}
> 
> Or, alternatively, use lassign to set the values.

Thanks for the review.

Updated version, the changes are:

  * Use {} to create a flattened list,
  * compile_flags and sizeof_result are extracted with lassign inside
    proc, and
  * Comment on vla_optimized_out explaining parameters.

Thanks,
Andrew

---

gdb: Merge similar tests into a single test script

The three test scripts:

  gdb/testsuite/gdb.base/vla-optimized-out.exp
  gdb/testsuite/gdb.base/vla-optimized-out-o3.exp
  gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp

are all pretty similar, with differences in the compile flags used,
and some of the expected results.

Instead of maintaining 3 files, merge them into a single test script,
and use parameters to control the test behaviour.

gdb/testsuite/ChangeLog:

	* gdb.base/vla-optimized-out-o3.exp: Delete.
	* gdb.base/vla-optimized-out-o3-strict.exp: Delete.
	* gdb.base/vla-optimized-out.exp: Extend to cover all of the
	deleted tests.
---
 gdb/testsuite/ChangeLog                            |  7 ++++
 .../gdb.base/vla-optimized-out-o3-strict.exp       | 40 ----------------------
 gdb/testsuite/gdb.base/vla-optimized-out-o3.exp    | 36 -------------------
 gdb/testsuite/gdb.base/vla-optimized-out.exp       | 35 ++++++++++++++-----
 4 files changed, 34 insertions(+), 84 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp
 delete mode 100644 gdb/testsuite/gdb.base/vla-optimized-out-o3.exp

diff --git a/gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp b/gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp
deleted file mode 100644
index 81ada875b37..00000000000
--- a/gdb/testsuite/gdb.base/vla-optimized-out-o3-strict.exp
+++ /dev/null
@@ -1,40 +0,0 @@
-# Copyright 2018 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/>.
-
-# Check whether we can determine the size of an optimized-out vla.
-
-standard_testfile
-
-if { [prepare_for_testing "failed to prepare" $testfile vla-optimized-out.c \
-	  {debug optimize=-O3 additional_flags=-gstrict-dwarf}] } {
-    return -1
-}
-
-proc vla_optimized_out { } {
-    if ![runto f1] {
-	fail "can't run to f1"
-	return
-    }
-
-    gdb_test "p a" \
-	{ = <optimized out>} \
-	"printed optimized out vla"
-
-    gdb_test "p sizeof (a)" \
-	{ = <optimized out>} \
-	"printed optimized out size of optimized out vla"
-}
-
-vla_optimized_out
diff --git a/gdb/testsuite/gdb.base/vla-optimized-out-o3.exp b/gdb/testsuite/gdb.base/vla-optimized-out-o3.exp
deleted file mode 100644
index 60707e7aff8..00000000000
--- a/gdb/testsuite/gdb.base/vla-optimized-out-o3.exp
+++ /dev/null
@@ -1,36 +0,0 @@
-# Copyright 2018 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/>.
-
-# Check whether we can print an optimized-out vla.
-
-standard_testfile
-
-if { [prepare_for_testing "failed to prepare" $testfile "vla-optimized-out.c" \
-	  {debug optimize=-O3}] } {
-    return -1
-}
-
-proc vla_optimized_out { } {
-    if ![runto f1] {
-	fail "can't run to f1"
-	return
-    }
-
-    gdb_test "p a" \
-	{ = <optimized out>} \
-	"printed optimized out vla"
-}
-
-vla_optimized_out
diff --git a/gdb/testsuite/gdb.base/vla-optimized-out.exp b/gdb/testsuite/gdb.base/vla-optimized-out.exp
index b27569ed1e1..298b689bfbf 100644
--- a/gdb/testsuite/gdb.base/vla-optimized-out.exp
+++ b/gdb/testsuite/gdb.base/vla-optimized-out.exp
@@ -17,24 +17,43 @@
 
 standard_testfile
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
-	  {debug optimize=-O1 additional_flags=-DNOCLONE}] } {
-    return -1
-}
+# The EXE_SUFFIX is a string appended to the name of the test binary
+# to make it unique per variation.
+# The OPTIONS is a two item list, the first item is a list of compiler
+# flags used for building the test binary, and the second item is a
+# pattern which matches some expected output within this proc.
+proc vla_optimized_out {exe_suffix options} {
+    global testfile srcfile
+
+    lassign $options compile_flags sizeof_result
+
+    if { [prepare_for_testing "failed to prepare" "$testfile-$exe_suffix" $srcfile \
+	      $compile_flags] } {
+	return -1
+    }
 
-proc vla_optimized_out { } {
     if ![runto f1] {
 	fail "can't run to f1"
 	return
     }
 
     gdb_test "p a" \
-	{ = <optimized out>} \
+	" = <optimized out>" \
 	"printed optimized out vla"
 
     gdb_test "p sizeof (a)" \
-	{ = 6} \
+	" = $sizeof_result" \
 	"printed size of optimized out vla"
 }
 
-vla_optimized_out
+foreach {test_prefix options} \
+    { "o1" {{debug optimize=-O1 additional_flags=-DNOCLONE} "6"} \
+      "o3" {{debug optimize=-O3} "<optimized out>"} \
+      "o3_strict" {{debug optimize=-O3 \
+			additional_flags=-gstrict-dwarf} \
+		       "<optimized out>"}} {
+    with_test_prefix $test_prefix {
+	vla_optimized_out $test_prefix $options
+    }
+}
+
-- 
2.14.4

Re: [PATCH 1/2] gdb: Merge similar tests into a single test script

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Thanks for the review.

Andrew> Updated version, the changes are:

Andrew>   * Use {} to create a flattened list,
Andrew>   * compile_flags and sizeof_result are extracted with lassign inside
Andrew>     proc, and
Andrew>   * Comment on vla_optimized_out explaining parameters.

Thank you, this is ok.

Tom