From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59095 invoked by alias); 6 Jul 2015 18:34:00 -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 59081 invoked by uid 89); 6 Jul 2015 18:34:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 06 Jul 2015 18:33:58 +0000 Received: from svr-orw-fem-04.mgc.mentorg.com ([147.34.97.41]) by relay1.mentorg.com with esmtp id 1ZCBDG-0001OW-AE from Luis_Gustavo@mentor.com ; Mon, 06 Jul 2015 11:33:54 -0700 Received: from [172.30.7.214] (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.3.224.2; Mon, 6 Jul 2015 11:33:53 -0700 Message-ID: <559ACA0F.7010306@codesourcery.com> Date: Mon, 06 Jul 2015 18:34:00 -0000 From: Luis Machado Reply-To: Luis Machado User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Pedro Alves , Subject: Re: [PATCH] Fix problems with finishing a dummy function call on simulators. References: <1433862056-18237-1-git-send-email-lgustavo@codesourcery.com> <55772797.802@redhat.com> <55805F52.20805@codesourcery.com> <55816AD5.6020605@redhat.com> <55817569.7060704@codesourcery.com> <5581798B.5080207@redhat.com> <5581D5A9.3070706@codesourcery.com> <559A998E.6010500@redhat.com> <559A9FCE.5070908@codesourcery.com> <559AA992.2020004@redhat.com> In-Reply-To: <559AA992.2020004@redhat.com> Content-Type: multipart/mixed; boundary="------------010909080709000302080002" X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00137.txt.bz2 --------------010909080709000302080002 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3773 On 07/06/2015 01:15 PM, Pedro Alves wrote: > On 07/06/2015 04:33 PM, Luis Machado wrote: > >> I'll take a look at it. I suppose this will block the branching? > > I think so, or at least the release. Broken infcalls seems > pretty nasty. > >> Then again, simply reverting this will still have bad results with some >> simulators. > > True. Might be the fix is simple though. I'm seeing this: > > (gdb) PASS: gdb.base/shlib-call.exp: step out of shr2 to main (stopped in shr2 epilogue) > step > main () at /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.base/shmain.c:37 > 37 g = mainshr1(g); > (gdb) PASS: gdb.base/shlib-call.exp: step out of shr2 epilogue to main > print mainshr1(1) > > Program received signal SIGSEGV, Segmentation fault. > mainshr1 (g=1) at /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.base/shmain.c:29 > 29 } > The program being debugged was signaled while in a function called from GDB. > GDB remains in the frame where the signal was received. > To change this behavior use "set unwindonsignal on". > Evaluation of the expression containing the function > (mainshr1) will be abandoned. > When the function is done executing, GDB will silently stop. > (gdb) FAIL: gdb.base/shlib-call.exp: print mainshr1(1) > step > > The SIGSEGV look scary until one remembers that the dummy breakpoints > are placed on the stack, which is non-executable. gdb translates those > SIGSEGVs back to SIGTRAPs, provided it knows there's a breakpoint at that > address. > > Looking a bit at breakpoint.c, I notice that a few ->permanent > checks seem to have been left behind, and as result we don't actually > remove from the target the breakpoints that were placed on top of the > permanent breakpoints? > > This seems to fix the FAILs here, but I didn't run full regression > testing. Could you take this, test it on qemu, and and finish it off? > > --- > From 9cbd03b61441072c71d2076c1deb6766fecf25d2 Mon Sep 17 00:00:00 2001 > From: Pedro Alves > Date: Mon, 6 Jul 2015 17:04:05 +0100 > Subject: [PATCH] remove left behind permanent breakpoint special casing > > --- > gdb/breakpoint.c | 11 +---------- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 1481112..af0d167 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -3892,10 +3892,6 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) > /* BL is never in moribund_locations by our callers. */ > gdb_assert (bl->owner != NULL); > > - if (bl->permanent) > - /* Permanent breakpoints cannot be inserted or removed. */ > - return 0; > - > /* The type of none suggests that owner is actually deleted. > This should not ever happen. */ > gdb_assert (bl->owner->type != bp_none); > @@ -4042,10 +4038,6 @@ remove_breakpoint (struct bp_location *bl, insertion_state_t is) > /* BL is never in moribund_locations by our callers. */ > gdb_assert (bl->owner != NULL); > > - if (bl->permanent) > - /* Permanent breakpoints cannot be inserted or removed. */ > - return 0; > - > /* The type of none suggests that owner is actually deleted. > This should not ever happen. */ > gdb_assert (bl->owner->type != bp_none); > @@ -4068,8 +4060,7 @@ mark_breakpoints_out (void) > struct bp_location *bl, **blp_tmp; > > ALL_BP_LOCATIONS (bl, blp_tmp) > - if (bl->pspace == current_program_space > - && !bl->permanent) > + if (bl->pspace == current_program_space) > bl->inserted = 0; > } > > I've confirmed it fixes the gdbserver-specific issue here and it still works correctly for other setups like native GDB and simulator-based debugging. I'll go ahead and push the following in, if it is ok. --------------010909080709000302080002 Content-Type: text/x-patch; name="0001-Fix-problems-with-finishing-a-dummy-function-call-on.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-Fix-problems-with-finishing-a-dummy-function-call-on.pa"; filename*1="tch" Content-length: 3863 >From c1bc516aae7a2293b4119f92432a73021865f538 Mon Sep 17 00:00:00 2001 From: Luis Machado Date: Mon, 6 Jul 2015 15:24:37 -0300 Subject: [PATCH] Fix problems with finishing a dummy function call on simulators. This fixes regressions introduced with the original change to not consider permanent breakpoints always inserted: 6ae8866180bf90e9ec76c2dd34c07fd826d11a83 is the first bad commit commit 6ae8866180bf90e9ec76c2dd34c07fd826d11a83 Author: Luis Machado Date: Wed Jun 17 16:50:57 2015 -0300 Fix problems with finishing a dummy function call on simulators. Some checks were mistakenly left out of the original patch, which caused the following failures: -PASS: gdb.base/shlib-call.exp: print mainshr1(1) -PASS: gdb.base/shlib-call.exp: step into mainshr1 +FAIL: gdb.base/shlib-call.exp: print mainshr1(1) +FAIL: gdb.base/shlib-call.exp: step into mainshr1 -PASS: gdb.cp/chained-calls.exp: q(p()) +FAIL: gdb.cp/chained-calls.exp: q(p()) -PASS: gdb.cp/chained-calls.exp: q(p() + r()) +FAIL: gdb.cp/chained-calls.exp: q(p() + r()) -PASS: gdb.cp/chained-calls.exp: g(f(g(f() + f())) + f()) +FAIL: gdb.cp/chained-calls.exp: g(f(g(f() + f())) + f()) -PASS: gdb.cp/chained-calls.exp: *c -PASS: gdb.cp/chained-calls.exp: *c + *c -PASS: gdb.cp/chained-calls.exp: q(*c + *c) +FAIL: gdb.cp/chained-calls.exp: *c +FAIL: gdb.cp/chained-calls.exp: *c + *c +FAIL: gdb.cp/chained-calls.exp: q(*c + *c) -PASS: gdb.cp/classes.exp: calling method for small class +FAIL: gdb.cp/classes.exp: calling method for small class The above is likely caused by GDB not removing the permanent breakpoints from the target, leading to the inferior executing the breakpoint instruction and tripping on a SIGSEGV. 2015-07-06 Luis Machado * breakpoint.c (remove_breakpoint_1): Don't handle permanent breakpoints in a special way. (remove_breakpoint): Likewise. (mark_breakpoints_out): Likewise. --- gdb/ChangeLog | 7 +++++++ gdb/breakpoint.c | 11 +---------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4636653..5688b4a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2015-07-06 Luis Machado + + * breakpoint.c (remove_breakpoint_1): Don't handle permanent + breakpoints in a special way. + (remove_breakpoint): Likewise. + (mark_breakpoints_out): Likewise. + 2015-07-06 Andrew Burgess * tui/tui-data.c (tui_partial_win_by_name): Window name is const. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 1481112..af0d167 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -3892,10 +3892,6 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) /* BL is never in moribund_locations by our callers. */ gdb_assert (bl->owner != NULL); - if (bl->permanent) - /* Permanent breakpoints cannot be inserted or removed. */ - return 0; - /* The type of none suggests that owner is actually deleted. This should not ever happen. */ gdb_assert (bl->owner->type != bp_none); @@ -4042,10 +4038,6 @@ remove_breakpoint (struct bp_location *bl, insertion_state_t is) /* BL is never in moribund_locations by our callers. */ gdb_assert (bl->owner != NULL); - if (bl->permanent) - /* Permanent breakpoints cannot be inserted or removed. */ - return 0; - /* The type of none suggests that owner is actually deleted. This should not ever happen. */ gdb_assert (bl->owner->type != bp_none); @@ -4068,8 +4060,7 @@ mark_breakpoints_out (void) struct bp_location *bl, **blp_tmp; ALL_BP_LOCATIONS (bl, blp_tmp) - if (bl->pspace == current_program_space - && !bl->permanent) + if (bl->pspace == current_program_space) bl->inserted = 0; } -- 1.9.1 --------------010909080709000302080002--