[PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL

[PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL

[James Hogan]

PTRACE_PEEKUSER can return -1, which is usually used to determine whether
a system call has reported an error, so errno must be used alone to
determine whether an error occurred. However errno isn't modified by a
successful system call so it must be reset to a known value (0) before the
syscall call.

Add the missing errno reset when reading the DSP_CONTROL register in the
native MIPS Linux backend and the MIPS gdbserver backend.

gdb/:
	* mips-linux-nat.c (mips_linux_read_description): Reset errno to 0
	prior to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.

gdb/gdbserver/:
	* linux-mips-low.c (mips_read_description): Reset errno to 0 prior
	to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
---
Changes in v2:
 - Wrap ChangeLog entries to 74 columns (Maciej W. Rozycki).
---
 gdb/ChangeLog                  | 5 +++++
 gdb/gdbserver/ChangeLog        | 5 +++++
 gdb/gdbserver/linux-mips-low.c | 1 +
 gdb/mips-linux-nat.c           | 1 +
 4 files changed, 12 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8963878eb8ca..e9e3f9e734e6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2014-09-01  James Hogan  <james.hogan@imgtec.com>
+
+	* mips-linux-nat.c (mips_linux_read_description): Reset errno to 0
+	prior to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
+
 2014-09-01  Maciej W. Rozycki  <macro@codesourcery.com>
 
 	* varobj.c (_initialize_varobj): Move to the end of file.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index caa243da5d7d..2fa9ec74c08a 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2014-09-01  James Hogan  <james.hogan@imgtec.com>
+
+	* linux-mips-low.c (mips_read_description): Reset errno to 0 prior
+	to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
+
 2014-08-29  Gary Benson  <gbenson@redhat.com>
 
 	* server.h (setjmp.h): Do not include.
diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index 1b2160b35371..377284be5422 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -128,6 +128,7 @@ mips_read_description (void)
     {
       int pid = lwpid_of (current_inferior);
 
+      errno = 0;
       ptrace (PTRACE_PEEKUSER, pid, DSP_CONTROL, 0);
       switch (errno)
 	{
diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
index 88faa1edf692..ffc8411ca3bc 100644
--- a/gdb/mips-linux-nat.c
+++ b/gdb/mips-linux-nat.c
@@ -439,6 +439,7 @@ mips_linux_read_description (struct target_ops *ops)
       if (tid == 0)
 	tid = ptid_get_pid (inferior_ptid);
 
+      errno = 0;
       ptrace (PTRACE_PEEKUSER, tid, DSP_CONTROL, 0);
       switch (errno)
 	{
-- 
1.8.5.5

Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL

[Maciej W. Rozycki]

On Mon, 1 Sep 2014, James Hogan wrote:

> PTRACE_PEEKUSER can return -1, which is usually used to determine whether
> a system call has reported an error, so errno must be used alone to
> determine whether an error occurred. However errno isn't modified by a
> successful system call so it must be reset to a known value (0) before the
> syscall call.
> 
> Add the missing errno reset when reading the DSP_CONTROL register in the
> native MIPS Linux backend and the MIPS gdbserver backend.
> 
> gdb/:
> 	* mips-linux-nat.c (mips_linux_read_description): Reset errno to 0
> 	prior to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
> 
> gdb/gdbserver/:
> 	* linux-mips-low.c (mips_read_description): Reset errno to 0 prior
> 	to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.

 This is fine, go ahead and apply it.  Thanks.

  Maciej

Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL

[James Hogan]

On Wed, Sep 03, 2014 at 01:41:29PM +0100, Maciej W. Rozycki wrote:
> On Mon, 1 Sep 2014, James Hogan wrote:
> 
> > PTRACE_PEEKUSER can return -1, which is usually used to determine whether
> > a system call has reported an error, so errno must be used alone to
> > determine whether an error occurred. However errno isn't modified by a
> > successful system call so it must be reset to a known value (0) before the
> > syscall call.
> > 
> > Add the missing errno reset when reading the DSP_CONTROL register in the
> > native MIPS Linux backend and the MIPS gdbserver backend.
> > 
> > gdb/:
> > 	* mips-linux-nat.c (mips_linux_read_description): Reset errno to 0
> > 	prior to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
> > 
> > gdb/gdbserver/:
> > 	* linux-mips-low.c (mips_read_description): Reset errno to 0 prior
> > 	to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
> 
>  This is fine, go ahead and apply it.  Thanks.

Thanks for reviewing it Maciej.

This is my first GDB patch submitted upstream (although I have a pile of
RFC patches for FR=1, FRE=1, MSA support I'm still getting into shape),
so I don't have push access yet. Is it easy to arrange?

Thanks
James

Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL

[Maciej W. Rozycki]

On Wed, 3 Sep 2014, James Hogan wrote:

> > > gdb/:
> > > 	* mips-linux-nat.c (mips_linux_read_description): Reset errno to 0
> > > 	prior to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
> > > 
> > > gdb/gdbserver/:
> > > 	* linux-mips-low.c (mips_read_description): Reset errno to 0 prior
> > > 	to reading DSP_CONTROL with PTRACE_PEEKUSER ptrace call.
> > 
> >  This is fine, go ahead and apply it.  Thanks.
> 
> Thanks for reviewing it Maciej.
> 
> This is my first GDB patch submitted upstream (although I have a pile of
> RFC patches for FR=1, FRE=1, MSA support I'm still getting into shape),
> so I don't have push access yet. Is it easy to arrange?

 Sure, that can be arranged, but you need to prove yourself by submitting 
a few more good patches first though.  And assuming you've got a copyright 
assignment in place with FSF, either personally or on behalf of your 
employer (I presume you do, but please confirm as you start sending larger 
changes).  You'll have to speak with one of the general maintainers when 
the right time comes.

 I have applied your change now, thanks.

 BTW, I've had some FR=1 stuff ongoing too, though regrettably stalled 
recently, see:

https://sourceware.org/bugzilla/show_bug.cgi?id=7518
http://sourceware.org/ml/gdb-patches/2012-06/msg00201.html

-- based on an old patch from MIPS Technologies that wasn't quite there, 
but still was a good starting point.  Bare iron only of course as there 
was no FR=1 Linux ABI up until recently.  It will be interesting to see 
how the two pieces compare.  Or actually it may make sense if I push my 
piece first, there was just some concern about regcache that remained
unresolved, so I'll see if I can reintegrate that change against current
trunk, retest and repost.

  Maciej

Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL

[Sergio Durigan Junior]

On Wednesday, September 03 2014, Maciej W. Rozycki wrote:

>  Sure, that can be arranged, but you need to prove yourself by submitting 
> a few more good patches first though.  And assuming you've got a copyright 
> assignment in place with FSF, either personally or on behalf of your 
> employer (I presume you do, but please confirm as you start sending larger 
> changes).  You'll have to speak with one of the general maintainers when 
> the right time comes.

Late notice: if you don't have copyright assignment on file, send me an
e-mail offlist and I can get you started on the process.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/