Re: [PATCH 1/2] This patch fixes GDBServer's run control for single stepping

[Yao Qi <qiyaoltc at gmail dot com>]

On 17-01-30 08:28:29, Antoine Tremblay wrote:
>
> Yao Qi writes:
>
> > On Fri, Jan 27, 2017 at 6:24 PM, Antoine Tremblay
> > <antoine.tremblay@ericsson.com> wrote:
> >>
> >> Yao Qi writes:
> >>
> >>> On Fri, Jan 27, 2017 at 4:06 PM, Antoine Tremblay
> >>> <antoine.tremblay@ericsson.com> wrote:
> >
> >>> If emulation is complex, probably
> >>> we can partially fix this problem by "always using 16-bit thumb instruction
> >>> if program is out of IT block".
> >>>
> >>
> >> I would be against that since it would leave the feature partially
> >> working and crashing the program when it fails...
> >>
> >> It would also be a regression compared to previous GDBServer.
> >
> > There won't be any regression.  16-bit thumb breakpoint works pretty well
> > for any thumb instructions (16-bit and 32-bit) if program is out of IT block.
> > The 32-bit thumb-2 breakpoint was added for single step IT block.
>
> Yes so there will be a regression for single step inside an IT block if
> we keep using the 32 bit breakpoint since this one can be non atomic if
> it's not aligned properly.
>

It does fail, but not a regression, because current GDBserver fails to
write 32-bit thumb breakpoint to 2-byte aligned 32-bit instruction
atomically, regardless the program is within IT block or not.  My
suggestion is that "let us fix this problem when program is out of IT
block by using 16-bit thumb breakpoint".  That will fix the issue
of atomic write when program is out of IT block, and leave the problem
there when program is within IT block.  Why is it a regression?

> We could write a test case for it and it would fail like schedlock
> did. But it would not with the single stepping controlled by GDB like it
> was before.
>
> >
> >> Also, IT blocks are a common place to have a breakpoint/tracepoint.
> >>
> >
> > We don't change anything when setting breakpoint inside IT block.
>
> Well that's a problem if we write a 32 bit thumb2 breakpoint aligned on
> 2 bytes like discussed before.
>

That is just the sub-set of the original problem.

> >
> >>>> I think it would be better to get the current single stepping working
> >>>> with the stop all threads logic since GDBServer was working like that
> >>>> when GDB was doing the single stepping anyway. This would fix the current
> >>>> regression.
> >>>>
> >>>> Then work could be done to improve GDBServer to be better at
> >>>> non-stopping.
> >>>>
> >>>> WDYT ?
> >>>>
> >>>
> >>> Sounds like we are applying the ARM linux limitation to a general
> >>> place.
> >>> Other software single step targets may write breakpoint in atomic way,
> >>> so we don't need to stop all threads.  Even in -marm mode, we don't
> >>> have to stop all threads on inserting breakpoints.
> >>
> >> Well ARM is the only software single stepping target, while I agreee
> >> that we would be affecting general code, I think that since there is
> >> no software single step breakpoint target that supports atomic
> >> breakpoint writes at the moment it's normal that the run control
> >> doesn't support that.
> >
> > No, ARM Linux ptrace POKETEXT is _atomic_ if the address is 4-byte
> > aligned, so 32-bit arm breakpoint and 16-bit thumb breakpoint can be
> > written atomically.  32-bit thumb-2 breakpoint can be written atomically
> > too if the address is 4-byte aligned.
> >
> > The only problem we have is inserting a breakpoint on a 2-byte aligned
> > 32-bit thumb-2 instruction inside IT block, we can not use neither 16-bit thumb
> > breakpoint nor 32-bit thumb breakpoint.  Everything works fine in other
> > cases.
> >
>
> I mean software single stepping as a whole, which means considering all
> cases, is not atomic. I don't see how we could leave that case
> unaddressed ?
>
> Especially since it will crash the inferior ?
>

I am thinking how do we make progress here.  Nowadays, the multi-threaded
program may crash almost everywhere, but we can partially fix it to an
extent that the multi-threaded program may only crash in side IT block.
Is it a progress?  I feel it is a good example to apply "divide and
conquer" to a complicated engineering problem.  We can easily fix the
first half of this problem, and then think about the second half.

> >>
> >> I don't count -marm as an arch since there no way to check that all the
> >> program including shared libs etc is -marm, I don't think we could make
> >> the distinction in GDBServer AFAIK.
> >
> > We can check with -mthumb.  My hack in last email fixes fails in
> > schedlock.exp in thumb mode.
>
> Yes but schedlock.exp is not made to test the 2 byte aligned thumb2
> 32-bit breakpoint IIRC.
>

We can write one test for single step 2-byte aligned thumb-2 32-bit
instruction.  No problem.

-- 
Yao (齐尧)