Only leave dprintf inserted if it is marked as persistent (PR breakpoints/17012)

On Linux native, if dprintfs are inserted when detaching, they are left
in the inferior which causes it to crash from a SIGTRAP. It also happens
with dprintfs on remote targets, when set disconnected-dprintf is off.

The rationale of the line modified by the patch was to leave dprintfs
inserted in order to support disconnected dprintfs. However, not all
dprintfs are persistent. Also, there's no reason other kinds of
breakpoints can't be persistent either. So this replaces the bp_dprintf
check with a check on whether the location is persistent.

bl->target_info.persist will be 1 only if disconnected-dprintf is on and
we are debugging a remote target. On native, it will always be 0,
regardless of the value of disconnected-dprintf. This makes sense, since
disconnected dprintfs are not supported by the native target.

One issue about the test is that it does not pass when using
--target_board=native-extended-gdbserver, partly due to bug 17302 [1].

One quick hack I tried for this was to add a useless "next" between the
call to getpid() and detach, which avoids the bug. There is still one
case where the test fails, and that is with:

- breakpoint always-inserted on
- dprintf-style agent
- disconnected-dprintf on

What happens is that my detach does not actually detach the process,
because some persistent commands (the disconnected dprintf) is present.
However since gdbserver is ran with --once, when gdb disconnects,
gdbserver goes down and takes with it all the processes it spawned and
that are still under its control (which includes my test process).
When the test checks if the test process is still alive, it obvisouly
fails. Investigating about that led me to ask a question on the ML [2]
about the behavior of detach.

Until the remote case is sorted out, the problematic test is marked as
KFAIL.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=17302
[2] https://sourceware.org/ml/gdb/2014-08/msg00115.html

gdb/Changelog:

	PR breakpoints/17012
	* breakpoint.c (remove_breakpoints_pid): Skip removing
	breakpoint if it is marked as persistent.

gdb/testsuite/ChangeLog:

	PR breakpoints/17012
	* gdb.base/dprintf-detach.c: New file.
	* gdb.base/dprintf-detach.exp: New file.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b6b885959860966a2134fcba5782bd96a9488001..441a20c0e61961e092bca7de99ad149b43dbef4c 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-12-10  Simon Marchi  <simon.marchi@ericsson.com>
+
+       PR breakpoints/17012
+       * breakpoint.c (remove_breakpoints_pid): Skip removing breakpoint if
+       it is marked as persistent.
+
 2014-12-10  Doug Evans  <xdje42@gmail.com>
 
        * cp-namespace.c (cp_lookup_symbol_nonlocal): Move definition,

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 574d06c3a7444561dde71ef2a587a0e53de671d8..8ccaf434f7d97011a0c8ae8b8a0ae8c07f8d5bff 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3220,10 +3220,7 @@ remove_breakpoints_pid (int pid)
     if (bl->pspace != inf->pspace)
       continue;
 
-    if (bl->owner->type == bp_dprintf)
-      continue;
-
-    if (bl->inserted)
+    if (bl->inserted && !bl->target_info.persist)
       {
        val = remove_breakpoint (bl, mark_uninserted);
        if (val != 0)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 4abd097aaa2e02ee2af7c57e3c701a65a0355acb..5ce30296a74210f4f2a97ffc91d6d31f940cc691 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2014-12-10  Simon Marchi  <simon.marchi@ericsson.com>
+
+       PR breakpoints/17012
+       * gdb.base/dprintf-detach.c: New file.
+       * gdb.base/dprintf-detach.exp: New file.
+
 2014-12-10  Simon Marchi  <simon.marchi@ericsson.com>
 
        * gdb.trace/qtro.exp: Replace gdbserver detection code by...

diff --git a/gdb/testsuite/gdb.base/dprintf-detach.c b/gdb/testsuite/gdb.base/dprintf-detach.c
new file mode 100644 (file)
index 0000000..16d7023
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-detach.c
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 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/>.  */
+
+#include <unistd.h>
+
+static void
+function (void)
+{
+  sleep (1);
+}
+
+int
+main (void)
+{
+  int i;
+
+  for (i = 0; i < 30; i++)
+    function ();
+}

diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
new file mode 100644 (file)
index 0000000..2cee42c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
@@ -0,0 +1,95 @@
+#   Copyright 2014 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 test checks that inserting a dprintf and detaching does not crash
+# the program.
+#
+# Related bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17012
+
+load_lib gdbserver-support.exp
+
+# The test relies on "detach/attach".
+if { [target_info exists use_gdb_stub] } then {
+    return 0
+}
+
+standard_testfile
+set escapedbinfile [string_to_regexp ${binfile}]
+
+if [prepare_for_testing "failed to prepare for dprintf-detach" \
+    ${testfile} ${srcfile} {debug}] {
+    return -1
+}
+
+proc dprintf_detach_test { breakpoint_always_inserted dprintf_style disconnected_dprintf } {
+    set test_prefix "bai=${breakpoint_always_inserted} ds=${dprintf_style} dd=${disconnected_dprintf}"
+    global binfile decimal gdb_prompt escapedbinfile
+
+    with_test_prefix "$test_prefix" {
+       # Start with a clean gdb
+       clean_restart ${binfile}
+
+       gdb_test_no_output "set breakpoint always-inserted ${breakpoint_always_inserted}"
+       gdb_test_no_output "set dprintf-style ${dprintf_style}"
+       gdb_test_no_output "set disconnected-dprintf ${disconnected_dprintf}"
+
+       if ![runto_main] {
+           fail "Can't run to main"
+           return -1
+       }
+
+       # Get PID of test program.
+       set inferior_pid -1
+       set test "get inferior process ID"
+       gdb_test_multiple "call getpid ()" $test {
+           -re ".* = ($decimal).*$gdb_prompt $" {
+               set inferior_pid $expect_out(1,string)
+               pass $test
+           }
+       }
+
+       if {$inferior_pid == -1} {
+           return
+       }
+
+       # Add a dprintf and detach.
+       gdb_test "dprintf function, \"hello\"" "Dprintf .*" "dprintf insertion"
+       gdb_test "detach" "Detaching from program: .*$escapedbinfile, .*" "detach program"
+
+       gdb_exit
+
+       # Check that the process still exists by attaching a new gdb to it.
+       clean_restart ${binfile}
+       set test "re-attach to inferior"
+       set is_gdbserver [target_is_gdbserver]
+
+       if { $is_gdbserver == 1 } {
+               setup_kfail "*-*-*" "server/17302"
+       } else {
+               # Give some time for the ex-inferior to run and hopefully not crash.
+               sleep 1
+       }
+
+       gdb_test "attach $inferior_pid" "Attaching to program: $escapedbinfile, process $inferior_pid.*Reading symbols from.*" "$test"
+    }
+}
+
+foreach breakpoint_always_inserted { "on" "off" } {
+    foreach dprintf_style { "gdb" "call" "agent" } {
+       foreach disconnected_dprintf { "on" "off" } {
+           dprintf_detach_test $breakpoint_always_inserted $dprintf_style $disconnected_dprintf
+       }
+    }
+}