From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27115 invoked by alias); 28 Apr 2011 15:18:49 -0000 Received: (qmail 27101 invoked by uid 22791); 28 Apr 2011 15:18:46 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate2.uk.ibm.com (HELO mtagate2.uk.ibm.com) (194.196.100.162) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 28 Apr 2011 15:18:28 +0000 Received: from d06nrmr1507.portsmouth.uk.ibm.com (d06nrmr1507.portsmouth.uk.ibm.com [9.149.38.233]) by mtagate2.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p3SFIH7n002352 for ; Thu, 28 Apr 2011 15:18:17 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1507.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p3SFJM1l1753316 for ; Thu, 28 Apr 2011 16:19:22 +0100 Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p3SFIHuk020014 for ; Thu, 28 Apr 2011 09:18:17 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p3SFIGBM019969; Thu, 28 Apr 2011 09:18:16 -0600 Message-Id: <201104281518.p3SFIGBM019969@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 28 Apr 2011 17:18:16 +0200 Subject: [patch v2] Re: [commit] Re: [rfc][1/2] Signal delivery + software single-step is broken To: pedro@codesourcery.com (Pedro Alves) Date: Thu, 28 Apr 2011 15:18:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <201104281301.04426.pedro@codesourcery.com> from "Pedro Alves" at Apr 28, 2011 01:01:04 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-04/txt/msg00543.txt.bz2 Pedro Alves wrote: > (I wish there was no need to undo stuff, and do the decision > before actually inserting sss breakpoints, but I see why you > did it. We need to call gdbarch_software_single_step to > know whether sss breakpoints will be inserted, which, actually > inserts them.) Fully agreed ... > I'm trying to think whether this works okay with nested > signals. [...] > Since you'll already have one step-resume breakpoint set (back in the > main code), and you can't set another -- you'd hit the > assert in insert_step_resume_breakpoint_at_sal trying to insert > a second one. Hmm, good point. Indeed I was able to trigger that assert. However, this is easily fixable just the same as in handle_inferior_event: if we already have a step-resume breakpoint, we don't need to insert another one, but just continue running until we hit the original one. Fixed in the patch below, together with a test case that triggers the assert with the original patch. Retested on armv7l-unknown-linux-gnueabi and i386-linux. Does this look OK to you? Thanks, Ulrich ChangeLog: gdb/ * infrun.c (proceed): Revert previous change. (resume): Instead, handle the case of signal delivery while stepping off a breakpoint location here, and only if software single-stepping is used. Handle nested signals. gdb/testsuite/ * gdb.base/signest.exp: New file. * gdb.base/signest.c: Likewise. Index: gdb/infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.476 diff -u -p -r1.476 infrun.c --- gdb/infrun.c 27 Apr 2011 17:08:41 -0000 1.476 +++ gdb/infrun.c 28 Apr 2011 14:24:44 -0000 @@ -1703,6 +1703,51 @@ a command like `return' or `jump' to con else if (step) step = maybe_software_singlestep (gdbarch, pc); + /* Currently, our software single-step implementation leads to different + results than hardware single-stepping in one situation: when stepping + into delivering a signal which has an associated signal handler, + hardware single-step will stop at the first instruction of the handler, + while software single-step will simply skip execution of the handler. + + For now, this difference in behavior is accepted since there is no + easy way to actually implement single-stepping into a signal handler + without kernel support. + + However, there is one scenario where this difference leads to follow-on + problems: if we're stepping off a breakpoint by removing all breakpoints + and then single-stepping. In this case, the software single-step + behavior means that even if there is a *breakpoint* in the signal + handler, GDB still would not stop. + + Fortunately, we can at least fix this particular issue. We detect + here the case where we are about to deliver a signal while software + single-stepping with breakpoints removed. In this situation, we + revert the decisions to remove all breakpoints and insert single- + step breakpoints, and instead we install a step-resume breakpoint + at the current address, deliver the signal without stepping, and + once we arrive back at the step-resume breakpoint, actually step + over the breakpoint we originally wanted to step over. */ + if (singlestep_breakpoints_inserted_p + && tp->control.trap_expected && sig != TARGET_SIGNAL_0) + { + /* If we have nested signals or a pending signal is delivered + immediately after a handler returns, might might already have + a step-resume breakpoint set on the earlier handler. We cannot + set another step-resume breakpoint; just continue on until the + original breakpoint is hit. */ + if (tp->control.step_resume_breakpoint == NULL) + { + insert_step_resume_breakpoint_at_frame (get_current_frame ()); + tp->step_after_step_resume_breakpoint = 1; + } + + remove_single_step_breakpoints (); + singlestep_breakpoints_inserted_p = 0; + + insert_breakpoints (); + tp->control.trap_expected = 0; + } + if (should_resume) { ptid_t resume_ptid; @@ -2064,6 +2109,24 @@ proceed (CORE_ADDR addr, enum target_sig /* prepare_to_proceed may change the current thread. */ tp = inferior_thread (); + if (oneproc) + { + tp->control.trap_expected = 1; + /* If displaced stepping is enabled, we can step over the + breakpoint without hitting it, so leave all breakpoints + inserted. Otherwise we need to disable all breakpoints, step + one instruction, and then re-add them when that step is + finished. */ + if (!use_displaced_stepping (gdbarch)) + remove_breakpoints (); + } + + /* We can insert breakpoints if we're not trying to step over one, + or if we are stepping over one but we're using displaced stepping + to do so. */ + if (! tp->control.trap_expected || use_displaced_stepping (gdbarch)) + insert_breakpoints (); + if (!non_stop) { /* Pass the last stop signal to the thread we're resuming, @@ -2133,42 +2196,6 @@ proceed (CORE_ADDR addr, enum target_sig /* Reset to normal state. */ init_infwait_state (); - /* Stepping over a breakpoint while at the same time delivering a signal - has a problem: we cannot use displaced stepping, but we also cannot - use software single-stepping, because we do not know where execution - will continue if a signal handler is installed. - - On the other hand, if there is a signal handler we'd have to step - over it anyway. So what we do instead is to install a step-resume - handler at the current address right away, deliver the signal without - stepping, and once we arrive back at the step-resume breakpoint, step - once more over the original breakpoint we wanted to step over. */ - if (oneproc && tp->suspend.stop_signal != TARGET_SIGNAL_0 - && execution_direction != EXEC_REVERSE) - { - insert_step_resume_breakpoint_at_frame (get_current_frame ()); - tp->step_after_step_resume_breakpoint = 1; - oneproc = 0; - } - - if (oneproc) - { - tp->control.trap_expected = 1; - /* If displaced stepping is enabled, we can step over the - breakpoint without hitting it, so leave all breakpoints - inserted. Otherwise we need to disable all breakpoints, step - one instruction, and then re-add them when that step is - finished. */ - if (!use_displaced_stepping (gdbarch)) - remove_breakpoints (); - } - - /* We can insert breakpoints if we're not trying to step over one, - or if we are stepping over one but we're using displaced stepping - to do so. */ - if (! tp->control.trap_expected || use_displaced_stepping (gdbarch)) - insert_breakpoints (); - /* Resume inferior. */ resume (oneproc || step || bpstat_should_step (), tp->suspend.stop_signal); --- /dev/null 2011-04-28 13:38:53.488264001 +0200 +++ gdb/testsuite/gdb.base/signest.exp 2011-04-28 16:15:16.000000000 +0200 @@ -0,0 +1,67 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2011 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 . + +set testfile "signest" +set srcfile ${testfile}.c + +if [target_info exists gdb,nosignals] { + verbose "Skipping ${testfile}.exp because of nosignals." + return -1 +} + +if [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug}] { + untested ${testfile}.exp + return -1 +} + +if ![runto_main] then { + untested ${testfile}.exp + return -1 +} + +# If we can examine what's at memory address 0, it is possible that we +# could also execute it. This could probably make us run away, +# executing random code, which could have all sorts of ill effects, +# especially on targets without an MMU. Don't run the tests in that +# case. + +gdb_test_multiple "x 0" "memory at address 0" { + -re "0x0:.*Cannot access memory at address 0x0.*$gdb_prompt $" { } + -re "0x0:.*Error accessing memory address 0x0.*$gdb_prompt $" { } + -re ".*$gdb_prompt $" { + untested "Memory at address 0 is possibly executable" + return -1 + } +} + +# Run until we hit the SIGSEGV (or SIGBUS on some platforms). +gdb_test "continue" \ + ".*Program received signal (SIGBUS|SIGSEGV).*bowler.*" \ + "continue to fault" + +# Insert conditional breakpoint at faulting instruction +gdb_test "break if 0" ".*" "set conditional breakpoint" + +# Set SIGSEGV/SIGBUS to pass+nostop +gdb_test "handle SIGSEGV nostop print pass" ".*" "pass SIGSEGV" +gdb_test "handle SIGBUS nostop print pass" ".*" "pass SIGBUS" + +# Step off the faulting instruction into the handler, triggering nested faults +gdb_test "continue" \ + ".*Program received signal (SIGBUS|SIGSEGV).*Program received signal (SIGBUS|SIGSEGV).*exited normally.*" \ + "run through nested faults" + --- /dev/null 2011-04-28 13:38:53.488264001 +0200 +++ gdb/testsuite/gdb.base/signest.c 2011-04-28 16:04:52.000000000 +0200 @@ -0,0 +1,53 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2011 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 + +volatile char *p = NULL; + +extern long +bowler (void) +{ + return *p; +} + +extern void +keeper (int sig) +{ + static int recurse = 0; + if (++recurse < 3) + bowler (); + + _exit (0); +} + +int +main (void) +{ + struct sigaction act; + memset (&act, 0, sizeof act); + act.sa_handler = keeper; + act.sa_flags = SA_NODEFER; + sigaction (SIGSEGV, &act, NULL); + sigaction (SIGBUS, &act, NULL); + + bowler (); + return 0; +} -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com