This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: libgo patch committed: Merge from revision 18783 of master
- From: Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>
- To: Ian Lance Taylor <iant at google dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, "gofrontend-dev\ at googlegroups dot com" <gofrontend-dev at googlegroups dot com>
- Date: Wed, 11 Jun 2014 15:57:07 +0200
- Subject: Re: libgo patch committed: Merge from revision 18783 of master
- Authentication-results: sourceware.org; auth=none
- References: <mcrioogdu1a dot fsf at iant-glaptop dot roam dot corp dot google dot com> <yddegz25rm9 dot fsf at lokon dot CeBiTec dot Uni-Bielefeld dot DE> <CAKOQZ8yi5MNgqu9MSC6MCyOAZDrM6tHBYGC_+=n4ds8DX4BxNg at mail dot gmail dot com> <yddbntz4pv2 dot fsf at lokon dot CeBiTec dot Uni-Bielefeld dot DE> <CAKOQZ8zOt67brm95-pz4N-3bTaDgU1O7nKC3ZG35ve3OnoqO-w at mail dot gmail dot com>
Ian Lance Taylor <iant@google.com> writes:
> On Wed, Jun 11, 2014 at 5:01 AM, Rainer Orth
> <ro@cebitec.uni-bielefeld.de> wrote:
>> Ian Lance Taylor <iant@google.com> writes:
>>
>>> On Fri, Jun 6, 2014 at 2:12 AM, Rainer Orth
>>> <ro@cebitec.uni-bielefeld.de> wrote:
>>>> Ian Lance Taylor <iant@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 H 0 COMMON go$zerovalue
index value size type bind oth ver shndx name
[27] 0x4 0x4 OBJT GLOB H 0 COMMON go$zerovalue
index value size type bind oth ver shndx name
[27] 0x4 0x4 OBJT GLOB H 0 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