Re: [RFA] Improve Sparc epilogue analysis

[Michael Snyder <msnyder at redhat dot com>]

"David S. Miller" wrote:
> 
>    From: Michael Snyder <msnyder@redhat.com>
>    Date: Tue, 23 Apr 2002 12:19:57 -0700
> 
>    No.  Examine prologue serves two roles.  One is simply
>    to skip to the end of the prologue, but the other is to
>    actually determine where the registers are saved by the
>    prologue.  You've short-circuited the second usage.
> 
>    Your idea is sound, though -- you've just implemented
>    it in the wrong place.  Instead of modifying examine_prologue,
>    how about if you modify sparc_gdbarch_skip_prologue?
> 
> Right.  There is even extra confusion arising from the
> fact that we have two functions for the same thing.
> 
> These are sparc_skip_prologue and sparc_gdbarch_skip_prologue.
> 
> I killed the latter, and we can "static" the former
> when all the sparc targets are multi-arch'd.
> 
> How does this revised patch look?

Not quite.  I think you're trying to do too much in one go.
Slow down, and make one change at a time.

In this case, I don't believe you got the frameless_p case 
right.  First, you don't seem to have changed the definition
of SKIP_PROLOGUE_FRAMELESS_P in tm-sparc.h.  Second, your
implementation of sparc_prologue_frameless_p is not 
functionally equivalent to the old one.  If it goes thru
the new, symbolic path, it will not return the same result.

Please try handling the simple rewrite of sparc_skip_prologue
first, and then work on combining the two functions in a
separate pass.  Smaller and simpler patches are also much
easier to review.

> 
> 2002-04-23  David S. Miller  <davem@redhat.com>
> 
>         * sparc-tdep.c (sparc_gdbarch_skip_prologue): Kill, duplicates
>         sparc_skip_prologue.
>         (sparc_skip_prologue): Kill frameless_p arg, and use line number
>         information to find prologue when possible.
>         (sparc_prologue_frameless_p): Use line number information.
>         (sparc_gdbarch_init): Update set_gdbarch_skip_prologue call.
>         (sparc_init_extra_frame_info): Use sparc_skip_prologue to find
>         prologue bounds instead of looking at line number info by hand.
>         * config/sparc/tm-sparc.h (sparc_skip_prologue): Update for killed
>         second argument.
>         (SKIP_PROLOGUE): Likewise.
> 
> --- ./config/sparc/tm-sparc.h.~1~       Sun Apr 21 19:18:59 2002
> +++ ./config/sparc/tm-sparc.h   Tue Apr 23 22:38:43 2002
> @@ -250,8 +250,8 @@ extern int sparc_intreg_size (void);
>  /* Advance PC across any function entry prologue instructions
>     to reach some "real" code.  */
> 
> -extern CORE_ADDR sparc_skip_prologue (CORE_ADDR, int);
> -#define SKIP_PROLOGUE(PC) sparc_skip_prologue (PC, 0)
> +extern CORE_ADDR sparc_skip_prologue (CORE_ADDR);
> +#define SKIP_PROLOGUE(PC) sparc_skip_prologue (PC)
> 
>  /* Immediately after a function call, return the saved pc.
>     Can't go through the frames for this because on some machines
> --- ./sparc-tdep.c.~1~  Sun Apr 21 20:51:34 2002
> +++ ./sparc-tdep.c      Tue Apr 23 23:00:46 2002
> @@ -279,6 +279,8 @@ struct frame_extra_info
>    int sp_offset;
>  };
> 
> +CORE_ADDR sparc_skip_prologue (CORE_ADDR);
> +
>  /* Call this for each newly created frame.  For SPARC, we need to
>     calculate the bottom of the frame, and do some extra work if the
>     prologue has been generated via the -mflat option to GCC.  In
> @@ -393,25 +395,20 @@ sparc_init_extra_frame_info (int fromlea
>              to the current value of the stack pointer and set
>              the in_prologue flag.  */
>           CORE_ADDR addr;
> -         struct symtab_and_line sal;
> 
> -         sal = find_pc_line (prologue_start, 0);
> -         if (sal.line == 0)    /* no line info, use PC */
> -           prologue_end = fi->pc;
> -         else if (sal.end < prologue_end)
> -           prologue_end = sal.end;
> +         prologue_end = sparc_skip_prologue (prologue_start);
>           if (fi->pc < prologue_end)
>             {
> -             for (addr = prologue_start; addr < fi->pc; addr += 4)
> +             for (addr = fi->pc; addr < prologue_end; addr += 4)
>                 {
>                   insn = read_memory_integer (addr, 4);
>                   if (X_OP (insn) == 2 && X_OP3 (insn) == 0x3c)
>                     break;      /* SAVE seen, stop searching */
>                 }
> -             if (addr >= fi->pc)
> +             if (addr < prologue_end)
>                 {
>                   fi->extra_info->in_prologue = 1;
> -                 fi->frame = read_register (SP_REGNUM);
> +                 fi->frame = read_sp ();
>                 }
>             }
>         }
> @@ -685,18 +682,50 @@ examine_prologue (CORE_ADDR start_pc, in
>    return pc;
>  }
> 
> +/* Advance PC across any function entry prologue instructions to reach
> +   some "real" code.  */
> +
>  CORE_ADDR
> -sparc_skip_prologue (CORE_ADDR start_pc, int frameless_p)
> +sparc_skip_prologue (CORE_ADDR start_pc)
>  {
> -  return examine_prologue (start_pc, frameless_p, NULL, NULL);
> +  struct symtab_and_line sal;
> +  CORE_ADDR func_start, func_end;
> +
> +  /* This is the preferred method, find the end of the prologue by
> +     using the debugging information.  */
> +  if (find_pc_partial_function (start_pc, NULL, &func_start, &func_end))
> +    {
> +      sal = find_pc_line (func_start, 0);
> +
> +      if (sal.end < func_end
> +         && start_pc <= sal.end)
> +       return sal.end
> +    }
> +
> +  /* Oh well, examine the code by hand.  */
> +  return examine_prologue (start_pc, 0, NULL, NULL);
>  }
> 
>  /* Is the prologue at IP frameless?  */
> 
>  int
> -sparc_prologue_frameless_p (CORE_ADDR ip)
> +sparc_prologue_frameless_p (CORE_ADDR start_pc)
>  {
> -  return ip == sparc_skip_prologue (ip, 1);
> +  struct symtab_and_line sal;
> +  CORE_ADDR func_start, func_end;
> +
> +  /* This is the preferred method, find the end of the prologue by
> +     using the debugging information.  */
> +  if (find_pc_partial_function (start_pc, NULL, &func_start, &func_end))
> +    {
> +      sal = find_pc_line (func_start, 0);
> +
> +      if (sal.end < func_end
> +         && start_pc <= sal.end)
> +       return start_pc == sal.end;
> +    }
> +
> +  return start_pc == examine_prologue (start_pc, 1, NULL, NULL);
>  }
> 
>  /* Check instruction at ADDR to see if it is a branch.
> @@ -2784,15 +2813,6 @@ sparc64_register_byte (int regno)
>      return 64 * 8 + (regno - 80) * 8;
>  }
> 
> -/* Advance PC across any function entry prologue instructions to reach
> -   some "real" code.  */
> -
> -static CORE_ADDR
> -sparc_gdbarch_skip_prologue (CORE_ADDR ip)
> -{
> -  return examine_prologue (ip, 0, NULL, NULL);
> -}
> -
>  /* Immediately after a function call, return the saved pc.
>     Can't go through the frames for this because on some machines
>     the new frame is not set up until the new function executes
> @@ -2993,7 +3013,7 @@ sparc_gdbarch_init (struct gdbarch_info
>    set_gdbarch_saved_pc_after_call (gdbarch, sparc_saved_pc_after_call);
>    set_gdbarch_prologue_frameless_p (gdbarch, sparc_prologue_frameless_p);
>    set_gdbarch_short_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> -  set_gdbarch_skip_prologue (gdbarch, sparc_gdbarch_skip_prologue);
> +  set_gdbarch_skip_prologue (gdbarch, sparc_skip_prologue);
>    set_gdbarch_sp_regnum (gdbarch, SPARC_SP_REGNUM);
>    set_gdbarch_use_generic_dummy_frames (gdbarch, 0);
>    set_gdbarch_write_pc (gdbarch, generic_target_write_pc);