Re: [patch/rfc] signal trampoline frames

[Daniel Jacobowitz <drow@false.org>]

On Sun, Mar 21, 2004 at 10:14:17PM -0500, Andrew Cagney wrote:
> Hello,
> 
> This adds "tramp-frame.[hc]" as a generic signal trampoline frame 
> unwinder.  The client architectures, per the revised change to 
> ppcnbsd-tdep.c (it superseeds my original), just needs to provide the 
> signal trampoline instruction sequence and a method to initialize the 
> trad_frame_cache.
> 
> Thoughts, I'll look to commit the tramp-frame parts in a few days.

Some minor comments.

> +static CORE_ADDR
> +tramp_frame_start (CORE_ADDR pc, const struct tramp_frame *tramp)
> +{
> +  int ti;
> +  /* Search through the trampoline for one that matches the
> +     instruction sequence around PC.  */
> +  for (ti = 0; tramp->insn[ti] != 0; ti++)
> +    {
> +      CORE_ADDR func = pc - tramp->insn_size * ti;
> +      int i;
> +      for (i = 0; 1; i++)
> +	{
> +	  bfd_byte buf[sizeof (LONGEST)];
> +	  CORE_ADDR insn;

tramp->insn is a ULONGEST.  Both of these should probably be ULONGEST
also.

> +	  if (tramp->insn[i] == 0)
> +	    return func;

So zeros in tramp->insn mark the end of the sequence?  Should document
that, zeros are valid instructions and some bizarre architecture might
use one as a syscall trap.

> +  /* If the function has a valid symbol name, it isn't a
> +     trampoline.  */
> +  find_pc_partial_function (pc, &name, NULL, NULL);
> +  if (name != NULL)
> +    return 0;
> +  /* If the function lives in a valid section (even without a starting
> +     point) it isn't a trampoline.  */
> +  if (find_pc_section (pc) != NULL)
> +    return 0;

I believe the first check is redundant to the second check; can we have
names without sections?  I may be wrong about this, remembering the
problem on IRIX with absolute sections.



-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer