Re: bug located (was: Re: gdb, libio, readline

[Roland McGrath]

My gcc-3.x cross-compiler does not produce that same bad code.
(My gcc build is from cvs as of March 15.)

Even if there is something wrong with the code, gcc is definitely behaving
badly.  Here is the code from your hurdselect.s that should correspond to
the initialization of INTTYPE.

.LM240:
        xorl    %eax, %eax
        andl    $-268369921, %eax
        orl     $65536, %eax
        movl    $0, -172(%ebp)
        movb    $2, -172(%ebp)
        movb    $32, -171(%ebp)
        movl    %eax, -172(%ebp)
        orb     $16, -169(%ebp)
        .stabn 68,0,309,.LM241-_hurd_select

That is just obviously bogus, no matter what it's supposed to be doing.
The first insn sets %eax to zero, so the andl does nothing and the orl has
the same effect as "movl $65536, %eax".  The next three instructions have
the effect of "movl $0x2020, -172(%ebp)", and then that location is
clobbered with the %eax value (65536).  Finally, the last instruction makes
the value at -172(%ebp) be 0x10002020.  Then nothing ever uses that stack
location at all.

So I definitely think you should report that as a gcc bug, though it might
well be a symptom of something already fixed in the current gcc code.

It's possible that the casts to int are not fully kosher in the
up-to-the-minute C standard.  I'm not really sure about that, though there
is absolutely no reason they should not be (there is aliasing going on
here, but it's all of the read-only variety).  Here is a patch to make it
use unions instead of casts.


Index: hurdselect.c
===================================================================
RCS file: /cvs/glibc/libc/hurd/hurdselect.c,v
retrieving revision 1.4
diff -u -b -p -r1.4 hurdselect.c
--- hurdselect.c        2001/10/28 22:10:26     1.4
+++ hurdselect.c        2002/03/31 21:31:10
@@ -64,6 +64,14 @@ _hurd_select (int nfds,
     } d[nfds];
   sigset_t oset;
 
+  union typeword               /* Use this to avoid unkosher casts.  */
+    {
+      mach_msg_type_t type;
+      uint32_t word;
+    };
+  assert (sizeof (union typeword) == sizeof (mach_msg_type_t));
+  assert (sizeof (uint32_t) == sizeof (mach_msg_type_t));
+
   if (sigmask && __sigprocmask (SIG_SETMASK, sigmask, &oset))
     return -1;
 
@@ -282,15 +290,15 @@ _hurd_select (int nfds,
          struct
            {
              mach_msg_header_t head;
-             mach_msg_type_t err_type;
+             union typeword err_type;
              error_t err;
            } error;
          struct
            {
              mach_msg_header_t head;
-             mach_msg_type_t err_type;
+             union typeword err_type;
              error_t err;
-             mach_msg_type_t result_type;
+             union typeword result_type;
              int result;
            } success;
        } msg;
@@ -303,13 +311,14 @@ _hurd_select (int nfds,
        {
          /* We got a message.  Decode it.  */
 #define IO_SELECT_REPLY_MSGID (21012 + 100) /* XXX */
-         const mach_msg_type_t inttype =
-           { MACH_MSG_TYPE_INTEGER_T, sizeof (MACH_MSG_TYPE_INTEGER_T) * 8,
-             1, 1, 0, 0 };
+         const union typeword inttype =
+         { type:
+           { MACH_MSG_TYPE_INTEGER_T, sizeof (integer_t) * 8, 1, 1, 0, 0 }
+         };
          if (msg.head.msgh_id == IO_SELECT_REPLY_MSGID &&
              msg.head.msgh_size >= sizeof msg.error &&
              !(msg.head.msgh_bits & MACH_MSGH_BITS_COMPLEX) &&
-             *(int *) &msg.error.err_type == *(int *) &inttype)
+             msg.error.err_type.word == inttype.word)
            {
              /* This is a properly formatted message so far.
                 See if it is a success or a failure.  */
@@ -323,7 +332,7 @@ _hurd_select (int nfds,
                }
              if (msg.error.err ||
                  msg.head.msgh_size != sizeof msg.success ||
-                 *(int *) &msg.success.result_type != *(int *) &inttype ||
+                 msg.success.result_type.word != inttype.word ||
                  (msg.success.result & SELECT_ALL) == 0)
                {
                  /* Error or bogus reply.  Simulate readiness.  */