Re: patch8.diff updated Was: Re: GCC's -fsplit-stack disturbing Mach's vm_allocate

2014-05-21 Thread Samuel Thibault 
Svante Signell, le Wed 21 May 2014 09:47:08 +0200, a écrit :
   +# Special treatment of EWOULDBLOCK for GNU/Hurd
   +# /usr/include/bits/errno.h: #define EWOULDBLOCK EAGAIN
   +if egrep 'define EWOULDBLOCK EAGAIN' gen-sysinfo.go  /dev/null 21; 
   then
   +  egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' ${OUT} | \
   +sed -i.bak -e 's/_EWOULDBLOCK/_EAGAIN/' ${OUT}
  
  I don't understand why you both pass the output of egrep to sed, and you
  give the -i option to sed. AIUI, the
  egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)'
  part is completely unused, so you can just drop it.
 
 Well, the -i option is to get a backup copy for debugging purposes, can
 safely be removed.

Err, no, -i completely changes the behavior of sed, which then doesn't
read its stdin at all any more, it will modify ${OUT} in-place instead.
It happens that it is what you want here.  But then just drop the egrep
just before, it really is useless now. And drop the .bak suffix, I don't
think the gnugo maintainers will want to see a .bak file lying behind.

 Good luck making the patch better. It worked for me, but as I wrote
 before I'm no sed/grep expert.

I'm not the one to be convinced, gnugo maintainers are the one to be.
I'm just telling you in advance what *they* will tell you.

Samuel

Re: patch8.diff updated Was: Re: GCC's -fsplit-stack disturbing Mach's vm_allocate

2014-05-21 Thread Ian Lance Taylor 
On Wed, May 21, 2014 at 12:47 AM, Svante Signell
svante.sign...@gmail.com wrote:
 On Wed, 2014-05-21 at 01:27 +0200, Samuel Thibault wrote:
 Svante Signell, le Fri 16 May 2014 10:03:05 +0200, a écrit :
  is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go:
  func dupCloseOnExec(fd int) (newfd int, err error) {
  if atomic.LoadInt32(tryDupCloexec) == 1  syscall.F_DUPFD_CLOEXEC!=0 {
  r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd),
  syscall.F_DUPFD_CLOEXEC, 0)

 That code can not work as it is, fcntl is not a system call on
 GNU/Hurd. Why isn't gccgo just using the C fcntl function?  That one
 will just work and be portable.

 I don't know, I'm not a go developer. Ask Ian.

It's a bug.  That code, like most of libgo, is simply copied from the
master Go library, and I never noticed the direct use of
syscall.Syscall here.

Ian

Re: patch8.diff updated Was: Re: GCC's -fsplit-stack disturbing Mach's vm_allocate

2014-05-20 Thread Samuel Thibault 
Svante Signell, le Fri 16 May 2014 10:03:05 +0200, a écrit :
 is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go:
 func dupCloseOnExec(fd int) (newfd int, err error) {
 if atomic.LoadInt32(tryDupCloexec) == 1  syscall.F_DUPFD_CLOEXEC!=0 {
 r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd),
 syscall.F_DUPFD_CLOEXEC, 0)

That code can not work as it is, fcntl is not a system call on
GNU/Hurd. Why isn't gccgo just using the C fcntl function?  That one
will just work and be portable.

 +# Special treatment of EWOULDBLOCK for GNU/Hurd
 +# /usr/include/bits/errno.h: #define EWOULDBLOCK EAGAIN
 +if egrep 'define EWOULDBLOCK EAGAIN' gen-sysinfo.go  /dev/null 21; then
 +  egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' ${OUT} | \
 +sed -i.bak -e 's/_EWOULDBLOCK/_EAGAIN/' ${OUT}

I don't understand why you both pass the output of egrep to sed, and you
give the -i option to sed. AIUI, the
egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)'
part is completely unused, so you can just drop it.

 @@ -528,6 +540,12 @@
 +# Special treatment of st_dev for GNU/Hurd
 +# /usr/include/i386-gnu/bits/stat.h: #define st_dev st_fsid
 +if grep 'define st_dev st_fsid' gen-sysinfo.go  /dev/null 21; then
 +  egrep '^type _stat ' gen-sysinfo.go  /dev/null 21| \
 +  sed -i.bak -e 's/; st_fsid/; st_dev/' gen-sysinfo.go
 +fi

The same remark about egrep | sed -i applies here.

And anyway, why not simply using the very first patch you proposed,
which was:

@@ -536,6 +548,7 @@
 fi | sed -e 's/type _stat64/type Stat_t/' \
  -e 's/type _stat/type Stat_t/' \  
  -e 's/st_dev/Dev/' \
+ -e 's/st_fsid/Dev/' \
  -e 's/st_ino/Ino/g' \
  -e 's/st_nlink/Nlink/' \
  -e 's/st_mode/Mode/' \

which I said several times that it should be completely fine.

Samuel

Re: patch8.diff updated Was: Re: GCC's -fsplit-stack disturbing Mach's vm_allocate

2014-05-16 Thread Svante Signell 
On Wed, 2014-05-07 at 10:18 +0200, Svante Signell wrote:
 On Tue, 2014-05-06 at 15:26 +0200, Samuel Thibault wrote:

Attached is an updated patch8.diff. Arch specific code to
src/libgo/mksysinfo.sh has been added, now other systems are not
affected by the patch except the SYS_FCNTL part.

For that part of the patch without it the build on GNU/Hurd fails. On
the other hand SYS_FCNTL is not defined for e.g. GNU/Linux either. This
is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go:
func dupCloseOnExec(fd int) (newfd int, err error) {
if atomic.LoadInt32(tryDupCloexec) == 1  syscall.F_DUPFD_CLOEXEC!=0 {
r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd),
syscall.F_DUPFD_CLOEXEC, 0)

It is yet unknown how the build succeeds on Linux without the SYS_FCNTL
being defined? Maybe any the conditions above are not met. 
--- a/src/libgo/mksysinfo.sh
+++ b/src/libgo/mksysinfo.sh
@@ -210,6 +210,13 @@
   egrep '#define E[A-Z0-9_]+ ' | \
   sed -e 's/^#define \(E[A-Z0-9_]*\) .*$/const \1 = Errno(_\1)/'  ${OUT}
 
+# Special treatment of EWOULDBLOCK for GNU/Hurd
+# /usr/include/bits/errno.h: #define EWOULDBLOCK EAGAIN
+if egrep 'define EWOULDBLOCK EAGAIN' gen-sysinfo.go  /dev/null 21; then
+  egrep '^const EWOULDBLOCK = Errno(_EWOULDBLOCK)' ${OUT} | \
+sed -i.bak -e 's/_EWOULDBLOCK/_EAGAIN/' ${OUT}
+fi
+
 # The O_xxx flags.
 egrep '^const _(O|F|FD)_' gen-sysinfo.go | \
   sed -e 's/^\(const \)_\([^= ]*\)\(.*\)$/\1\2 = _\2/'  ${OUT}
@@ -225,6 +232,11 @@
   echo const F_DUPFD_CLOEXEC = 0  ${OUT}
 fi
 
+# Special treatment of SYS_FCNTL for GNU/Hurd
+if ! grep '^const SYS_FCNTL' ${OUT}  /dev/null 21; then
+  echo const SYS_FCNTL = 0  ${OUT}
+fi
+
 # These flags can be lost on i386 GNU/Linux when using
 # -D_FILE_OFFSET_BITS=64, because we see #define F_SETLK F_SETLK64
 # before we see the definition of F_SETLK64.
@@ -528,6 +540,12 @@
 
 # The stat type.
 # Prefer largefile variant if available.
+# Special treatment of st_dev for GNU/Hurd
+# /usr/include/i386-gnu/bits/stat.h: #define st_dev st_fsid
+if grep 'define st_dev st_fsid' gen-sysinfo.go  /dev/null 21; then
+  egrep '^type _stat ' gen-sysinfo.go  /dev/null 21| \
+  sed -i.bak -e 's/; st_fsid/; st_dev/' gen-sysinfo.go
+fi
 stat=`grep '^type _stat64 ' gen-sysinfo.go || true`
 if test $stat != ; then
   grep '^type _stat64 ' gen-sysinfo.go

Re: patch8.diff updated Was: Re: GCC's -fsplit-stack disturbing Mach's vm_allocate

2014-05-16 Thread Ian Lance Taylor 
On Fri, May 16, 2014 at 1:03 AM, Svante Signell
svante.sign...@gmail.com wrote:

 For that part of the patch without it the build on GNU/Hurd fails. On
 the other hand SYS_FCNTL is not defined for e.g. GNU/Linux either. This
 is used in gcc-4.9-4.9.0/src/libgo/go/net/fd_unix.go:
 func dupCloseOnExec(fd int) (newfd int, err error) {
 if atomic.LoadInt32(tryDupCloexec) == 1  syscall.F_DUPFD_CLOEXEC!=0 {
 r0, _, e1 := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd),
 syscall.F_DUPFD_CLOEXEC, 0)

 It is yet unknown how the build succeeds on Linux without the SYS_FCNTL
 being defined? Maybe any the conditions above are not met.

On GNU/Linux SYS_FCNTL is defined by the generated file sysinfo.go,
because SYS_fcntl is defined by syscall.h.

Ian