Patchwork Fix remaining libgo testsuite failures on Solaris 2

login
register
mail settings
Submitter Rainer Orth
Date Jan. 24, 2011, 1:48 p.m.
Message ID <yddvd1ewgz0.fsf@manam.CeBiTec.Uni-Bielefeld.DE>
Download mbox | patch
Permalink /patch/80157/
State New
Headers show

Comments

Rainer Orth - Jan. 24, 2011, 1:48 p.m.
The recent merge of libgo from upstream broke bootstrap with Go on
Solaris: unless _XOPEN_SOURCE is defined as 400 or above, struct msghdr
has no msg_control, msg_controllen, and msg_flags members, but uses
msg_accrights and msg_accrightslen instead.

I've fixed this (and the bootstrap) like this in mksysinfo.sh:

I think we really need to introduce the target_triplet arg to
mksysinfo.sh again which I already had in my original Solaris libgo
patch.  I really doubt that we can find macros which make those
parts/versions of libc visible that are needed in libgo, but have no ill
effect on other platforms.

While this restores libgo bootstrap, there are several other issues that
need to be dealt with:

* In go/time/zoneinfo_unix.go, zoneDir is /usr/share/lib/zoneinfo/ on
  Solaris, not /usr/share/zoneinfo/.  Hardcoding this change fixes
  time/check, but of course this isn't appropriate.

* We need to avoid unconditionally using -Wl,--whole-archive, which is
  only available with GNU ld (and, rarely, vendor linkers like the
  Solaris 11 ld that also implement it).

* Currently, go/os/sys_linux.go is used everywhere which doesn't work
  since only Linux has /proc/sys/kernel/hostname.  I wonder what the
  best solution is, though:

  Solaris /bin/hostname uses sysinfo(SI_HOSTNAME), there's also
  gethostname(3C) and of course uname(2).

* There are several more functions that need largefile variants on
  32-bit Solaris:

  readdir_r needs to use readdir64_r and Dirent needs to correspond to
  _dirent64

  stat, fstat and lstat need to use *stat64, and Stat needs to become
  _stat64.

  There may be more instances, but those are the easy ones so far.  I'm
  unsure where best to place those declarations:

  *stat might go into syscalls/sysfile_stat{regfile, largefile}.go

  readdir_r is currently in go/os/dir.go, but the variants obviously
  should go somewhere into syscalls.

There are also a couple of testsuite issues:

* At least the 64-bit compress/gzip/check hangs on Solaris 11/x86, which
  hangs the whole testsuite run.  This is already dealt with nicely by
  DejaGnu.

* The non-DejaGnu make check only prints it output to stdout, so no
  record of the testsuite run is kept that would show up in
  mail-report.log.

* Even a sequential make check run with use_dejagnu=yes is pretty
  useless: the individal tests are all run with they own runtest
  invocation, but inside the same directory (testsuite).  This way,
  libgo.{log,sum} from the next testcase overwrites the previous one.
  Even so, every test is called go, so all you see is

  FAIL: go

  Not very useful ;-(

Thanks.
	Rainer
Ralf Wildenhues - Jan. 24, 2011, 6:47 p.m.
* Rainer Orth wrote on Mon, Jan 24, 2011 at 02:48:03PM CET:
> * The non-DejaGnu make check only prints it output to stdout, so no
>   record of the testsuite run is kept that would show up in
>   mail-report.log.

You can use the parallel-tests driver.  It creates a test-suite.log file
with a summary of skipped and failed tests, as well as per-test .log
files.  (It also enables parallel tests execution ...)

Cheers,
Ralf
Ian Taylor - Jan. 24, 2011, 8:23 p.m.
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> The recent merge of libgo from upstream broke bootstrap with Go on
> Solaris: unless _XOPEN_SOURCE is defined as 400 or above, struct msghdr
> has no msg_control, msg_controllen, and msg_flags members, but uses
> msg_accrights and msg_accrightslen instead.
>
> I've fixed this (and the bootstrap) like this in mksysinfo.sh:
>
> diff -r 77ad95adb9e1 libgo/mksysinfo.sh
> --- a/libgo/mksysinfo.sh	Sun Jan 23 23:05:50 2011 +0100
> +++ b/libgo/mksysinfo.sh	Sun Jan 23 23:08:31 2011 +0100
> @@ -57,7 +57,8 @@
>  #include <unistd.h>
>  EOF
>  
> -${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
> +#${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
> +${CC} -std=gnu99 -D_XOPEN_SOURCE=600 -D__EXTENSIONS__ -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c

Why did you remove the _D_GNU_SOURCE?  Does it hurt?


> I think we really need to introduce the target_triplet arg to
> mksysinfo.sh again which I already had in my original Solaris libgo
> patch.  I really doubt that we can find macros which make those
> parts/versions of libc visible that are needed in libgo, but have no ill
> effect on other platforms.

I agree that we can't make all systems work the same way, but I think
that testing target_triplet is wrong.  We should instead test for the
relevant features in configure.ac, and use those tests in mksysinfo.sh.
E.g., the way mksysinfo.sh already tests HAVE_SYSCALL_H,
HAVE_SYS_EPOLL_H, etc.


> * Currently, go/os/sys_linux.go is used everywhere which doesn't work
>   since only Linux has /proc/sys/kernel/hostname.  I wonder what the
>   best solution is, though:
>
>   Solaris /bin/hostname uses sysinfo(SI_HOSTNAME), there's also
>   gethostname(3C) and of course uname(2).

The best solution here is to grab os/sys_bsd.go from the master library
and use it on Solaris.

Thanks for the list of items to address, and thanks for testing it.

Ian
Rainer Orth - Jan. 25, 2011, 6:01 p.m.
Ian Lance Taylor <iant@google.com> writes:

>> I've fixed this (and the bootstrap) like this in mksysinfo.sh:
>>
>> diff -r 77ad95adb9e1 libgo/mksysinfo.sh
>> --- a/libgo/mksysinfo.sh	Sun Jan 23 23:05:50 2011 +0100
>> +++ b/libgo/mksysinfo.sh	Sun Jan 23 23:08:31 2011 +0100
>> @@ -57,7 +57,8 @@
>>  #include <unistd.h>
>>  EOF
>>  
>> -${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
>> +#${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
>> +${CC} -std=gnu99 -D_XOPEN_SOURCE=600 -D__EXTENSIONS__ -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
>
> Why did you remove the _D_GNU_SOURCE?  Does it hurt?

Probably not, it should have no effect on Solaris, but the rest
(especially _XOPEN_SOURCE=600) will likely hurt on other platforms.
As I (should have) mentioned, this `patch' was just a hack to let me
continue at all.

>> I think we really need to introduce the target_triplet arg to
>> mksysinfo.sh again which I already had in my original Solaris libgo
>> patch.  I really doubt that we can find macros which make those
>> parts/versions of libc visible that are needed in libgo, but have no ill
>> effect on other platforms.
>
> I agree that we can't make all systems work the same way, but I think
> that testing target_triplet is wrong.  We should instead test for the
> relevant features in configure.ac, and use those tests in mksysinfo.sh.
> E.g., the way mksysinfo.sh already tests HAVE_SYSCALL_H,
> HAVE_SYS_EPOLL_H, etc.

That would mean testing for msg_control, msg_controllen in struct msghdr
in this case, and trying various feature test macros until a match is
found (if any).  E.g., Tru64 UNIX seems to use _POSIX_PII_SOCKET for
that.  libjava already has some of that mess.

>> * Currently, go/os/sys_linux.go is used everywhere which doesn't work
>>   since only Linux has /proc/sys/kernel/hostname.  I wonder what the
>>   best solution is, though:
>>
>>   Solaris /bin/hostname uses sysinfo(SI_HOSTNAME), there's also
>>   gethostname(3C) and of course uname(2).
>
> The best solution here is to grab os/sys_bsd.go from the master library
> and use it on Solaris.

At least the version currently in the GCC tree uses sysctl which doesn't
exist on Solaris.  I suppose uname(2) should be most portable, but I'd
have to check.

> Thanks for the list of items to address, and thanks for testing it.

My pleasure.  If you can make some suggestions for the best route to fix
these issues, I can try (some of) this myself, but given the often
trivial nature of the issues, it might just be a wast of time.

	Rainer
Ian Taylor - March 31, 2011, 11:04 p.m.
I believe that all of these issues have now been fixed.  Thanks.

Ian

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> The recent merge of libgo from upstream broke bootstrap with Go on
> Solaris: unless _XOPEN_SOURCE is defined as 400 or above, struct msghdr
> has no msg_control, msg_controllen, and msg_flags members, but uses
> msg_accrights and msg_accrightslen instead.
>
> I've fixed this (and the bootstrap) like this in mksysinfo.sh:
>
> diff -r 77ad95adb9e1 libgo/mksysinfo.sh
> --- a/libgo/mksysinfo.sh	Sun Jan 23 23:05:50 2011 +0100
> +++ b/libgo/mksysinfo.sh	Sun Jan 23 23:08:31 2011 +0100
> @@ -57,7 +57,8 @@
>  #include <unistd.h>
>  EOF
>  
> -${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
> +#${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
> +${CC} -std=gnu99 -D_XOPEN_SOURCE=600 -D__EXTENSIONS__ -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
>  
>  echo 'package syscall' > ${OUT}
>  
> I think we really need to introduce the target_triplet arg to
> mksysinfo.sh again which I already had in my original Solaris libgo
> patch.  I really doubt that we can find macros which make those
> parts/versions of libc visible that are needed in libgo, but have no ill
> effect on other platforms.
>
> While this restores libgo bootstrap, there are several other issues that
> need to be dealt with:
>
> * In go/time/zoneinfo_unix.go, zoneDir is /usr/share/lib/zoneinfo/ on
>   Solaris, not /usr/share/zoneinfo/.  Hardcoding this change fixes
>   time/check, but of course this isn't appropriate.
>
> * We need to avoid unconditionally using -Wl,--whole-archive, which is
>   only available with GNU ld (and, rarely, vendor linkers like the
>   Solaris 11 ld that also implement it).
>
> * Currently, go/os/sys_linux.go is used everywhere which doesn't work
>   since only Linux has /proc/sys/kernel/hostname.  I wonder what the
>   best solution is, though:
>
>   Solaris /bin/hostname uses sysinfo(SI_HOSTNAME), there's also
>   gethostname(3C) and of course uname(2).
>
> * There are several more functions that need largefile variants on
>   32-bit Solaris:
>
>   readdir_r needs to use readdir64_r and Dirent needs to correspond to
>   _dirent64
>
>   stat, fstat and lstat need to use *stat64, and Stat needs to become
>   _stat64.
>
>   There may be more instances, but those are the easy ones so far.  I'm
>   unsure where best to place those declarations:
>
>   *stat might go into syscalls/sysfile_stat{regfile, largefile}.go
>
>   readdir_r is currently in go/os/dir.go, but the variants obviously
>   should go somewhere into syscalls.
>
> There are also a couple of testsuite issues:
>
> * At least the 64-bit compress/gzip/check hangs on Solaris 11/x86, which
>   hangs the whole testsuite run.  This is already dealt with nicely by
>   DejaGnu.
>
> * The non-DejaGnu make check only prints it output to stdout, so no
>   record of the testsuite run is kept that would show up in
>   mail-report.log.
>
> * Even a sequential make check run with use_dejagnu=yes is pretty
>   useless: the individal tests are all run with they own runtest
>   invocation, but inside the same directory (testsuite).  This way,
>   libgo.{log,sum} from the next testcase overwrites the previous one.
>   Even so, every test is called go, so all you see is
>
>   FAIL: go
>
>   Not very useful ;-(
>
> Thanks.
> 	Rainer

Patch

diff -r 77ad95adb9e1 libgo/mksysinfo.sh
--- a/libgo/mksysinfo.sh	Sun Jan 23 23:05:50 2011 +0100
+++ b/libgo/mksysinfo.sh	Sun Jan 23 23:08:31 2011 +0100
@@ -57,7 +57,8 @@ 
 #include <unistd.h>
 EOF
 
-${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
+#${CC} -D_GNU_SOURCE -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
+${CC} -std=gnu99 -D_XOPEN_SOURCE=600 -D__EXTENSIONS__ -fdump-go-spec=gen-sysinfo.go -S -o sysinfo.s sysinfo.c
 
 echo 'package syscall' > ${OUT}