From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17736 invoked by alias); 7 May 2013 17:11:22 -0000 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 Received: (qmail 17727 invoked by uid 89); 7 May 2013 17:11:21 -0000 X-Spam-SWARE-Status: No, score=-8.4 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 07 May 2013 17:11:20 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r47HBHM8007727 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 7 May 2013 13:11:17 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r47HBFVD016979; Tue, 7 May 2013 13:11:16 -0400 Message-ID: <518935B3.5020101@redhat.com> Date: Tue, 07 May 2013 17:11:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Philippe Waroquiers CC: gdb-patches@sourceware.org Subject: Re: RFC: gdb_assert with 'catch signal ...' and fork References: <1367620645.2243.49.camel@soleil> <1367621018.14613.2.camel@soleil> In-Reply-To: <1367621018.14613.2.camel@soleil> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00249.txt.bz2 Hi Philippe, Thanks for working on this. On 05/03/2013 11:37 PM, Philippe Waroquiers wrote: > gdb 7.6 'catch signal' interacts badly with fork, causing a gdb_assert > such as > break-catch-sig.c:152: internal-error: signal_catchpoint_remove_location: Assertion `signal_catch_counts[iter] > 0' failed. > A problem internal to GDB has been detected, > further debugging may prove unreliable. > Quit this debugging session? (y or n) > > This can be reproduced with a (patched) gdb.base/catch-signal.c. > The problem is that the signal_catch_counts is decremented > by detach_breakpoints. > The patch below modifies catch-signal.c, catch-signal.exp > and modifies breakpoint.c so as to avoid the assert. > > 2 questions: > 1. The fix is based on the assumption that detach_breakpoints > only has to detach breakpoints and watchpoints, > but not catchpoints (see breakpoints.h comments, > that do not fully match the implementation which > e.g. also detach the single step breakpoints). > Unclear to me if this is the good fix ? > (the fix causes no regtest failure) Unfortunately not. A catchpoint can be a breakpoint behind the scenes. See break-catch-throw.c: /* We need to reset 'type' in order for code in breakpoint.c to do the right thing. */ cp->base.type = bp_breakpoint; (I don't know exactly why this is needed though.) It's better to instead look at the location type, and skip bp_loc_other locations. See init_bp_location and: enum bp_loc_type { bp_loc_software_breakpoint, bp_loc_hardware_breakpoint, bp_loc_hardware_watchpoint, bp_loc_other /* Miscellaneous... */ }; I don't think any current catchpoint type needs to be detached per inferior, so I think that'll be sufficient. > 2. when running the modified catch-signal test with an > unpatched gdb, the test fails but gdb.log does not contain > the assert. Rather, the test fails with 'the program exited'. > When doing the same commands manually, it shows the gdb assert. > No idea why this difference of behaviour. I tried it, and the test doesn't trigger a gdb_assert for me. Instead, the program exists, as when running it manually: Running ../../../src/gdb/testsuite/gdb.base/catch-signal.exp ... FAIL: gdb.base/catch-signal.exp: SIGHUP: got SIGHUP after fork (the program exited) FAIL: gdb.base/catch-signal.exp: 1: got SIGHUP after fork (the program exited) FAIL: gdb.base/catch-signal.exp: SIGHUP SIGUSR2: got SIGHUP after fork (the program exited) You must have had the test do a "next" or "step" or some such instead of a "continue" at some point, because that's one way I can trigger the assert: main () at ../../../src/gdb/testsuite/gdb.base/foll-fork.c:23 23 int v = 5; (gdb) n 25 pid = fork (); (gdb) catch signal SIGINT Catchpoint 2 (signal SIGINT) (gdb) n ../../src/gdb/break-catch-sig.c:152: internal-error: signal_catchpoint_remove_location: Assertion `signal_catch_counts[iter] > 0' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) > Index: gdb/testsuite/gdb.base/catch-signal.c > =================================================================== > RCS file: /cvs/src/src/gdb/testsuite/gdb.base/catch-signal.c,v > retrieving revision 1.3 > diff -u -p -r1.3 catch-signal.c > --- gdb/testsuite/gdb.base/catch-signal.c 3 May 2013 19:16:56 -0000 1.3 > +++ gdb/testsuite/gdb.base/catch-signal.c 3 May 2013 22:39:48 -0000 > @@ -15,6 +15,7 @@ > You should have received a copy of the GNU General Public License > along with this program. If not, see . */ > > +#include > #include > #include > > @@ -44,5 +45,17 @@ main () > raise (SIGHUP); /* fourth HUP */ > > raise (SIGINT); /* first INT */ > + > + signal (SIGCHLD, handle); > + switch (fork()) /* fork marker */ > + { > + case -1: > + perror ("fork"); > + exit (1); > + case 0: > + exit (0); > + } > + (void) wait(NULL); > + raise (SIGHUP); /* fifth HUP */ > } "catch signal" works on all targets, but some targets, like mingw32 don't have 'fork', so this will make the test not compile there, losing test coverage. The easy way out is to split this to a new catch-signal-fork.c|exp test. > > Index: gdb/testsuite/gdb.base/catch-signal.exp > =================================================================== > RCS file: /cvs/src/src/gdb/testsuite/gdb.base/catch-signal.exp,v > retrieving revision 1.4 > diff -u -p -r1.4 catch-signal.exp > --- gdb/testsuite/gdb.base/catch-signal.exp 3 May 2013 19:16:56 -0000 1.4 > +++ gdb/testsuite/gdb.base/catch-signal.exp 3 May 2013 22:39:48 -0000 > @@ -83,7 +83,16 @@ proc test_catch_signal {signame} { > y > gdb_test "catch signal SIGINT" "Catchpoint .*" > gdb_test "continue" "Catchpoint .* SIGINT.*" > + > + delete_breakpoints > > + # Test interaction with fork. This used to cause a gdb_assert. As above, this set of steps does not cause the gdb_assert, so best remove or redo this comment. Also, two spaces after period. > + gdb_breakpoint ${srcfile}:[gdb_get_line_number "fork marker"] > + gdb_continue_to_breakpoint "fork marker" > + gdb_test "handle SIGHUP nostop noprint pass" \ > + "SIGHUP.*No.*No.*Yes.*" > + gdb_test "catch signal SIGHUP" "Catchpoint .*" > + gdb_test "continue" "Catchpoint .* SIGHUP.*" "got SIGHUP after fork" > } > } -- Pedro Alves