libgo patch committed: Change build procedure to use build tags

29 views
Skip to first unread message

Ian Lance Taylor

unread,
Aug 5, 2016, 8:36:40 PM8/5/16
to gcc-patches, gofront...@googlegroups.com
Go packages use build tags (see the section on Build Constraints at
https://golang.org/pkg/go/build/) to select which files to build on
specific systems.

Previously the libgo Makefile explicitly listed the set of files to
compile for each package. For packages that use build tags, this
required a lot of awkward automake conditionals in the Makefile.

This patch changes the build to look at the build tags in the files.
The new shell script libgo/match.sh does the matching. This required
adjusting a lot of build tags, and removing some files that are never
used. I verified that the exact same sets of files are compiled on
x86_64-pc-linux-gnu. I also tested the build on i386-sun-solaris
(building for both 32-bit and 64-bit).

Writing match.sh revealed some bugs in the build tag handling that
already exists, in a slightly different form, in the gotest shell
script. This patch fixes those problems as well.

The old code used automake conditionals to handle systems that were
missing strerror_r and wait4. Rather than deal with those in Go,
those functions are now implemented in runtime/go-nosys.c when
necessary, so the Go code can simply assume that they exist.

The os testsuite looked for dir_unix.go, which was never built for
gccgo and has now been removed. I changed the testsuite to look for
dir.go instead.

Note that if you have an existing build directory, you will have to
remove all the .dep files in TARGET/libgo after updating to this
patch. There isn't anything that will force them to update
automatically.

Bootstrapped on x86_64-pc-linux-gnu and i386-sun-solaris. Ran Go
testsuite on x86_64-pc-linux-gnu. Committed to mainline.

Ian
patch.txt.bz2

Andreas Schwab

unread,
Aug 7, 2016, 8:14:40 AM8/7/16
to Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com
That breaks ia64:

../../../libgo/go/internal/syscall/unix/getrandom_linux.go:29:5: error: reference to undefined name 'randomTrap'
if randomTrap == 0 {
^
../../../libgo/go/internal/syscall/unix/getrandom_linux.go:38:34: error: reference to undefined name 'randomTrap'
r1, _, errno := syscall.Syscall(randomTrap,
^
make[4]: *** [internal/syscall/unix.lo] Error 1

Andreas.

--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Matthias Klose

unread,
Aug 7, 2016, 11:18:36 AM8/7/16
to Andreas Schwab, Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com
On 07.08.2016 14:14, Andreas Schwab wrote:
> That breaks ia64:
>
> ../../../libgo/go/internal/syscall/unix/getrandom_linux.go:29:5: error: reference to undefined name 'randomTrap'
> if randomTrap == 0 {
> ^
> ../../../libgo/go/internal/syscall/unix/getrandom_linux.go:38:34: error: reference to undefined name 'randomTrap'
> r1, _, errno := syscall.Syscall(randomTrap,
> ^
> make[4]: *** [internal/syscall/unix.lo] Error 1

same on s390x.

Ian Lance Taylor

unread,
Aug 7, 2016, 6:32:55 PM8/7/16
to Matthias Klose, Andreas Schwab, gcc-patches, gofront...@googlegroups.com
Thanks for the reports. This patch should fix these problems, and
also fix the build for Alpha GNU/Linux. Bootstrapped on
x86_64-pc-linux-gnu and verified that the ia64, s390, and alpha should
pick the right files. Committed to mainline.

Ian
patch.txt

Ian Lance Taylor

unread,
Aug 8, 2016, 2:26:29 PM8/8/16
to Lynn A. Boger, gcc-patches, gofront...@googlegroups.com
On Mon, Aug 8, 2016 at 11:14 AM, Lynn A. Boger
<lab...@linux.vnet.ibm.com> wrote:
>
> The libgo tests on ppc64le and ppc64 have all been failing in
> gcc-testresults since this change went in and continues to fail after the
> recent fixes for failures on other platforms.
>
> Built myself and got the same failures. I set keep=true in gotest to save
> the test dirs. Just running a single package:
>
> make bufio/check
> file /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go not
> found
> Keeping gotest9734
> FAIL: bufio
> Makefile:3645: recipe for target 'bufio/check' failed
> make: *** [bufio/check] Error 1
> boger@willow3:~/gccgo.work/trunk/bld/powerpc64le-linux/libgo$ ls
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go
> boger@willow3:~/gccgo.work/trunk/bld/powerpc64le-linux/libgo$ file
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go: ASCII text

I do not know what would cause that and I'm not seeing it. I don't
see how it could be related to the most recent patch (SVN revision
239225); was your build working before that patch and failing
afterward?

There is another report of the same problem on IRC so it's not just you.

Ian

Lynn A. Boger

unread,
Aug 8, 2016, 3:07:32 PM8/8/16
to Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com
Sorry if I was unclear. Failures started happening with r239189, and it
continues to fail in the most recent commit, so none of the later
changes fixed the problem.

Only happens on trunk, ppc64le & ppc64 (m32 also). I did my build on a
different machine from the gcc-testresults build just to be sure there
wasn't something flaky with the system.

Ian Lance Taylor

unread,
Aug 8, 2016, 4:34:11 PM8/8/16
to Lynn A. Boger, gcc-patches, gofront...@googlegroups.com, Segher Boessenkool
On Mon, Aug 8, 2016 at 11:14 AM, Lynn A. Boger
<lab...@linux.vnet.ibm.com> wrote:
> The libgo tests on ppc64le and ppc64 have all been failing in
> gcc-testresults since this change went in and continues to fail after the
> recent fixes for failures on other platforms.
>
> Built myself and got the same failures. I set keep=true in gotest to save
> the test dirs. Just running a single package:
>
> make bufio/check
> file /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go not
> found
> Keeping gotest9734
> FAIL: bufio
> Makefile:3645: recipe for target 'bufio/check' failed
> make: *** [bufio/check] Error 1
> boger@willow3:~/gccgo.work/trunk/bld/powerpc64le-linux/libgo$ ls
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go
> boger@willow3:~/gccgo.work/trunk/bld/powerpc64le-linux/libgo$ file
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go
> /home/boger/gccgo.work/trunk/bld/../src/libgo/go/bufio/bufio.go: ASCII text

I found the problem. I always configure with a relative srcdir. You
are using an absolute srcdir. The recent changes to use build tags
changed the libgo Makefile so that when invoked with with an absolute
srcdir it would pass absolute path names to the gotest script. That
never worked. This patch makes it work, and should fix your problem.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu with a
relative srcdir and an absolute srcdir. Committed to mainline.

Ian
patch.txt

Lynn A. Boger

unread,
Aug 9, 2016, 11:46:56 AM8/9/16
to Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com, Segher Boessenkool
libgo test results in gcc-testresults for ppc64le & ppc64 are back to
what they were before the change.

Thanks.

Rainer Orth

unread,
Aug 11, 2016, 11:15:47 AM8/11/16
to Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com
Hi Ian,
this patch broke i386-pc-solaris2.12 and sparc-sun-solaris2.12
bootstrap, however: in both cases, the 64-bit build of os.lo fails like this:

/vol/gcc/src/hg/trunk/local/libgo/go/os/dir.go:82:8: error: reference to undefined name 'libc_readdir_r'
i := libc_readdir_r(file.dirinfo.dir, entryDirent, pr)
^

Neither dir_largefile.go (which is correctly omitted, being 32-bit only)
nor dir_regfile.go (which is needed here) is included in the
compilation.

Rainer

--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Ian Lance Taylor

unread,
Aug 11, 2016, 5:36:18 PM8/11/16
to Rainer Orth, gcc-patches, gofront...@googlegroups.com
Sorry, I don't know what I messed up in my testing. I committed the
appended patch, which should fix the problem.

Ian
patch.txt

Rainer Orth

unread,
Aug 12, 2016, 5:15:20 AM8/12/16
to Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com
Hi Ian,

>> this patch broke i386-pc-solaris2.12 and sparc-sun-solaris2.12
>> bootstrap, however: in both cases, the 64-bit build of os.lo fails like this:
>>
>> /vol/gcc/src/hg/trunk/local/libgo/go/os/dir.go:82:8: error: reference to undefined name 'libc_readdir_r'
>> i := libc_readdir_r(file.dirinfo.dir, entryDirent, pr)
>> ^
>>
>> Neither dir_largefile.go (which is correctly omitted, being 32-bit only)
>> nor dir_regfile.go (which is needed here) is included in the
>> compilation.
>
> Sorry, I don't know what I messed up in my testing. I committed the
> appended patch, which should fix the problem.

I had found a different one to the same effect, but running match.sh
with yours looked right, too.

There's now one new failure, 32 and 64-bit, sparc and x86:

+FAIL: syscall

libgo.log shows

libcalls.go:825:1: error: redefinition of 'Getpgid'
func Getpgid(pid int) (pgid int, err error) {
^
exec_solaris_test.go:22:1: note: previous definition of 'Getpgid' was here
func Getpgid(pid int) (pgid int, err error) {
^
libcalls.go:843:1: error: redefinition of 'Getpgrp'
func Getpgrp() (pid int) {
^
exec_solaris_test.go:31:1: note: previous definition of 'Getpgrp' was here
func Getpgrp() (pgrp int) {
^
exec_solaris_test.go:14:3: error: libc_Getpgid is not a function; //go:linkname is only supported for functions
//go:linkname libc_Getpgid libc_Getpgid
^
exec_solaris_test.go:15:3: error: libc_Getpgrp is not a function; //go:linkname is only supported for functions
//go:linkname libc_Getpgrp libc_Getpgrp
^
exec_solaris_test.go:23:15: error: reference to undefined name 'sysvicall6'
r0, _, e1 := sysvicall6(uintptr(unsafe.Pointer(&libc_Getpgid)), 1, uintptr(pid), 0, 0, 0, 0, 0)
^
exec_solaris_test.go:32:14: error: reference to undefined name 'sysvicall6'
r0, _, _ := sysvicall6(uintptr(unsafe.Pointer(&libc_Getpgrp)), 0, 0, 0, 0, 0, 0, 0)
^
exec_solaris_test.go:37:13: error: reference to undefined name 'ioctl'
var Ioctl = ioctl
^
exec_solaris_test.go:19:15: error: use of undefined type 'libcFunc'
libc_Getpgrp libcFunc
^

I've no idea what exec_solaris_test.go is about: e.g.I couldn't find any
other reference to sysvicall6 elsewhere in the gcc tree.

Rainer Orth

unread,
Aug 12, 2016, 9:56:31 AM8/12/16
to Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com
Hi Ian,

>>> this patch broke i386-pc-solaris2.12 and sparc-sun-solaris2.12
>>> bootstrap, however: in both cases, the 64-bit build of os.lo fails like this:
>>>
>>> /vol/gcc/src/hg/trunk/local/libgo/go/os/dir.go:82:8: error: reference to undefined name 'libc_readdir_r'
>>> i := libc_readdir_r(file.dirinfo.dir, entryDirent, pr)
>>> ^
>>>
>>> Neither dir_largefile.go (which is correctly omitted, being 32-bit only)
>>> nor dir_regfile.go (which is needed here) is included in the
>>> compilation.
>>
>> Sorry, I don't know what I messed up in my testing. I committed the
>> appended patch, which should fix the problem.
>
> I had found a different one to the same effect, but running match.sh
> with yours looked right, too.

here's another issue found during wider testing: on Solaris 10 with
/bin/ksh as CONFIG_SHELL, the build aborts like this:

objcopy: 'bufio.o': No such file
Makefile:5072: recipe for target 'bufio.gox' failed
make[4]: *** [bufio.gox] Error 1

It turns out that no bufio.o is created indeed:

/vol/gcc/src/hg/trunk/local/libgo/../install-sh -c -d .; files=`echo | sed -e 's/[^ ]*\.gox//g' -e 's/[^ ]*\.dep//'`; /bin/ksh ./libtool --tag GO --mode=compile /var/gcc/regression/trunk/10-gcc/build/./gcc/gccgo -B/var/gcc/regression/trunk/10-gcc/build/./gcc/ -B/vol/gcc/i386-pc-solaris2.10/bin/ -B/vol/gcc/i386-pc-solaris2.10/lib/ -isystem /vol/gcc/i386-pc-solaris2.10/include -isystem /vol/gcc/i386-pc-solaris2.10/sys-include -minline-all-stringops -g -O2 -I . -c -fgo-pkgpath=`echo bufio.lo | sed -e 's/.lo$//' -e 's/-go$//'` -o bufio.lo $files

The list of input files, output from the likes of

/bin/ksh /vol/gcc/src/hg/trunk/local/libgo/match.sh --goarch=386 --goos=solaris --srcdir=/vol/gcc/src/hg/trunk/local/libgo/go/bufio --extrafiles=""

is empty. Running match.sh with /bin/ksh -x reveals

+ set -e
+ unset LANG

where the script aborts. It turns out that /bin/ksh errors out on an
unset of a variable that isn't set.

Maybe one could explicitly set LANG=C here, or use the `portable unset'
used in configure?

Thanks.

Ian Lance Taylor

unread,
Aug 12, 2016, 8:14:23 PM8/12/16
to Rainer Orth, gcc-patches, gofront...@googlegroups.com
The file syscall/exec_solaris_test.go is verifying that the somewhat
unusual way that Solaris support is implemented in the gc toolchain
works correctly. It is meaningless and unnecessary for gccgo. I
removed it. Patch bootstrapped and tested on x86_64-pc-linux-gnu,
which I admit means nothing. Committed to mainline.

Ian
patch.txt

Ian Lance Taylor

unread,
Aug 12, 2016, 10:52:46 PM8/12/16
to Rainer Orth, gcc-patches, gofront...@googlegroups.com
That is a shell portability problem I was not aware of. I took your
LANG=C suggestion. Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu. Committed to mainline.

Ian
patch.txt

Matthias Klose

unread,
Sep 4, 2016, 10:51:15 AM9/4/16
to Andreas Schwab, Ian Lance Taylor, gcc-patches, gofront...@googlegroups.com
same on mips, mipsel, mips64el and sparc64.

Matthias

Ian Lance Taylor

unread,
Sep 23, 2016, 5:00:59 PM9/23/16
to Matthias Klose, Andreas Schwab, gcc-patches, gofront...@googlegroups.com
Sorry, I somehow misunderstood this message. This patch should fix
it. Bootstrapped on x86_64-pc-linux-gnu, which proves little.
Committed to mainline.

Ian
patch.txt
Reply all
Reply to author
Forward
0 new messages