diff mbox

Fix crash in cp_print_value_fields

Message ID 20190517201814.27951-1-tromey@adacore.com
State New
Headers show

Commit Message

Tom Tromey May 17, 2019, 8:18 p.m. UTC
From: Tom Tromey <tom@tromey.com>

PR c++/20020 concerns a crash in cp_print_value_fields.  The immediate
cause is that cp_print_value_fields does not handle the case where
value_static_field fails.  This is fixed in this patch by calling
cp_print_static_field from the "try" block.

Digging a bit deeper, the error occurs because GCC does not emit a
DW_AT_const_value for a static constexpr member appearing in a
template class.  I've filed a GCC bug for this.

Tested on x86-64 Fedora 29.

gdb/ChangeLog
2019-05-17  Tom Tromey  <tromey@adacore.com>

	PR c++/20020:
	* cp-valprint.c (cp_print_value_fields): Call
	cp_print_static_field inside "try".

gdb/testsuite/ChangeLog
2019-05-17  Tom Tromey  <tromey@adacore.com>

	PR c++/20020:
	* gdb.cp/constexpr-field.exp: New file.
	* gdb.cp/constexpr-field.cc: New file.
---
 gdb/ChangeLog                            |  6 ++++
 gdb/cp-valprint.c                        | 12 ++++----
 gdb/testsuite/ChangeLog                  |  6 ++++
 gdb/testsuite/gdb.cp/constexpr-field.cc  | 37 ++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/constexpr-field.exp | 36 +++++++++++++++++++++++
 5 files changed, 90 insertions(+), 7 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/constexpr-field.cc
 create mode 100644 gdb/testsuite/gdb.cp/constexpr-field.exp

Comments

Tom Tromey May 29, 2019, 3:34 p.m. UTC | #1
Tom> 2019-05-17  Tom Tromey  <tromey@adacore.com>

Tom> 	PR c++/20020:
Tom> 	* cp-valprint.c (cp_print_value_fields): Call
Tom> 	cp_print_static_field inside "try".

I'm checking this in now.

Tom
Pedro Alves May 29, 2019, 7:19 p.m. UTC | #2
On 5/17/19 9:18 PM, Tom Tromey wrote:
> +# "x" sometimes isn't available due to
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90526
> +gdb_test "print x" " = {static f = .*}"
> +

How about making that expect the correct value, and then use
"setup_xfail gcc/90526" ?

Thanks,
Pedro Alves
Tom de Vries June 18, 2019, 1:50 p.m. UTC | #3
On 17-05-19 22:18, Tom Tromey wrote:
> From: Tom Tromey <tom@tromey.com>
> 
> PR c++/20020 concerns a crash in cp_print_value_fields.  The immediate
> cause is that cp_print_value_fields does not handle the case where
> value_static_field fails.  This is fixed in this patch by calling
> cp_print_static_field from the "try" block.
> 
> Digging a bit deeper, the error occurs because GCC does not emit a
> DW_AT_const_value for a static constexpr member appearing in a
> template class.  I've filed a GCC bug for this.
> 
> Tested on x86-64 Fedora 29.
> 
> gdb/ChangeLog
> 2019-05-17  Tom Tromey  <tromey@adacore.com>
> 
> 	PR c++/20020:
> 	* cp-valprint.c (cp_print_value_fields): Call
> 	cp_print_static_field inside "try".
> 
> gdb/testsuite/ChangeLog
> 2019-05-17  Tom Tromey  <tromey@adacore.com>
> 
> 	PR c++/20020:
> 	* gdb.cp/constexpr-field.exp: New file.
> 	* gdb.cp/constexpr-field.cc: New file.
> ---
>  gdb/ChangeLog                            |  6 ++++
>  gdb/cp-valprint.c                        | 12 ++++----
>  gdb/testsuite/ChangeLog                  |  6 ++++
>  gdb/testsuite/gdb.cp/constexpr-field.cc  | 37 ++++++++++++++++++++++++
>  gdb/testsuite/gdb.cp/constexpr-field.exp | 36 +++++++++++++++++++++++
>  5 files changed, 90 insertions(+), 7 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/constexpr-field.cc
>  create mode 100644 gdb/testsuite/gdb.cp/constexpr-field.exp
> 
> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
> index ff860df499a..d8d5c06fd1e 100644
> --- a/gdb/cp-valprint.c
> +++ b/gdb/cp-valprint.c
> @@ -329,22 +329,20 @@ cp_print_value_fields (struct type *type, struct type *real_type,
>  		}
>  	      else if (field_is_static (&TYPE_FIELD (type, i)))
>  		{
> -		  struct value *v = NULL;
> -
>  		  try
>  		    {
> -		      v = value_static_field (type, i);
> -		    }
> +		      struct value *v = value_static_field (type, i);
>  
> +		      cp_print_static_field (TYPE_FIELD_TYPE (type, i),
> +					     v, stream, recurse + 1,
> +					     options);
> +		    }

Here options is used ...

>  		  catch (const gdb_exception_error &ex)
>  		    {
>  		      fprintf_filtered (stream,
>  					_("<error reading variable: %s>"),
>  					ex.what ());
>  		    }
> -
> -		  cp_print_static_field (TYPE_FIELD_TYPE (type, i),
> -					 v, stream, recurse + 1, opts);

while here opts used to be used.

Is this change intentional? It's not mentioned anywhere.

Thanks,
- Tom

>  		}
>  	      else if (i == vptr_fieldno && type == vptr_basetype)
>  		{
Tom Tromey June 18, 2019, 3:34 p.m. UTC | #4
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> On 5/17/19 9:18 PM, Tom Tromey wrote:
>> +# "x" sometimes isn't available due to
>> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90526
>> +gdb_test "print x" " = {static f = .*}"
>> +

Pedro> How about making that expect the correct value, and then use
Pedro> "setup_xfail gcc/90526" ?

Sorry about the delay on this.  I made this change, and I'll send it soon.

Tom
Tom Tromey June 18, 2019, 3:35 p.m. UTC | #5
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

>> +		      cp_print_static_field (TYPE_FIELD_TYPE (type, i),
>> +					     v, stream, recurse + 1,
>> +					     options);
>> +		    }

Tom> Here options is used ...

>> catch (const gdb_exception_error &ex)
>> {
>> fprintf_filtered (stream,
>> _("<error reading variable: %s>"),
>> ex.what ());
>> }
>> -
>> -		  cp_print_static_field (TYPE_FIELD_TYPE (type, i),
>> -					 v, stream, recurse + 1, opts);

Tom> while here opts used to be used.

Tom> Is this change intentional? It's not mentioned anywhere.

Thanks for catching that.  It was not intentional.  I'll send a fix shortly.

Tom
diff mbox

Patch

diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index ff860df499a..d8d5c06fd1e 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -329,22 +329,20 @@  cp_print_value_fields (struct type *type, struct type *real_type,
 		}
 	      else if (field_is_static (&TYPE_FIELD (type, i)))
 		{
-		  struct value *v = NULL;
-
 		  try
 		    {
-		      v = value_static_field (type, i);
-		    }
+		      struct value *v = value_static_field (type, i);
 
+		      cp_print_static_field (TYPE_FIELD_TYPE (type, i),
+					     v, stream, recurse + 1,
+					     options);
+		    }
 		  catch (const gdb_exception_error &ex)
 		    {
 		      fprintf_filtered (stream,
 					_("<error reading variable: %s>"),
 					ex.what ());
 		    }
-
-		  cp_print_static_field (TYPE_FIELD_TYPE (type, i),
-					 v, stream, recurse + 1, opts);
 		}
 	      else if (i == vptr_fieldno && type == vptr_basetype)
 		{
diff --git a/gdb/testsuite/gdb.cp/constexpr-field.cc b/gdb/testsuite/gdb.cp/constexpr-field.cc
new file mode 100644
index 00000000000..ac2e514cb38
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/constexpr-field.cc
@@ -0,0 +1,37 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+template<typename T>
+struct X
+{
+  static constexpr bool f = true;
+};
+
+X<int> x;
+
+struct Y
+{
+  static constexpr bool f = true;
+};
+
+Y y;
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/constexpr-field.exp b/gdb/testsuite/gdb.cp/constexpr-field.exp
new file mode 100644
index 00000000000..e4849efeae9
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/constexpr-field.exp
@@ -0,0 +1,36 @@ 
+# Copyright 2019 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/>.
+
+# This file is part of the gdb testsuite.
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 {c++ debug additional_flags=-std=c++17}]} {
+    return -1
+}
+
+if {![runto_main]} {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# "x" sometimes isn't available due to
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90526
+gdb_test "print x" " = {static f = .*}"
+
+gdb_test "print y" " = {static f = true}"