Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: <gdb-patches@sourceware.org>
Subject: [PATCH] Linux/gdbserver: Factor out usr_store_inferior_registers
Date: Tue, 22 Nov 2011 16:06:00 -0000	[thread overview]
Message-ID: <alpine.DEB.1.10.1111212008190.4191@tp.orcam.me.uk> (raw)

Hi,

 While working on a MIPS/Linux DSP support change I am about to submit I 
have tripped over usr_fetch_inferior_registers and 
usr_store_inferior_registers being structured in a different way even 
though the make complementing actions and there's no obvious reason for 
them to be designed like this.  Specifically usr_fetch_inferior_registers 
uses a helper to retrieve individual registers but 
usr_store_inferior_registers recurses into itself instead.

 The change below makes the two functions consistent, 
usr_store_inferior_registers now uses a store_register helper exactly like 
usr_fetch_inferior_registers uses fetch_register.

 No regressions on i686-linux-gnu or mips-linux-gnu (I had to force 
srv_linux_regsets to "no" and deal with some bitrot of that configuration 
to get indicative results).  OK to apply?

2011-11-22  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/gdbserver/
	* linux-low.c (usr_store_inferior_registers): Factor out code
	to handle individual registers into...
	(store_register): ... this new function.

  Maciej

gdb-gdbserver-linux-store-inferior.diff
Index: gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/gdbserver/linux-low.c	2011-11-17 19:36:30.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c	2011-11-17 19:47:38.345612518 +0000
@@ -3768,6 +3768,60 @@ fetch_register (struct regcache *regcach
     supply_register (regcache, regno, buf);
 }
 
+/* Store one register.  */
+static void
+store_register (struct regcache *regcache, int regno)
+{
+  CORE_ADDR regaddr;
+  int i, size;
+  char *buf;
+  int pid;
+
+  if (regno >= the_low_target.num_regs)
+    return;
+
+  if ((*the_low_target.cannot_store_register) (regno) == 1)
+    return;
+
+  regaddr = register_addr (regno);
+  if (regaddr == -1)
+    return;
+  errno = 0;
+  size = (register_size (regno) + sizeof (PTRACE_XFER_TYPE) - 1)
+	 & - sizeof (PTRACE_XFER_TYPE);
+  buf = alloca (size);
+  memset (buf, 0, size);
+
+  if (the_low_target.collect_ptrace_register)
+    the_low_target.collect_ptrace_register (regcache, regno, buf);
+  else
+    collect_register (regcache, regno, buf);
+
+  pid = lwpid_of (get_thread_lwp (current_inferior));
+  for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE))
+    {
+      errno = 0;
+      ptrace (PTRACE_POKEUSER, pid,
+	    /* Coerce to a uintptr_t first to avoid potential gcc warning
+	       about coercing an 8 byte integer to a 4 byte pointer.  */
+	      (PTRACE_ARG3_TYPE) (uintptr_t) regaddr,
+	      (PTRACE_ARG4_TYPE) *(PTRACE_XFER_TYPE *) (buf + i));
+      if (errno != 0)
+	{
+	  /* At this point, ESRCH should mean the process is
+	     already gone, in which case we simply ignore attempts
+	     to change its registers.  See also the related
+	     comment in linux_resume_one_lwp.  */
+	  if (errno == ESRCH)
+	    return;
+
+	  if ((*the_low_target.cannot_store_register) (regno) == 0)
+	    error ("writing register %d: %s", regno, strerror (errno));
+	}
+      regaddr += sizeof (PTRACE_XFER_TYPE);
+    }
+}
+
 /* Fetch all registers, or just one, from the child process.  */
 static void
 usr_fetch_inferior_registers (struct regcache *regcache, int regno)
@@ -3785,60 +3839,11 @@ usr_fetch_inferior_registers (struct reg
 static void
 usr_store_inferior_registers (struct regcache *regcache, int regno)
 {
-  CORE_ADDR regaddr;
-  int i, size;
-  char *buf;
-  int pid;
-
-  if (regno >= 0)
-    {
-      if (regno >= the_low_target.num_regs)
-	return;
-
-      if ((*the_low_target.cannot_store_register) (regno) == 1)
-	return;
-
-      regaddr = register_addr (regno);
-      if (regaddr == -1)
-	return;
-      errno = 0;
-      size = (register_size (regno) + sizeof (PTRACE_XFER_TYPE) - 1)
-	     & - sizeof (PTRACE_XFER_TYPE);
-      buf = alloca (size);
-      memset (buf, 0, size);
-
-      if (the_low_target.collect_ptrace_register)
-	the_low_target.collect_ptrace_register (regcache, regno, buf);
-      else
-	collect_register (regcache, regno, buf);
-
-      pid = lwpid_of (get_thread_lwp (current_inferior));
-      for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE))
-	{
-	  errno = 0;
-	  ptrace (PTRACE_POKEUSER, pid,
-		/* Coerce to a uintptr_t first to avoid potential gcc warning
-		   about coercing an 8 byte integer to a 4 byte pointer.  */
-		  (PTRACE_ARG3_TYPE) (uintptr_t) regaddr,
-		  (PTRACE_ARG4_TYPE) *(PTRACE_XFER_TYPE *) (buf + i));
-	  if (errno != 0)
-	    {
-	      /* At this point, ESRCH should mean the process is
-		 already gone, in which case we simply ignore attempts
-		 to change its registers.  See also the related
-		 comment in linux_resume_one_lwp.  */
-	      if (errno == ESRCH)
-		return;
-
-	      if ((*the_low_target.cannot_store_register) (regno) == 0)
-		error ("writing register %d: %s", regno, strerror (errno));
-	    }
-	  regaddr += sizeof (PTRACE_XFER_TYPE);
-	}
-    }
-  else
+  if (regno == -1)
     for (regno = 0; regno < the_low_target.num_regs; regno++)
-      usr_store_inferior_registers (regcache, regno);
+      store_register (regcache, regno);
+  else
+    store_register (regcache, regno);
 }
 #endif /* HAVE_LINUX_USRREGS */
 


             reply	other threads:[~2011-11-22 16:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-22 16:06 Maciej W. Rozycki [this message]
2011-12-06 19:14 ` Pedro Alves
2011-12-06 23:10   ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.1.10.1111212008190.4191@tp.orcam.me.uk \
    --to=macro@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox