This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [pushed 1/2] PR gdb/18002: Fix reinsert of a permanent breakpoints
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 06 Mar 2015 14:31:17 +0000
- Subject: Re: [pushed 1/2] PR gdb/18002: Fix reinsert of a permanent breakpoints
- Authentication-results: sourceware.org; auth=none
- References: <1425598969-7666-1-git-send-email-palves at redhat dot com> <1425598969-7666-2-git-send-email-palves at redhat dot com>
Pedro Alves <palves@redhat.com> writes:
> Permanent breakpoints
> are always marked as inserted. So if the permanent breakpoint doesn't
> have a shadow yet in its shadow buffer, but we set shadow_len before
> calling target_read_memory, then the still clear shadow_contents
> buffer will be used by the breakpoint masking code... And then from
> there on, the permanent breakpoint has a broken shadow buffer, and
> thus any memory read out of that address will read bogus code, and
> many random bad things fall out from that.
Yes, that is what I observed on aarch64-linux too.
>
> The fix is just to set shadow_len at the same time shadow_contents is
> set, not one before and another after...
>
> Fixes all gdb.base/bp-permanent.exp FAILs on PPC64 GNU/Linux gdbserver
> and probably any other gdbserver port that doesn't do z0 breakpoints.
This patch fixes gdb.base/bp-permanent.exp FAILs on aarch64-linux too,
but there are some remains, which are not related.
> diff --git a/gdb/mem-break.c b/gdb/mem-break.c
> index aeffc93..0fb53cf 100644
> --- a/gdb/mem-break.c
> +++ b/gdb/mem-break.c
> @@ -53,12 +53,21 @@ default_memory_insert_breakpoint (struct gdbarch *gdbarch,
>
> /* Save the memory contents in the shadow_contents buffer and then
> write the breakpoint instruction. */
> - bp_tgt->shadow_len = bplen;
> readbuf = alloca (bplen);
> val = target_read_memory (addr, readbuf, bplen);
> if (val == 0)
> {
> + /* These must be set together, either before or after the shadow
> + read, so that if we're "reinserting" a breakpoint that
> + doesn't have a shadow yet, the breakpoint masking code inside
> + target_read_memory doesn't mask out this breakpoint using an
> + unfilled shadow buffer. The core may be trying to reinsert a
> + permanent breakpoint, for targets that support breakpoint
> + conditions/commands on the target side for some types of
> + breakpoints, such as target remote. */
> + bp_tgt->shadow_len = bplen;
> memcpy (bp_tgt->shadow_contents, readbuf, bplen);
> +
Your fix looks right to me, although I am testing a different one, in
which bp_location_has_shadow returns false if bl->permanent is true.
Anyway, Thanks for fixing this bug, Pedro.
--
Yao (éå)