From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24057 invoked by alias); 23 Apr 2014 16:27:28 -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 24047 invoked by uid 89); 23 Apr 2014 16:27:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 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 ESMTP; Wed, 23 Apr 2014 16:27:27 +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 s3NGRPke012104 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 23 Apr 2014 12:27: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 s3NGROpJ014934; Wed, 23 Apr 2014 12:27:24 -0400 Message-ID: <5357E9EB.9090005@redhat.com> Date: Wed, 23 Apr 2014 16:27:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: "Blanc, Nicolas" CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH] Stale breakpoint instructions, spurious SIGTRAPS. References: <1397585886-29261-1-git-send-email-palves@redhat.com> <388084C8C1E6A64FA36AD1D656E4856635AE8EB5@IRSMSX106.ger.corp.intel.com> In-Reply-To: <388084C8C1E6A64FA36AD1D656E4856635AE8EB5@IRSMSX106.ger.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-04/txt/msg00447.txt.bz2 On 04/16/2014 06:18 PM, Blanc, Nicolas wrote: > Hi Pedro, > >> Then, the reason that disable_breakpoints_in_freed_objfile was clearing the inserted flag isn't clear, but likely to avoid breakpoint removal errors, assuming remove-symbol-file was called after the dynamic object was already unmapped from the inferior. > > Indeed that was the intend. The clearing of the flag copied from disable_breakpoints_in_unloaded_shlib. I see. > >> if (val) >> @@ -7666,7 +7693,7 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) >> ? /* If the file is a shared library not loaded by the user then >> solib_unloaded was notified and disable_breakpoints_in_unloaded_shlib >> was called. In that case there is no need to take action again. */ >> - if ((objfile->flags & OBJF_SHARED) && !(objfile->flags & OBJF_USERLOADED)) >> + if ((objfile->flags & OBJF_USERLOADED) == 0) >> return; > > The comment above should be updated. The condition is now weaker. I've updated the comment. > > > ALL_BREAKPOINTS (b) > @@ -7698,7 +7725,11 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) >> if (is_addr_in_objfile (loc_addr, objfile)) >> { >> loc->shlib_disabled = 1; >> - loc->inserted = 0; >> + /* At this point, we don't know whether the object was >> + unmapped from the inferior or not, so leave the >> + inserted flag alone. We'll handle failure to >> + uninsert quietly, in case the object was indeed >> + unmapped. */ > > Would it work to simplify disable_breakpoints_in_unloaded_shlib in the same way? > Note that I understand it might be unnecessary risky to fix a function that is not broken. Yeah. In fact, it seems to me that by just clearing the inserted flag, we leave stale HW breakpoints planted in the target, just like in the "remove-symbol-file" case. So we should either fix it in the same way, or only clear the flag for software breakpoints. > >> + /* Make sure we see the memory breakpoints. */ cleanup = >> + make_show_memory_breakpoints_cleanup (1); val = target_read_memory >> + (addr, old_contents, bplen); > > It was not immediately clear to me that old_contents means current content. Indeed. I had just copied this from ppc_linux_memory_remove_breakpoint and not noticed that. I've renamed it in the version pushed. Thanks, -- Pedro Alves