Re: [rfa] ARM prologue parsing support for Thumb-2 instructions

[Richard Earnshaw <rearnsha at arm dot com>]

On Fri, 2010-10-08 at 14:54 +0200, Ulrich Weigand wrote:
> Daniel Jacobowitz wrote:
> > On Mon, Jul 19, 2010 at 12:59:05PM +0200, Ulrich Weigand wrote:
> > > > Trunk thumb_analyze_prologue does have support for Thumb-2.
> > > 
> > > Well, all the support for Thumb-2 I can see is in this block:
> > > 
> > >       else if ((insn & 0xe000) == 0xe000 && cache == NULL)
> > >         {
> > >           /* Only recognize 32-bit instructions for prologue skipping.  */
> > > 
> > > which, as the comment says, is active *only* if this routine is
> > > called from arm_skip_prologue (with cache == NULL), but not if the
> > > routine is called from arm_scan_prologue (with cache != NULL),
> > > which is what is used during unwinding.
> > 
> > IIRC, it would not be hard to fill in the missing pieces; I just
> > didn't need them at the time, and could not easily test them.  So
> > rather than risk them being wrong, I left them for later.
> 
> I've finally gotten around to finish my implementation of this.
> 
> The main part of this patch is support in thumb_analyze_prologue for
> various Thumb-2 instructions.  In addition, the patch improves support
> for optimized code by skipping unrecognized instructions instead of
> aborting the prologue scan, and by not relying on line number data to
> determine an upper bound for the end of the prologue.  (Both these
> changes are already in place for the ARM decoder; the patch simply
> brings them over to the Thumb decode as well.)
> 
> Finally, the patch adds support for a couple of more function calls
> that may happen within the prologue: __tls_get_addr and __aeabi_read_tp.
> 
> Tested on armv7l-linux-gnueabi with no regressions.
> 
> In addition, I've run a test with the following line disabled:
>   dwarf2_append_unwinders (gdbarch);
> This causes GDB to ignore DWARF CFI even if present, and always
> fall back to prologue parsing.  With GDB before this patch, that
> test introduces a large number of extra regressions.  *With* this
> patch, nearly all of those regressions are fixed again.  The
> following failures still remain:
> 
> FAIL: gdb.base/call-signal-resume.exp: backtrace
> FAIL: gdb.base/call-signal-resume.exp: dummy stack frame number
> FAIL: gdb.base/call-signal-resume.exp: continue to breakpoint at stop_two
> FAIL: gdb.base/call-signal-resume.exp: continue to receipt of signal
> FAIL: gdb.base/call-signal-resume.exp: continue to breakpoint at handle_signal
> FAIL: gdb.base/call-signal-resume.exp: continue to program exit
> 
> FAIL: gdb.base/foll-vfork.exp: (timeout) vfork parent follow, finish after tcatch vfork
> FAIL: gdb.base/foll-vfork.exp: vfork child follow, finish after tcatch vfork
> 
> FAIL: gdb.threads/attach-stopped.exp: threaded: attach2 to stopped bt
> FAIL: gdb.threads/attachstop-mt.exp: attach0 to sleeping bt
> FAIL: gdb.threads/attachstop-mt.exp: attach3 to stopped bt
> FAIL: gdb.threads/attachstop-mt.exp: attach4 to stopped bt
> FAIL: gdb.threads/pthreads.exp: check backtrace from thread 1
> FAIL: gdb.threads/pthreads.exp: check backtrace from thread 2
> FAIL: gdb.threads/pthreads.exp: apply backtrace command to all three threads
> 
> All these are caused by failures to parse the prologues of hand-written
> *ARM* assembler routines in glibc (system call handlers like nanosleep).
> These simply deviate too far from the usual rules (e.g. by intermixing
> stack saving/restoring of registers with conditional branches, or by
> temporarily saving registers into other registers instead of the stack)
> for the ARM prologue parser to be able to handle them.
> 
> It seems to me that there is not much sense in attempting to support
> even this type of code.  I guess we should strongly recommend to have
> (at least) glibc debuginfo files installed if you want to debug.
> 
> OK for mainline?
> 
> Bye,
> Ulrich
> 
> ChangeLog:
> 
> 	* arm-tdep.c (thumb_expand_immediate): New function.
> 	(thumb_instruction_changes_pc): Likewise.
> 	(thumb2_instruction_changes_pc): Likewise.
> 	(thumb_analyze_prologue): Handle 32-bit Thumb instructions during
> 	prologue parsing.  Improved support for optimized code.
> 	(thumb_scan_prologue): Do not reply on line-number information,
> 	use same heuristics as arm_scan_prologue insead.
> 	(skip_prologue_function): Accept functions
> 	"__tls_get_addr" and "__aeabi_read_tp".

OK.

R.