Re: libgo patch committed: Merge from revision 18783 of master
Ian Lance Taylor i...@google.com writes:
On Fri, Jun 6, 2014 at 2:12 AM, Rainer Orth r...@cebitec.uni-bielefeld.de
wrote:
Ian Lance Taylor i...@google.com writes:
I have committed a patch to libgo to merge from revision
18783:00cce3a34d7e of the master library. This revision was committed
January 7. I picked this revision to merge to because the next revision
deleted a file that is explicitly merged in by the libgo/merge.sh
script.
Among other things, this patch changes type descriptors to add a new
pointer to a zero value. In gccgo this is implemented as a common
variable, and that requires some changes to the compiler and a small
change to go-gcc.cc.
This change introduced many failures on Solaris with /bin/ld, e.g.
FAIL: go.test/test/bom.go -O (test for excess errors)
ld: warning: symbol 'go$zerovalue' has differing sizes:
(file bom.o value=0x8; file
/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo/.libs/libgo.so
value=0x800);
bom.o definition taken and updated with larger size
Interesting. This is working as intended, except for the warning.
go$zerovalue is a common symbol, and the linker is supposed to use the
larger version. From the error message I'm guessing the Solaris
linker supports this when linking object files, but not when linking
an object file against a shared library.
I wonder if we could avoid this warning by giving go$zerovalue hidden
visibility. That would mean something like this patch.
Ian
Index: go-gcc.cc
===
--- go-gcc.cc (revision 211315)
+++ go-gcc.cc (working copy)
@@ -2521,6 +2521,8 @@ Gcc_backend::implicit_variable(const std
DECL_COMMON(decl) = 1;
TREE_PUBLIC(decl) = 1;
gcc_assert(init_tree == NULL_TREE);
+ DECL_VISIBILITY(decl) = VISIBILITY_HIDDEN;
+ DECL_VISIBILITY_SPECIFIED(decl) = 1;
}
else if (is_constant)
{
Unfortunately, this doesn't make a difference. I've now found that ld
supports
-t
Turns off the warning for multiply-defined tentative
(common block) data symbols that have different sizes or
different alignments. This option is equivalent to
specifying the -z relax=common option.
But I'm reluctant to enable this globally. Since Go uses no specs file,
support for target-specific (linker) options would have to go into gccgo
somehow.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: libgo patch committed: Merge from revision 18783 of master
On Wed, Jun 11, 2014 at 5:01 AM, Rainer Orth
r...@cebitec.uni-bielefeld.de wrote:
Ian Lance Taylor i...@google.com writes:
On Fri, Jun 6, 2014 at 2:12 AM, Rainer Orth r...@cebitec.uni-bielefeld.de
wrote:
Ian Lance Taylor i...@google.com writes:
I have committed a patch to libgo to merge from revision
18783:00cce3a34d7e of the master library. This revision was committed
January 7. I picked this revision to merge to because the next revision
deleted a file that is explicitly merged in by the libgo/merge.sh
script.
Among other things, this patch changes type descriptors to add a new
pointer to a zero value. In gccgo this is implemented as a common
variable, and that requires some changes to the compiler and a small
change to go-gcc.cc.
This change introduced many failures on Solaris with /bin/ld, e.g.
FAIL: go.test/test/bom.go -O (test for excess errors)
ld: warning: symbol 'go$zerovalue' has differing sizes:
(file bom.o value=0x8; file
/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo/.libs/libgo.so
value=0x800);
bom.o definition taken and updated with larger size
Interesting. This is working as intended, except for the warning.
go$zerovalue is a common symbol, and the linker is supposed to use the
larger version. From the error message I'm guessing the Solaris
linker supports this when linking object files, but not when linking
an object file against a shared library.
I wonder if we could avoid this warning by giving go$zerovalue hidden
visibility. That would mean something like this patch.
Ian
Index: go-gcc.cc
===
--- go-gcc.cc (revision 211315)
+++ go-gcc.cc (working copy)
@@ -2521,6 +2521,8 @@ Gcc_backend::implicit_variable(const std
DECL_COMMON(decl) = 1;
TREE_PUBLIC(decl) = 1;
gcc_assert(init_tree == NULL_TREE);
+ DECL_VISIBILITY(decl) = VISIBILITY_HIDDEN;
+ DECL_VISIBILITY_SPECIFIED(decl) = 1;
}
else if (is_constant)
{
Unfortunately, this doesn't make a difference.
That is peculiar. Can you verify that in libgo.so the symbol
go$zerovalue is marked as hidden? I don't understand why the linker
would say anything about a symbol in a .o file not matching a hidden
symbol in a .so file. That seems like a linker bug, since the symbols
are not being linked together.
I've now found that ld
supports
-t
Turns off the warning for multiply-defined tentative
(common block) data symbols that have different sizes or
different alignments. This option is equivalent to
specifying the -z relax=common option.
But I'm reluctant to enable this globally. Since Go uses no specs file,
support for target-specific (linker) options would have to go into gccgo
somehow.
Does using that option fix the problem?
The place to add a target-specific linker option is gcc/go/gospec.c.
See, for example, the ways it adds -Wl,-u,pthread_create on some
systems.
Ian
Re: libgo patch committed: Merge from revision 18783 of master
Ian Lance Taylor i...@google.com writes:
On Wed, Jun 11, 2014 at 5:01 AM, Rainer Orth
r...@cebitec.uni-bielefeld.de wrote:
Ian Lance Taylor i...@google.com writes:
On Fri, Jun 6, 2014 at 2:12 AM, Rainer Orth
r...@cebitec.uni-bielefeld.de wrote:
Ian Lance Taylor i...@google.com writes:
I have committed a patch to libgo to merge from revision
18783:00cce3a34d7e of the master library. This revision was committed
January 7. I picked this revision to merge to because the next revision
deleted a file that is explicitly merged in by the libgo/merge.sh
script.
Among other things, this patch changes type descriptors to add a new
pointer to a zero value. In gccgo this is implemented as a common
variable, and that requires some changes to the compiler and a small
change to go-gcc.cc.
This change introduced many failures on Solaris with /bin/ld, e.g.
FAIL: go.test/test/bom.go -O (test for excess errors)
ld: warning: symbol 'go$zerovalue' has differing sizes:
(file bom.o value=0x8; file
/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo/.libs/libgo.so
value=0x800);
bom.o definition taken and updated with larger size
Interesting. This is working as intended, except for the warning.
go$zerovalue is a common symbol, and the linker is supposed to use the
larger version. From the error message I'm guessing the Solaris
linker supports this when linking object files, but not when linking
an object file against a shared library.
I wonder if we could avoid this warning by giving go$zerovalue hidden
visibility. That would mean something like this patch.
Ian
Index: go-gcc.cc
===
--- go-gcc.cc (revision 211315)
+++ go-gcc.cc (working copy)
@@ -2521,6 +2521,8 @@ Gcc_backend::implicit_variable(const std
DECL_COMMON(decl) = 1;
TREE_PUBLIC(decl) = 1;
gcc_assert(init_tree == NULL_TREE);
+ DECL_VISIBILITY(decl) = VISIBILITY_HIDDEN;
+ DECL_VISIBILITY_SPECIFIED(decl) = 1;
}
else if (is_constant)
{
Unfortunately, this doesn't make a difference.
That is peculiar. Can you verify that in libgo.so the symbol
go$zerovalue is marked as hidden? I don't understand why the linker
would say anything about a symbol in a .o file not matching a hidden
symbol in a .so file. That seems like a linker bug, since the symbols
are not being linked together.
It's not about libgo.so. E.g. for the bug191 testcase, I get
ro@arenal 118
/var/gcc/regression/trunk/11-gcc/build/gcc/testsuite/go1/../../gccgo
-B/var/gcc/regression/trunk/11-gcc/build/gcc/testsuite/go1/../../
/vol/gcc/src/hg/trunk/local/gcc/testsuite/go.test/test/fixedbugs/bug191.dir/main.go
-fno-diagnostics-show-caret -fdiagnostics-color=never
-I/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo -w -O2 -g
a.o b.o -L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo
-L/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo/.libs -lm
-o main.x -save-temps
ld: warning: symbol 'go$zerovalue' has differing sizes:
(file main.o value=0x8; file a.o value=0x4);
main.o definition taken
ld: warning: symbol 'go$zerovalue' has differing sizes:
(file main.o value=0x8; file b.o value=0x4);
main.o definition taken
ro@arenal 120 elfdump -s main.o a.o b.o|egrep 'index|zerov'
index value size type bind oth ver shndx name
[38]0x4 0x8 OBJT GLOB H0 COMMON go$zerovalue
index value size type bind oth ver shndx name
[27]0x4 0x4 OBJT GLOB H0 COMMON go$zerovalue
index value size type bind oth ver shndx name
[27]0x4 0x4 OBJT GLOB H0 COMMON go$zerovalue
I've now found that ld
supports
-t
Turns off the warning for multiply-defined tentative
(common block) data symbols that have different sizes or
different alignments. This option is equivalent to
specifying the -z relax=common option.
But I'm reluctant to enable this globally. Since Go uses no specs file,
support for target-specific (linker) options would have to go into gccgo
somehow.
Does using that option fix the problem?
Yup, it does, I tried linking the testcase above with -Wl,-t added.
The place to add a target-specific linker option is gcc/go/gospec.c.
See, for example, the ways it adds -Wl,-u,pthread_create on some
systems.
Ok, I'll have a look unless some other solution can be found.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: libgo patch committed: Merge from revision 18783 of master
On Wed, Jun 11, 2014 at 6:57 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Ok, I'll have a look unless some other solution can be found. There are other, less efficient, ways that this could be compiled, but a common symbol seems like the right solution. Basically, every type descriptor now points to a zero value for the type. The zero value in Go is always the all-bytes-zero value. So we can have every type descriptor point to the same block of memory, as long as we can ensure that that memory is large enough. Using a common symbol does exactly what we want: the linker sees all the go$zerovalue symbols, and sets the final one to the largest go$zerovalue that it sees. I can't think of any other solution that doesn't involve some sort of runtime initialization. I do wonder what gfortran does on Solaris. Ian
Re: libgo patch committed: Merge from revision 18783 of master
Ian Lance Taylor i...@google.com writes: On Wed, Jun 11, 2014 at 6:57 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Ok, I'll have a look unless some other solution can be found. There are other, less efficient, ways that this could be compiled, but a common symbol seems like the right solution. Basically, every type descriptor now points to a zero value for the type. The zero value in Go is always the all-bytes-zero value. So we can have every type descriptor point to the same block of memory, as long as we can ensure that that memory is large enough. Using a common symbol does exactly what we want: the linker sees all the go$zerovalue symbols, and sets the final one to the largest go$zerovalue that it sees. I can't think of any other solution that doesn't involve some sort of runtime initialization. I do wonder what gfortran does on Solaris. This case only occurs in a few LTO testcases, where they are pruned by lto.exp (lto_prune_warns). Apart from that, the only testcase I could find is gfortran.dg/bind_c_coms.f90 with gfortran.dg/bind_c_coms_driver.c, and there all common symbols are the same size in the C and F90 cases. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: libgo patch committed: Merge from revision 18783 of master
On 06/04/14 18:28:17, Ian Lance Taylor wrote:
I have committed a patch to libgo to merge from revision
18783:00cce3a34d7e of the master library.
Based on trunk rev. 211365, we're seeing this warning:
libgo/runtime/chan.c:484:7: error: ‘received’ may be used uninitialized
in this function [-Werror=maybe-uninitialized]
bool received;
^
Here:
481 _Bool
482 runtime_chanrecv2(ChanType *t, Hchan* c, byte* v)
483 {
484 bool received;
485
486 chanrecv(t, c, v, true, received);
487 return received;
488 }
Re: libgo patch committed: Merge from revision 18783 of master
On Mon, Jun 9, 2014 at 1:12 PM, Gary Funck g...@intrepid.com wrote: On 06/04/14 18:28:17, Ian Lance Taylor wrote: I have committed a patch to libgo to merge from revision 18783:00cce3a34d7e of the master library. Based on trunk rev. 211365, we're seeing this warning: libgo/runtime/chan.c:484:7: error: ‘received’ may be used uninitialized in this function [-Werror=maybe-uninitialized] bool received; ^ Thanks for the report. There is no bug here, the control flow is just too complicated for the compiler to sort out. I don't know why I'm not seeing the warning, but in any case the fix is simple. This patch bootstrapped and tested on x86_64-unknown-linux-gnu. Committed to mainline. Ian
Re: libgo patch committed: Merge from revision 18783 of master
Forgot to CC gofrontend-dev. On Mon, Jun 9, 2014 at 5:36 PM, Ian Lance Taylor i...@google.com wrote: On Mon, Jun 9, 2014 at 1:12 PM, Gary Funck g...@intrepid.com wrote: On 06/04/14 18:28:17, Ian Lance Taylor wrote: I have committed a patch to libgo to merge from revision 18783:00cce3a34d7e of the master library. Based on trunk rev. 211365, we're seeing this warning: libgo/runtime/chan.c:484:7: error: ‘received’ may be used uninitialized in this function [-Werror=maybe-uninitialized] bool received; ^ Thanks for the report. There is no bug here, the control flow is just too complicated for the compiler to sort out. I don't know why I'm not seeing the warning, but in any case the fix is simple. This patch bootstrapped and tested on x86_64-unknown-linux-gnu. Committed to mainline. Ian
Re: libgo patch committed: Merge from revision 18783 of master
And I also forgot to attach the patch. Sorry about this.
On Mon, Jun 9, 2014 at 5:37 PM, Ian Lance Taylor i...@google.com wrote:
Forgot to CC gofrontend-dev.
On Mon, Jun 9, 2014 at 5:36 PM, Ian Lance Taylor i...@google.com wrote:
On Mon, Jun 9, 2014 at 1:12 PM, Gary Funck g...@intrepid.com wrote:
On 06/04/14 18:28:17, Ian Lance Taylor wrote:
I have committed a patch to libgo to merge from revision
18783:00cce3a34d7e of the master library.
Based on trunk rev. 211365, we're seeing this warning:
libgo/runtime/chan.c:484:7: error: ‘received’ may be used uninitialized
in this function [-Werror=maybe-uninitialized]
bool received;
^
Thanks for the report. There is no bug here, the control flow is just
too complicated for the compiler to sort out. I don't know why I'm
not seeing the warning, but in any case the fix is simple. This patch
bootstrapped and tested on x86_64-unknown-linux-gnu. Committed to
mainline.
Ian
diff -r e632610ff06a libgo/runtime/chan.c
--- a/libgo/runtime/chan.c Fri Jun 06 14:52:01 2014 -0700
+++ b/libgo/runtime/chan.c Mon Jun 09 17:34:47 2014 -0700
@@ -481,7 +481,7 @@
_Bool
runtime_chanrecv2(ChanType *t, Hchan* c, byte* v)
{
- bool received;
+ bool received = false;
chanrecv(t, c, v, true, received);
return received;
Re: libgo patch committed: Merge from revision 18783 of master
On Mon, Jun 9, 2014 at 5:36 PM, Ian Lance Taylor i...@google.com wrote: There is no bug here, the control flow is just too complicated for the compiler to sort out. I don't know why I'm not seeing the warning [...] We have these compilation flags set: CFLAGS='-g3 -O3' CFLAGS_FOR_BUILD='-g3 -O3' CFLAGS_FOR_TARGET='-g3 -O3' I tried make CFLAGS='-g3 -O2' chan.lo (the default) and it compiled without complaint.
Re: libgo patch committed: Merge from revision 18783 of master
Ian Lance Taylor i...@google.com writes: I have committed a patch to libgo to merge from revision 18783:00cce3a34d7e of the master library. This revision was committed January 7. I picked this revision to merge to because the next revision deleted a file that is explicitly merged in by the libgo/merge.sh script. Among other things, this patch changes type descriptors to add a new pointer to a zero value. In gccgo this is implemented as a common variable, and that requires some changes to the compiler and a small change to go-gcc.cc. This change introduced many failures on Solaris with /bin/ld, e.g. FAIL: go.test/test/bom.go -O (test for excess errors) ld: warning: symbol 'go$zerovalue' has differing sizes: (file bom.o value=0x8; file /var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo/.libs/libgo.so value=0x800); bom.o definition taken and updated with larger size Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: libgo patch committed: Merge from revision 18783 of master
On Fri, Jun 6, 2014 at 2:12 AM, Rainer Orth r...@cebitec.uni-bielefeld.de
wrote:
Ian Lance Taylor i...@google.com writes:
I have committed a patch to libgo to merge from revision
18783:00cce3a34d7e of the master library. This revision was committed
January 7. I picked this revision to merge to because the next revision
deleted a file that is explicitly merged in by the libgo/merge.sh
script.
Among other things, this patch changes type descriptors to add a new
pointer to a zero value. In gccgo this is implemented as a common
variable, and that requires some changes to the compiler and a small
change to go-gcc.cc.
This change introduced many failures on Solaris with /bin/ld, e.g.
FAIL: go.test/test/bom.go -O (test for excess errors)
ld: warning: symbol 'go$zerovalue' has differing sizes:
(file bom.o value=0x8; file
/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/./libgo/.libs/libgo.so
value=0x800);
bom.o definition taken and updated with larger size
Interesting. This is working as intended, except for the warning.
go$zerovalue is a common symbol, and the linker is supposed to use the
larger version. From the error message I'm guessing the Solaris
linker supports this when linking object files, but not when linking
an object file against a shared library.
I wonder if we could avoid this warning by giving go$zerovalue hidden
visibility. That would mean something like this patch.
Ian
Index: go-gcc.cc
===
--- go-gcc.cc (revision 211315)
+++ go-gcc.cc (working copy)
@@ -2521,6 +2521,8 @@ Gcc_backend::implicit_variable(const std
DECL_COMMON(decl) = 1;
TREE_PUBLIC(decl) = 1;
gcc_assert(init_tree == NULL_TREE);
+ DECL_VISIBILITY(decl) = VISIBILITY_HIDDEN;
+ DECL_VISIBILITY_SPECIFIED(decl) = 1;
}
else if (is_constant)
{
Re: libgo patch committed: Merge from revision 18783 of master
Am 05.06.2014 03:28, schrieb Ian Lance Taylor: I have committed a patch to libgo to merge from revision 18783:00cce3a34d7e of the master library. This revision was committed January 7. I picked this revision to merge to because the next revision deleted a file that is explicitly merged in by the libgo/merge.sh script. Among other things, this patch changes type descriptors to add a new pointer to a zero value. In gccgo this is implemented as a common variable, and that requires some changes to the compiler and a small change to go-gcc.cc. As usual the patch is too large to include in this e-mail message. I've appended the changes to parts of libgo that are more gccgo-specific. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Is it time to bump the soname on trunk?
Re: libgo patch committed: Merge from revision 18783 of master
On Thu, Jun 5, 2014 at 3:24 AM, Matthias Klose d...@ubuntu.com wrote: Am 05.06.2014 03:28, schrieb Ian Lance Taylor: I have committed a patch to libgo to merge from revision 18783:00cce3a34d7e of the master library. This revision was committed January 7. I picked this revision to merge to because the next revision deleted a file that is explicitly merged in by the libgo/merge.sh script. Among other things, this patch changes type descriptors to add a new pointer to a zero value. In gccgo this is implemented as a common variable, and that requires some changes to the compiler and a small change to go-gcc.cc. As usual the patch is too large to include in this e-mail message. I've appended the changes to parts of libgo that are more gccgo-specific. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Is it time to bump the soname on trunk? Yes, I'll do that when I've merged gccgo all the way up to the Go 1.3 release. Ian
Re: libgo patch committed: Merge from revision 18783 of master
Hello! I have committed a patch to libgo to merge from revision 18783:00cce3a34d7e of the master library. This revision was committed January 7. I picked this revision to merge to because the next revision deleted a file that is explicitly merged in by the libgo/merge.sh script. crypto/x509 fails on x86 Fedora20 with: --- FAIL: TestImports (0.00 seconds) testing.go:228: failed to run x509_test_import.go: exec: go: executable file not found in $PATH FAIL FAIL: crypto/x509 Uros.
Re: libgo patch committed: Merge from revision 18783 of master
On Thu, Jun 5, 2014 at 9:38 AM, Uros Bizjak ubiz...@gmail.com wrote: I have committed a patch to libgo to merge from revision 18783:00cce3a34d7e of the master library. This revision was committed January 7. I picked this revision to merge to because the next revision deleted a file that is explicitly merged in by the libgo/merge.sh script. crypto/x509 fails on x86 Fedora20 with: --- FAIL: TestImports (0.00 seconds) testing.go:228: failed to run x509_test_import.go: exec: go: executable file not found in $PATH FAIL FAIL: crypto/x509 Thanks. I'll fix this in the next import. Ian
