From 1b61134393256db280b968d24ddf7bb22b6aabcc Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Wed, 29 Feb 2012 19:46:48 +0000 Subject: [PATCH] [Ada] avoid error message pollution with uninitialized tagged variable Consider the following function... 3 procedure Foo is 4 I : Integer := Ident (10); 5 Obj : Base; 6 begin 7 Obj.X := I; 8 Do_Nothing (Obj.X'Address); 9 end Foo; ... where type "Base" is defined as a plain tagged record. If the user stops execution before "Obj" gets initialized (for example, by inserting a breakpoint "on" the function - or in other words, by inserting a breakpoint using the function name as the location), one might get the following of output if you try printing the value of obj: (gdb) p obj object size is larger than varsize-limit object size is larger than varsize-limit object size is larger than varsize-limit $1 = object size is larger than varsize-limit (x => 4204154) Same thing with "info locals": (gdb) info locals i = 0 obj = object size is larger than varsize-limit (x => 4204154) We have also seen different error messages such as "Cannot read memory at 0x...". The error happens because we are trying to read the dispatch table of a tagged type variable before it gets initialized. So the errors might legitimately occur, and are supposed to be be contained. However, the way things are written in ada-lang.c:ada_tag_name, although the exception is in fact contained, the error message still gets to be printed out. This patch prevents this from happening by eliminating the use of catch_errors, and using a TRY_CATCH block instead. Doing this removed the need to use functions specifically fitted for catch_errors, and thus some other simplifications could me made. In the end, the code got reorganized a bit to better show the logic behind it, as well as the common patterns. gdb/ChangeLog: * ada-lang.c (struct tag_args): Delete. (ada_get_tsd_type): Function body moved up in source file. (ada_tag_name_1, ada_tag_name_2): Delete. (ada_get_tsd_from_tag): New function. (ada_tag_name_from_tsd): New function. (ada_tag_name): Use a TRY_CATCH block instead of catch_errors to determine the tag name. gdb/testsuite/ChangeLog: * gdb.ada/tagged_not_init: New testcase. --- gdb/ChangeLog | 10 ++ gdb/ada-lang.c | 139 +++++++++--------- gdb/testsuite/ChangeLog | 4 + gdb/testsuite/gdb.ada/tagged_not_init.exp | 36 +++++ gdb/testsuite/gdb.ada/tagged_not_init/foo.adb | 24 +++ gdb/testsuite/gdb.ada/tagged_not_init/pck.adb | 26 ++++ gdb/testsuite/gdb.ada/tagged_not_init/pck.ads | 31 ++++ 7 files changed, 203 insertions(+), 67 deletions(-) create mode 100644 gdb/testsuite/gdb.ada/tagged_not_init.exp create mode 100644 gdb/testsuite/gdb.ada/tagged_not_init/foo.adb create mode 100644 gdb/testsuite/gdb.ada/tagged_not_init/pck.adb create mode 100644 gdb/testsuite/gdb.ada/tagged_not_init/pck.ads diff --git a/gdb/ChangeLog b/gdb/ChangeLog index a937673b270..2681f17f71a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2012-02-29 Joel Brobecker + + * ada-lang.c (struct tag_args): Delete. + (ada_get_tsd_type): Function body moved up in source file. + (ada_tag_name_1, ada_tag_name_2): Delete. + (ada_get_tsd_from_tag): New function. + (ada_tag_name_from_tsd): New function. + (ada_tag_name): Use a TRY_CATCH block instead of catch_errors + to determine the tag name. + 2012-02-29 Joel Brobecker * ada-lang.h (ada_get_decoded_value, ada_get_decoded_type): Add diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 430e7067052..1b467789c35 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -6029,105 +6029,110 @@ type_from_tag (struct value *tag) return NULL; } -struct tag_args +/* Return the "ada__tags__type_specific_data" type. */ + +static struct type * +ada_get_tsd_type (struct inferior *inf) { - struct value *tag; - char *name; -}; + struct ada_inferior_data *data = get_ada_inferior_data (inf); + if (data->tsd_type == 0) + data->tsd_type = ada_find_any_type ("ada__tags__type_specific_data"); + return data->tsd_type; +} -static int ada_tag_name_1 (void *); -static int ada_tag_name_2 (struct tag_args *); +/* Return the TSD (type-specific data) associated to the given TAG. + TAG is assumed to be the tag of a tagged-type entity. -/* Wrapper function used by ada_tag_name. Given a struct tag_args* - value ARGS, sets ARGS->name to the tag name of ARGS->tag. - The value stored in ARGS->name is valid until the next call to - ada_tag_name_1. */ + May return NULL if we are unable to get the TSD. */ -static int -ada_tag_name_1 (void *args0) +static struct value * +ada_get_tsd_from_tag (struct value *tag) { - struct tag_args *args = (struct tag_args *) args0; - static char name[1024]; - char *p; struct value *val; + struct type *type; - args->name = NULL; - val = ada_value_struct_elt (args->tag, "tsd", 1); - if (val == NULL) - return ada_tag_name_2 (args); - val = ada_value_struct_elt (val, "expanded_name", 1); - if (val == NULL) - return 0; - read_memory_string (value_as_address (val), name, sizeof (name) - 1); - for (p = name; *p != '\0'; p += 1) - if (isalpha (*p)) - *p = tolower (*p); - args->name = name; - return 0; -} + /* First option: The TSD is simply stored as a field of our TAG. + Only older versions of GNAT would use this format, but we have + to test it first, because there are no visible markers for + the current approach except the absence of that field. */ -/* Return the "ada__tags__type_specific_data" type. */ + val = ada_value_struct_elt (tag, "tsd", 1); + if (val) + return val; -static struct type * -ada_get_tsd_type (struct inferior *inf) -{ - struct ada_inferior_data *data = get_ada_inferior_data (inf); + /* Try the second representation for the dispatch table (in which + there is no explicit 'tsd' field in the referent of the tag pointer, + and instead the tsd pointer is stored just before the dispatch + table. */ - if (data->tsd_type == 0) - data->tsd_type = ada_find_any_type ("ada__tags__type_specific_data"); - return data->tsd_type; + type = ada_get_tsd_type (current_inferior()); + if (type == NULL) + return NULL; + type = lookup_pointer_type (lookup_pointer_type (type)); + val = value_cast (type, tag); + if (val == NULL) + return NULL; + return value_ind (value_ptradd (val, -1)); } -/* Utility function for ada_tag_name_1 that tries the second - representation for the dispatch table (in which there is no - explicit 'tsd' field in the referent of the tag pointer, and instead - the tsd pointer is stored just before the dispatch table. */ - -static int -ada_tag_name_2 (struct tag_args *args) +/* Given the TSD of a tag (type-specific data), return a string + containing the name of the associated type. + + The returned value is good until the next call. May return NULL + if we are unable to determine the tag name. */ + +static char * +ada_tag_name_from_tsd (struct value *tsd) { - struct type *info_type; static char name[1024]; char *p; - struct value *val, *valp; + struct value *val; - args->name = NULL; - info_type = ada_get_tsd_type (current_inferior()); - if (info_type == NULL) - return 0; - info_type = lookup_pointer_type (lookup_pointer_type (info_type)); - valp = value_cast (info_type, args->tag); - if (valp == NULL) - return 0; - val = value_ind (value_ptradd (valp, -1)); - if (val == NULL) - return 0; - val = ada_value_struct_elt (val, "expanded_name", 1); + val = ada_value_struct_elt (tsd, "expanded_name", 1); if (val == NULL) - return 0; + return NULL; read_memory_string (value_as_address (val), name, sizeof (name) - 1); for (p = name; *p != '\0'; p += 1) if (isalpha (*p)) *p = tolower (*p); - args->name = name; - return 0; + return name; } /* The type name of the dynamic type denoted by the 'tag value TAG, as - a C string. */ + a C string. + + Return NULL if the TAG is not an Ada tag, or if we were unable to + determine the name of that tag. The result is good until the next + call. */ const char * ada_tag_name (struct value *tag) { - struct tag_args args; + volatile struct gdb_exception e; + char *name = NULL; if (!ada_is_tag_type (value_type (tag))) return NULL; - args.tag = tag; - args.name = NULL; - catch_errors (ada_tag_name_1, &args, NULL, RETURN_MASK_ALL); - return args.name; + + /* It is perfectly possible that an exception be raised while trying + to determine the TAG's name, even under normal circumstances: + The associated variable may be uninitialized or corrupted, for + instance. We do not let any exception propagate past this point. + instead we return NULL. + + We also do not print the error message either (which often is very + low-level (Eg: "Cannot read memory at 0x[...]"), but instead let + the caller print a more meaningful message if necessary. */ + TRY_CATCH (e, RETURN_MASK_ERROR) + { + struct value *tsd = ada_get_tsd_from_tag (tag); + + if (tsd != NULL) + name = ada_tag_name_from_tsd (tsd); + } + + return name; } /* The parent type of TYPE, or NULL if none. */ diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 2ff685aa972..9d6beea2e27 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2012-02-29 Joel Brobecker + + * gdb.ada/tagged_not_init: New testcase. + 2012-02-29 Joel Brobecker * gdb.ada/arrayidx.exp: Adjust expected output for p_one_two_three. diff --git a/gdb/testsuite/gdb.ada/tagged_not_init.exp b/gdb/testsuite/gdb.ada/tagged_not_init.exp new file mode 100644 index 00000000000..013d76ab578 --- /dev/null +++ b/gdb/testsuite/gdb.ada/tagged_not_init.exp @@ -0,0 +1,36 @@ +# Copyright 2012 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 . + +load_lib "ada.exp" + +if { [skip_ada_tests] } { return -1 } + +set testdir "tagged_not_init" +set testfile "${testdir}/foo" +set srcfile ${srcdir}/${subdir}/${testfile}.adb +set binfile ${objdir}/${subdir}/${testfile} + +file mkdir ${objdir}/${subdir}/${testdir} +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} { + return -1 +} + +clean_restart ${testfile} + +set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb] +runto "foo.adb:$bp_location" + +gdb_test "print obj" " = \\(x => -?$decimal\\)" + diff --git a/gdb/testsuite/gdb.ada/tagged_not_init/foo.adb b/gdb/testsuite/gdb.ada/tagged_not_init/foo.adb new file mode 100644 index 00000000000..aa345a2076d --- /dev/null +++ b/gdb/testsuite/gdb.ada/tagged_not_init/foo.adb @@ -0,0 +1,24 @@ +-- Copyright 2012 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 . + +with Pck; use Pck; + +procedure Foo is + I : Integer := Ident (10); -- STOP + Obj : Base; +begin + Obj.X := I; + Do_Nothing (Obj.X'Address); +end Foo; diff --git a/gdb/testsuite/gdb.ada/tagged_not_init/pck.adb b/gdb/testsuite/gdb.ada/tagged_not_init/pck.adb new file mode 100644 index 00000000000..5d6e7606935 --- /dev/null +++ b/gdb/testsuite/gdb.ada/tagged_not_init/pck.adb @@ -0,0 +1,26 @@ +-- Copyright 2012 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 . + +package body Pck is + function Ident (I : Integer) return Integer is + begin + return I; + end Ident; + + procedure Do_Nothing (A : System.Address) is + begin + null; + end Do_Nothing; +end Pck; diff --git a/gdb/testsuite/gdb.ada/tagged_not_init/pck.ads b/gdb/testsuite/gdb.ada/tagged_not_init/pck.ads new file mode 100644 index 00000000000..19d92951153 --- /dev/null +++ b/gdb/testsuite/gdb.ada/tagged_not_init/pck.ads @@ -0,0 +1,31 @@ +-- Copyright 2012 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 . + +with System; + +package Pck is + type Base is tagged record + X : Integer := 42; + end record; + + type Extension is new Base with + record + Y : Float := 42.0; + end record; + + function Ident (I : Integer) return Integer; + + procedure Do_Nothing (A : System.Address); +end Pck; -- 2.30.2