From patchwork Wed Sep 17 22:47:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 2897 Received: (qmail 26502 invoked by alias); 17 Sep 2014 22:47:20 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 26441 invoked by uid 89); 17 Sep 2014 22:47:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 17 Sep 2014 22:47:16 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8HMlEFf000813 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 17 Sep 2014 18:47:14 -0400 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8HMl9Wh030382 for ; Wed, 17 Sep 2014 18:47:13 -0400 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 3/3] Fix "breakpoint always-inserted off"; remove "breakpoint always-inserted auto" Date: Wed, 17 Sep 2014 23:47:08 +0100 Message-Id: <1410994028-24282-4-git-send-email-palves@redhat.com> In-Reply-To: <1410994028-24282-1-git-send-email-palves@redhat.com> References: <1410994028-24282-1-git-send-email-palves@redhat.com> By default, GDB removes all breakpoints from the target when the target stops and the prompt is given back to the user. This is useful in case GDB crashes while the user is interacting, as otherwise, there's a higher chance breakpoints would be left planted on the target. But, as long as any thread is running free, we need to make sure to keep breakpoints inserted, lest a thread misses a breakpoint. With that in mind, in preparation for non-stop mode, we added a "breakpoint always-inserted on" mode. This traded off the extra crash protection for never having threads miss breakpoints, and in addition is more efficient if there's a ton of breakpoints to remove/insert at each user command (e.g., at each "step"). When we added non-stop mode, and for a period, we required users to manually set "always-inserted on" when they enabled non-stop mode, as otherwise GDB removes all breakpoints from the target as soon as any thread stops, even if other threads are still running. That soon revealed a nuisance, and so later we added an extra "breakpoint always-inserted auto" mode, that made GDB behave like "always-inserted on" when non-stop was enabled, and "always-inserted off" when non-stop was disabled. "auto" was made the default at the same time. In hindsight, this "auto" setting was unnecessary, and not the ideal solution. Non-stop mode does depends on breakpoints always-inserted mode, but only as long as any thread is running. If no thread is running, no breakpoint can be missed. The same is true for all-stop too. E.g., if, in all-stop mode, and the user does: (gdb) c& (gdb) b foo That breakpoint at "foo" should be inserted immediately, but it currently isn't -- currently it'll end up inserted only if the target happens to trip on some event, and is re-resumed, e.g., an internal breakpoint triggers that doesn't cause a user-visible stop, and so we end up in keep_going calling insert_breakpoints. The patch adds a new test that covers this. It fails before this patch. IOW, no matter whether in non-stop or all-stop, if the target fully stops, we can remove breakpoints. And no matter whether in all-stop or non-stop, if any thread is running in the target, then we need breakpoints to be immediately inserted. And then, if the target has global breakpoints, we need to keep breakpoints even when the target is stopped. So with that in mind, and aiming at reducing all-stop vs non-stop differences for all-stop-on-stop-of-non-stop, this patch fixes "breakpoint always-inserted off" to not remove breakpoints from the target until it fully stops, and then removes the "auto" setting as unnecessary. I propose removing it straight away rather than keeping it as an alias, unless someone complains they have scripts that need it and that can't adjust. Tested on x86_64 Fedora 20. gdb/ 2014-09-17 Pedro Alves * NEWS: Mention merge of "breakpoint always-inserted" modes "off" and "auto" merged. * breakpoint.c (enum ugll_insert_mode): New enum. (always_inserted_mode): Now a plain boolean. (show_always_inserted_mode): No longer handle AUTO_BOOLEAN_AUTO. (breakpoints_always_inserted_mode): Delete. (breakpoints_should_be_inserted_now): New function. (insert_breakpoints): Pass UGLL_INSERT to update_global_location_list instead of calling insert_breakpoint_locations manually. (create_solib_event_breakpoint_1): New, factored out from ... (create_solib_event_breakpoint): ... this. (create_and_insert_solib_event_breakpoint): Use create_solib_event_breakpoint_1 instead of calling insert_breakpoint_locations manually. (update_global_location_list): Change parameter type from boolean to enum ugll_insert_mode. All callers adjusted. Adjust to use breakpoints_should_be_inserted_now and handle UGLL_INSERT. (update_global_location_list_nothrow): Change parameter type from boolean to enum ugll_insert_mode. (_initialize_breakpoint): "breakpoint always-inserted" option is now a boolean command. Update help text. * breakpoint.h (breakpoints_always_inserted_mode): Delete declaration. (breakpoints_should_be_inserted_now): New declaration. * infrun.c (handle_inferior_event) : Remove breakpoints_always_inserted_mode check. (normal_stop): Adjust to use breakpoints_should_be_inserted_now. * remote.c (remote_start_remote): Likewise. gdb/doc/ 2014-09-17 Pedro Alves * gdb.texinfo (Set Breaks): Document that "set breakpoint always-inserted off" is the default mode now. Delete documentation of "set breakpoint always-inserted auto". gdb/testsuite/ 2014-09-17 Pedro Alves * gdb.threads/break-while-running.exp: New file. * gdb.threads/break-while-running.c: New file. --- gdb/NEWS | 7 ++ gdb/breakpoint.c | 90 +++++++++++++---------- gdb/breakpoint.h | 2 +- gdb/doc/gdb.texinfo | 13 +--- gdb/infrun.c | 5 +- gdb/remote.c | 5 +- gdb/testsuite/gdb.threads/break-while-running.c | 51 +++++++++++++ gdb/testsuite/gdb.threads/break-while-running.exp | 87 ++++++++++++++++++++++ 8 files changed, 205 insertions(+), 55 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/break-while-running.c create mode 100644 gdb/testsuite/gdb.threads/break-while-running.exp diff --git a/gdb/NEWS b/gdb/NEWS index 343ee49..ac94874 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -31,6 +31,13 @@ queue-signal signal-name-or-number confirmation if the program had stopped for a signal and the user switched threads meanwhile. +* "breakpoint always-inserted" modes "off" and "auto" merged. + + Now, when 'breakpoint always-inserted mode' is set to "off", GDB + won't remove breakpoints from the target until all threads stop, + even in non-stop mode. The "auto" mode has been removed, and "off" + is now the default mode. + *** Changes in GDB 7.8 * New command line options diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 3f372de..3675b4f 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -258,12 +258,17 @@ enum ugll_insert_mode the inferior. */ UGLL_DONT_INSERT, - /* May insert breakpoints if breakpoints_always_inserted_mode is - true. */ + /* May insert breakpoints iff breakpoints_should_be_inserted_now + claims breakpoints should be inserted now. */ UGLL_MAY_INSERT, - /* Insert locations now, even if breakpoints_always_inserted_mode is - false. */ + /* Insert locations now, irrespective of + breakpoints_should_be_inserted_now. E.g., say all threads are + stopped right now, and the user did "continue". We need to + insert breakpoints _before_ resuming the target, but + UGLL_MAY_INSERT wouldn't insert them, because + breakpoints_should_be_inserted_now returns false at that point, + as no thread is running yet. */ UGLL_INSERT }; @@ -451,34 +456,51 @@ show_automatic_hardware_breakpoints (struct ui_file *file, int from_tty, value); } -/* If on, gdb will keep breakpoints inserted even as inferior is - stopped, and immediately insert any new breakpoints. If off, gdb - will insert breakpoints into inferior only when resuming it, and - will remove breakpoints upon stop. If auto, GDB will behave as ON - if in non-stop mode, and as OFF if all-stop mode.*/ - -static enum auto_boolean always_inserted_mode = AUTO_BOOLEAN_AUTO; +/* If on, GDB keeps breakpoints inserted even if the inferior is + stopped, and immediately inserts any new breakpoints as soon as + they're created. If off (default), GDB keeps breakpoints off of + the target as long as possible. That is, it delays inserting + breakpoints until the next resume, and removes them again when the + target fully stops. This is a bit safer in case GDB crashes while + processing user input. */ +static int always_inserted_mode = 0; static void show_always_inserted_mode (struct ui_file *file, int from_tty, struct cmd_list_element *c, const char *value) { - if (always_inserted_mode == AUTO_BOOLEAN_AUTO) - fprintf_filtered (file, - _("Always inserted breakpoint " - "mode is %s (currently %s).\n"), - value, - breakpoints_always_inserted_mode () ? "on" : "off"); - else - fprintf_filtered (file, _("Always inserted breakpoint mode is %s.\n"), - value); + fprintf_filtered (file, _("Always inserted breakpoint mode is %s.\n"), + value); } int -breakpoints_always_inserted_mode (void) +breakpoints_should_be_inserted_now (void) { - return (always_inserted_mode == AUTO_BOOLEAN_TRUE - || (always_inserted_mode == AUTO_BOOLEAN_AUTO && non_stop)); + if (gdbarch_has_global_breakpoints (target_gdbarch ())) + { + /* If breakpoints are global, they should be inserted even if no + thread under gdb's control is running, or even if there are + no threads under GDB's control yet. */ + return 1; + } + else if (target_has_execution) + { + struct thread_info *tp; + + if (always_inserted_mode) + { + /* The user wants breakpoints inserted even if all threads + are stopped. */ + return 1; + } + + ALL_NON_EXITED_THREADS (tp) + { + if (tp->executing) + return 1; + } + } + return 0; } static const char condition_evaluation_both[] = "host or target"; @@ -12930,10 +12952,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode) "a permanent breakpoint")); } - if (insert_mode == UGLL_INSERT - || (breakpoints_always_inserted_mode () - && (have_live_inferiors () - || (gdbarch_has_global_breakpoints (target_gdbarch ()))))) + if (insert_mode == UGLL_INSERT || breakpoints_should_be_inserted_now ()) { if (insert_mode != UGLL_DONT_INSERT) insert_breakpoint_locations (); @@ -17020,18 +17039,15 @@ a warning will be emitted for such breakpoints."), &breakpoint_set_cmdlist, &breakpoint_show_cmdlist); - add_setshow_auto_boolean_cmd ("always-inserted", class_support, - &always_inserted_mode, _("\ + add_setshow_boolean_cmd ("always-inserted", class_support, + &always_inserted_mode, _("\ Set mode for inserting breakpoints."), _("\ Show mode for inserting breakpoints."), _("\ -When this mode is off, breakpoints are inserted in inferior when it is\n\ -resumed, and removed when execution stops. When this mode is on,\n\ -breakpoints are inserted immediately and removed only when the user\n\ -deletes the breakpoint. When this mode is auto (which is the default),\n\ -the behaviour depends on the non-stop setting (see help set non-stop).\n\ -In this case, if gdb is controlling the inferior in non-stop mode, gdb\n\ -behaves as if always-inserted mode is on; if gdb is controlling the\n\ -inferior in all-stop mode, gdb behaves as if always-inserted mode is off."), +When this mode is on, breakpoints are inserted immediately as soon as\n\ +they're created, kept inserted even when execution stops, and removed\n\ +only when the user deletes them. When this mode is off (the default),\n\ +breakpoints are inserted only when execution continues, and removed\n\ +when execution stops."), NULL, &show_always_inserted_mode, &breakpoint_set_cmdlist, diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 00c8802..e191c10 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1491,7 +1491,7 @@ extern void breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf, const gdb_byte *writebuf_org, ULONGEST memaddr, LONGEST len); -extern int breakpoints_always_inserted_mode (void); +extern int breakpoints_should_be_inserted_now (void); /* Called each time new event from target is processed. Retires previously deleted breakpoint locations that diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 1bb1c0c..537fae8 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -3850,22 +3850,13 @@ This behavior can be controlled with the following commands:: @item set breakpoint always-inserted off All breakpoints, including newly added by the user, are inserted in the target only when the target is resumed. All breakpoints are -removed from the target when it stops. +removed from the target when it stops. This is the default mode. @item set breakpoint always-inserted on Causes all breakpoints to be inserted in the target at all times. If the user adds a new breakpoint, or changes an existing breakpoint, the breakpoints in the target are updated immediately. A breakpoint is -removed from the target only when breakpoint itself is removed. - -@cindex non-stop mode, and @code{breakpoint always-inserted} -@item set breakpoint always-inserted auto -This is the default mode. If @value{GDBN} is controlling the inferior -in non-stop mode (@pxref{Non-Stop Mode}), gdb behaves as if -@code{breakpoint always-inserted} mode is on. If @value{GDBN} is -controlling the inferior in all-stop mode, @value{GDBN} behaves as if -@code{breakpoint always-inserted} mode is off. -@end table +removed from the target only when breakpoint itself is deleted. @value{GDBN} handles conditional breakpoints by evaluating these conditions when a breakpoint breaks. If the condition is true, then the process being diff --git a/gdb/infrun.c b/gdb/infrun.c index c18267f..3725f2d 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3448,8 +3448,7 @@ handle_inferior_event (struct execution_control_state *ecs) { /* Loading of shared libraries might have changed breakpoint addresses. Make sure new breakpoints are inserted. */ - if (stop_soon == NO_STOP_QUIETLY - && !breakpoints_always_inserted_mode ()) + if (stop_soon == NO_STOP_QUIETLY) insert_breakpoints (); resume (0, GDB_SIGNAL_0); prepare_to_wait (ecs); @@ -6110,7 +6109,7 @@ normal_stop (void) printf_filtered (_("No unwaited-for children left.\n")); } - if (!breakpoints_always_inserted_mode () && target_has_execution) + if (!breakpoints_should_be_inserted_now () && target_has_execution) { if (remove_breakpoints ()) { diff --git a/gdb/remote.c b/gdb/remote.c index 357e9f2..41ea012 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -3604,9 +3604,8 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p) up. */ rs->starting_up = 0; - /* If breakpoints are global, insert them now. */ - if (gdbarch_has_global_breakpoints (target_gdbarch ()) - && breakpoints_always_inserted_mode ()) + /* Maybe breakpoints are global and need to be inserted now. */ + if (breakpoints_should_be_inserted_now ()) insert_breakpoints (); } diff --git a/gdb/testsuite/gdb.threads/break-while-running.c b/gdb/testsuite/gdb.threads/break-while-running.c new file mode 100644 index 0000000..b900996 --- /dev/null +++ b/gdb/testsuite/gdb.threads/break-while-running.c @@ -0,0 +1,51 @@ +/* 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 . */ + +#include +#include +#include +#include +#include + + +volatile unsigned int counter = 1; + +void * +child_function (void *arg) +{ + while (counter > 0) + { + counter++; /* breakpoint A child here */ + usleep (1); /* breakpoint B child here */ + } + + pthread_exit (NULL); +} + +int +main (void) +{ + pthread_t child_thread; + int res; + + res = pthread_create (&child_thread, NULL, child_function, NULL); + assert (res == 0); + + pthread_join (child_thread, NULL); + + exit (EXIT_SUCCESS); +} diff --git a/gdb/testsuite/gdb.threads/break-while-running.exp b/gdb/testsuite/gdb.threads/break-while-running.exp new file mode 100644 index 0000000..163c0ec --- /dev/null +++ b/gdb/testsuite/gdb.threads/break-while-running.exp @@ -0,0 +1,87 @@ +# Copyright (C) 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 . + +# Test that setting a breakpoint while a thread is running results in +# the breakpoint being inserted. + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} { + return -1 +} + +# The test proper. + +proc test {} { + global srcfile binfile + global gdb_prompt + global decimal + + clean_restart $binfile + + if ![runto_main] { + continue + } + + # Check whether we're testing with the remote or extended-remote + # targets. If so, skip the tests, as with the RSP, we can't issue + # commands until the target replies to vCont. + if [gdb_is_target_remote] { + return -1 + } + + gdb_breakpoint [gdb_get_line_number "breakpoint A child here"] + gdb_continue_to_breakpoint "run to child breakpoint" + gdb_test "info threads" "\\\* 2 .* 1.*" "info threads shows all threads" + + delete_breakpoints + + # Leave one thread stopped, so GDB + gdb_test_no_output "set scheduler-locking on" + + gdb_test "continue &" "Continuing\." + + # Switch to a stopped thread, so GDB can poke at memory freely. + gdb_test "thread 1" "Switching to .*" "switch to stopped thread" + + gdb_test "info threads" "2.*running.*\\\* 1.*" "info threads shows running thread" + + set linenum [gdb_get_line_number "breakpoint B child here"] + + # Don't use gdb_test as it's racy in this case -- gdb_test matches + # the prompt with an end anchor. Sometimes expect will manage to + # read the breakpoint hit output while still processing this test, + # defeating the anchor. + set test "set breakpoint while a thread is running" + gdb_test_multiple "break $srcfile:$linenum" $test { + -re "Breakpoint $decimal at .*: file .*$srcfile, line $linenum.\r\n$gdb_prompt " { + pass $test + } + -re "$gdb_prompt " { + fail $test + } + } + + # Check that the breakpoint is hit. Can't use gdb_test here, as + # no prompt is expected to come out. + set test "breakpoint is hit" + gdb_test_multiple "" $test { + -re "Breakpoint .*, child_function.*break-while-running.c:$linenum.*breakpoint B child here.*" { + pass $test + } + } +} + +test