This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] MIPS Linux signals


On 06/06/2012 03:52 PM, Maciej W. Rozycki wrote:
On Fri, 1 Jun 2012, Michael Eager wrote:

Some of the awkward code in translating REALTIME signals is due to
the fact that the GDB_SIGNAL_REALTIME block is not contiguous.

OK, fair enough, it's always good to include any such explanations with patch submissions so as to make the review process easier for the reviewers. It makes sense to note the peculiarity around code concerned too.

I recast the section of code copied from signals.c in terms of
MIPS_SIGRTMIN&  MIPS_SIGRTMAX, but it isn't clear to me that this
is an improvement, since it is now different from the similar code
in signals.c.

This is target-specific code, its goal is not to be as close as to generic code as possible, but (as with any code, except where critical performance or any other requirements mandate otherwise) straightforward and matching the requirements of the specific target concerned.

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14309
diff -u -p -r1.14309 ChangeLog
--- gdb/ChangeLog	1 Jun 2012 16:37:56 -0000	1.14309
+++ gdb/ChangeLog	1 Jun 2012 22:43:51 -0000
@@ -1,3 +1,8 @@
+2012-06-01  Michael Eager<eager@eagercon.com>
+
+	* mips-linux-tdep.c (mips_gdb_signal_from_target): New
+	* mips-linux-tdep.h (mips_signals): New
+
  2012-06-01  Siddhesh Poyarekar<siddhesh@redhat.com>

* target.c (target_read_memory): Make LEN argument as size_t.

Please supply ChangeLog entries separately, they're not going to work when included as a diff and will require hand-editing anyway.

I forgot to edit the ChangeLog out of the diff.



Index: gdb/mips-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v
retrieving revision 1.94
diff -u -p -r1.94 mips-linux-tdep.c
--- gdb/mips-linux-tdep.c	22 May 2012 17:12:07 -0000	1.94
+++ gdb/mips-linux-tdep.c	1 Jun 2012 22:43:51 -0000
@@ -1330,6 +1331,98 @@ mips_linux_get_syscall_number (struct gd
[...]
+
+  if (signo>= MIPS_SIGRTMIN&&  signo<= MIPS_SIGRTMAX)
+    {
+      /* This block of GDB_SIGNAL_REALTIME value is in order.  */
+      if (MIPS_SIGRTMIN<= signo&&  signo<= (MIPS_SIGRTMIN + 32))
+	return ((enum gdb_signal)
+		(signo - MIPS_SIGRTMIN + 1 + (int) GDB_SIGNAL_REALTIME_33));
+      else if (signo == MIPS_SIGRTMIN)
+	return GDB_SIGNAL_REALTIME_32;
+      else if ((MIPS_SIGRTMIN + 32)<= signo&&  signo<= MIPS_SIGRTMAX)
+	return ((enum gdb_signal)
+		(signo - (MIPS_SIGRTMIN + 32) + (int) GDB_SIGNAL_REALTIME_64));
+      else
+	error ("GDB bug: unrecognized real-time signal");
+    }
+
+  return GDB_SIGNAL_UNKNOWN;
+}
+
  /* Initialize one of the GNU/Linux OS ABIs.  */

static void

I find this unreadable (and buggy too, which is likely a consequence), per observations above please rewrite this as below:

   if (signo>= MIPS_SIGRTMIN&&  signo<= MIPS_SIGRTMAX)
     {
       /* GDB_SIGNAL_REALTIME values are not contiguous, map parts of
          the MIPS block to the respective GDB_SIGNAL_REALTIME blocks.  */
       signo -= MIPS_SIGRTMIN;
       if (signo == 0)
	return GDB_SIGNAL_REALTIME_32;
       else if (signo<  32)
	return ((enum gdb_signal) (signo - 1 + (int) GDB_SIGNAL_REALTIME_33));
       else
	return ((enum gdb_signal) (signo - 32 + (int) GDB_SIGNAL_REALTIME_64));
     }

   return GDB_SIGNAL_UNKNOWN;
}

OK.



-- the casts are probably redundant, but let them stay.


  OK with this change, the rest is fine with me.  Thanks for tackling this
problem.

Thanks.



-- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]