From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32357 invoked by alias); 29 Sep 2014 20:56:08 -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 32347 invoked by uid 89); 29 Sep 2014 20:56:07 -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,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 (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 29 Sep 2014 20:56:02 +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 s8TKtx95025516 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 29 Sep 2014 16:56:00 -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 s8TKtvuk031463; Mon, 29 Sep 2014 16:55:58 -0400 Message-ID: <5429C75C.3000309@redhat.com> Date: Mon, 29 Sep 2014 20:56:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: "Maciej W. Rozycki" CC: gdb-patches@sourceware.org, Luis Machado Subject: Re: [PATCH] Avoid software breakpoint's instruction shadow inconsistency References: <5421B1B3.7010106@redhat.com> <5429A4E3.40108@redhat.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-09/txt/msg00852.txt.bz2 On 09/29/2014 08:11 PM, Maciej W. Rozycki wrote: > On Mon, 29 Sep 2014, Pedro Alves wrote: >> It doesn't look like to me that this is really the problem, since >> default_memory_insert_breakpoint adjusts bp_tgt->placed_address >> before reading memory. > > Not true (from `mips_breakpoint_from_pc'): > > insn = mips_fetch_instruction (gdbarch, ISA_MICROMIPS, pc, &status); > size = status ? 2 > : mips_insn_size (ISA_MICROMIPS, insn) == 2 ? 2 : 4; > *pcptr = unmake_compact_addr (pc); > > (hmm, weird indentation here, will have to fix) -- as you can see > `mips_fetch_instruction' (that reads the instruction under `pc') is called > before the ISA bit is stripped as `pc' is written back to `*pcptr', and > `pc' has to have the ISA bit set for the reasons I stated in the last > mail. Ah! That's the part that I was missing. I see now. > > Maybe I could work it around by writing `*pcptr' back first (and still > calling `mips_fetch_instruction' with the original `pc'), but that looks > hackish to me; first of all there is no contract in the API between the > implementation of `gdbarch_breakpoint_from_pc' and its callers that memory > behind `pcptr' is the address used for breakpoint shadowing. I think the > data structures used for shadowing should simply be consistent all the > time. Agreed. So, we could fix this by not ever trying to re-insert a memory breakpoint that has a shadow buffer. But, if we ever decide we want to record a shadow buffer for target-managed breakpoint that ends up reinserted, we'll end up with the same problem again. So might as well go with your patch. >> would be unnecessary. > > But as I noted that breaks mips_breakpoint_from_pc, you must not > overwrite `placed_address' once the instruction shadow has been made. > Indeed. >> I could be missing something else, of course. That's what I was missing... Patch is OK. Please push. Thanks, Pedro Alves