This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC/7.1] Reset breakpoints after load
- From: Pedro Alves <pedro at codesourcery dot com>
- To: Daniel Jacobowitz <dan at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org, Joel Brobecker <brobecker at adacore dot com>
- Date: Mon, 15 Mar 2010 19:27:05 +0000
- Subject: Re: [RFC/7.1] Reset breakpoints after load
- References: <20100314162107.GG9310@caradoc.them.org>
On Sunday 14 March 2010 16:21:07, Daniel Jacobowitz wrote:
> Since this patch:
>
> 2009-06-17 Pierre Muller <muller@ics.u-strasbg.fr>
> Pedro Alves <pedro@codesourcery.com>
>
> * infcmd.c (post_create_inferior): Call breakpoint_re_set after target
> is pushed for watchpoint promotion to hardware watchpoint.
>
> GDB performs this sequence:
>
> % gdb -quiet file
> (gdb) break main
> [Breakpoint set after prologue]
> (gdb) target remote :PORT
> [Connect to remote target]
> [breakpoint_re_set called]
> (gdb) load
> (gdb) continue
>
> If the prologue skipping logic reads from memory, then when
> breakpoint_re_set is called, it will read garbage. Many of the
> prologue analyzers do, although the effect is mitigated by
> skip_prologue_using_sal, which is used in preference if possible.
>
> I believe we worked around this bug locally for MIPS. I've also just
> encountered it while testing a patch for ARM that changes the prologue
> skipping behavior.
>
> I can think of three solutions.
>
> * Don't reset breakpoints here. Promote watchpoints and make no other
> changes. A bit twisty to implement, unfortunately.
This would only papers over the issue. Imagine that the patch
that introduced the new breakpoint_re_set call was reverted. You
can still trigger the issue at hand easily. E.g.:
$ gdb
(gdb) target remote :PORT
[Connect to remote target]
(gdb) file FILE
[breakpoint_re_set called]
(gdb) load
(gdb) continue
>
> * Don't read from the target during prologue analyzers; only read from
> the executable file. I like this solution best, and it has other
> merits (it's faster!). But it's the most work.
"only" would be too strong. You'd want "prefer", like
trust-readonly-sections. We may have debug info available but
no pure memory contents to read from.
> * The easy solution: Reset breakpoints again once we know that target
> memory is valid.
>
> Any comments on this patch? It has no effect on test results on
> arm-none-eabi today, and fixes two hundred or so failures with another
> patch that required reading from the target during prologue analysis.
Given that I proposed exactly this at least a couple of times
already, I don't expect you to be waiting for me to say go
ahead. :-) I'm not sure I agree in calling this a workaround
though. IMO, this situation is analog to an "exec".
The previous memory image is supposedly replaced by
"load". Consider breakpoints always-inserted mode: the
previous traps are simply overwriten by "load" behind the
breakpoint's module. Calling:
- mark_breakpoints_out ();
- update_breakpoints_after_exec ();
- breakpoint_re_set ();
Wouldn't be such a stretch, although just
- remove_breakpoints ();
<do actual load>
- breakpoint_re_set ();
would work too, and be simpler.
The comment reads a bit like the post_create_inferior
path is the only that causes this, but as shown in the example abov
, any breakpoint_re_set call would trigger the issue, so maybe
I'd rephrase it a bit in that direction.
IMO.
--
Pedro Alves