From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20014 invoked by alias); 29 Sep 2014 20:22:19 -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 19998 invoked by uid 89); 29 Sep 2014 20:22:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00 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, 29 Sep 2014 20:22:13 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1XYhSR-0002pI-Ar from Maciej_Rozycki@mentor.com ; Mon, 29 Sep 2014 13:22:07 -0700 Received: from localhost (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server (TLS) id 14.3.181.6; Mon, 29 Sep 2014 21:22:05 +0100 Date: Mon, 29 Sep 2014 20:22:00 -0000 From: "Maciej W. Rozycki" To: Pedro Alves CC: , Luis Machado Subject: Re: [PATCH] Avoid software breakpoint's instruction shadow inconsistency In-Reply-To: Message-ID: References: <5421B1B3.7010106@redhat.com> <5429A4E3.40108@redhat.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2014-09/txt/msg00850.txt.bz2 On Mon, 29 Sep 2014, Maciej W. Rozycki wrote: > > I'm confused on this part of your original description: > > > > > The issue is `insert_bp_location' overwrites the previously adjusted > > > value in `placed_address' with the original address, that is only replaced > > > back with the correct adjusted address later on when > > > `gdbarch_breakpoint_from_pc' is called. Meanwhile there's a window where > > > the value in `placed_address' does not correspond to data stored in > > > `shadow_contents', leading to incorrect instruction bytes being supplied > > > when `one_breakpoint_xfer_memory' is called to supply the instruction > > > overlaid by the breakpoint. > > > > 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. > > 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. > > > Instead, the issue is that because the breakpoint is supposed to be > > inserted (we're re-inserting it), one_breakpoint_xfer_memory needs > > to store the breakpoint instruction on top of the memory we're > > about to write. And then one_breakpoint_xfer_memory gets the > > breakpoint instruction wrong exactly because it lost the ISA bit. As a further note -- as long as symbol information is available the instruction used for the breakpoint is always a valid encoding for the ISA being used at the breakpoint location even if the ISA bit has been lost, because the ISA bit is only used to determine the encoding as the last resort. The ELF `st_other' symbol flags are a preferred way to determine the ISA and are available for the majority of failures I see because of this defect. Only cases like software watchpoint tests that single-step through possibly stripped system library code will trip on the lost ISA bit. Of course getting the ISA right does not guarantee the right size of the breakpoint instruction, which is the matter of this bug. > > I could be missing something else, of course. > > > > The patch below is what I'd like to push on top of the software single-step > > rework (which I've meanwhile slit and posted here > > https://sourceware.org/ml/gdb-patches/2014-09/msg00755.html) > > > > I've pushed that series with this patch on top here, for convenience: > > > > git@github.com:palves/gdb.git palves/mips_instruction_shadow_inconsistency > > > > Obviously, the mips-linux-gnu testing mentioned in the log is tentative. :-) > > I'll push it through testing, although given what I wrote above I have > little hope, so it'll be just a smoke test with a microMIPS multilib and > the offending test case first. So I smoke-tested gdb.base/break.exp that fails horribly with our current trunk and the `-EL -mmicromips' multilib and it still fails the same way with your tree: (gdb) PASS: gdb.base/break.exp: run until file:function(1) breakpoint continue Continuing. 720 Breakpoint 2, marker2 (a=43) at /scratch/macro/mips-linux/src/gdb-trunk/gdb/testsuite/gdb.base/break1.c:63 63 int marker2 (a) int a; { return (1); } /* set breakpoint 9 here */ (gdb) PASS: gdb.base/break.exp: run until quoted breakpoint continue Continuing. Program received signal SIGILL, Illegal instruction. 0x0040098d in marker2 (a=43) at /scratch/macro/mips-linux/src/gdb-trunk/gdb/testsuite/gdb.base/break1.c:63 63 int marker2 (a) int a; { return (1); } /* set breakpoint 9 here */ (gdb) FAIL: gdb.base/break.exp: run until file:linenum breakpoint break +1 Breakpoint 10 at 0x4009a3: file /scratch/macro/mips-linux/src/gdb-trunk/gdb/testsuite/gdb.base/break1.c, line 64. (gdb) FAIL: gdb.base/break.exp: breakpoint offset +1 step Program terminated with signal SIGILL, Illegal instruction. The program no longer exists. (gdb) FAIL: gdb.base/break.exp: step onto breakpoint Child terminated with signal = 0x4 (SIGILL) GDBserver exiting [...] Sorry. This test script scores "all passed" with my change. Maciej