Re: [PATCH] [SPARC64] Figure out where a longjmp will land

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> From: jose.marchesi@oracle.com (Jose E. Marchesi)
> Date: Thu, 17 Oct 2013 13:33:43 +0200
> 
>     > 2013-10-08  Jose E. Marchesi  <jose.marchesi@oracle.com>
>     > 
>     > 	* sparc-tdep.c (sparc_is_annulled_branch_insn): New function.
>     > 	* sparc-tdep.h: And its prototype.
>     > 
>     > 	* sparc64-linux-tdep.c (sparc64_linux_get_longjmp_target): New
>     > 	function.
>     > 	(sparc64_linux_init_abi): Register the get_longjmp_target hook.
>     
>     This looks reasonable to me, with a minor correction requested below.
>     But Sparc patches normally get reviewed by MarkK, so can you give it
>     another week before checking the final version in? (remember that
>     when you check something in that is different from the patch already
>     posted, an updated patch should be sent to the list, for the
>     record).
> 
> Noted, but I don't have write access to the CVS so I can't check
> anything in.
>     
>     I assume that this was validated against our testsuite, and that
>     no new regressions were detected?
> 
> Yes.  I always run the testsuite for regressions before submitting any
> patch.  I tested this on sparc64-unknown-linux-gnu.
>     
>     > +/* Figure out where a longjmp will land.  Get the args out of the
>     > +   output registers.  We expect the first arg to be a pointer to the
>     > +   jmp_buf structure from which we extract the address that we will
>     > +   land at.  This address is copied into PC.  This routine returns
>     > +   non-zero on success.  */
>     > +
>     > +static int
>     > +sparc64_linux_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
>     
>     This function is expected to implement a gdbarch callback, so you do
>     not need to repeat the spec, which is expected to already be documented
>     in gdbarch.h. Instead, use:
>     
>         /* Implement the "get_longjmp_target" gdbarch method.  */
> 
> Amended patch below.
> 
> 2013-10-08  Jose E. Marchesi  <jose.marchesi@oracle.com>
> 
> 	* sparc-tdep.c (sparc_is_annulled_branch_insn): New function.
> 	* sparc-tdep.h: And its prototype.
> 
> 	* sparc64-linux-tdep.c (sparc64_linux_get_longjmp_target): New
> 	function.
> 	(sparc64_linux_init_abi): Register the get_longjmp_target hook.

Diff looks good to me, although I'm not really familliar with the
glibc longjmp implementation.  See my note about pinging Dave Miller.

One nit though...

> Index: sparc-tdep.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/sparc-tdep.h,v
> retrieving revision 1.33
> diff -u -r1.33 sparc-tdep.h
> --- sparc-tdep.h	1 Jan 2013 06:32:51 -0000	1.33
> +++ sparc-tdep.h	17 Oct 2013 11:27:05 -0000
> @@ -202,6 +202,12 @@
>  extern void sparc_collect_rwindow (const struct regcache *regcache,
>  				   CORE_ADDR sp, int regnum);
>  
> +\f
> +
> +extern int sparc_is_annulled_branch_insn (CORE_ADDR pc);
> +
> +\f
> +

Seems a bit excessive to put these additional ^L's in there.