Downgrade linker error on protected symbols in .dynbss to a warning

[H.J. Lu]

On Fri, Apr 10, 2015 at 6:21 AM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Apr 10, 2015 at 05:49:05AM -0700, H.J. Lu wrote:
>> On Fri, Apr 10, 2015 at 4:58 AM, Alan Modra <amodra@gmail.com> wrote:
>> > On Fri, Apr 10, 2015 at 03:49:23AM -0700, H.J. Lu wrote:
>> >> Adding a warning is wrong since it is OK to have copy relocation against
>> >> protected symbol.  It works with glibc 2.22.
>> >
>> > Not without gcc changes, and the gcc changes you posted will generate
>> > code that is wrong if using glibc 2.21.  Somehow you even got your
>>
>> GCC 5 is no more wrong than before with older glibc.
>
> This is false.  Your gcc5 change with glibc 2.22 fixes just one
> particular use of protected visibility variables, a definition in a
> shared library and a reference in a non-PIC executable.
>
> Previous versions of gcc work properly with glibc-2.21 and earlier for
> - more than one shared library with definitions of a protected
>   visibility variable,
> - a shared library with a definition of a protected variable, and an
>   executable with a definition of the same variable.
> Those cases are both broken with gcc-5 and glibc-2.21, on x86.

It is broken only if a shared library expects that the protected definition
won't be preempted by another definition.

> Note that I'm not against the overall idea of your changes.  In fact,
> I think the idea is quite reasonable.  What I'm complaining about is
> the complete lack of any checking for older glibc in the case of the
> gcc change, and lack of checks for older gcc in the binutils change.

We need to start somewhere.  Otherwise, it will never be fixed.

>> > changes past review into gcc-5!  That's sad for gcc-5 on x86_64.
>>
>> That is a matter of opinion.
>
> I've given you two regressions above.  Another one is that shared
> library code using protected variables is now worse than it was
> before, not that that matters too much.

Yes, I can make shared library run even faster, but I can't guarantee
it works correctly.

>> >>  Totally revert my patch is
>> >> also wrong as indicated by tests I added since protected symbols
>> >> should reference globally on targets with copy relocation. It will also fail
>> >> the new protected symbol tests in glibc.
>> >
>> > Please show me who approved your patch in the first place.
>>
>> Since my patch is limited to x86, I thought it was OK.
>>
>> > I'll OK a patch that leaves the warning enabled for previous gcc code
>> > but disables it when detecting code that is safe to use with .dynbss
>> > copies of protected visibility variables.  Otherwise you are just
>> > hiding a real problem, as reported in PR15228.  Exactly how you detect
>> > the safe code is up to you.
>>
>> It was about the incorrect shared library with protected symbols and
>> it is a run-time issue. How can linker know if the run-time shared library
>> is safe at link-time?
>>
>> The real problem on x86 in GCC and glibc has been fixed on x86.
>> I will re-install my patch which is limited to x86.
>
> Please don't.  Your x86 users won't thank you when they trip over
> pr15228 when using older versions of gcc.
>

I have backported my glibc fix up to glibc 2.18.  I can backport my
GCC change if request.

I will leave binutils 2.25 alone and only fix master branch. Otherwise,
glibc tests will fail with binutils master branch.

-- 
H.J.