From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 34009 invoked by alias); 17 Jun 2015 12:41:03 -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 33948 invoked by uid 89); 17 Jun 2015 12:41:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_WEIRDTRICK1,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=no 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 Jun 2015 12:40:59 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id C2FE5376B73; Wed, 17 Jun 2015 12:40:57 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t5HCesPd004135; Wed, 17 Jun 2015 08:40:55 -0400 Message-ID: <55816AD5.6020605@redhat.com> Date: Wed, 17 Jun 2015 12:41:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Luis Machado , gdb-patches@sourceware.org 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> In-Reply-To: <55805F52.20805@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-06/txt/msg00359.txt.bz2 On 06/09/2015 07:22 PM, Luis Machado wrote: > Both the above and the mail sound like workaround ideas. You mentioned > even more special casing in the mail. It is the amount of special casing > that i'm afraid of. I was actually proposing to remove the special casing. :-) On 06/16/2015 06:39 PM, Luis Machado wrote: > I gave the strategy of not marking permanent breakpoints/locations as > inserted a try, and it fixes the simulator problems i've been seeing > with the permanent breakpoint locations. Thanks. > > One strange side effect of this change on my local machine (x86-64) is > that gdb.threads/attach-many-short-lived-threads.exp gives me PASS > instead of FAIL when always-inserted mode is ON. I didn't investigate > this further though. You mean you _always_ get a FAIL before your patch? This test sometimes FAILs for an unknown reason, but it's racy -- it should be passing most of the time. > Is it known that this testcase is affected by > permanent breakpoint locations? No. > Is this patch what you had in mind? Yep. Close, but also remove the bp_call_dummy check in bp_loc_is_permanent, and merge in its comment, like ... > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index eb3df02..768ce59 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -7440,15 +7440,16 @@ make_breakpoint_permanent (struct breakpoint *b) > struct bp_location *bl; > > /* By definition, permanent breakpoints are already present in the > - code. Mark all locations as inserted. For now, > - make_breakpoint_permanent is called in just one place, so it's > - hard to say if it's reasonable to have permanent breakpoint with > - multiple locations or not, but it's easy to implement. */ > + code. For now, make_breakpoint_permanent is called in just one place, so > + it's hard to say if it's reasonable to have permanent breakpoint with > + multiple locations or not, but it's easy to implement. > + > + Permanent breakpoints are not marked as inserted so we allow other > + non-permanent locations at the same address to be inserted on top > + of it. This is required due to some targets, simulators mostly, not > + dealing properly with hardwired breakpoints in the code. */ ... this: /* While by definition, permanent breakpoints are already present in the code, we don't mark the location as inserted. Normally one would expect that GDB could rely on that breakpoint instruction to stop the program, thus removing the need to insert its own breakpoint, except that executing the breakpoint instruction can kill the target instead of reporting a SIGTRAP. E.g., on SPARC, when interrupts are disabled, executing the instruction resets the CPU, so QEMU 2.0.0 for SPARC correspondingly dies with "Trap 0x02 while interrupts disabled, Error state". Letting the breakpoint be inserted normally results in QEMU knowing about the GDB breakpoint, and thus trap before the breakpoint instruction is executed. (If GDB later needs to continue execution past the permanent breakpoint, it manually increments the PC, thus avoiding executing the breakpoint instruction.) > for (bl = b->loc; bl; bl = bl->next) > - { > - bl->permanent = 1; > - bl->inserted = 1; > - } > + bl->permanent = 1; > } > Actually, make_breakpoint_permanent is dead and should be deleted. The last remaining caller is finally gone - it was one of the old Unix ports we removed. So the comment should be moved to add_location_to_breakpoint instead. Thanks, Pedro Alves