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

[Yao Qi <qiyaoltc@gmail.com>]

On 16-11-29 07:07:01, Antoine Tremblay wrote:
> ** Note these patches depend on:
> https://sourceware.org/ml/gdb-patches/2016-11/msg00914.html
> 
> Before, installing single-step breakpoints was done in proceed_one_lwp as
> each thread was started.
> 
> This caused a problem on ARM, which is the only architecture using
> software single-step on which it is not safe to modify an instruction
> to insert a breakpoint in one thread while the other threads are running.
> See the architecture manual section "A3.5.4 Concurrent modification and
> execution of instructions":
> 
> "The ARMv7 architecture limits the set of instructions that can be
> executed by one thread of execution as they are being modified by another
> thread of execution without requiring explicit synchronization.  Except
> for the instructions identified in this section, the effect of the
> concurrent modification and execution of an instruction is UNPREDICTABLE
> ."

This doesn't apply to ptrace.  PTRACE_POKETEXT on a word aligned address
is atomic, so threads observe the coherent result, either breakpoint
instruction or the original instruction.  We don't see any fails in -marm
mode, do we?

In -mthumb mode, breakpoint can be 16-bit or 32-bit, but GDBserver still
PTRACE_POKETEXT on a word aligned address (in linux-low.c:linux_write_memory)
and write a word each time.  It should be atomic, however, if the 32-bit
thumb-2 instruction is 2-byte aligned, we end up two PTRACE_POKETEXT,
which is non-atomic.  That is the root cause of the problem, AFAICS.

32-bit thumb-2 breakpoint was invented for single step 32-bit thumb-2
instruction in IT block,
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-January/007488.html
but we can use 16-bit thumb breakpoint instruction anywhere else.  If I
force GDBserver to use 16-bit thumb breakpoint instruction even on 32-bit
thumb-2 instruction, I can't see any fails in schedlock.exp anymore!  See
the patch below.

Out of IT block, we can use 16-bit thumb breakpoint for any thumb code.  In
IT block, we can use 16-bit thumb breakpoint for 16-bit instruction, and
32-bit thumb-2 breakpoint for 4-byte aligned 32-bit thumb-2 instruction.
However, we can not use either 16-bit or 32-bit breakpoint for 2-byte
aligned 32-bit thumb-2 instruction in IT block.

The reason we can't use 16-bit breakpoint on a 32-bit instruction in IT
block is that 16-bit breakpoint instruction makes the second half of
32-bit instruction to another different 16-bit instruction, and the 16-bit
breakpoint instruction may not be executed at all, so the second half
of instruction may be executed, and the result is completely unknown.
GDB sets two breakpoints on two branches, "if" branch and "else" branch,
because GDB doesn't know how does the instruction to be executed affect
the flag.

	      /* There are conditional instructions after this one.
		 If this instruction modifies the flags, then we can
		 not predict what the next executed instruction will
		 be.  Fortunately, this instruction is architecturally
		 forbidden to branch; we know it will fall through.
		 Start by skipping past it.  */

GDB/GDBserver has to emulate the instruction on how does it affect the
flag, and only insert the breakpoint on the "true" branch.  Since the
target instruction will be definitely executed, we can safely use
16-bit breakpoint instruction.

-- 
Yao (齐尧)

diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c
index 2b710ba..789e0e6 100644
--- a/gdb/gdbserver/linux-aarch32-low.c
+++ b/gdb/gdbserver/linux-aarch32-low.c
@@ -243,7 +243,7 @@ arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
 
 	  target_read_memory (*pcptr, (gdb_byte *) &inst1, 2);
 	  if (thumb_insn_size (inst1) == 4)
-	    return ARM_BP_KIND_THUMB2;
+	    return ARM_BP_KIND_THUMB;
 	}
       return ARM_BP_KIND_THUMB;
     }