Re: C++ vs. glibc/Hurd/Mach headers

[Samuel Thibault]

Thomas Schwinge, on Fri 25 Nov 2016 12:46:33 +0100, wrote:
> Additionally to the issues Samuel pointed
> out in 
> (still unresolved),

?

I can include hurd/signal.h fine from c++. The second part of my patch
was applied, and IIRC we made some changes to error_t.

Samuel

Re: gdb handling of Mach exceptions

[Justus Winter]

"Brent W. Baccala"  writes:

> On Wed, Nov 23, 2016 at 10:03 PM, Brent W. Baccala 
> wrote:
>
>>
>> Any comments?
>>
>
> Well, yes, actually.  :-)
>
> gdb's hurd target has a poorly documented command "set noninvasive".  I
> don't completely understand it, but...

gdb in noninvasive mode does not stop the target.  This is important if
you want to use it on core servers (like the root filesystem) that your
gdb process, your shell, etc.  uses.  Blocking such a server likely
causes a deadlock.

I don't know what else it does, or how it is implemented.

Justus


signature.asc
Description: PGP signature

Re: C++ vs. glibc/Hurd/Mach headers

[Thomas Schwinge]

Hi!

On Sat, 26 Nov 2016 13:36:22 +0100, Samuel Thibault  
wrote:
> Thomas Schwinge, on Fri 25 Nov 2016 12:46:33 +0100, wrote:
> > Additionally to the issues Samuel pointed
> > out in 
> > (still unresolved),
> 
> ?
> 
> I can include hurd/signal.h fine from c++. The second part of my patch
> was applied

Right, hurd/signal.h's hurd_self_sigstate, _hurd_critical_section_lock,
and _hurd_critical_section_unlock have been fixed, but...

> and IIRC we made some changes to error_t.

... I don't see any such changes?

For GDB, the following is expected to work:

$ echo -e '#include \n#include \n#include 
\n#include \nvoid f(){ error_t err = 0; err = 
thread_get_state(0,0,0,0); err = HURD_MSGPORT_RPC(0,0,0,0); }' | g++ 
-D_GNU_SOURCE -x c++ - -S -o /dev/null -O2
In file included from /usr/include/errno.h:35:0,
 from :1:
/usr/include/hurd.h: In function ‘int __hurd_fail(error_t)’:
/usr/include/hurd.h:60:13: error: invalid conversion from ‘int’ to ‘error_t 
{aka __error_t_codes}’ [-fpermissive]
   err = EIEIO;
 ^
/usr/include/hurd.h:64:13: error: invalid conversion from ‘int’ to ‘error_t 
{aka __error_t_codes}’ [-fpermissive]
   err = ENOMEM;
 ^
/usr/include/hurd.h:68:13: error: invalid conversion from ‘int’ to ‘error_t 
{aka __error_t_codes}’ [-fpermissive]
   err = EINVAL;
 ^
: In function ‘void f()’:
:5:25: error: invalid conversion from ‘int’ to ‘error_t {aka 
__error_t_codes}’ [-fpermissive]
:5:50: error: invalid conversion from ‘kern_return_t {aka int}’ to 
‘error_t {aka __error_t_codes}’ [-fpermissive]
In file included from /usr/include/hurd/userlink.h:27:0,
 from /usr/include/hurd/port.h:25,
 from /usr/include/hurd.h:41,
 from :3:
:5:67: error: invalid conversion from ‘int’ to ‘error_t {aka 
__error_t_codes}’ [-fpermissive]
:5:67: error: invalid conversion from ‘int’ to ‘error_t {aka 
__error_t_codes}’ [-fpermissive]
:5:67: error: invalid conversion from ‘int’ to ‘error_t {aka 
__error_t_codes}’ [-fpermissive]

When changing the GDB source code to use kern_return_t (or int) instead
of error_t, I still see hurd.h:__hurd_fail and
hurd/signal.h:HURD_MSGPORT_RPC choke on their own error_t usage:

$ echo -e '#include \n#include \n#include 
\n#include \nvoid f(){ kern_return_t err = 0; err = 
thread_get_state(0,0,0,0); err = HURD_MSGPORT_RPC(0,0,0,0); }' | g++ 
-D_GNU_SOURCE -x c++ - -S -o /dev/null -O2
In file included from /usr/include/errno.h:35:0,
 from :1:
/usr/include/hurd.h: In function ‘int __hurd_fail(error_t)’:
/usr/include/hurd.h:60:13: error: invalid conversion from ‘int’ to ‘error_t 
{aka __error_t_codes}’ [-fpermissive]
   err = EIEIO;
 ^
/usr/include/hurd.h:64:13: error: invalid conversion from ‘int’ to ‘error_t 
{aka __error_t_codes}’ [-fpermissive]
   err = ENOMEM;
 ^
/usr/include/hurd.h:68:13: error: invalid conversion from ‘int’ to ‘error_t 
{aka __error_t_codes}’ [-fpermissive]
   err = EINVAL;
 ^
In file included from /usr/include/hurd/userlink.h:27:0,
 from /usr/include/hurd/port.h:25,
 from /usr/include/hurd.h:41,
 from :3:
: In function ‘void f()’:
:5:73: error: invalid conversion from ‘int’ to ‘error_t {aka 
__error_t_codes}’ [-fpermissive]
:5:73: error: invalid conversion from ‘int’ to ‘error_t {aka 
__error_t_codes}’ [-fpermissive]
:5:73: error: invalid conversion from ‘int’ to ‘error_t {aka 
__error_t_codes}’ [-fpermissive]


Grüße
 Thomas

Re: Hurd port for gcc go PATCH 1-4(23)

[Samuel Thibault]

Hello,

Nice work!

Mmm, why making changes in each file in a separate patch?  I don't think
it makes things easier to review, I'd say rather send a big patch, it's
altogether that it makes sense anyway.

> Index: gcc-6-6.2.1-4.1/src/libgo/Makefile.in
> ===
> --- gcc-6-6.2.1-4.1.orig/src/libgo/Makefile.in
> +++ gcc-6-6.2.1-4.1/src/libgo/Makefile.in

Makefile.in is a generated file!  Send the patch against Makefile.am
instead.  I hope you didn't modify Makefile.in by hand?!


Svante Signell, on Fri 25 Nov 2016 20:59:27 +0100, wrote:
> Index: gcc-6-6.2.1-4.1/src/libgo/go/syscall/wait.c
> ===
> --- gcc-6-6.2.1-4.1.orig/src/libgo/go/syscall/wait.c
> +++ gcc-6-6.2.1-4.1/src/libgo/go/syscall/wait.c
> @@ -8,6 +8,9 @@
> OS-independent.  */
>  
>  #include 
> +#ifndef WCONTINUED
> +#define WCONTINUED 0
> +#endif
>  #include 
>  
>  #include "runtime.h"

That looks odd at best: WCONTINUED can't be defined at this place
anyway, so it'll get defined to 0. Fortunately the sys/wait.h definition
would override this. But then wait.c will define something which does
not make sense since WCONTINUED is not implemented. Better use #ifdef
WCONTINUED inside the Continued function, to always return 0 when it's
not defined.


> @@ -744,6 +752,14 @@ go_net_sockoptip_file = go/net/sockoptip
>  go_net_cgo_sock_file = go/net/cgo_sockold.go
>  go_net_cgo_res_file = go/net/cgo_resnew.go
>  else
> +if LIBGO_IS_GNU
> +go_net_cgo_file = go/net/cgo_linux.go

I'd say rather rename to cgo_glibc.go . The getaddrinfo implementation
is simply exactly the same between linux, GNU/Hurd, kFreeBSD etc., there
is nothing Linux-specific here.

There are quite a few other files which could be renamed to glibc:

os/pipe_linux.go
crypto/x509/root_linux.go
syscall/errstr_linux.go


> @@ -768,11 +785,15 @@ else
>  if LIBGO_IS_SOLARIS
>  go_net_sendfile_file = go/net/sendfile_solaris.go
>  else
> +if LIBGO_IS_GNU
> +go_net_sendfile_file = go/net/sendfile_gnu.go
> +else
>  go_net_sendfile_file = go/net/sendfile_stub.go
>  endif

There is no difference with the Linux version... I'd say rather propose
to rename sendfile_linux.go into sendfile_glibc.go and just use that.


> +go_net_sock_file = go/net/sock_gnu.go

> Index: gcc-6-6.2.1-4.1/src/libgo/go/net/sock_gnu.go
> ===
> --- /dev/null
> +++ gcc-6-6.2.1-4.1/src/libgo/go/net/sock_gnu.go
> @@ -0,0 +1,14 @@
> +// Copyright 2014 The Go Authors.  All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +// +build gnu
> +
> +package net
> +
> +import "syscall"
> +
> +func maxListenerBacklog() int {
> +   // From /usr/include/i386-gnu/bits/socket.h
> +   return syscall.SOMAXCONN
> +}

Why not just using sock_stub.go which is the same?


> @@ -2028,11 +2093,20 @@ else
>  syscall_os_file = go/syscall/libcall_bsd.go
>  endif
>  
> +# GNU/Hurd specific library calls support.
> +if LIBGO_IS_GNU
> +syscall_libcall_file = go/syscall/libcall_posix-1.go
> +syscall_os_test_file = go/syscall/syscall_gnu_test.go
> +else
> +syscall_libcall_file = go/syscall/libcall_posix.go
> +syscall_os_test_file = go/syscall/syscall_unix_test.go
> +endif

That will be probably be very much frowned upon...

What you said is:
- Removed the mount call for GNU/Hurd, it exists but use translators.
- Removed the mlockall/munlockall calls for GNU/Hurd, not yet implemented.
- Removed the madvise call for GNU/Hurd, not yet implemented.

I'd say you'd much better see with upstream how to make these
conditional inside the file itself, using ifdef GNU for mount,
HAVE_MLOCKALL and such for mlockall, munlockall, madvise, rather than
forking it all...

And
src_libgo_go_syscall_syscall_gnu_test.go: New file:
  Define Type and Whence as 32bit in syscall.Flock_t

Again, you'll probably have to discuss with upstream to see how they
prefer to make it configurable rather than forking the whole file.


> @@ -4431,7 +4505,7 @@ mostlyclean-local:
>   find . -name '*-testsum' -print | xargs rm -f
>   find . -name '*-testlog' -print | xargs rm -f
>  
> -CLEANFILES = *.go *.gox goc2c *.c s-version libgo.sum libgo.log
> +CLEANFILES = *.go *.gox goc2c *.c s-* libgo.sum libgo.log

This seems unrelated?


> Index: gcc-6-6.2.1-4.1/src/libgo/mksysinfo.sh
> ===
> --- gcc-6-6.2.1-4.1.orig/src/libgo/mksysinfo.sh
> +++ gcc-6-6.2.1-4.1/src/libgo/mksysinfo.sh
> @@ -304,6 +304,13 @@ echo '#include ' | ${CC} -x c -
>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 2>&1; then
> +  egrep '^con