Date: Wed, 3 Feb 2010 18:56:20 -0800 (PST) From: Neelkanth Natu <neelnatu@yahoo.com> To: "C. Jayachandran" <c.jayachandran@gmail.com> Cc: freebsd-mips@freebsd.org Subject: Re: mips ptrace.S fix Message-ID: <940379.6969.qm@web34402.mail.mud.yahoo.com> In-Reply-To: <98a59be81002031809p70f0154dnd9a1744ade7aac33@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi JC,
--- On Wed, 2/3/10, C. Jayachandran <c.jayachandran@gmail.com> wrote:
> From: C. Jayachandran <c.jayachandran@gmail.com>
> Subject: Re: mips ptrace.S fix
> To: "Neelkanth Natu" <neelnatu@yahoo.com>
> Cc: "Rui Paulo" <rpaulo@freebsd.org>, freebsd-mips@freebsd.org
> Date: Wednesday, February 3, 2010, 6:09 PM
> On Thu, Feb 4, 2010 at 1:01 AM,
> Neelkanth Natu <neelnatu@yahoo.com>
> wrote:
> > Your patch looks good. I have a few comments though.
> See inline:
> >
> > Index: lib/libc/mips/sys/ptrace.S
> >
> ===================================================================
> > --- lib/libc/mips/sys/ptrace.S (revision 203379)
> > +++ lib/libc/mips/sys/ptrace.S (working copy)
> > @@ -42,14 +42,26 @@
> > #endif /* LIBC_SCCS and not lint */
> >
> > LEAF(ptrace)
> > + .frame sp,40,ra
> >
> >>> space missing after the ','
> >
> > + .mask 0x80000000, -8
> > #ifdef __ABICALLS__
> > .set noreorder
> > .cpload t9
> > .set reorder
> > #endif
> > + subu sp, sp, 40
> > + sw ra, 32(sp)
> > +#ifdef __ABICALLS__
> > + .cprestore 16
> > +#endif
> > la t9, _C_LABEL(__error) #
> locate address of errno
> > - jalr t9
> > + jalr t9
> >
> >>> this change is not required - the newly added
> line has a tab at the end.
> >
> > +#ifdef __ABICALLS__
> > + lw gp, 16(sp)
> > +#endif
> >
> >>> this is redundant - the assembler will
> generate exactly the same line of
> >>> code due to the .cprestore directive above.
>
> Are you sure about this? I think the assembler
> generates the gp load
> only for the 'jal' and 'bal' not for 'jalr'.
>
> Probably the two lines(la and jalr) can be replaced by a
> jal.
>
You are right. My eyes glossed over between the 'jal' and 'jalr'. I think
combining the two instructions into a single 'jal' is the way to go.
best
Neel
> Regards,
> JC.
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?940379.6969.qm>