From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 741 invoked by alias); 6 Mar 2012 18:56:39 -0000 Received: (qmail 697 invoked by uid 22791); 6 Mar 2012 18:56:37 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 06 Mar 2012 18:56:17 +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 (8.14.4/8.14.4) with ESMTP id q26Iu3kw005892 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 6 Mar 2012 13:56:14 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q26IeZwD024839; Tue, 6 Mar 2012 13:40:36 -0500 Message-ID: <4F565A23.1070009@redhat.com> Date: Tue, 06 Mar 2012 18:56:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 MIME-Version: 1.0 To: "Maciej W. Rozycki" CC: gdb-patches@sourceware.org Subject: Re: [PATCH] remote: Fix hw watchpoint address matching References: <201112071724.06407.pedro@codesourcery.com> <201112091415.08804.pedro@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 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: 2012-03/txt/msg00199.txt.bz2 On 02/24/2012 11:48 PM, Maciej W. Rozycki wrote: > Hi Pedro, > > Back to this change, was distracted by something else. So was I. :-) >>> This makes me nervous. I think we should be liberal on what we accept. >>> In particular ILP32 ABIs on 64-bit targets may be affected. An example is >>> the MIPS n64 ABI where the width of general registers is 64 bits and >>> addresses are sign-extended 32 bits. When bit #31 is set in the address, >>> the remote stub may possibly report the value as truncated to 32 bits or >>> as a properly sign-extended 64-bit value. Not that I observed this >>> anywhere, but I think we should accept both. >> >> If such thing were possible, then wouldn't breakpoints break? >> We store the (masked) address of where we ended up putting >> the breakpoint in bp_tgt->placed_address (remote_insert_breakpoint), >> and if the target reported an address not exactly bp_tgt->placed_address, >> we wouldn't be able to match it up, resulting in spurious SIGTRAPs. >> Hmm, actually, it looks like breakpoint.c:bkpt_breakpoint_hit is broken >> in that it should be using bl->target_info.placed_address instead >> of bl->address ? How is this not breaking on cases that need >> breakpoint adjustment? I'm probably missing something. > > Yes, this is about watchpoints, not breakpoints. ;) I fully understand that. What I was saying is that if that is a concern, then breakpoints with bit #31 set are quite likely to suffer from exactly the same issue, with the remote possibly reporting the trapped address as truncated to 32 bits or as a properly sign-extended 64-bit value. And I'd imagine the code that handles the low level traps/exceptions to handle both watchpoint and breakpoint addresses the same, and the stub code that marshals those breakpoint addresses to rsp to be just as susceptible to the issue as code that marshals watchpoint addresses. Now, I'm not saying we should run to fix that... I'm still curious why isn't bkpt_breakpoint_hit broken like I described above, when a breakpoint is subject to adjustment, such as that done by mips_breakpoint_from_pc. I probably need to stare more at the code. ;-) >> /* Implementation of target method FOO. */ > > Umm, there aren't that many comments of this kind there actually... We started enforcing that rule not too long ago. Anyway, thanks for the fixes, and congratulations on your new role. :-) -- Pedro Alves