Re: [rfa] Always define all of TARGET_SIGNAL_*

[Daniel Jacobowitz <drow@mvista.com>]

On Mon, Mar 11, 2002 at 11:00:56PM -0500, Andrew Cagney wrote:
> >On Mon, Mar 11, 2002 at 09:32:56PM -0500, Andrew Cagney wrote:
> >
> >>
> >
> >>>Now that we've got a "gdb/signals.h", there's a place to fix this
> >>>properly in the trunk.  How does this patch look?
> >
> >>
> >>Can I suggest sitting on this until after 5.2 is out!  If the change is 
> >>put on hold, then it will be easier to refresh the branch gdbserver from 
> >>the trunk.
> >>
> >>The addition of things like a src/include/gdb/ directory to the branch 
> >>shouldn't be an issue provided GDB proper doesn't try (get modified) to 
> >>use it.
> >
> >
> >I think you're thinking of my other pending signals patch?  This is the
> >one that has nothing to do with gdbserver, and I was just waiting for
> >the branch to be cut.  If you'd rather I just hold it a bit longer
> >that's fine by me, though.
> 
> I'm thinking of the net effect pulling both this patch and the other 
> pending signal patch onto the 5.1 would have.  (I'm not sure if you ment 
> this or that I was confusing this patch with the other pending patches :-)

The only patch I'd particularly like in 5.2 (not 5.1 :)) is the one to
make gdbserver use signals.h, which I was going to address after this
one was done (just to reduce confusion, not because of any dependence). 
I personally believe that pulling the others, particularly this one,
into 5.2 will do no more harm than pulling them into the trunk; every
day we leave them there is just a greater chance they'll be used this
way.

> Changing this enum scares me!

Me, too.  Especially since this patch is wrong; I forgot to include the
matching change in signals.c :(

> Anyway, looking at the change:
> 
> >-#if defined(MACH) || defined(__MACH__)
> >     /* Mach exceptions */
> >     TARGET_EXC_BAD_ACCESS,
> >     TARGET_EXC_BAD_INSTRUCTION,
> >@@ -216,7 +216,7 @@ enum target_signal
> >     TARGET_EXC_EMULATION,
> >     TARGET_EXC_SOFTWARE,
> >     TARGET_EXC_BREAKPOINT,
> >-#endif
> >+
> >     TARGET_SIGNAL_INFO,
> 
> I think the code formally wrapped in #if should be moved to after 
> TARGET_SIGNAL_INFO.
> 
> Try sketching out all the possible combinations of mach X host X target 
> X mach (I got confused after two but a guess is 8). I think you find 
> that either we scramble (fix and break) some of the mach cases xor we 
> break everything else.
> 
> Hmm, no wonder that enum scares me :-)
> 
> enjoy,
> Andrew
> 
> --
> 
> Ex: Hardwired remote target built for GNU/Linux but with signals 
> numbered to include the MACH values.  It only works with a MACH host. GDB.

Fortunately for us, I know of _no_ remote stub that sends any signal
this high.  Unless something is actively using SIGINFO that supports
remote debugging?  Must of the existing stubs are lower-level than
that.

Moving it after TARGET_SIGNAL_INFO is fine with me, though.  That'll
preserve the most non-Mach cases.  In that case, I think it is
appropriate to move it after TARGET_SIGNAL_UNKNOWN and
TARGET_SIGNAL_DEFAULT as well.  TARGET_SIGNAL_UNKNOWN is reasonably
likely to occur someday in a stub.

Revised patch attached; is this OK for the trunk?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2002-03-16  Daniel Jacobowitz  <drow@mvista.com>

	* signals.h: Update comments.
	(enum target_signal): Remove conditional compilation around
	Mach-specific signals.  Move them to after TARGET_SIGNAL_DEFAULT.

2002-03-16  Daniel Jacobowitz  <drow@mvista.com>

	* signals.c (signals): Remove conditional compilation around
	Mach-specific signals.  Move them to after TARGET_SIGNAL_DEFAULT.
	(target_signal_from_name): Loop until TARGET_SIGNAL_LAST.

Index: gdb/signals.c
===================================================================
RCS file: /cvs/src/src/gdb/signals.c,v
retrieving revision 1.3
diff -u -p -r1.3 signals.c
--- signals.c	2002/01/13 21:58:16	1.3
+++ signals.c	2002/03/17 02:20:58
@@ -1,6 +1,6 @@
 /* Target signal translation functions for GDB.
    Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
-   2000, 2001 Free Software Foundation, Inc.
+   2000, 2001, 2002 Free Software Foundation, Inc.
    Contributed by Cygnus Support.
 
    This file is part of GDB.
@@ -175,7 +175,11 @@ static struct {
   {"SIG126", "Real-time event 126"},
   {"SIG127", "Real-time event 127"},
 
-#if defined(MACH) || defined(__MACH__)
+  {"SIGINFO", "Information request"},
+
+  {NULL, "Unknown signal"},
+  {NULL, "Internal error: printing TARGET_SIGNAL_DEFAULT"},
+
   /* Mach exceptions */
   {"EXC_BAD_ACCESS", "Could not access memory"},
   {"EXC_BAD_INSTRUCTION", "Illegal instruction/operand"},
@@ -183,11 +187,6 @@ static struct {
   {"EXC_EMULATION", "Emulation instruction"},
   {"EXC_SOFTWARE", "Software generated exception"},
   {"EXC_BREAKPOINT", "Breakpoint"},
-#endif
-  {"SIGINFO", "Information request"},
-
-  {NULL, "Unknown signal"},
-  {NULL, "Internal error: printing TARGET_SIGNAL_DEFAULT"},
 
   /* Last entry, used to check whether the table is the right size.  */
   {NULL, "TARGET_SIGNAL_MAGIC"}
@@ -232,9 +231,10 @@ target_signal_from_name (char *name)
 
   /* This ugly cast brought to you by the native VAX compiler.  */
   for (sig = TARGET_SIGNAL_HUP;
-       signals[sig].name != NULL;
+       sig < TARGET_SIGNAL_LAST;
        sig = (enum target_signal) ((int) sig + 1))
-    if (STREQ (name, signals[sig].name))
+    if (signals[sig].name
+	&& strcmp (name, signals[sig].name) == 0)
       return sig;
   return TARGET_SIGNAL_UNKNOWN;
 }
Index: include/gdb/signals.h
===================================================================
RCS file: /cvs/src/src/include/gdb/signals.h,v
retrieving revision 1.1
diff -u -p -r1.1 signals.h
--- signals.h	2002/03/11 00:01:11	1.1
+++ signals.h	2002/03/17 02:20:58
@@ -32,7 +32,8 @@
    Since these numbers have actually made it out into other software
    (stubs, etc.), you mustn't disturb the assigned numbering.  If you
    need to add new signals here, add them to the end of the explicitly
-   numbered signals.
+   numbered signals, at the comment marker.  Add them unconditionally,
+   not within any #if or #ifdef.
 
    This is based strongly on Unix/POSIX signals for several reasons:
    (1) This set of signals represents a widely-accepted attempt to
@@ -208,15 +209,6 @@ enum target_signal
     TARGET_SIGNAL_REALTIME_126,
     TARGET_SIGNAL_REALTIME_127,
 
-#if defined(MACH) || defined(__MACH__)
-    /* Mach exceptions */
-    TARGET_EXC_BAD_ACCESS,
-    TARGET_EXC_BAD_INSTRUCTION,
-    TARGET_EXC_ARITHMETIC,
-    TARGET_EXC_EMULATION,
-    TARGET_EXC_SOFTWARE,
-    TARGET_EXC_BREAKPOINT,
-#endif
     TARGET_SIGNAL_INFO,
 
     /* Some signal we don't know about.  */
@@ -225,6 +217,18 @@ enum target_signal
     /* Use whatever signal we use when one is not specifically specified
        (for passing to proceed and so on).  */
     TARGET_SIGNAL_DEFAULT,
+
+    /* Mach exceptions.  In versions of GDB before 5.2, these were just before
+       TARGET_SIGNAL_INFO if you were compiling on a Mach host (and missing
+       otherwise).  */
+    TARGET_EXC_BAD_ACCESS,
+    TARGET_EXC_BAD_INSTRUCTION,
+    TARGET_EXC_ARITHMETIC,
+    TARGET_EXC_EMULATION,
+    TARGET_EXC_SOFTWARE,
+    TARGET_EXC_BREAKPOINT,
+
+    /* If you are adding a new signal, add it just above this comment.  */
 
     /* Last and unused enum value, for sizing arrays, etc.  */
     TARGET_SIGNAL_LAST