[go]: Port to ALPHA arch - epoll problems

[go]: Port to ALPHA arch - epoll problems

[Uros Bizjak]

On Tue, Jul 5, 2011 at 10:12 PM, Ian Lance Taylor <iant@google.com> wrote:

>> What remains is a couple of unrelated failures in the testsuite:
>>
>> Epoll unexpected fd=0
>> pollServer: unexpected wakeup for fd=0 mode=w
>> panic: test timed out
>> ../../../gcc-svn/trunk/libgo/testsuite/gotest: line 388:  7123 Aborted
>>                 ./a.out -test.short -test.timeout=$timeout "$@"
>> FAIL: http
>> gmake[2]: *** [http/check] Error 1
>>
>> 2011/07/05 18:43:28 Test RPC server listening on 127.0.0.1:50334
>> 2011/07/05 18:43:28 Test HTTP RPC server listening on 127.0.0.1:49010
>> 2011/07/05 18:43:28 rpc.Serve: accept:accept tcp 127.0.0.1:50334:
>> Resource temporarily unavailable
>> FAIL: rpc
>> gmake[2]: *** [rpc/check] Error 1
>>
>> 2011/07/05 18:44:22 Test WebSocket server listening on 127.0.0.1:40893
>> Epoll unexpected fd=0
>> pollServer: unexpected wakeup for fd=0 mode=w
>> panic: test timed out
>> ../../../gcc-svn/trunk/libgo/testsuite/gotest: line 388: 12993 Aborted
>>                 ./a.out -test.short -test.timeout=$timeout "$@"
>> FAIL: websocket
>> gmake[2]: *** [websocket/check] Error 1
>>
>> ../../../gcc-svn/trunk/libgo/testsuite/gotest: line 388: 13945
>> Segmentation fault      ./a.out -test.short -test.timeout=$timeout
>> "$@"
>> FAIL: compress/flate
>> gmake[2]: *** [compress/flate/check] Error 1
>>
>> Any ideas how to attack these?
>
> None of these look familiar to me.
>
> An "Epoll unexpected fd" error means that epoll returned information
> about a file descriptor which the program didn't ask about.  Not sure
> why that would happen.  Particularly for fd 0, since epoll is only used
> for network connections, which fd 0 presumably is not.
>
> The way to look into these is to cd to TARGET/libgo and run "make
> GOTESTFLAGS=--keep http/check" (or whatever/check).  That will leave a
> directory gotestNNNN in your libgo directory.  The executable a.out in
> that directory is the test case.  You can debug the test case using gdb
> in more or less the usual way.  It's a bit painful to set breakpoints by
> function name, but setting breakpoints by file:line works fine.
> Printing variables works as well as it ever does, but the variables are
> printed in C form rather than Go form.

It turned out that the EpollEvent definition in
libgo/syscalls/epoll/socket_epoll.go is non-portable (if not outright
dangerous...). The definition does have a FIXME comment, but does not
take into account the effects of __attribute__((__packed__)) from
system headers. Contrary to alpha header, x86 has
__attribute__((__packed__)) added to struct epoll_event definition in
sys/epoll.h header.

To illustrate the problem, please run following test:

--cut here--
#include <stdint.h>
#include <stdio.h>

typedef union epoll_data
{
  void *ptr;
  int fd;
  uint32_t u32;
  uint64_t u64;
} epoll_data_t;

struct epoll_event
{
  uint32_t events;
  epoll_data_t data;
};

struct packed_epoll_event
{
  uint32_t events;
  epoll_data_t data;
} __attribute__ ((__packed__));

struct fake_epoll_event
{
  uint32_t events;
  int32_t fd;
  int32_t pad;
};

int
main ()
{
  struct epoll_event *ep;
  struct packed_epoll_event *pep;

  struct fake_epoll_event fep;

  fep.events = 0xfe;
  fep.fd = 9;
  fep.pad = 0;

  ep = (struct epoll_event *) &fep;
  pep = (struct packed_epoll_event *) &fep;

  printf ("%#x %i\n", ep->events, ep->data.fd);
  printf ("%#x %i\n", pep->events, pep->data.fd);
  return 0;
}
--cut here--

./a.out
0xfe 0
0xfe 9

So, the first line simulates the alpha, the second simulates x86_64.
32bit targets are OK in both cases:

./a.out
0xfe 9
0xfe 9

By changing the definition of EpollEvent to the form that suits alpha:

type EpollEvent struct {
  Events uint32;
  Pad int32;
  Fd int32;
};

both timeouts got fixed and correct FD was passed to and from the syscall.

Uros.

Re: [go]: Port to ALPHA arch - epoll problems

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

Uros Bizjak <ubizjak@gmail.com> writes:

> It turned out that the EpollEvent definition in
> libgo/syscalls/epoll/socket_epoll.go is non-portable (if not outright
> dangerous...). The definition does have a FIXME comment, but does not
> take into account the effects of __attribute__((__packed__)) from
> system headers. Contrary to alpha header, x86 has
> __attribute__((__packed__)) added to struct epoll_event definition in
> sys/epoll.h header.

I couldn't work out a way to handle this correctly in mksysinfo.sh or
-fdump-go-spec, so I did it in configure instead.  Bootstrapped and
tested on x86_64-unknown-linux-gnu.  Committed to mainline.  Let me know
if it seems to do the right sort of thing on Alpha GNU/Linux--see if the
generated file TARGET/libgo/epoll.h looks OK.

Ian


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 3527 bytes --]

Index: libgo/configure.ac
===================================================================
--- libgo/configure.ac	(revision 180345)
+++ libgo/configure.ac	(working copy)
@@ -505,6 +505,28 @@ CFLAGS="$CFLAGS -D_LARGEFILE_SOURCE -D_L
 AC_CHECK_TYPES(off64_t)
 CFLAGS=$CFLAGS_hold
 
+dnl Work out the size of the epoll_events struct on GNU/Linux.
+AC_CACHE_CHECK([epoll_event size],
+[libgo_cv_c_epoll_event_size],
+[AC_COMPUTE_INT(libgo_cv_c_epoll_event_size,
+[sizeof (struct epoll_event)],
+[#include <sys/epoll.h>],
+[libgo_cv_c_epoll_event_size=0])])
+SIZEOF_STRUCT_EPOLL_EVENT=${libgo_cv_c_epoll_event_size}
+AC_SUBST(SIZEOF_STRUCT_EPOLL_EVENT)
+
+dnl Work out the offset of the fd field in the epoll_events struct on
+dnl GNU/Linux.
+AC_CACHE_CHECK([epoll_event data.fd offset],
+[libgo_cv_c_epoll_event_fd_offset],
+[AC_COMPUTE_INT(libgo_cv_c_epoll_event_fd_offset,
+[offsetof (struct epoll_event, data.fd)],
+[#include <stddef.h>
+#include <sys/epoll.h>],
+[libgo_cv_c_epoll_event_fd_offset=0])])
+STRUCT_EPOLL_EVENT_FD_OFFSET=${libgo_cv_c_epoll_event_fd_offset}
+AC_SUBST(STRUCT_EPOLL_EVENT_FD_OFFSET)
+
 AC_CACHE_SAVE
 
 if test ${multilib} = yes; then
Index: libgo/go/syscall/socket_linux.go
===================================================================
--- libgo/go/syscall/socket_linux.go	(revision 180552)
+++ libgo/go/syscall/socket_linux.go	(working copy)
@@ -164,15 +164,6 @@ func anyToSockaddrOS(rsa *RawSockaddrAny
 	return nil, EAFNOSUPPORT
 }
 
-// We don't take this type directly from the header file because it
-// uses a union.  FIXME.
-
-type EpollEvent struct {
-	Events uint32
-	Fd int32
-	Pad int32
-}
-
 //sysnb	EpollCreate(size int) (fd int, errno int)
 //epoll_create(size int) int
 
Index: libgo/Makefile.am
===================================================================
--- libgo/Makefile.am	(revision 180552)
+++ libgo/Makefile.am	(working copy)
@@ -1498,7 +1498,7 @@ endif # !LIBGO_IS_LINUX
 
 # Define socket sizes and types.
 if LIBGO_IS_LINUX
-syscall_socket_file = go/syscall/socket_linux.go
+syscall_socket_file = go/syscall/socket_linux.go epoll.go
 else
 if LIBGO_IS_SOLARIS
 syscall_socket_file = go/syscall/socket_solaris.go
@@ -1582,6 +1582,34 @@ s-sysinfo: $(srcdir)/mksysinfo.sh config
 	$(SHELL) $(srcdir)/../move-if-change tmp-sysinfo.go sysinfo.go
 	$(STAMP) $@
 
+# The epoll struct has an embedded union and is packed on x86_64,
+# which is too complicated for mksysinfo.sh.  We find the offset of
+# the only field we care about in configure.ac, and generate the
+# struct here.
+epoll.go: s-epoll; @true
+s-epoll: Makefile
+	rm -f epoll.go.tmp
+	echo 'package syscall' > epoll.go.tmp
+	echo 'type EpollEvent struct {' >> epoll.go.tmp
+	echo '	Events uint32' >> epoll.go.tmp
+	case "$(SIZEOF_STRUCT_EPOLL_EVENT),$(STRUCT_EPOLL_EVENT_FD_OFFSET)" in \
+	0,0) echo 1>&2 "*** struct epoll_event data.fd offset unknown"; \
+	   exit 1; ;; \
+	8,4) echo '	Fd int32' >> epoll.go.tmp; ;; \
+	12,4) echo '	Fd int32' >> epoll.go.tmp; \
+	   echo '	Pad [4]byte' >> epoll.go.tmp; ;; \
+	12,8) echo '	Pad [4]byte' >> epoll.go.tmp; \
+	   echo '	Fd int32' >> epoll.go.tmp; ;; \
+	16,8) echo '	Pad [4]byte' >> epoll.go.tmp; \
+	   echo '	Fd int32' >> epoll.go.tmp; \
+	   echo '	Pad2 [4]byte' >> epoll.go.tmp; ;; \
+	*) echo 1>&2 "*** struct epoll_event unsupported"; \
+	   exit 1; ;; \
+	esac
+	echo '}' >> epoll.go.tmp
+	$(SHELL) $(srcdir)/../move-if-change epoll.go.tmp epoll.go
+	$(STAMP) $@
+
 if LIBGO_IS_LINUX
 # os_lib_inotify_lo = os/inotify.lo
 os_lib_inotify_lo =