This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] [SPARC64] Figure out where a longjmp will land
- From: Mark Kettenis <mark dot kettenis at xs4all dot nl>
- To: jose dot marchesi at oracle dot com
- Cc: brobecker at adacore dot com, tromey at redhat dot com, gdb-patches at sourceware dot org
- Date: Sun, 20 Oct 2013 21:32:16 +0200 (CEST)
- Subject: Re: [PATCH] [SPARC64] Figure out where a longjmp will land
- Authentication-results: sourceware.org; auth=none
- References: <87y563k71b dot fsf at oracle dot com> <87d2nffr53 dot fsf at fleche dot redhat dot com> <87txgrjupm dot fsf at oracle dot com> <20131017055736 dot GC3302 at adacore dot com> <87r4bk1694 dot fsf at oracle dot com>
> 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);
>
> +
> +
> +extern int sparc_is_annulled_branch_insn (CORE_ADDR pc);
> +
> +
> +
Seems a bit excessive to put these additional ^L's in there.