Dandling memory pointers in Ada catchpoints with GDB/MI.
authorJoel Brobecker <brobecker@adacore.com>
Wed, 30 Oct 2013 10:18:24 +0000 (11:18 +0100)
committerJoel Brobecker <brobecker@adacore.com>
Mon, 11 Nov 2013 15:19:07 +0000 (19:19 +0400)
When using the GDB/MI commands to insert a catchpoint on a specific
Ada exception, any re-evaluation of that catchpoint (for instance
a re-evaluation performed after a shared library got mapped by the
inferior) fails. For instance, with any Ada program:

    (gdb)
    -catch-exception -e program_error
    ^done,bkptno="1",bkpt={[...]}
    (gdb)
    -exec-run
    =thread-group-started,id="i1",pid="28315"
    =thread-created,id="1",group-id="i1"
    ^running
    *running,thread-id="all"
    (gdb)
    =library-loaded,[...]
    &"warning: failed to reevaluate internal exception condition for catchpoint 1: No definition of \"exec\" in current context.\n"
    &"warning: failed to reevaluate internal exception condition for catchpoint 1: No definition of \"exec\" in current context.\n"
    [...]

The same is true if using an Ada exception catchpoint.

The problem comes from the fact that that we deallocate the strings
given as arguments to create_ada_exception_catchpoint, while the latter
just makes shallow copies of those strings, thus creating dandling
pointers.

This patch fixes the issue by passing freshly allocated strings to
create_ada_exception_catchpoint, while at the same time updating
create_ada_exception_catchpoint's documentation to make it clear
that deallocating the strings is no longer the responsibility of
the caller.

gdb/ChangeLog:

        * ada-lang.c (create_ada_exception_catchpoint): Enhance
        the documentation of fields "except_string" and "condition".
        * mi/mi-cmd-catch.c (mi_cmd_catch_assert): Reallocate
        CONDITION on the heap before passing it to
        create_ada_exception_catchpoint.
        (mi_cmd_catch_exception): Likewise for EXCEPTION_NAME and
        CONDITION.

gdb/testsuite/ChangeLog:

        * gdb.ada/mi_ex_cond: New testcase.

Tested on x86_64-linux.  The "-break-list" test FAILs without
this patch.

gdb/ChangeLog
gdb/ada-lang.c
gdb/mi/mi-cmd-catch.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.ada/mi_ex_cond.exp [new file with mode: 0644]
gdb/testsuite/gdb.ada/mi_ex_cond/foo.adb [new file with mode: 0644]
gdb/testsuite/gdb.ada/mi_ex_cond/pck.ads [new file with mode: 0644]

index c35ed65a83477f25e7309b2cbd58e858b704b1fe..3445b04c2afb8f9520dc68e1b12324154a9f0528 100644 (file)
@@ -1,3 +1,13 @@
+2013-11-11  Joel Brobecker  <brobecker@adacore.com>
+
+       * ada-lang.c (create_ada_exception_catchpoint): Enhance
+       the documentation of fields "except_string" and "condition".
+       * mi/mi-cmd-catch.c (mi_cmd_catch_assert): Reallocate
+       CONDITION on the heap before passing it to
+       create_ada_exception_catchpoint.
+       (mi_cmd_catch_exception): Likewise for EXCEPTION_NAME and
+       CONDITION.
+
 2013-11-11  Tom Tromey  <tromey@redhat.com>
 
        * config.in, configure: Rebuild.
index 4b554608d8dce78a5439f4d259660d7fb05c5ec8..781713266ede2b00b35f9cedeaa241c67fe3fb56 100644 (file)
@@ -12170,11 +12170,15 @@ ada_exception_sal (enum ada_exception_catchpoint_kind ex, char *excep_string,
 
    EX_KIND is the kind of exception catchpoint to be created.
 
-   EXCEPT_STRING, if not NULL, indicates the name of the exception
-   to which this catchpoint applies.  If NULL, this catchpoint is
-   expected to trigger for all exceptions.
-
-   COND_STRING, if not NULL, is the catchpoint condition.
+   If EXCEPT_STRING is NULL, this catchpoint is expected to trigger
+   for all exceptions.  Otherwise, EXCEPT_STRING indicates the name
+   of the exception to which this catchpoint applies.  When not NULL,
+   the string must be allocated on the heap, and its deallocation
+   is no longer the responsibility of the caller.
+
+   COND_STRING, if not NULL, is the catchpoint condition.  This string
+   must be allocated on the heap, and its deallocation is no longer
+   the responsibility of the caller.
 
    TEMPFLAG, if nonzero, means that the underlying breakpoint
    should be temporary.
index 23e30d086e0f8cf4ae74dd2c2c11f9cd5356aa94..bcfc1bad2044291b8d5126e60380b225f30dc9eb 100644 (file)
@@ -81,6 +81,10 @@ mi_cmd_catch_assert (char *cmd, char *argv[], int argc)
     error (_("Invalid argument: %s"), argv[oind]);
 
   setup_breakpoint_reporting ();
+  /* create_ada_exception_catchpoint needs CONDITION to be xstrdup'ed,
+     and will assume control of its lifetime.  */
+  if (condition != NULL)
+    condition = xstrdup (condition);
   create_ada_exception_catchpoint (gdbarch, ada_catch_assert,
                                   NULL, condition, temp, enabled, 0);
 }
@@ -154,6 +158,12 @@ mi_cmd_catch_exception (char *cmd, char *argv[], int argc)
     error (_("\"-e\" and \"-u\" are mutually exclusive"));
 
   setup_breakpoint_reporting ();
+  /* create_ada_exception_catchpoint needs EXCEPTION_NAME and CONDITION
+     to be xstrdup'ed, and will assume control of their lifetime.  */
+  if (exception_name != NULL)
+    exception_name = xstrdup (exception_name);
+  if (condition != NULL)
+    condition = xstrdup (condition);
   create_ada_exception_catchpoint (gdbarch, ex_kind,
                                   exception_name, condition,
                                   temp, enabled, 0);
index 6a88d6c658854b599d976e078d3cf91fedba3c7d..fb1dc5745ffef35e72ae42687968a0d87627a24a 100644 (file)
@@ -1,3 +1,7 @@
+2013-11-11  Joel Brobecker  <brobecker@adacore.com>
+
+       * gdb.ada/mi_ex_cond: New testcase.
+
 2013-11-07  Doug Evans  <dje@google.com>
 
        PR 11786
diff --git a/gdb/testsuite/gdb.ada/mi_ex_cond.exp b/gdb/testsuite/gdb.ada/mi_ex_cond.exp
new file mode 100644 (file)
index 0000000..9144102
--- /dev/null
@@ -0,0 +1,94 @@
+# Copyright 2013 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
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional_flags=-bargs additional_flags=-static additional_flags=-margs ]] != "" } {
+  return -1
+}
+
+# # Some global variables used to simplify the maintenance of some of
+# # the regular expressions below.
+set any_nb "\[0-9\]+"
+set eol "\[\r\n\]+"
+
+# Before going any further, verify that we can insert exception
+# catchpoints...  That way, we won't have to do this while doing
+# the actual GDB/MI testing.
+
+clean_restart ${testfile}
+
+if ![runto_main] then {
+   fail "Cannot run to main, testcase aborted"
+   return 0
+}
+
+set msg "insert catchpoint on all Ada exceptions"
+gdb_test_multiple "catch exception" $msg {
+    -re "Catchpoint $any_nb: all Ada exceptions$eol$gdb_prompt $" {
+       pass $msg
+    }
+    -re "Your Ada runtime appears to be missing some debugging information.*\[\r\n\]+$gdb_prompt $" {
+       # If the runtime was not built with enough debug information,
+       # or if it was stripped, we can not test exception
+       # catchpoints.
+       unsupported $msg
+       return -1
+    }
+}
+
+# Now, we can start the GDB/MI testing itself...
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+# And finally, the meat of the testcase... Insert an Ada exception
+# catchpoint that uses both conditions and exception name.
+
+mi_gdb_test "-catch-exception -c \"i = 2\" -e constraint_error" \
+            "\\^done,bkptno=\"$decimal\",bkpt={.*disp=\"keep\",enabled=\"y\",addr=\"$hex\",what=\"`constraint_error' Ada exception\",.*,cond=\"i = 2\",.*}" \
+            "catch C_E if i = 2"
+
+# It is important that we start the program's execution after having
+# inserted the exception catchpoint above.  We want to verify that
+# we are able to re-evaluate the exception catchpoint exception
+# address and stop condition without problems when new shared libraries
+# get mapped (during program startup).
+
+mi_run_cmd
+
+mi_expect_stop \
+    "breakpoint-hit\",disp=\"keep\",bkptno=\"$any_nb\",exception-name=\"CONSTRAINT_ERROR" \
+    "foo" "" ".*" ".*" \
+    ".*" \
+    "run to exception catchpoint hit"
+
+# Make sure that any of the catchpoint re-evaluations didn't cause
+# a clobbering of some of the exeption's info.
+
+mi_gdb_test "-break-list" \
+    "\\^done,.*,what=\"`constraint_error' Ada exception\",.*,cond=\"i = 2\",.*" \
+    "-break-list"
diff --git a/gdb/testsuite/gdb.ada/mi_ex_cond/foo.adb b/gdb/testsuite/gdb.ada/mi_ex_cond/foo.adb
new file mode 100644 (file)
index 0000000..9db1233
--- /dev/null
@@ -0,0 +1,29 @@
+--  Copyright 2013 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 Pck; use Pck;
+
+procedure Foo is
+begin
+   while I <= 3 loop
+      begin
+         raise Constraint_Error;
+      exception
+         when others =>
+            null;
+      end;
+      I := I + 1;
+   end loop;
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/mi_ex_cond/pck.ads b/gdb/testsuite/gdb.ada/mi_ex_cond/pck.ads
new file mode 100644 (file)
index 0000000..0ac131c
--- /dev/null
@@ -0,0 +1,18 @@
+--  Copyright 2013 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 Pck is
+   I : Integer := 1;
+end Pck;