Re: [rfc] Strip Thumb bit from PC returned by arm_get_longjmp_target

Matthew Gretton-Dann
2010-08-18T09:01:06Z

On Tue, 2010-08-17 at 21:15 +0200, Ulrich Weigand wrote:
> Hello,
>
> when stepping over code performing a longjmp on Thumb code,
> I'm seeing warning messages along the lines of:
> warning: Breakpoint address adjusted from 0x4002d29d to 0x4002d29c.
>
> (This happens e.g. in gdb.threads/threxit-hop-specific.exp.)
>
> This is caused by arm_get_longjmp_target returning a value with
> the Thumb bit set, which the rest of the code doesn't expect.
>
> Fixed by calling arm_addr_bits_remove on the PC value returned
> from arm_get_longjmp_target.
>
> Tested on armv7l-linux-gnueabi with no regressions.
>
> Any comments? I'm planning on committing this within a couple
> of days.

The problem with the patch is it removes what may be the only way we
have of telling the instruction state of the longjmp target. If you
have debugging information (mapping symbols at the very least)
everything is fine, but if you don't then how do you tell what the state
is? arm_pc_is_thumb does use this bit to detect the instruction state
(and arm_breakpoint_from_pc then uses this result to determine the
breakpoint type).

In the case above I think the correct fix is to make
arm_adjust_breakpoint_address not strip out the address bits (which it
does when trying to work out whether we are single stepping through an
IT block).

However, I think there is a more fundamental issue to be solved in the
ARM backend. For ARM (and possibly other architectures) you do not have
enough information in the PC to be able to accurately set a breakpoint,
you also need to know the instruction set in use at that address.

Currently this is being done in a fairly ad hoc and inconsistent fashion
by using bit-0 to indicate Thumb or ARM state. But this troublesome to
maintain, and can make the debugger behave in a non-obvious manner to
the user. I think it would be cleaner to change the interfaces to
various functions (for example breakpoint_from_pc) to take an
'instruction set state' parameter as well as the pc (or change the pc
from a CORE_ADDR to a { CORE_ADDR pc; enum instruction_set isa; } pair),
but this would be a large scale change with impacts throughout the code
base - which probably makes it impractical, and certainly in need of
discussion.

Any thoughts?

Thanks,

Matt

--
Matthew Gretton-Dann
Principal Engineer - PDSW Tools
ARM Ltd