From patchwork Mon Oct 27 19:44:22 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 3434 Received: (qmail 31084 invoked by alias); 27 Oct 2014 19:44:31 -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 31071 invoked by uid 89); 27 Oct 2014 19:44:30 -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; Mon, 27 Oct 2014 19:44:28 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9RJiPmX017191 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 27 Oct 2014 15:44:25 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9RJiNgi002516; Mon, 27 Oct 2014 15:44:24 -0400 Message-ID: <544EA096.5050803@redhat.com> Date: Mon, 27 Oct 2014 19:44:22 +0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Eli Zaretskii CC: gdb-patches@sourceware.org Subject: Re: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives References: <1413308910-30423-1-git-send-email-palves@redhat.com> <83ppdu5wx7.fsf@gnu.org> <543D7044.2000703@redhat.com> <83oate5uec.fsf@gnu.org> <543E7961.5090002@redhat.com> <838ukh5rry.fsf@gnu.org> <544BAD08.1050601@redhat.com> <83tx2p2z12.fsf@gnu.org> In-Reply-To: <83tx2p2z12.fsf@gnu.org> On 10/27/2014 05:38 PM, Eli Zaretskii wrote: >> Date: Sat, 25 Oct 2014 15:00:40 +0100 >> From: Pedro Alves > Thanks, this is a huge improvement. I have only a couple of minor > stylistic suggestions: > Thanks. >> +@cindex stepping and signal handlers >> +@anchor{stepping and signal handlers} >> + >> +@value{GDBN} optimizes for stepping the mainline code. If a signal >> +that has @code{handle nostop} and @code{handle pass} set arrives while >> +a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is >> +in progress, @value{GDBN} lets the signal handler run and then resumes >> +stepping the mainline code once the signal handler returns. In other >> +words, @value{GDBN} steps over the signal handler. If the signal has >> +@code{handle noprint} set, then you won't even hear about it. This >> +prevents signals that you've specified as not interesting (with > > I would suggest to use a semi-colon, not a period, before the last > "This". That's because the last sentence is logically an immediate > continuation of the one before it. By putting a full stop between > them we create a potential for misunderstanding to what "this" refers, > since the previous text described 2 different situations. Using a > semi-colon removes that danger. Hmm, I don't think I agree with that. I'm not really trying to describ 2 situations. The "this" refers to GDB stepping over the signal handler. The noprint issue is secondary here, and I guess an example shows better what I'm talking about: (gdb) list 28 28 p = 0; 29 p = 0; 30 p = 0; (gdb) n 28 p = 0; (gdb) info inferiors Num Description Executable * 1 process 25468 /home/pedro/gdb/tests/signal (gdb) shell kill -SIGUSR1 25468 (gdb) handle SIGUSR1 nostop pass print Signal Stop Print Pass to program Description SIGUSR1 No Yes Yes User defined signal 1 (gdb) si Program received signal SIGUSR1, User defined signal 1. 29 p = 0; (gdb) That stepped over the signal handler, from line 28 to 29, but we still heard about the signal. Vs: (gdb) 28 p = 0; (gdb) handle SIGUSR1 nostop noprint pass Signal Stop Print Pass to program Description SIGUSR1 No No Yes User defined signal 1 (gdb) info inferiors Num Description Executable * 1 process 25484 /home/pedro/gdb/tests/signal (gdb) shell kill -SIGUSR1 25484 (gdb) si 29 p = 0; (gdb) That stepped over the signal handler, from line 28 to 29, and we didn't even hear about the signal. So, perhaps this variant is clearer? @value{GDBN} optimizes for stepping the mainline code. If a signal that has @code{handle nostop} and @code{handle pass} set arrives while a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is in progress, @value{GDBN} lets the signal handler run and then resumes stepping the mainline code once the signal handler returns. In other words, @value{GDBN} steps over the signal handler. This prevents signals that you've specified as not interesting (with @code{handle nostop}) from changing the focus of debugging unexpectedly. Note that the signal handler itself may still hit a breakpoint, stop for another signal that has @code{handle stop} in effect, or for any other event that normally results in stopping the stepping command sooner. Also note that @value{GDBN} still informs you that the program received a signal if @code{handle print} is set. > > For the same reason, it might be better to make "If the signal has > 'handle noprint' ..." start a new paragraph. Yes, though perhaps a new paragraph is unnecessary. See above. > >> +@cindex stepping into signal handlers >> +@anchor{stepping into signal handlers} > > I would remove this @cindex entry: it doesn't add anything useful to > the previous one, and will likely point to the same page. I'd prefer to keep it, if you don't mind. The queue-signal reference wants to point here directly, and I can imagine the text above expanding in directions not relevant for that cross reference. I'd like to have a place where I can point at when the topic of wanting to debug a handler without knowing exactly which function that is comes up. > >> +If the program was stopped for a signal (that is, stopped before the >> +program sees it), due to @code{handle stop} being set, and >> +@code{handle pass} is in effect for that signal too, and your program >> +handles the signal, a stepping command such as for example >> +@code{stepi} or @code{step} steps @emph{into} the signal's handler (if >> +the target supports it). > > This is a mouthful, not in the least because of excessive use of past > tense. How about this variant instead: > > If you set @code{handle stop} for a signal, @value{GDBN} stops your > program and announces the signal when it arrives, before the program > sees it. I think this part ends up being redundant with what is already said further above, around: "When a signal stops your program, the signal is not visible to the program until you continue. Your program sees the signal then, if @code{pass} is in effect for the signal in question @emph{at that time}. " I'm now thinking that we can just remove that part, and use the rest of your paragraph below: > If you also set @code{handle pass} for that signal, and > your program sets up a handler for it, then issuing a stepping > command, such as @code{step} or @code{stepi}, when your program is > stopped due to the signal will step @emph{into} the signal handler > (if the target supports that). Sounds clear enough to me (with a minor tweak to make it stand on its own). > >> +Likewise, if the @code{queue-signal} command was used to queue a >> +signal to be delivered to the current thread when execution of the > > Please reword in active tens ("... if you use the @code{queue-signal} > command to queue ..."). > Done. >> +thread resumes (@pxref{Signaling, ,Giving your Program a Signal}), >> +then a stepping command steps into the signal's handler. > > Not sure I understand the sequence here. First, I queue-signal, then > the signal is delivered and the thread stops, and _then_ I issue si? > I guess the "when execution of the thread resumes" confused me. Sounds like you're thinking of "queue-signal" like "kill" from the shell, but that's not how "queue-signal" works. "queue-signal" instead passes the signal to the program immediately as if the thread had _already_ stopped for the signal. GDB doesn't intercept the signal anymore. I've added a queue-signal example. Hopefully that makes things clearer. Let me know how this version looks. From fd292ad2a1c10ec5a4f902535c7d3c29d9cbc1e1 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 27 Oct 2014 19:27:10 +0000 Subject: [PATCH] stepi/nexti: skip signal handler if "handle nostop" signal arrives I noticed that "si" behaves differently when a "handle nostop" signal arrives while the step is in progress, depending on whether the program was stopped at a breakpoint when "si" was entered. Specifically, in case GDB needs to step off a breakpoint, the handler is skipped and the program stops in the next "mainline" instruction. Otherwise, the "si" stops in the first instruction of the signal handler. I was surprised the testsuite doesn't catch this difference. Turns out gdb.base/sigstep.exp covers a bunch of cases related to stepping and signal handlers, but does not test stepi nor nexti, only step/next/continue. My first reaction was that stopping in the signal handler was the correct thing to do, as it's where the next user-visible instruction that is executed is. I considered then "nexti" -- a signal handler could be reasonably considered a subroutine call to step over, it'd seem intuitive to me that "nexti" would skip it. But then, I realized that signals that arrive while a plain/line "step" is in progress _also_ have their handler skipped. A user might well be excused for being confused by this, given: (gdb) help step Step program until it reaches a different source line. And the signal handler's sources will be in different source lines, after all. I think that having to explain that "stepi" steps into handlers, (and that "nexti" wouldn't according to my reasoning above), while "step" does not, is a sign of an awkward interface. E.g., if a user truly is interested in stepping into signal handlers, then it's odd that she has to either force the signal to "handle stop", or recall to do "stepi" whenever such a signal might be delivered. For that use case, it'd seem nicer to me if "step" also stepped into handlers. This suggests to me that we either need a global "step-into-handlers" setting, or perhaps better, make "handle pass/nopass stop/nostop print/noprint" have have an additional axis - "handle stepinto/nostepinto", so that the user could configure whether handlers for specific signals should be stepped into. In any case, I think it's simpler (and thus better) for all step commands to behave the same. This commit thus makes "si/ni" skip handlers for "handle nostop" signals that arrive while the command was already in progress, like step/next do. To be clear, nothing changes if the program was stopped for a signal, and the user enters a stepping command _then_ -- GDB still steps into the handler. The change concerns signals that don't cause a stop and that arrive while the step is in progress. Tested on x86_64 Fedora 20, native and gdbserver. gdb/ 2014-10-27 Pedro Alves * infrun.c (handle_signal_stop): Also skip handlers when a random signal arrives while handling a "stepi" or a "nexti". Set the thread's 'step_after_step_resume_breakpoint' flag. gdb/doc/ 2014-10-27 Pedro Alves * gdb.texinfo (Continuing and Stepping): Add cross reference to info on stepping and signal handlers. (Signals): Explain stepping and signal handlers. Add context index entries, and cross references. gdb/testsuite/ 2014-10-27 Pedro Alves * gdb.base/sigstep.c (dummy): New global. (main): Issue a couple writes to the new global. * gdb.base/sigstep.exp (get_next_pc, test_skip_handler): New procedures. (skip_over_handler): Use test_skip_handler. (top level): Call skip_over_handler for stepi and nexti too. (breakpoint_over_handler): Use test_skip_handler. (top level): Call breakpoint_over_handler for stepi and nexti too. --- gdb/doc/gdb.texinfo | 68 +++++++++++++++++++++++++++++++++++++- gdb/infrun.c | 7 ++-- gdb/testsuite/gdb.base/sigstep.c | 7 ++-- gdb/testsuite/gdb.base/sigstep.exp | 55 ++++++++++++++++++++++-------- 4 files changed, 118 insertions(+), 19 deletions(-) diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index a1b8ac7..9b32217 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -5079,7 +5079,9 @@ line of source code, or one machine instruction (depending on what particular command you use). Either when continuing or when stepping, your program may stop even sooner, due to a breakpoint or a signal. (If it stops due to a signal, you may want to use @code{handle}, or use -@samp{signal 0} to resume execution. @xref{Signals, ,Signals}.) +@samp{signal 0} to resume execution (@pxref{Signals, ,Signals}), +or you may step into the signal's handler (@pxref{stepping and signal +handlers}).) @table @code @kindex continue @@ -5573,6 +5575,67 @@ a result of the fatal signal once it saw the signal. To prevent this, you can continue with @samp{signal 0}. @xref{Signaling, ,Giving your Program a Signal}. +@cindex stepping and signal handlers +@anchor{stepping and signal handlers} + +@value{GDBN} optimizes for stepping the mainline code. If a signal +that has @code{handle nostop} and @code{handle pass} set arrives while +a stepping command (e.g., @code{stepi}, @code{step}, @code{next}) is +in progress, @value{GDBN} lets the signal handler run and then resumes +stepping the mainline code once the signal handler returns. In other +words, @value{GDBN} steps over the signal handler. This prevents +signals that you've specified as not interesting (with @code{handle +nostop}) from changing the focus of debugging unexpectedly. Note that +the signal handler itself may still hit a breakpoint, stop for another +signal that has @code{handle stop} in effect, or for any other event +that normally results in stopping the stepping command sooner. Also +note that @value{GDBN} still informs you that the program received a +signal if @code{handle print} is set. + +@cindex stepping into signal handlers +@anchor{stepping into signal handlers} + +If you set @code{handle pass} for a signal, and your program sets up a +handler for it, then issuing a stepping command, such as @code{step} +or @code{stepi}, when your program is stopped due to the signal will +step @emph{into} the signal handler (if the target supports that). + +Likewise, if you use the @code{queue-signal} command to queue a signal +to be delivered to the current thread when execution of the thread +resumes (@pxref{Signaling, ,Giving your Program a Signal}), then a +stepping command will step into the signal handler. + +Here's an example, using @code{stepi} to step to the first instruction +of @code{SIGUSR1}'s handler: + +@smallexample +(@value{GDBP}) handle SIGUSR1 +Signal Stop Print Pass to program Description +SIGUSR1 Yes Yes Yes User defined signal 1 +(@value{GDBP}) c +Continuing. + +Program received signal SIGUSR1, User defined signal 1. +main () sigusr1.c:28 +28 p = 0; +(@value{GDBP}) si +sigusr1_handler () at sigusr1.c:9 +9 @{ +@end smallexample + +The same, but using @code{queue-signal} instead of waiting for the +program to receive the signal first: + +@smallexample +(@value{GDBP}) n +28 p = 0; +(@value{GDBP}) queue-signal SIGUSR1 +(@value{GDBP}) si +sigusr1_handler () at sigusr1.c:9 +9 @{ +(@value{GDBP}) +@end smallexample + @cindex extra signal information @anchor{extra signal information} @@ -16654,6 +16717,9 @@ be used to pass a signal whose handling state has been set to @code{nopass} @end table @c @end group +@xref{stepping into signal handlers}, for information on how stepping +commands behave when the thread has a signal queued. + @node Returning @section Returning from a Function diff --git a/gdb/infrun.c b/gdb/infrun.c index 90a3123..df053e2 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4463,9 +4463,9 @@ handle_signal_stop (struct execution_control_state *ecs) return; } - if (ecs->event_thread->control.step_range_end != 0 - && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0 - && pc_in_thread_step_range (stop_pc, ecs->event_thread) + if (ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0 + && (pc_in_thread_step_range (stop_pc, ecs->event_thread) + || ecs->event_thread->control.step_range_end == 1) && frame_id_eq (get_stack_frame_id (frame), ecs->event_thread->control.step_stack_frame_id) && ecs->event_thread->control.step_resume_breakpoint == NULL) @@ -4485,6 +4485,7 @@ handle_signal_stop (struct execution_control_state *ecs) "single-step range\n"); insert_hp_step_resume_breakpoint_at_frame (frame); + ecs->event_thread->step_after_step_resume_breakpoint = 1; /* Reset trap_expected to ensure breakpoints are re-inserted. */ ecs->event_thread->control.trap_expected = 0; keep_going (ecs); diff --git a/gdb/testsuite/gdb.base/sigstep.c b/gdb/testsuite/gdb.base/sigstep.c index aa2384a..cc69184 100644 --- a/gdb/testsuite/gdb.base/sigstep.c +++ b/gdb/testsuite/gdb.base/sigstep.c @@ -24,6 +24,7 @@ #include static volatile int done; +static volatile int dummy; static void handler (int sig) @@ -74,8 +75,10 @@ main () return 1; } } - /* Wait. */ - while (!done); + /* Wait. Issue a couple writes to a dummy volatile var to be + reasonably sure our simple "get-next-pc" logic doesn't + stumble on branches. */ + dummy = 0; dummy = 0; while (!done); done = 0; } return 0; diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp index 184d46e..53152b8 100644 --- a/gdb/testsuite/gdb.base/sigstep.exp +++ b/gdb/testsuite/gdb.base/sigstep.exp @@ -269,9 +269,38 @@ proc skip_to_handler_entry { i } { gdb_test "clear *handler" ".*" "$prefix; clear handler" } -skip_to_handler_entry step -skip_to_handler_entry next -skip_to_handler_entry continue +foreach cmd {"stepi" "nexti" "step" "next" "continue"} { + skip_to_handler_entry $cmd +} + +# Get the address of where a single-step should land. + +proc get_next_pc {test} { + global gdb_prompt + global hex + + set next "" + gdb_test_multiple "x/2i \$pc" $test { + -re "$hex .*:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" { + set next $expect_out(1,string) + pass $test + } + } + + return $next +} + +# Test that the command skipped over the handler. + +proc test_skip_handler {prefix i} { + if {$i == "stepi" || $i == "nexti"} { + set next_pc [get_next_pc "$prefix; get next PC"] + gdb_test "$i" "dummy = 0.*" "$prefix; performing $i" + gdb_test "p /x \$pc" " = $next_pc" "$prefix; advanced" + } else { + gdb_test "$i" "done = 0.*" "$prefix; performing $i" + } +} # Try stepping when there's a signal pending but no breakpoints. # Should skip the handler advancing to the next line. @@ -295,13 +324,13 @@ proc skip_over_handler { i } { # Make the signal pending sleep 1 - - gdb_test "$i" "done = 0.*" "$prefix; performing $i" + + test_skip_handler $prefix $i } -skip_over_handler step -skip_over_handler next -skip_over_handler continue +foreach cmd {"stepi" "nexti" "step" "next" "continue"} { + skip_over_handler $cmd +} # Try stepping when there's a signal pending, a pre-existing # breakpoint at the current instruction, and a breakpoint in the @@ -385,7 +414,7 @@ breakpoint_to_handler_entry continue # Try stepping when there's a signal pending, and a pre-existing # breakpoint at the current instruction, and no breakpoint in the -# handler. Should advance to the next line. +# handler. Should advance to the next line/instruction. proc breakpoint_over_handler { i } { global gdb_prompt @@ -409,10 +438,10 @@ proc breakpoint_over_handler { i } { # Make the signal pending sleep 1 - gdb_test "$i" "done = 0.*" "$prefix; performing $i" + test_skip_handler $prefix $i gdb_test "clear $infinite_loop" ".*" "$prefix; clear infinite loop" } -breakpoint_over_handler step -breakpoint_over_handler next -breakpoint_over_handler continue +foreach cmd {"stepi" "nexti" "step" "next" "continue"} { + breakpoint_over_handler $cmd +}