This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH] Avoid software breakpoint's instruction shadow inconsistency
- From: Pedro Alves <palves at redhat dot com>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org, Luis Machado <lgustavo at codesourcery dot com>
- Date: Mon, 29 Sep 2014 21:55:56 +0100
- Subject: Re: [PATCH] Avoid software breakpoint's instruction shadow inconsistency
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 1 dot 10 dot 1409140447170 dot 27075 at tp dot orcam dot me dot uk> <5421B1B3 dot 7010106 at redhat dot com> <alpine dot DEB dot 1 dot 10 dot 1409231850520 dot 27075 at tp dot orcam dot me dot uk> <5429A4E3 dot 40108 at redhat dot com> <alpine dot DEB dot 1 dot 10 dot 1409291950540 dot 4971 at tp dot orcam dot me dot uk>
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
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
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.
>> I could be missing something else, of course.
That's what I was missing...
Patch is OK. Please push.