Re: [PATCH RFA] Misc fixes for GNU/Linux/ARM target

["Scott Bambrough" <scottb at netwinder dot org>]

Hi Kevin,

These patches are fine.  Sorry to take so long to evaluate them.

Scott

----- Original Message -----
From: Kevin Buettner <kevinb@cygnus.com>
To: Scott Bambrough <scottb@netwinder.org>
Cc: <gdb-patches@sourceware.cygnus.com>
Sent: Thursday, August 10, 2000 3:24 AM
Subject: [PATCH RFA] Misc fixes for GNU/Linux/ARM target


> The patches below add support for backtracing through signal handlers
> and makes the prologue scanning code somewhat less naive about
> optimized code on GNU/Linux/ARM.  In addition, there are several small
> fixes so that arm-linux-tdep.c will build without warnings.
>
> These patches fix the following failures on GNU/Linux/ARM:
>
>     FAIL: gdb.base/annota1.exp: backtrace @ signal handler
>     FAIL: gdb.base/mips_pro.exp: backtrace
>     FAIL: gdb.base/selftest.exp: backtrace through signal handler
>     FAIL: gdb.base/signals.exp: bt in signals.exp
>
> They do not introduce any new failures.
>
> Okay to commit?
>
> * config/arm/tm-linux.h (arm_linux_sigcontext_register_address,
> arm_linux_in_sigtramp): Declare.
> (IN_SIGTRAMP, SIGCONTEXT_REGISTER_ADDRESS): Define.
> * arm-tdep.c (SIGCONTEXT_REGISTER_ADDRESS): Define to be 0
> if not already defined by tm.h.
> (arm_scan_prologue): Don't assume that the prologue instructions
> will be in a contiguous clump.
> (arm_init_extra_frame_info): Add support for sigtramp frames.
> (arm_pc_is_thumb, arm_pc_is_thumb_dummy): Change type of
> `memaddr' from bfd_vma to CORE_ADDR.
> * arm-linux-tdep.c (gdbcore.h, frame.h): Include.
> (arm_pc_is_thumb): Declare.
> (arm_linux_skip_solib_resolver): Fix printf() statement.  [Which
> shouldn't be there anyway.]
> (ARM_LINUX_SIGRETURN_INSTR, ARM_LINUX_RT_SIGRETURN_INSTR): New
> defines.
> (arm_linux_in_sigtramp, arm_linux_sigcontext_register_address):
> New functions.
>
> Index: arm-linux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-linux-tdep.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 arm-linux-tdep.c
> --- arm-linux-tdep.c 2000/05/25 18:24:33 1.5
> +++ arm-linux-tdep.c 2000/08/10 07:05:40
> @@ -23,12 +23,18 @@
>  #include "value.h"
>  #include "gdbtypes.h"
>  #include "floatformat.h"
> +#include "gdbcore.h"
> +#include "frame.h"
>
>  /* For arm_linux_skip_solib_resolver.  */
>  #include "symtab.h"
>  #include "symfile.h"
>  #include "objfiles.h"
>
> +/* FIXME: Put in common header file shared between arm-tdep.c and
> +   arm-linux-tdep.c */
> +int arm_pc_is_thumb (CORE_ADDR memaddr);
> +
>  #ifdef GET_LONGJMP_TARGET
>
>  /* Figure out where the longjmp will land.  We expect that we have
> @@ -425,12 +431,93 @@ arm_linux_skip_solib_resolver (CORE_ADDR
>
>    /* Plug in functions for other kinds of resolvers here.  */
>    result = skip_hurd_resolver (pc);
> -  printf ("Result = 0x%08x\n");
> +  printf ("Result = 0x%08lx\n", result);
>    if (result)
>      return result;
>
>
>    return 0;
> +}
> +
> +/* The constants below were determined by examining the following files
> +   in the linux kernel sources:
> +
> +      arch/arm/kernel/signal.c
> +   - see SWI_SYS_SIGRETURN and SWI_SYS_RT_SIGRETURN
> +      include/asm-arm/unistd.h
> +   - see __NR_sigreturn, __NR_rt_sigreturn, and __NR_SYSCALL_BASE */
> +
> +#define ARM_LINUX_SIGRETURN_INSTR 0xef900077
> +#define ARM_LINUX_RT_SIGRETURN_INSTR 0xef9000ad
> +
> +/* arm_linux_in_sigtramp determines if PC points at one of the
> +   instructions which cause control to return to the Linux kernel upon
> +   return from a signal handler.  FUNC_NAME is unused.  */
> +
> +int
> +arm_linux_in_sigtramp (CORE_ADDR pc, char *func_name)
> +{
> +  unsigned long inst;
> +
> +  inst = read_memory_integer (pc, 4);
> +
> +  return (inst == ARM_LINUX_SIGRETURN_INSTR
> +   || inst == ARM_LINUX_RT_SIGRETURN_INSTR);
> +
> +}
> +
> +/* arm_linux_sigcontext_register_address returns the address in the
> +   sigcontext of register REGNO given a stack pointer value SP and
> +   program counter value PC.  The value 0 is returned if PC is not
> +   pointing at one of the signal return instructions or if REGNO is
> +   not saved in the sigcontext struct.  */
> +
> +CORE_ADDR
> +arm_linux_sigcontext_register_address (CORE_ADDR sp, CORE_ADDR pc, int
regno)
> +{
> +  unsigned long inst;
> +  CORE_ADDR reg_addr = 0;
> +
> +  inst = read_memory_integer (pc, 4);
> +
> +  if (inst == ARM_LINUX_SIGRETURN_INSTR || inst ==
ARM_LINUX_RT_SIGRETURN_INSTR)
> +    {
> +      CORE_ADDR sigcontext_addr;
> +
> +      /* The sigcontext structure is at different places for the two
> +         signal return instructions.  For ARM_LINUX_SIGRETURN_INSTR,
> + it starts at the SP value.  For ARM_LINUX_RT_SIGRETURN_INSTR,
> + it is at SP+8.  For the latter instruction, it may also be
> + the case that the address of this structure may be determined
> + by reading the 4 bytes at SP, but I'm not convinced this is
> + reliable.
> +
> + In any event, these magic constants (0 and 8) may be
> + determined by examining struct sigframe and struct
> + rt_sigframe in arch/arm/kernel/signal.c in the Linux kernel
> + sources.  */
> +
> +      if (inst == ARM_LINUX_RT_SIGRETURN_INSTR)
> + sigcontext_addr = sp + 8;
> +      else /* inst == ARM_LINUX_SIGRETURN_INSTR */
> +        sigcontext_addr = sp + 0;
> +
> +      /* The layout of the sigcontext structure for ARM GNU/Linux is
> +         in include/asm-arm/sigcontext.h in the Linux kernel sources.
> +
> + There are three 4-byte fields which precede the saved r0
> + field.  (This accounts for the 12 in the code below.)  The
> + sixteen registers (4 bytes per field) follow in order.  The
> + PSR value follows the sixteen registers which accounts for
> + the constant 19 below. */
> +
> +      if (0 <= regno && regno <= PC_REGNUM)
> + reg_addr = sigcontext_addr + 12 + (4 * regno);
> +      else if (regno == PS_REGNUM)
> + reg_addr = sigcontext_addr + 19 * 4;
> +    }
> +
> +  return reg_addr;
>  }
>
>  void
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 arm-tdep.c
> --- arm-tdep.c 2000/06/08 00:52:56 1.7
> +++ arm-tdep.c 2000/08/10 07:05:43
> @@ -30,6 +30,31 @@
>  #include "dis-asm.h" /* For register flavors. */
>  #include <ctype.h> /* for isupper () */
>
> +/* Each OS has a different mechanism for accessing the various
> +   registers stored in the sigcontext structure.
> +
> +   SIGCONTEXT_REGISTER_ADDRESS should be defined to the name (or
> +   function pointer) which may be used to determine the addresses
> +   of the various saved registers in the sigcontext structure.
> +
> +   For the ARM target, there are three parameters to this function.
> +   The first is the pc value of the frame under consideration, the
> +   second the stack pointer of this frame, and the last is the
> +   register number to fetch.
> +
> +   If the tm.h file does not define this macro, then it's assumed that
> +   no mechanism is needed and we define SIGCONTEXT_REGISTER_ADDRESS to
> +   be 0.
> +
> +   When it comes time to multi-arching this code, see the identically
> +   named machinery in ia64-tdep.c for an example of how it could be
> +   done.  It should not be necessary to modify the code below where
> +   this macro is used.  */
> +
> +#ifndef SIGCONTEXT_REGISTER_ADDRESS
> +#define SIGCONTEXT_REGISTER_ADDRESS 0
> +#endif
> +
>  extern void _initialize_arm_tdep (void);
>
>  /* Number of different reg name sets (options). */
> @@ -226,7 +251,7 @@ static int caller_is_thumb;
>     function.  */
>
>  int
> -arm_pc_is_thumb (bfd_vma memaddr)
> +arm_pc_is_thumb (CORE_ADDR memaddr)
>  {
>    struct minimal_symbol *sym;
>
> @@ -250,7 +275,7 @@ arm_pc_is_thumb (bfd_vma memaddr)
>     dummy being called from a Thumb function.  */
>
>  int
> -arm_pc_is_thumb_dummy (bfd_vma memaddr)
> +arm_pc_is_thumb_dummy (CORE_ADDR memaddr)
>  {
>    CORE_ADDR sp = read_sp ();
>
> @@ -725,14 +750,42 @@ arm_scan_prologue (struct frame_info *fi
>       the symbol table, peek in the stack frame to find the PC.  */
>    if (find_pc_partial_function (fi->pc, NULL, &prologue_start,
&prologue_end))
>      {
> -      /* Assume the prologue is everything between the first instruction
> -         in the function and the first source line.  */
> -      struct symtab_and_line sal = find_pc_line (prologue_start, 0);
> +      /* One way to find the end of the prologue (which works well
> +         for unoptimized code) is to do the following:
>
> -      if (sal.line == 0) /* no line info, use current PC */
> - prologue_end = fi->pc;
> -      else if (sal.end < prologue_end) /* next line begins after fn end
*/
> - prologue_end = sal.end; /* (probably means no prologue)  */
> +     struct symtab_and_line sal = find_pc_line (prologue_start, 0);
> +
> +     if (sal.line == 0)
> +       prologue_end = fi->pc;
> +     else if (sal.end < prologue_end)
> +       prologue_end = sal.end;
> +
> + This mechanism is very accurate so long as the optimizer
> + doesn't move any instructions from the function body into the
> + prologue.  If this happens, sal.end will be the last
> + instruction in the first hunk of prologue code just before
> + the first instruction that the scheduler has moved from
> + the body to the prologue.
> +
> + In order to make sure that we scan all of the prologue
> + instructions, we use a slightly less accurate mechanism which
> + may scan more than necessary.  To help compensate for this
> + lack of accuracy, the prologue scanning loop below contains
> + several clauses which'll cause the loop to terminate early if
> + an implausible prologue instruction is encountered.
> +
> + The expression
> +
> +       prologue_start + 64
> +
> + is a suitable endpoint since it accounts for the largest
> + possible prologue plus up to five instructions inserted by
> + the scheduler. */
> +
> +      if (prologue_end > prologue_start + 64)
> + {
> +   prologue_end = prologue_start + 64; /* See above. */
> + }
>      }
>    else
>      {
> @@ -740,10 +793,7 @@ arm_scan_prologue (struct frame_info *fi
>           PC is the address of the stmfd + 8.  */
>        prologue_start = ADDR_BITS_REMOVE (read_memory_integer (fi->frame,
4))
>   - 8;
> -      prologue_end = prologue_start + 64; /* This is all the insn's
> -    that could be in the prologue,
> -    plus room for 5 insn's inserted
> -    by the scheduler.  */
> +      prologue_end = prologue_start + 64; /* See above. */
>      }
>
>    /* Now search the prologue looking for instructions that set up the
> @@ -833,6 +883,10 @@ arm_scan_prologue (struct frame_info *fi
>     fi->fsr.regs[fp_start_reg++] = sp_offset;
>   }
>       }
> +   else if ((insn & 0xf0000000) != 0xe0000000)
> +     break; /* Condition not true, exit early */
> +   else if ((insn & 0xfe200000) == 0xe8200000) /* ldm? */
> +     break; /* Don't scan past a block load */
>     else
>       /* The optimizer might shove anything into the prologue,
>          so we just skip what we don't recognize. */
> @@ -973,6 +1027,40 @@ arm_init_extra_frame_info (int fromleaf,
>      }
>    else
>  #endif
> +
> +  /* Determine whether or not we're in a sigtramp frame.
> +     Unfortunately, it isn't sufficient to test
> +     fi->signal_handler_caller because this value is sometimes set
> +     after invoking INIT_EXTRA_FRAME_INFO.  So we test *both*
> +     fi->signal_handler_caller and IN_SIGTRAMP to determine if we need
> +     to use the sigcontext addresses for the saved registers.
> +
> +     Note: If an ARM IN_SIGTRAMP method ever needs to compare against
> +     the name of the function, the code below will have to be changed
> +     to first fetch the name of the function and then pass this name
> +     to IN_SIGTRAMP. */
> +
> +  if (SIGCONTEXT_REGISTER_ADDRESS
> +      && (fi->signal_handler_caller || IN_SIGTRAMP (fi->pc, 0)))
> +    {
> +      CORE_ADDR sp;
> +
> +      if (!fi->next)
> + sp = read_sp();
> +      else
> + sp = fi->next->frame - fi->next->frameoffset + fi->next->framesize;
> +
> +      for (reg = 0; reg < NUM_REGS; reg++)
> + fi->fsr.regs[reg] = SIGCONTEXT_REGISTER_ADDRESS (sp, fi->pc, reg);
> +
> +      /* FIXME: What about thumb mode? */
> +      fi->framereg = SP_REGNUM;
> +      fi->frame = read_memory_integer (fi->fsr.regs[fi->framereg], 4);
> +      fi->framesize = 0;
> +      fi->frameoffset = 0;
> +
> +    }
> +  else
>      {
>        arm_scan_prologue (fi);
>
> Index: config/arm/tm-linux.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/arm/tm-linux.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 tm-linux.h
> --- tm-linux.h 2000/05/25 18:24:33 1.5
> +++ tm-linux.h 2000/08/10 07:05:44
> @@ -135,4 +135,26 @@ extern CORE_ADDR in_svr4_dynsym_resolve_
>  #define IN_SOLIB_DYNSYM_RESOLVE_CODE  in_svr4_dynsym_resolve_code */
>  #endif
>
> +/* When the ARM Linux kernel invokes a signal handler, the return
> +   address points at a special instruction which'll trap back into
> +   the kernel.  These definitions are used to identify this bit of
> +   code as a signal trampoline in order to support backtracing
> +   through calls to signal handlers. */
> +
> +int arm_linux_in_sigtramp (CORE_ADDR pc, char *name);
> +#define IN_SIGTRAMP(pc, name) arm_linux_in_sigtramp (pc, name)
> +
> +/* Each OS has different mechanisms for accessing the various
> +   registers stored in the sigcontext structure.  These definitions
> +   provide a mechanism by which the generic code in arm-tdep.c can
> +   find the addresses at which various registers are saved at in the
> +   sigcontext structure.  If SIGCONTEXT_REGISTER_ADDRESS is not
> +   defined, arm-tdep.c will define it to be 0.  (See ia64-tdep.c and
> +   ia64-linux-tdep.c to see what a similar mechanism looks like when
> +   multi-arched.) */
> +
> +extern CORE_ADDR arm_linux_sigcontext_register_address (CORE_ADDR,
CORE_ADDR,
> +                                                        int);
> +#define SIGCONTEXT_REGISTER_ADDRESS arm_linux_sigcontext_register_address
> +
>  #endif /* TM_ARMLINUX_H */
>
>