[rfc,arm] Always use correct execution state for single-step breakpoints

Hello,

about a year ago, Matthew Gretton-Dann added improved heuristics in an
attempt to ensure the breakpoints for software single-singlestepping
are correct for the target execution mode:
http://sourceware.org/ml/gdb-patches/2010-03/msg00168.html

Now in general, GDB needs to make some assumptions (usually by consulting
symbol tables) to determine the target execution mode for an arbitrary
code address.  However, when single-stepping, GDB detects the target
address by decoding the branch instruction anyway, and can at the same
time reliably detect the target execution mode.

With Matthew's patch, this information is used whenever GDB does not
have symbol information for the target address.  When GDB *does* have
symbol information, that still takes precedence.  Now usually this is
OK because the execution mode determined from symbol tables is still
correct.

However, we've had a bug report where symbols tables were present,
but contained the *wrong* execution mode for a symbol (due to an
error in hand-written assembler code).  Now this is certainly a bug
in the code (and led to the program crashing at run-time).  However,
you'd still expect a debugger to get this right -- bugs in the program
are why you need a debugger in the first place :-)

The following patch attempts to fix this test case by making sure that
for single-step breakpoints, the symbol table is *never* consulted,
but instead the known target execution mode is used.  This is done
by means of a new global "arm_override_mode" which is respected in
arm_pc_is_thumb unconditionally (similar to arm_force_mode_string),
and set by a new routine arm_insert_single_step_breakpoint which is
then used whenever setting a single-step breakpoint.  This routine
operates on the execution mode encoded in the LSB of the address;
to get at that, arm_get_next_pc is modified to return a PC in that
form.

(The global is a bit ugly, but the concept still seems right to me:
if at some point in the future, we actually do use two different
gdbarch's to represent Thumb vs. ARM, the single-step routines would
determine the proper gdbarch to set the breakpoint in and simply
use insert_single_step_breakpoint with that arch.)

The old single-step heuristics in arm_pc_is_thumb introduced by
Matthew is no longer needed, and removed by the patch.

As a final part of the patch, there was one case where 
arm_linux_syscall_next_pc would not properly set the Thumb bit;
this is fixed as well.

Tested on armv7l-unknown-linux-gnueabi with no regressions;
fixes the newly added test case.

Any comments?   I'm planning on committing this in a week or so.

Bye,
Ulrich

ChangeLog:

gdb/
	* arm-tdep.h (arm_insert_single_step_breakpoint): Add prototype.
	* arm-tdep.c (arm_override_mode): New global.
	(arm_pc_is_thumb): Respect arm_override_mode.  Remove single-step
	execution mode heuristics.
	(thumb_get_next_pc_raw): Remove INSERT_BKTP argument; always insert
	second single-step breakpoint if needed, using
	arm_insert_single_step_breakpoint.
	(arm_get_next_pc_raw): Remove INSERT_BKTP argument.  Only handle
	ARM execution mode, do not call thumb_get_next_pc_raw.
	(arm_get_next_pc): Encode execution mode in return value.  Call
	either arm_get_next_pc_raw or thumb_get_next_pc_raw.
	(arm_insert_single_step_breakpoint): New function.
	(arm_software_single_step): Call it.
	* arm-linux-tdep.c (arm_linux_sigreturn_return_addr): Add IS_THUMB
	argument to return execution mode of sigreturn target.
	(arm_linux_syscall_next_pc): Use it.
	(arm_linux_copy_svc): Update call.
	(arm_linux_software_single_step): Call
	arm_insert_single_step_breakpoint.

gdb/testsuite/
	* gdb.arch/thumb-singlestep.S: New file.
	* gdb.arch/thumb-singlestep.exp: Likewise.

[rfc,arm] Always use correct execution state for single-step breakpoints

Commit Message

Comments

Patch