Print non-Ada unions without crashing
Commit Message
ada-lang.c is a bit too eager trying to decode unions in the Ada style
-- looking for discriminants and such. This causes crashes when
printing a non-Ada union in Ada mode, something that can easily happen
when printing a value from history or certain registers on AArch64.
This patch fixes the bug by changing ada-lang.c to only apply special
Ada treatment to types coming from an Ada CU. This in turn required a
couple of surprising changes.
First, some of the Ada code was already using HAVE_GNAT_AUX_INFO to
decide whether a type had already been fixed -- such types had
INIT_CPLUS_SPECIFIC called on them. This patch changes these spots to
use the "none" identifier instead.
This then required changing value_rtti_type to avoid changing the
language-specific object attached to an Ada type, which seems like a
good change regardless.
Tested on x86-64 Fedora 29.
gdb/ChangeLog
2019-04-09 Tom Tromey <tromey@adacore.com>
* ada-lang.c (ada_is_variant_part, ada_to_fixed_type_1):
Check ADA_TYPE_P.
(empty_record, ada_template_to_fixed_record_type_1)
(template_to_static_fixed_type)
(to_record_with_fixed_variant_part): Use INIT_NONE_SPECIFIC.
* cp-abi.c (value_rtti_type): Check HAVE_CPLUS_STRUCT.
* gdbtypes.h (INIT_NONE_SPECIFIC, ADA_TYPE_P): New
macros.
gdb/testsuite/ChangeLog
2019-04-09 Tom Tromey <tromey@adacore.com>
* gdb.ada/ptype_union.c: New file.
* gdb.ada/ptype_union.exp: New file.
---
gdb/ChangeLog | 11 +++++++
gdb/ada-lang.c | 17 ++++++++---
gdb/cp-abi.c | 3 +-
gdb/gdbtypes.h | 10 +++++++
gdb/testsuite/ChangeLog | 5 ++++
gdb/testsuite/gdb.ada/ptype_union.c | 41 +++++++++++++++++++++++++++
gdb/testsuite/gdb.ada/ptype_union.exp | 34 ++++++++++++++++++++++
7 files changed, 116 insertions(+), 5 deletions(-)
create mode 100644 gdb/testsuite/gdb.ada/ptype_union.c
create mode 100644 gdb/testsuite/gdb.ada/ptype_union.exp
Comments
> ada-lang.c is a bit too eager trying to decode unions in the Ada style
> -- looking for discriminants and such. This causes crashes when
> printing a non-Ada union in Ada mode, something that can easily happen
> when printing a value from history or certain registers on AArch64.
>
> This patch fixes the bug by changing ada-lang.c to only apply special
> Ada treatment to types coming from an Ada CU. This in turn required a
> couple of surprising changes.
>
> First, some of the Ada code was already using HAVE_GNAT_AUX_INFO to
> decide whether a type had already been fixed -- such types had
> INIT_CPLUS_SPECIFIC called on them. This patch changes these spots to
> use the "none" identifier instead.
>
> This then required changing value_rtti_type to avoid changing the
> language-specific object attached to an Ada type, which seems like a
> good change regardless.
>
> Tested on x86-64 Fedora 29.
>
> gdb/ChangeLog
> 2019-04-09 Tom Tromey <tromey@adacore.com>
>
> * ada-lang.c (ada_is_variant_part, ada_to_fixed_type_1):
> Check ADA_TYPE_P.
> (empty_record, ada_template_to_fixed_record_type_1)
> (template_to_static_fixed_type)
> (to_record_with_fixed_variant_part): Use INIT_NONE_SPECIFIC.
> * cp-abi.c (value_rtti_type): Check HAVE_CPLUS_STRUCT.
> * gdbtypes.h (INIT_NONE_SPECIFIC, ADA_TYPE_P): New
> macros.
>
> gdb/testsuite/ChangeLog
> 2019-04-09 Tom Tromey <tromey@adacore.com>
>
> * gdb.ada/ptype_union.c: New file.
> * gdb.ada/ptype_union.exp: New file.
I reviewed this patch, and it looks good to me. Someone else may want
to take a look at it as well, e.g. for the non-Ada parts?
> ---
> gdb/ChangeLog | 11 +++++++
> gdb/ada-lang.c | 17 ++++++++---
> gdb/cp-abi.c | 3 +-
> gdb/gdbtypes.h | 10 +++++++
> gdb/testsuite/ChangeLog | 5 ++++
> gdb/testsuite/gdb.ada/ptype_union.c | 41 +++++++++++++++++++++++++++
> gdb/testsuite/gdb.ada/ptype_union.exp | 34 ++++++++++++++++++++++
> 7 files changed, 116 insertions(+), 5 deletions(-)
> create mode 100644 gdb/testsuite/gdb.ada/ptype_union.c
> create mode 100644 gdb/testsuite/gdb.ada/ptype_union.exp
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index c141adbef3e..a5bb997783d 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -6961,6 +6961,10 @@ ada_is_wrapper_field (struct type *type, int field_num)
> int
> ada_is_variant_part (struct type *type, int field_num)
> {
> + /* Only Ada types are eligible. */
> + if (!ADA_TYPE_P (type))
> + return 0;
> +
> struct type *field_type = TYPE_FIELD_TYPE (type, field_num);
>
> return (TYPE_CODE (field_type) == TYPE_CODE_UNION
> @@ -8258,7 +8262,7 @@ empty_record (struct type *templ)
> TYPE_CODE (type) = TYPE_CODE_STRUCT;
> TYPE_NFIELDS (type) = 0;
> TYPE_FIELDS (type) = NULL;
> - INIT_CPLUS_SPECIFIC (type);
> + INIT_NONE_SPECIFIC (type);
> TYPE_NAME (type) = "<empty>";
> TYPE_LENGTH (type) = 0;
> return type;
> @@ -8312,7 +8316,7 @@ ada_template_to_fixed_record_type_1 (struct type *type,
>
> rtype = alloc_type_copy (type);
> TYPE_CODE (rtype) = TYPE_CODE_STRUCT;
> - INIT_CPLUS_SPECIFIC (rtype);
> + INIT_NONE_SPECIFIC (rtype);
> TYPE_NFIELDS (rtype) = nfields;
> TYPE_FIELDS (rtype) = (struct field *)
> TYPE_ALLOC (rtype, nfields * sizeof (struct field));
> @@ -8587,7 +8591,7 @@ template_to_static_fixed_type (struct type *type0)
> {
> TYPE_TARGET_TYPE (type0) = type = alloc_type_copy (type0);
> TYPE_CODE (type) = TYPE_CODE (type0);
> - INIT_CPLUS_SPECIFIC (type);
> + INIT_NONE_SPECIFIC (type);
> TYPE_NFIELDS (type) = nfields;
> TYPE_FIELDS (type) = (struct field *)
> TYPE_ALLOC (type, nfields * sizeof (struct field));
> @@ -8636,7 +8640,7 @@ to_record_with_fixed_variant_part (struct type *type, const gdb_byte *valaddr,
>
> rtype = alloc_type_copy (type);
> TYPE_CODE (rtype) = TYPE_CODE_STRUCT;
> - INIT_CPLUS_SPECIFIC (rtype);
> + INIT_NONE_SPECIFIC (rtype);
> TYPE_NFIELDS (rtype) = nfields;
> TYPE_FIELDS (rtype) =
> (struct field *) TYPE_ALLOC (rtype, nfields * sizeof (struct field));
> @@ -9005,6 +9009,11 @@ ada_to_fixed_type_1 (struct type *type, const gdb_byte *valaddr,
> CORE_ADDR address, struct value *dval, int check_tag)
> {
> type = ada_check_typedef (type);
> +
> + /* Only un-fixed types need to be handled here. */
> + if (!HAVE_GNAT_AUX_INFO (type))
> + return type;
> +
> switch (TYPE_CODE (type))
> {
> default:
> diff --git a/gdb/cp-abi.c b/gdb/cp-abi.c
> index d95b1404dce..bbb74d42638 100644
> --- a/gdb/cp-abi.c
> +++ b/gdb/cp-abi.c
> @@ -109,7 +109,8 @@ value_rtti_type (struct value *v, int *full,
> {
> struct type *ret = NULL;
>
> - if ((current_cp_abi.rtti_type) == NULL)
> + if ((current_cp_abi.rtti_type) == NULL
> + || !HAVE_CPLUS_STRUCT (check_typedef (value_type (v))))
> return NULL;
> try
> {
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 2125ed991d0..a5f6afc6fad 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1255,6 +1255,10 @@ extern void allocate_cplus_struct_type (struct type *);
> (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_CPLUS_STUFF \
> && TYPE_RAW_CPLUS_SPECIFIC (type) != &cplus_struct_default)
>
> +#define INIT_NONE_SPECIFIC(type) \
> + (TYPE_SPECIFIC_FIELD (type) = TYPE_SPECIFIC_NONE, \
> + TYPE_MAIN_TYPE (type)->type_specific = {})
> +
> extern const struct gnat_aux_type gnat_aux_default;
>
> extern void allocate_gnat_aux_type (struct type *);
> @@ -1268,6 +1272,12 @@ extern void allocate_gnat_aux_type (struct type *);
> #define HAVE_GNAT_AUX_INFO(type) \
> (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_GNAT_STUFF)
>
> +/* * True if TYPE is known to be an Ada type of some kind. */
> +#define ADA_TYPE_P(type) \
> + (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_GNAT_STUFF \
> + || (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_NONE \
> + && TYPE_FIXED_INSTANCE (type)))
> +
> #define INIT_FUNC_SPECIFIC(type) \
> (TYPE_SPECIFIC_FIELD (type) = TYPE_SPECIFIC_FUNC, \
> TYPE_MAIN_TYPE (type)->type_specific.func_stuff = (struct func_type *) \
> diff --git a/gdb/testsuite/gdb.ada/ptype_union.c b/gdb/testsuite/gdb.ada/ptype_union.c
> new file mode 100644
> index 00000000000..eef9c5a92d3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/ptype_union.c
> @@ -0,0 +1,41 @@
> +/* This test program 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/>. */
> +
> +union a
> +{
> + int l;
> + double m;
> +};
> +
> +union b
> +{
> + char *n;
> + float o;
> +};
> +
> +struct s
> +{
> + union a af;
> + union b bf;
> +};
> +
> +struct s global;
> +
> +int main ()
> +{
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.ada/ptype_union.exp b/gdb/testsuite/gdb.ada/ptype_union.exp
> new file mode 100644
> index 00000000000..4f33721c817
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/ptype_union.exp
> @@ -0,0 +1,34 @@
> +# 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/>.
> +
> +standard_testfile .c
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
> + return -1
> +}
> +
> +# The test case is written in C, because it was easy to make the
> +# required type there; but the bug itself only happens in Ada.
> +gdb_test "set lang ada" ""
> +
> +gdb_test "ptype global" \
> + [multi_line \
> + "type = record" \
> + "\[ \t\]*af: a;" \
> + "\[ \t\]*bf: b;" \
> + "end record"]
> +
> +gdb_test "print global" \
> + " = \\(af => \\(l => 0, m => 0.0\\), bf => \\(n => 0x0, o => 0.0\\)\\)"
> --
> 2.20.1
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
>> 2019-04-09 Tom Tromey <tromey@adacore.com>
>>
>> * ada-lang.c (ada_is_variant_part, ada_to_fixed_type_1):
>> Check ADA_TYPE_P.
>> (empty_record, ada_template_to_fixed_record_type_1)
>> (template_to_static_fixed_type)
>> (to_record_with_fixed_variant_part): Use INIT_NONE_SPECIFIC.
>> * cp-abi.c (value_rtti_type): Check HAVE_CPLUS_STRUCT.
>> * gdbtypes.h (INIT_NONE_SPECIFIC, ADA_TYPE_P): New
>> macros.
>>
>> gdb/testsuite/ChangeLog
>> 2019-04-09 Tom Tromey <tromey@adacore.com>
>>
>> * gdb.ada/ptype_union.c: New file.
>> * gdb.ada/ptype_union.exp: New file.
Joel> I reviewed this patch, and it looks good to me. Someone else may want
Joel> to take a look at it as well, e.g. for the non-Ada parts?
I'm checking it in now.
Tom
@@ -6961,6 +6961,10 @@ ada_is_wrapper_field (struct type *type, int field_num)
int
ada_is_variant_part (struct type *type, int field_num)
{
+ /* Only Ada types are eligible. */
+ if (!ADA_TYPE_P (type))
+ return 0;
+
struct type *field_type = TYPE_FIELD_TYPE (type, field_num);
return (TYPE_CODE (field_type) == TYPE_CODE_UNION
@@ -8258,7 +8262,7 @@ empty_record (struct type *templ)
TYPE_CODE (type) = TYPE_CODE_STRUCT;
TYPE_NFIELDS (type) = 0;
TYPE_FIELDS (type) = NULL;
- INIT_CPLUS_SPECIFIC (type);
+ INIT_NONE_SPECIFIC (type);
TYPE_NAME (type) = "<empty>";
TYPE_LENGTH (type) = 0;
return type;
@@ -8312,7 +8316,7 @@ ada_template_to_fixed_record_type_1 (struct type *type,
rtype = alloc_type_copy (type);
TYPE_CODE (rtype) = TYPE_CODE_STRUCT;
- INIT_CPLUS_SPECIFIC (rtype);
+ INIT_NONE_SPECIFIC (rtype);
TYPE_NFIELDS (rtype) = nfields;
TYPE_FIELDS (rtype) = (struct field *)
TYPE_ALLOC (rtype, nfields * sizeof (struct field));
@@ -8587,7 +8591,7 @@ template_to_static_fixed_type (struct type *type0)
{
TYPE_TARGET_TYPE (type0) = type = alloc_type_copy (type0);
TYPE_CODE (type) = TYPE_CODE (type0);
- INIT_CPLUS_SPECIFIC (type);
+ INIT_NONE_SPECIFIC (type);
TYPE_NFIELDS (type) = nfields;
TYPE_FIELDS (type) = (struct field *)
TYPE_ALLOC (type, nfields * sizeof (struct field));
@@ -8636,7 +8640,7 @@ to_record_with_fixed_variant_part (struct type *type, const gdb_byte *valaddr,
rtype = alloc_type_copy (type);
TYPE_CODE (rtype) = TYPE_CODE_STRUCT;
- INIT_CPLUS_SPECIFIC (rtype);
+ INIT_NONE_SPECIFIC (rtype);
TYPE_NFIELDS (rtype) = nfields;
TYPE_FIELDS (rtype) =
(struct field *) TYPE_ALLOC (rtype, nfields * sizeof (struct field));
@@ -9005,6 +9009,11 @@ ada_to_fixed_type_1 (struct type *type, const gdb_byte *valaddr,
CORE_ADDR address, struct value *dval, int check_tag)
{
type = ada_check_typedef (type);
+
+ /* Only un-fixed types need to be handled here. */
+ if (!HAVE_GNAT_AUX_INFO (type))
+ return type;
+
switch (TYPE_CODE (type))
{
default:
@@ -109,7 +109,8 @@ value_rtti_type (struct value *v, int *full,
{
struct type *ret = NULL;
- if ((current_cp_abi.rtti_type) == NULL)
+ if ((current_cp_abi.rtti_type) == NULL
+ || !HAVE_CPLUS_STRUCT (check_typedef (value_type (v))))
return NULL;
try
{
@@ -1255,6 +1255,10 @@ extern void allocate_cplus_struct_type (struct type *);
(TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_CPLUS_STUFF \
&& TYPE_RAW_CPLUS_SPECIFIC (type) != &cplus_struct_default)
+#define INIT_NONE_SPECIFIC(type) \
+ (TYPE_SPECIFIC_FIELD (type) = TYPE_SPECIFIC_NONE, \
+ TYPE_MAIN_TYPE (type)->type_specific = {})
+
extern const struct gnat_aux_type gnat_aux_default;
extern void allocate_gnat_aux_type (struct type *);
@@ -1268,6 +1272,12 @@ extern void allocate_gnat_aux_type (struct type *);
#define HAVE_GNAT_AUX_INFO(type) \
(TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_GNAT_STUFF)
+/* * True if TYPE is known to be an Ada type of some kind. */
+#define ADA_TYPE_P(type) \
+ (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_GNAT_STUFF \
+ || (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_NONE \
+ && TYPE_FIXED_INSTANCE (type)))
+
#define INIT_FUNC_SPECIFIC(type) \
(TYPE_SPECIFIC_FIELD (type) = TYPE_SPECIFIC_FUNC, \
TYPE_MAIN_TYPE (type)->type_specific.func_stuff = (struct func_type *) \
new file mode 100644
@@ -0,0 +1,41 @@
+/* This test program 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/>. */
+
+union a
+{
+ int l;
+ double m;
+};
+
+union b
+{
+ char *n;
+ float o;
+};
+
+struct s
+{
+ union a af;
+ union b bf;
+};
+
+struct s global;
+
+int main ()
+{
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,34 @@
+# 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/>.
+
+standard_testfile .c
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
+ return -1
+}
+
+# The test case is written in C, because it was easy to make the
+# required type there; but the bug itself only happens in Ada.
+gdb_test "set lang ada" ""
+
+gdb_test "ptype global" \
+ [multi_line \
+ "type = record" \
+ "\[ \t\]*af: a;" \
+ "\[ \t\]*bf: b;" \
+ "end record"]
+
+gdb_test "print global" \
+ " = \\(af => \\(l => 0, m => 0.0\\), bf => \\(n => 0x0, o => 0.0\\)\\)"