From: Yao Qi <yao@codesourcery.com>
To: Pedro Alves <palves@redhat.com>
Cc: <gdb-patches@sourceware.org>
Subject: Re: [PATCH 3/3] fix skipping permanent breakpoints
Date: Wed, 05 Nov 2014 12:26:00 -0000 [thread overview]
Message-ID: <87y4rp7s02.fsf@codesourcery.com> (raw)
In-Reply-To: <1415127790-15091-4-git-send-email-palves@redhat.com> (Pedro Alves's message of "Tue, 4 Nov 2014 19:03:10 +0000")
Pedro Alves <palves@redhat.com> writes:
> The patch also adds a new gdb.base/bp-permanent.exp test that
> exercises many different code paths related to stepping permanent
> breakpoints, including the stepping with signals cases. The test uses
> "hack/trick" to make it work on all (or most) platforms -- it doesn't
> really hard code a breakpoint instruction.
It is great to test permanent breakpoint in an arch-independent way. It
exposes an existing issue on permanent breakpoint which is found when I
test your patch series on arm. breakpoint.c:bp_location_has_shadow
should return false for permanent breakpoint because permanent
breakpoint doesn't have shadow, otherwise, breakpoint_xfer_memory
destroys contents read from memory_xfer_partial_1 and the internal error
is triggered as below,
(gdb) PASS: gdb.base/bp-permanent.exp: always_inserted=off, sw_watchpoint=0: stepi signal with handler: queue-signal SIGUSR1
stepi^M
../../../git/gdb/infrun.c:2112: internal-error: resume: Assertion `tp->control.step_resume_breakpoint->loc->permanent' failed.^M
A problem internal to GDB has been detected,
> + else
> + {
> + /* There's no signal to pass. Skip the permanent
> + breakpoint, and arrange for any breakpoint at the new PC
> + to be reported back to handle_inferior_event. */
> +
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: resume: skipping permanent breakpoint\n");
> + gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
> + /* Update pc to reflect the new address from which we will
> + execute instructions. */
> + pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
Is get_thread_regcache (inferior_ptid) equivalent to regcache? If it
is, we can use regcache which is shorter.
> + tp->prev_pc = pc;
> +
> + clear_step_over_info ();
> + tp->control.trap_expected = 0;
> +
> + /* There may or not be a breakpoint at the new address. If
> + we're stepping then we're done. Otherwise, make sure
> + there's a breakpoint at the current address, and let it
> + trigger. */
Sorry, I can't map the comments to the code. Is "Otherwise" in the
comments about the else branch which doesn't exist? Can you
elaborate?
> + if (step)
> + {
> + /* If we already have a step-resume breakpoint set, then
> + we can just continue until that one is hit. */
When I read the comments, the code in my mind is like:
if (tp->control.step_resume_breakpoint != NULL)
step = 0;
which is different from your code below.
> + if (tp->control.step_resume_breakpoint == NULL)
> + insert_step_resume_breakpoint_at_frame (get_current_frame ());
> + step = 0;
> + }
> +
> + insert_breakpoints ();
> + }
> }
>
> +
> + with_test_prefix "next trips on permanent bp" {
> + delete_breakpoints
> +
> + gdb_breakpoint "test_next"
> + gdb_continue_to_breakpoint "test_next"
> +
> + gdb_breakpoint "$line_bp"
> + gdb_test "condition \$bpnum 0"
> +
> + gdb_test "next" "after next .*"
> + }
> +
> + if ![target_info exists gdb,nosignals] {
> +
We need also check can_single_step_to_signal_handler.
--
Yao (齐尧)
next prev parent reply other threads:[~2014-11-05 12:26 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-04 19:03 [PATCH 0/3] fix " Pedro Alves
2014-11-04 19:03 ` [PATCH 3/3] fix skipping " Pedro Alves
2014-11-05 12:26 ` Yao Qi [this message]
2014-11-07 19:53 ` Pedro Alves
2014-11-12 0:54 ` Yao Qi
2014-11-12 10:53 ` Pedro Alves
2014-11-04 19:03 ` [PATCH 1/3] add a default method for gdbarch_skip_permanent_breakpoint Pedro Alves
2014-11-04 19:03 ` [PATCH 2/3] make "permanent breakpoints" per location and disableable Pedro Alves
2014-11-05 6:37 ` Yao Qi
2014-11-12 10:55 ` Pedro Alves
2014-11-20 16:41 ` [RFA] Always consider infcall breakpoints as non-permanent Joel Brobecker
2014-11-21 11:38 ` Pedro Alves
2014-11-23 10:44 ` Joel Brobecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y4rp7s02.fsf@codesourcery.com \
--to=yao@codesourcery.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).