Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yao Qi <qiyaoltc@gmail.com>
To: Antoine Tremblay <antoine.tremblay@ericsson.com>
Cc: Pedro Alves <palves@redhat.com>,
	 "gdb-patches\@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH 1/2] This patch fixes GDBServer's run control for single stepping
Date: Fri, 31 Mar 2017 16:31:00 -0000	[thread overview]
Message-ID: <86d1cxwgpk.fsf@gmail.com> (raw)
In-Reply-To: <wwokvaqqlipt.fsf@ericsson.com> (Antoine Tremblay's message of	"Thu, 30 Mar 2017 14:31:26 -0400")

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

> You have to add if the current instruction is an IT instruction in wich
> case the next instruction will be in an IT block.
>

Oh, you are right.

> Also if you have a conditional instruction that would evalutate to
> true and is not the last one, get_next_pcs may return an instruction
> after the IT block, arm_breakpoint_kind_from_current_state will be
> called from the IT block with that PC and return a THUMB2_KIND when it
> should not. See the else case in arm-get-next-pcs.c:~351

With the current PC and CPSR, it is not difficult to know whether
next_pc is still within IT block nor not, because all instructions in IT
block should be sequentially executed or skipped.

>
> My point was that we should use get_next_pc directly since it's the best
> place to detect if the next_pc is in the IT block. And the intent would
> be clear.

Yeah, we can record the information of breakpoint type in the return
value of get_next_pc, ...

>
> It would give something like the patch below. (Note the GDB part of this
> is missing but it works with GDBServer)
>

... but using extra bit in CORE_ADDR is not a good idea to me.

>> The problem of this patch is that we end up inserting different
>> kinds of breakpoints on the same instruction.  For a given 32-bit thumb
>> instruction, GDB and GDBserver knows 32-bit thumb breakpoint instruction
>> is used for GDB breakpoint, but only GDBserver knows 16-bit thumb
>> breakpoint is used for GDBserver single-step breakpoint, so GDB will be
>> confused on this.  I stopped here, and start to do something else.
>
> Humm but how will the GDBServer 16-bit breakpoint be reported to GDB ?
> Won't it always be hit and handled by GDBServer ?
>
> And if you have a GDB breakpoint on an instruction and GDBServer puts
> a single step breakpoint on that GDB breakpoint instruction, GDBServer
> still knows of the GDB and GDBServer breakpoint types.
>
> So how does GDB get confused ?

That was my conclusion at that point.  I got some regressions in
gdb.threads/*.exp when I tested my patch (gdb running is on
x86_64-linux), but I can't remember more details.

I am also wondering that we can use some code in
arm_adjust_breakpoint_address about detecting BPADDR is within IT block
or not by scanning instructions backward, if none of two bytes (can be
16-bit thumb instruction or the 2nd half of 32-bit thumb instruction)
matches IT instruction, the PC is not within IT block.

-- 
Yao (齐尧)


  reply	other threads:[~2017-03-31 16:31 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-29 12:07 Antoine Tremblay
2016-11-29 12:07 ` [PATCH 2/2] Avoid step-over infinite loop in GDBServer Antoine Tremblay
2017-01-16 17:27   ` Antoine Tremblay
2017-01-18 16:31     ` Antoine Tremblay
2017-02-03 16:21   ` Pedro Alves
2017-02-17  3:39     ` Antoine Tremblay
2017-02-22 10:15   ` Yao Qi
2016-11-29 12:12 ` [PATCH 1/2] This patch fixes GDBServer's run control for single stepping Antoine Tremblay
2017-01-16 17:28 ` Antoine Tremblay
2017-01-27 15:01 ` Yao Qi
2017-01-27 16:07   ` Antoine Tremblay
     [not found]     ` <CAH=s-PP-i3v_Fr=QeWt9BQeJzjCHtW79nGYpJ9hF-Bb=OBo89Q@mail.gmail.com>
2017-01-27 18:24       ` Antoine Tremblay
2017-01-29 21:41         ` Yao Qi
2017-01-30 13:29           ` Antoine Tremblay
2017-02-16 22:32             ` Yao Qi
2017-02-17  2:17               ` Antoine Tremblay
     [not found]             ` <2255ed6f-a146-026c-f871-00e9a33dfcf0@redhat.com>
2017-02-17  1:42               ` Antoine Tremblay
2017-02-17  2:05                 ` Pedro Alves
2017-02-17  3:06                   ` Antoine Tremblay
2017-02-17 22:19                     ` Yao Qi
2017-02-18  0:19                       ` Antoine Tremblay
2017-02-18 22:49                         ` Yao Qi
2017-02-19 19:40                           ` Antoine Tremblay
2017-02-19 20:31                             ` Antoine Tremblay
2017-03-29 12:41                           ` Antoine Tremblay
2017-03-29 14:11                             ` Antoine Tremblay
2017-03-29 17:54                               ` Antoine Tremblay
     [not found]                             ` <86d1cy4umo.fsf@gmail.com>
2017-03-30 18:31                               ` Antoine Tremblay
2017-03-31 16:31                                 ` Yao Qi [this message]
2017-03-31 18:22                                   ` Antoine Tremblay
2017-04-03 12:41                                     ` Yao Qi
2017-04-03 13:18                                       ` Antoine Tremblay
2017-04-03 15:18                                         ` Yao Qi
2017-04-03 16:57                                           ` Antoine Tremblay

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=86d1cxwgpk.fsf@gmail.com \
    --to=qiyaoltc@gmail.com \
    --cc=antoine.tremblay@ericsson.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