This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] [Ada] GDB crash during "finish" of function with out parameters


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=dddc0e16ef5d77e4f97d02ee0e2d4234c97dae0e

commit dddc0e16ef5d77e4f97d02ee0e2d4234c97dae0e
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Mon Nov 9 09:58:16 2015 -0800

    [Ada] GDB crash during "finish" of function with out parameters
    
    Consider a function with the following signature...
    
       function F (R : out Rec_Type) return Enum_Type;
    
    ... where Rec_Type is a simple record:
    
       type Rec_Type is record
          Cur : Integer;
       end record;
    
    Trying to "finish" from that function causes GDB to SEGV:
    
        (gdb) fin
        Run till exit from #0  bar.f (r=...) at bar.adb:5
        0x00000000004022fe in foo () at foo.adb:5
        5          I : Enum_Type := F (R);
        [1]    18949 segmentation fault (core dumped)  /[..]/gdb
    
    This is related to the fact that funtion F has a parameter (R)
    which is an "out" parameter being passed by copy. For those,
    GNAT transforms the return value to be a record with multiple
    fields: The first one is called "RETVAL" and contains the return
    value shown in the source, and the remaining fields have the same
    name as the "out" or "in out" parameters which are passed by copy.
    So, in the example above, function F returns a struct that has
    one field who name is "r".
    
    Because "RETVAL" starts with "R", GDB thinks it's a wrapper field,
    because it looks like the encoding used for  variant records:
    
       --    member_name ::= {choice} | others_choice
       --    choice ::= simple_choice | range_choice
       --    simple_choice ::= S number
       --    range_choice  ::= R number T number   <<<<<-----  here
       --    number ::= {decimal_digit} [m]
       --    others_choice ::= O (upper case letter O)
    
    See ada_is_wrapper_field:
    
      return (name != NULL
              && (startswith (name, "PARENT")
                  || strcmp (name, "REP") == 0
                  || startswith (name, "_parent")
                  || name[0] == 'S' || name[0] == 'R' || name[0] == 'O'));
    
    As a result of this, when trying to print the RETURN value,
    we think that RETVAL is a wrapper, and thus recurse into
    print_field_values...
    
          if (ada_is_wrapper_field (type, i))
            {
              comma_needed =
                print_field_values (TYPE_FIELD_TYPE (type, i),
                                    valaddr,
                                    (offset
                                     + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
                                    stream, recurse, val, options,
                                    comma_needed, type, offset, language);
    
    ... which is a problem since print_field_values assumes that
    the type it is given ("TYPE_FIELD_TYPE (type, i)" here), is also
    a record type. However, that's not the case, since RETVAL is
    an enum. That eventually leads GDB to a NULL type when trying to
    extract fields out of the enum, which then leads to a SEGV when
    trying to dereference it.
    
    Ideally, we'd want to be a little more careful in identifying
    wrapper fields, by enhancing ada_is_wrapper_field to be a little
    more complete in its analysis of the field name before declaring
    it a variant record wrapper. However, it's not super easy to do
    so, considering that the choices can be combined together when
    complex choices are used. Eg:
    
       -- [...] the choice 1 .. 4 | 7 | -10 would be represented by
       --    R1T4S7S10m
    
    Given that we are working towards getting rid of GNAT encodings,
    which means that the above will eventually disappear, we took
    the more pragmatic approach is just treating  RETVAL as a special
    case.
    
    gdb/ChangeLog:
    
            * ada-lang.c (ada_is_wrapper_field): Add special handling
            for fields called "RETVAL".
    
    gdb/testsuite/ChangeLog:
    
            * gdb.ada/fin_fun_out: New testcase.

Diff:
---
 gdb/ChangeLog                                      |  5 +++
 gdb/ada-lang.c                                     | 11 +++++++
 gdb/testsuite/ChangeLog                            |  4 +++
 gdb/testsuite/gdb.ada/fin_fun_out.exp              | 38 ++++++++++++++++++++++
 gdb/testsuite/gdb.ada/fin_fun_out/bar.adb          | 23 +++++++++++++
 gdb/testsuite/gdb.ada/fin_fun_out/bar.ads          | 25 ++++++++++++++
 gdb/testsuite/gdb.ada/fin_fun_out/foo_o525_013.adb | 23 +++++++++++++
 7 files changed, 129 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e198c93..beb604f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2015-11-09  Joel Brobecker  <brobecker@adacore.com>
+
+	* ada-lang.c (ada_is_wrapper_field): Add special handling
+	for fields called "RETVAL".
+
 2015-11-09  Yao Qi  <yao.qi@linaro.org>
 
 	* arm-tdep.c (arm_exidx_new_objfile): Use
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index fff4862..1f2d014 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -6984,6 +6984,17 @@ ada_is_wrapper_field (struct type *type, int field_num)
 {
   const char *name = TYPE_FIELD_NAME (type, field_num);
 
+  if (name != NULL && strcmp (name, "RETVAL") == 0)
+    {
+      /* This happens in functions with "out" or "in out" parameters
+	 which are passed by copy.  For such functions, GNAT describes
+	 the function's return type as being a struct where the return
+	 value is in a field called RETVAL, and where the other "out"
+	 or "in out" parameters are fields of that struct.  This is not
+	 a wrapper.  */
+      return 0;
+    }
+
   return (name != NULL
           && (startswith (name, "PARENT")
               || strcmp (name, "REP") == 0
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 31a59f4..dfc9b77 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2015-11-09  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.ada/fin_fun_out: New testcase.
+
 2015-11-07  Kevin Buettner  <kevinb@redhat.com>
 
 	* gdb.dwarf2/data-loc.exp (Dwarf::assemble): Don't hardcode
diff --git a/gdb/testsuite/gdb.ada/fin_fun_out.exp b/gdb/testsuite/gdb.ada/fin_fun_out.exp
new file mode 100644
index 0000000..e989d87
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/fin_fun_out.exp
@@ -0,0 +1,38 @@
+# Copyright 2015 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/>.
+
+load_lib "ada.exp"
+
+standard_ada_testfile foo_o525_013
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+runto "bar.f"
+
+# Perform a "finish". The output, and in particular the value of
+# the return value depends on the target, as sometime the compiler
+# will transform it into a struct, which we may or may not be able
+# to display, depending on the ABI.  The objective of the test is
+# to verify that we don't crash, so keep the expected output simple...
+gdb_test "finish" \
+         ".*Value returned.*"
+
+# Verify that GDB is still alive...
+gdb_test "print 1" \
+         "= 1"
diff --git a/gdb/testsuite/gdb.ada/fin_fun_out/bar.adb b/gdb/testsuite/gdb.ada/fin_fun_out/bar.adb
new file mode 100644
index 0000000..1b74f13
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/fin_fun_out/bar.adb
@@ -0,0 +1,23 @@
+--  Copyright 2015 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/>.
+
+package body Bar is
+   function F (R : out Rec_Type) return Enum_Type
+   is
+   begin
+      R.Cur := 0;
+      return A;
+   end F;
+end Bar;
diff --git a/gdb/testsuite/gdb.ada/fin_fun_out/bar.ads b/gdb/testsuite/gdb.ada/fin_fun_out/bar.ads
new file mode 100644
index 0000000..cbd6504
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/fin_fun_out/bar.ads
@@ -0,0 +1,25 @@
+--  Copyright 2015 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/>.
+
+package Bar is
+   type Enum_Type is (A, B, C);
+
+   type Rec_Type is record
+      Cur : Integer;
+   end record;
+
+   function F (R : out Rec_Type) return Enum_Type;
+
+end Bar;
diff --git a/gdb/testsuite/gdb.ada/fin_fun_out/foo_o525_013.adb b/gdb/testsuite/gdb.ada/fin_fun_out/foo_o525_013.adb
new file mode 100644
index 0000000..46f89ec
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/fin_fun_out/foo_o525_013.adb
@@ -0,0 +1,23 @@
+--  Copyright 2015 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/>.
+
+with Bar; use Bar;
+
+procedure Foo_O525_013 is
+   R : Rec_Type;
+   I : Enum_Type := F (R);
+begin
+   null;
+end Foo_O525_013;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]