public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Downgrade linker error on protected symbols in .dynbss to a warning
@ 2015-04-10  9:46 Alan Modra
  2015-04-10 10:49 ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2015-04-10  9:46 UTC (permalink / raw)
  To: binutils

PR18222 has convinced me I ought to downgrade the linker error for
"copy reloc against protected <symbol> is invalid" to a warning.

The thing is that the protected visibility variable in libunwind is in
fact a constant.  libunwind doesn't change it from its initial value,
nor do any of the testsuite testcases.  Neither is the variable's
address taken.  So there isn't a real problem here in use of protected
variables, just a potential problem.

Since I'm downgrading the error to a warning I'm also going to revert
HJ's patch on master that turned off the error completely for x86.
Something like HJ's patch might be appropriate to turn off the warning
if the linker can be taught how to detect "safe" copying of protected
visibility variables into .dynbss.  Or better, edit code to PIC so no
copying into .dynbss is needed.

The following shows the changelog for the reversion + patch, then the
patch as I assume no one is particularly interested in the details of
a reversion.

bfd/
	Revert 2015-03-05  H.J. Lu  <hongjiu.lu@intel.com>
	PR ld/pr15228
	PR ld/pr17709
	* elf-bfd.h (elf_backend_data): Delete extern_protected_data.
	* elf32-i386.c (elf_backend_extern_protected_data): Delete.
	* elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
	* elflink.c (_bfd_elf_adjust_dynamic_copy): Remove
	extern_protected_data test.
	(_bfd_elf_symbol_refs_local_p): Likewise.
	* elfxx-target.h (elf_backend_extern_protected_data): Delete.
	(elfNN_bed): Delete elf_backend_extern_protected_data init.
/ld/testsuite/
	Revert 2015-03-05  H.J. Lu  <hongjiu.lu@intel.com>
	PR ld/pr15228
	PR ld/pr17709
	* ld-i386/i386.exp (i386tests): Remove test for PR ld/17709.
	* ld-i386/pr17709-nacl.rd: Delete.
	* ld-i386/pr17709.rd: Likewise.
	* ld-i386/pr17709a.s: Likewise.
	* ld-i386/pr17709b.s: Likewise.
	* ld-i386/protected3.d: Updated.
	* ld-i386/protected3.s: Likewise.
	* ld-x86-64/pr17709-nacl.rd: Delete.
	* ld-x86-64/pr17709.rd: Likewise.
	* ld-x86-64/pr17709a.s: Likewise.
	* ld-x86-64/pr17709b.s: Likewise.
	* ld-x86-64/protected3.d: Updated.
	* ld-x86-64/protected3.s: Likewise.
	* ld-x86-64/x86-64.exp (x86_64tests): Remove test for PR ld/17709.

bfd/
	PR ld/18222
	* elflink.c (_bfd_elf_adjust_dynamic_copy): Don't report an error
	on adding a protected visibility variable to .dynbss.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 183b313..5bc9e9b 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2674,13 +2674,9 @@ _bfd_elf_adjust_dynamic_copy (struct bfd_link_info *info,
   dynbss->size += h->size;
 
   if (h->protected_def)
-    {
-      info->callbacks->einfo
-	(_("%P: copy reloc against protected `%T' is invalid\n"),
-	 h->root.root.string);
-      bfd_set_error (bfd_error_bad_value);
-      return FALSE;
-    }
+    info->callbacks->einfo
+      (_("%P: copy reloc against protected `%T' is dangerous\n"),
+       h->root.root.string);
 
   return TRUE;
 }

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10  9:46 Downgrade linker error on protected symbols in .dynbss to a warning Alan Modra
@ 2015-04-10 10:49 ` H.J. Lu
  2015-04-10 12:25   ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2015-04-10 10:49 UTC (permalink / raw)
  To: Binutils

On Fri, Apr 10, 2015 at 2:45 AM, Alan Modra <amodra@gmail.com> wrote:
> PR18222 has convinced me I ought to downgrade the linker error for
> "copy reloc against protected <symbol> is invalid" to a warning.
>
> The thing is that the protected visibility variable in libunwind is in
> fact a constant.  libunwind doesn't change it from its initial value,
> nor do any of the testsuite testcases.  Neither is the variable's
> address taken.  So there isn't a real problem here in use of protected
> variables, just a potential problem.
>
> Since I'm downgrading the error to a warning I'm also going to revert
> HJ's patch on master that turned off the error completely for x86.
> Something like HJ's patch might be appropriate to turn off the warning
> if the linker can be taught how to detect "safe" copying of protected
> visibility variables into .dynbss.  Or better, edit code to PIC so no
> copying into .dynbss is needed.
>
> The following shows the changelog for the reversion + patch, then the
> patch as I assume no one is particularly interested in the details of
> a reversion.
>
> bfd/
>         Revert 2015-03-05  H.J. Lu  <hongjiu.lu@intel.com>
>         PR ld/pr15228
>         PR ld/pr17709
>         * elf-bfd.h (elf_backend_data): Delete extern_protected_data.
>         * elf32-i386.c (elf_backend_extern_protected_data): Delete.
>         * elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
>         * elflink.c (_bfd_elf_adjust_dynamic_copy): Remove
>         extern_protected_data test.
>         (_bfd_elf_symbol_refs_local_p): Likewise.
>         * elfxx-target.h (elf_backend_extern_protected_data): Delete.
>         (elfNN_bed): Delete elf_backend_extern_protected_data init.
> /ld/testsuite/
>         Revert 2015-03-05  H.J. Lu  <hongjiu.lu@intel.com>
>         PR ld/pr15228
>         PR ld/pr17709
>         * ld-i386/i386.exp (i386tests): Remove test for PR ld/17709.
>         * ld-i386/pr17709-nacl.rd: Delete.
>         * ld-i386/pr17709.rd: Likewise.
>         * ld-i386/pr17709a.s: Likewise.
>         * ld-i386/pr17709b.s: Likewise.
>         * ld-i386/protected3.d: Updated.
>         * ld-i386/protected3.s: Likewise.
>         * ld-x86-64/pr17709-nacl.rd: Delete.
>         * ld-x86-64/pr17709.rd: Likewise.
>         * ld-x86-64/pr17709a.s: Likewise.
>         * ld-x86-64/pr17709b.s: Likewise.
>         * ld-x86-64/protected3.d: Updated.
>         * ld-x86-64/protected3.s: Likewise.
>         * ld-x86-64/x86-64.exp (x86_64tests): Remove test for PR ld/17709.
>
> bfd/
>         PR ld/18222
>         * elflink.c (_bfd_elf_adjust_dynamic_copy): Don't report an error
>         on adding a protected visibility variable to .dynbss.

Adding a warning is wrong since it is OK to have copy relocation against
protected symbol.  It works with glibc 2.22.  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.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 10:49 ` H.J. Lu
@ 2015-04-10 12:25   ` Alan Modra
  2015-04-10 12:49     ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2015-04-10 12:25 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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
changes past review into gcc-5!  That's sad for gcc-5 on x86_64.

>  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.

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.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 12:25   ` Alan Modra
@ 2015-04-10 12:49     ` H.J. Lu
  2015-04-10 13:21       ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2015-04-10 12:49 UTC (permalink / raw)
  To: Binutils

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.

> changes past review into gcc-5!  That's sad for gcc-5 on x86_64.

That is a matter of opinion.

>>  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.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 12:49     ` H.J. Lu
@ 2015-04-10 13:21       ` Alan Modra
  2015-04-10 13:39         ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2015-04-10 13:21 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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.

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.

> > 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.

> >>  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.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 13:21       ` Alan Modra
@ 2015-04-10 13:39         ` H.J. Lu
  2015-04-10 14:59           ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2015-04-10 13:39 UTC (permalink / raw)
  To: Binutils

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 13:39         ` H.J. Lu
@ 2015-04-10 14:59           ` Alan Modra
  2015-04-10 18:00             ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2015-04-10 14:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils, Richard Henderson

On Fri, Apr 10, 2015 at 06:39:10AM -0700, H.J. Lu wrote:
> 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:
> >> 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.

If you don't require protected visibility variables to behave as they
are supposed to behave, then of course everything is fine.

> > 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.

Maybe, but you should admit that without the checks you have broken
the toolchain.  When one part of the toolchain requires other parts to
change for correct behaviour we usually at least want configure
checks.

Your gcc change ought to have emitted an attribute or somesuch into
relocatable object files marking them as having protected visibility
variables **with code accessing them no different than that for
default visibility variables**.  That change requires glibc-2.22 for
correctness.

The attribute could then be copied to a note when building a shared
library, and also be used by the linker to warn if linking with an
older ld.so.  The note then could be used by the linker to disable
"copy reloc against protected <symbol> is dangerous".

Ideally, it would also be possible to disable your gcc change with
configury and command line options, for people with older systems who
want to upgrade gcc but not glibc.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 14:59           ` Alan Modra
@ 2015-04-10 18:00             ` H.J. Lu
  2015-04-10 23:53               ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2015-04-10 18:00 UTC (permalink / raw)
  To: Binutils, Richard Henderson

On Fri, Apr 10, 2015 at 7:59 AM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Apr 10, 2015 at 06:39:10AM -0700, H.J. Lu wrote:
>> 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:
>> >> 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.
>
> If you don't require protected visibility variables to behave as they
> are supposed to behave, then of course everything is fine.
>
>> > 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.
>
> Maybe, but you should admit that without the checks you have broken
> the toolchain.  When one part of the toolchain requires other parts to
> change for correct behaviour we usually at least want configure
> checks.
>
> Your gcc change ought to have emitted an attribute or somesuch into
> relocatable object files marking them as having protected visibility
> variables **with code accessing them no different than that for
> default visibility variables**.  That change requires glibc-2.22 for
> correctness.

GCC, glibc and binutils were never correct before.  If applications
depend on the correct behavior of protected data symbol,  they should
update GCC, glibc and binutils.

> The attribute could then be copied to a note when building a shared
> library, and also be used by the linker to warn if linking with an
> older ld.so.  The note then could be used by the linker to disable
> "copy reloc against protected <symbol> is dangerous".

This won't work since link-time shared library != run-time shared
library.

> Ideally, it would also be possible to disable your gcc change with
> configury and command line options, for people with older systems who
> want to upgrade gcc but not glibc.
>

If they care about correctness of protected data symbol, they
should update GCC, glibc and binutils.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 18:00             ` H.J. Lu
@ 2015-04-10 23:53               ` H.J. Lu
  2015-04-11 15:28                 ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2015-04-10 23:53 UTC (permalink / raw)
  To: Binutils, Richard Henderson

On Fri, Apr 10, 2015 at 11:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Apr 10, 2015 at 7:59 AM, Alan Modra <amodra@gmail.com> wrote:
>> On Fri, Apr 10, 2015 at 06:39:10AM -0700, H.J. Lu wrote:
>>> 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:
>>> >> 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.
>>
>> If you don't require protected visibility variables to behave as they
>> are supposed to behave, then of course everything is fine.
>>
>>> > 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.
>>
>> Maybe, but you should admit that without the checks you have broken
>> the toolchain.  When one part of the toolchain requires other parts to
>> change for correct behaviour we usually at least want configure
>> checks.
>>
>> Your gcc change ought to have emitted an attribute or somesuch into
>> relocatable object files marking them as having protected visibility
>> variables **with code accessing them no different than that for
>> default visibility variables**.  That change requires glibc-2.22 for
>> correctness.
>
> GCC, glibc and binutils were never correct before.  If applications
> depend on the correct behavior of protected data symbol,  they should
> update GCC, glibc and binutils.
>
>> The attribute could then be copied to a note when building a shared
>> library, and also be used by the linker to warn if linking with an
>> older ld.so.  The note then could be used by the linker to disable
>> "copy reloc against protected <symbol> is dangerous".
>
> This won't work since link-time shared library != run-time shared
> library.
>
>> Ideally, it would also be possible to disable your gcc change with
>> configury and command line options, for people with older systems who
>> want to upgrade gcc but not glibc.
>>
>
> If they care about correctness of protected data symbol, they
> should update GCC, glibc and binutils.

I will add a linker switch, -z extern-protected-data, to control
linker behavior.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-10 23:53               ` H.J. Lu
@ 2015-04-11 15:28                 ` H.J. Lu
  2015-04-13  3:33                   ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2015-04-11 15:28 UTC (permalink / raw)
  To: Binutils, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 3286 bytes --]

On Fri, Apr 10, 2015 at 4:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Apr 10, 2015 at 11:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Apr 10, 2015 at 7:59 AM, Alan Modra <amodra@gmail.com> wrote:
>>> On Fri, Apr 10, 2015 at 06:39:10AM -0700, H.J. Lu wrote:
>>>> 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:
>>>> >> 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.
>>>
>>> If you don't require protected visibility variables to behave as they
>>> are supposed to behave, then of course everything is fine.
>>>
>>>> > 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.
>>>
>>> Maybe, but you should admit that without the checks you have broken
>>> the toolchain.  When one part of the toolchain requires other parts to
>>> change for correct behaviour we usually at least want configure
>>> checks.
>>>
>>> Your gcc change ought to have emitted an attribute or somesuch into
>>> relocatable object files marking them as having protected visibility
>>> variables **with code accessing them no different than that for
>>> default visibility variables**.  That change requires glibc-2.22 for
>>> correctness.
>>
>> GCC, glibc and binutils were never correct before.  If applications
>> depend on the correct behavior of protected data symbol,  they should
>> update GCC, glibc and binutils.
>>
>>> The attribute could then be copied to a note when building a shared
>>> library, and also be used by the linker to warn if linking with an
>>> older ld.so.  The note then could be used by the linker to disable
>>> "copy reloc against protected <symbol> is dangerous".
>>
>> This won't work since link-time shared library != run-time shared
>> library.
>>
>>> Ideally, it would also be possible to disable your gcc change with
>>> configury and command line options, for people with older systems who
>>> want to upgrade gcc but not glibc.
>>>
>>
>> If they care about correctness of protected data symbol, they
>> should update GCC, glibc and binutils.
>
> I will add a linker switch, -z extern-protected-data, to control
> linker behavior.
>

Here is a patch to add such an option.  OK for master?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Add-z-no-extern-protected-data-to-ld.patch --]
[-- Type: text/x-patch, Size: 8256 bytes --]

From 4edbf4492560418a8981389f2a121f5d413c2956 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 11 Apr 2015 07:34:49 -0700
Subject: [PATCH] Add -z [no]extern-protected-data to ld

Address of protected data defined in the shared library may be external,
i.e., due to copy relocation.  By default, linker backend checks if
relocations against protected data symbols are valid for building shared
library and issues an error if relocation isn't allowed.  The new options
override linker backend default.  When -z noextern-protected-data is used,
updates on protected data symbols by another module may not be visibile
to the resulting shared library.

bfd/

	* elflink.c (_bfd_elf_adjust_dynamic_copy): Check
	info->extern_protected_data when warning copy relocs against
	protected symbols.
	(_bfd_elf_symbol_refs_local_p): Check info->extern_protected_data
	when checking protected non-function symbols.

include/

	* bfdlink.h (bfd_link_info): Add extern_protected_data.

ld/

	* ld.texinfo: Document "-z [no]extern-protected-data".
	* ldmain.c (main): Initialize link_info.extern_protected_data
	to -1.
	* lexsup.c (elf_shlib_list_options): Add
	"-z [no]extern-protected-data".
	* emultempl/elf32.em (gld${EMULATION_NAME}_handle_option): Handle
	"-z [no]extern-protected-data".

ld/testsuite/

	* ld-i386/i386.exp: Run protected6b.
	* ld-i386/protected6b.d: New file.
	* ld-x86-64/protected6b.d: Likewise.
	* ld-x86-64/x86-64.exp:  Run protected6b.
---
 bfd/elflink.c                        |  9 +++++++--
 include/bfdlink.h                    |  5 +++++
 ld/emultempl/elf32.em                |  4 ++++
 ld/ld.texinfo                        | 13 +++++++++++++
 ld/ldmain.c                          |  1 +
 ld/lexsup.c                          |  4 ++++
 ld/testsuite/ld-i386/i386.exp        |  1 +
 ld/testsuite/ld-i386/protected6b.d   |  6 ++++++
 ld/testsuite/ld-x86-64/protected6b.d |  6 ++++++
 ld/testsuite/ld-x86-64/x86-64.exp    |  1 +
 10 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-i386/protected6b.d
 create mode 100644 ld/testsuite/ld-x86-64/protected6b.d

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 98d3108..5d45687 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2675,7 +2675,9 @@ _bfd_elf_adjust_dynamic_copy (struct bfd_link_info *info,
 
   /* No error if extern_protected_data is true.  */
   if (h->protected_def
-      && !get_elf_backend_data (dynbss->owner)->extern_protected_data)
+      && (!info->extern_protected_data
+	  || (info->extern_protected_data < 0
+	      && !get_elf_backend_data (dynbss->owner)->extern_protected_data)))
     info->callbacks->einfo
       (_("%P: copy reloc against protected `%T' is dangerous\n"),
        h->root.root.string);
@@ -2837,7 +2839,10 @@ _bfd_elf_symbol_refs_local_p (struct elf_link_hash_entry *h,
 
   /* If extern_protected_data is false, STV_PROTECTED non-function
      symbols are local.  */
-  if (!bed->extern_protected_data && !bed->is_function_type (h->type))
+  if ((!info->extern_protected_data
+       || (info->extern_protected_data < 0
+	   && !bed->extern_protected_data))
+      && !bed->is_function_type (h->type))
     return TRUE;
 
   /* Function pointer equality tests may require that STV_PROTECTED
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 6a02a3c..1b15826 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -517,6 +517,11 @@ struct bfd_link_info
      relaxation returning true in *AGAIN.  */
   int relax_trip;
 
+  /* > 0 to treat protected data defined in the shared library as
+     reference external.  0 to treat it as internal.  -1 to let
+     backend to decide.  */
+  int extern_protected_data;
+
   /* Non-zero if auto-import thunks for DATA items in pei386 DLLs
      should be generated/linked against.  Set to 1 if this feature
      is explicitly requested by the user, -1 if enabled by default.  */
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 5db5a93..3f99035 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -2335,6 +2335,10 @@ fragment <<EOF
 	link_info.error_textrel = FALSE;
       else if (strcmp (optarg, "textoff") == 0)
 	link_info.error_textrel = FALSE;
+      else if (strcmp (optarg, "extern-protected-data") == 0)
+	link_info.extern_protected_data = 1;
+      else if (strcmp (optarg, "noextern-protected-data") == 0)
+	link_info.extern_protected_data = 0;
 EOF
 fi
 
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 5384c98..62be2f9 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -1054,6 +1054,12 @@ shared libraries are still allowed.
 @item execstack
 Marks the object as requiring executable stack.
 
+@item extern-protected-data
+Treat protected data symbol as external when building shared library.
+Address of protected data defined in the shared library may be external,
+i.e., due to copy relocation.  This option overrides linker backend
+default.
+
 @item global
 This option is only meaningful when building a shared object.  It makes
 the symbols defined by this shared object available for symbol resolution
@@ -1107,6 +1113,13 @@ Marks the object can not be dumped by @code{dldump}.
 @item noexecstack
 Marks the object as not requiring executable stack.
 
+@item noextern-protected-data
+Don't treat protected data symbol as external when building shared
+library.  This option overrides linker backend default.  It can be used
+to workaround incorrect relocations against protected data symbols
+generated by compiler.  Updates on protected data symbols by another
+module aren't visibile to the resulting shared library.
+
 @item text
 Treat DT_TEXTREL in shared object as error.
 
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 6674a80..2ecb92d 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -285,6 +285,7 @@ main (int argc, char **argv)
   link_info.init_function = "_init";
   link_info.fini_function = "_fini";
   link_info.relax_pass = 1;
+  link_info.extern_protected_data = -1;
   link_info.pei386_auto_import = -1;
   link_info.spare_dynamic_tags = 5;
   link_info.path_separator = ':';
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 4a71ba4..adad7a8 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -1677,6 +1677,10 @@ elf_shlib_list_options (FILE *file)
   fprintf (file, _("\
   -z nocombreloc              Don't merge dynamic relocs into one section\n"));
   fprintf (file, _("\
+  -z extern-protected-data    Treat protected data symbol as external\n"));
+  fprintf (file, _("\
+  -z noextern-protected-data  Don't treat protected data symbol as external\n"));
+  fprintf (file, _("\
   -z global                   Make symbols in DSO available for subsequently\n\
 			       loaded objects\n"));
   fprintf (file, _("\
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 0c0fd96..f214d89 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -237,6 +237,7 @@ run_dump_test "protected3"
 run_dump_test "protected4"
 run_dump_test "protected5"
 run_dump_test "protected6a"
+run_dump_test "protected6b"
 run_dump_test "tlspie1"
 run_dump_test "tlspie2"
 run_dump_test "nogot1"
diff --git a/ld/testsuite/ld-i386/protected6b.d b/ld/testsuite/ld-i386/protected6b.d
new file mode 100644
index 0000000..5642c60
--- /dev/null
+++ b/ld/testsuite/ld-i386/protected6b.d
@@ -0,0 +1,6 @@
+#source: protected6.s
+#as: --32
+#ld: -shared -melf_i386 -z noextern-protected-data
+#readelf: -r
+
+There are no relocations in this file.
diff --git a/ld/testsuite/ld-x86-64/protected6b.d b/ld/testsuite/ld-x86-64/protected6b.d
new file mode 100644
index 0000000..8b44331
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected6b.d
@@ -0,0 +1,6 @@
+#source: protected6.s
+#as: --64
+#ld: -shared -melf_x86_64 -z noextern-protected-data
+#readelf: -r
+
+There are no relocations in this file.
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 213a4c0..8352ad9 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -221,6 +221,7 @@ run_dump_test "protected3-l1om"
 run_dump_test "protected4"
 run_dump_test "protected5"
 run_dump_test "protected6a"
+run_dump_test "protected6b"
 run_dump_test "protected7a"
 run_dump_test "protected7b"
 run_dump_test "tlsle1"
-- 
2.1.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-11 15:28                 ` H.J. Lu
@ 2015-04-13  3:33                   ` Alan Modra
  2015-04-13 11:55                     ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2015-04-13  3:33 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils, Richard Henderson

On Sat, Apr 11, 2015 at 08:27:58AM -0700, H.J. Lu wrote:
> On Fri, Apr 10, 2015 at 4:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > I will add a linker switch, -z extern-protected-data, to control
> > linker behavior.
> 
> Here is a patch to add such an option.  OK for master?

Why is this option needed?

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-13  3:33                   ` Alan Modra
@ 2015-04-13 11:55                     ` H.J. Lu
  2015-04-14  2:09                       ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2015-04-13 11:55 UTC (permalink / raw)
  To: Binutils, Richard Henderson

On Sun, Apr 12, 2015 at 8:32 PM, Alan Modra <amodra@gmail.com> wrote:
> On Sat, Apr 11, 2015 at 08:27:58AM -0700, H.J. Lu wrote:
>> On Fri, Apr 10, 2015 at 4:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> > I will add a linker switch, -z extern-protected-data, to control
>> > linker behavior.
>>
>> Here is a patch to add such an option.  OK for master?
>
> Why is this option needed?
>

To work around the GCC bug.  Otherwise, we got

[hjl@gnu-tools-1 pr17709]$ cat bar.c
int a;

__attribute__((visibility("protected"))) int a;

void
bar ()
{
  a = 30;
}
[hjl@gnu-tools-1 pr17709]$ make libbar.so
gcc  -m32 -fPIC   -c -o bar.o bar.c
./ld -m elf_i386 -shared -o libbar.so bar.o
./ld: bar.o: relocation R_386_GOTOFF against protected data `a' can
not be used when making a shared object
./ld: final link failed: Bad value
Makefile:20: recipe for target 'libbar.so' failed
make: *** [libbar.so] Error 1
[hjl@gnu-tools-1 pr17709]$


-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-13 11:55                     ` H.J. Lu
@ 2015-04-14  2:09                       ` Alan Modra
  2015-04-14 11:16                         ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2015-04-14  2:09 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils, Richard Henderson

On Mon, Apr 13, 2015 at 04:55:47AM -0700, H.J. Lu wrote:
> On Sun, Apr 12, 2015 at 8:32 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Sat, Apr 11, 2015 at 08:27:58AM -0700, H.J. Lu wrote:
> >> On Fri, Apr 10, 2015 at 4:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >> > I will add a linker switch, -z extern-protected-data, to control
> >> > linker behavior.
> >>
> >> Here is a patch to add such an option.  OK for master?
> >
> > Why is this option needed?
> >
> 
> To work around the GCC bug.  Otherwise, we got
> 
> [hjl@gnu-tools-1 pr17709]$ cat bar.c
> int a;
> 
> __attribute__((visibility("protected"))) int a;
> 
> void
> bar ()
> {
>   a = 30;
> }
> [hjl@gnu-tools-1 pr17709]$ make libbar.so
> gcc  -m32 -fPIC   -c -o bar.o bar.c
> ./ld -m elf_i386 -shared -o libbar.so bar.o
> ./ld: bar.o: relocation R_386_GOTOFF against protected data `a' can
> not be used when making a shared object

I presume this isn't gcc-5.  So for code generated by older versions
of gcc you now emit an error when linking shared libraries that access
protected visibility variables?  And the reason you emit an error when
building the shared library is that you'd like everyone to use gcc-5?

I really dislike the way this whole saga is playing out.  Especially
since you know that there is absolutely nothing wrong with using
R_386_GOTOFF against protected visibility data.  In fact, code using a
relative access is more efficient, and I wouldn't be surprised if
someone raises a gcc-5 regression over *not* using R_386_GOTOFF in a
shared library making heavy use of protected visibility.

If you want to go ahead with this patch, you need to make it x86
specific.  -z extern-protected-data is not appropriate as a general
ld option.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Downgrade linker error on protected symbols in .dynbss to a warning
  2015-04-14  2:09                       ` Alan Modra
@ 2015-04-14 11:16                         ` H.J. Lu
  0 siblings, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2015-04-14 11:16 UTC (permalink / raw)
  To: Binutils, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]

On Mon, Apr 13, 2015 at 7:09 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Apr 13, 2015 at 04:55:47AM -0700, H.J. Lu wrote:
>> On Sun, Apr 12, 2015 at 8:32 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Sat, Apr 11, 2015 at 08:27:58AM -0700, H.J. Lu wrote:
>> >> On Fri, Apr 10, 2015 at 4:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >> > I will add a linker switch, -z extern-protected-data, to control
>> >> > linker behavior.
>> >>
>> >> Here is a patch to add such an option.  OK for master?
>> >
>> > Why is this option needed?
>> >
>>
>> To work around the GCC bug.  Otherwise, we got
>>
>> [hjl@gnu-tools-1 pr17709]$ cat bar.c
>> int a;
>>
>> __attribute__((visibility("protected"))) int a;
>>
>> void
>> bar ()
>> {
>>   a = 30;
>> }
>> [hjl@gnu-tools-1 pr17709]$ make libbar.so
>> gcc  -m32 -fPIC   -c -o bar.o bar.c
>> ./ld -m elf_i386 -shared -o libbar.so bar.o
>> ./ld: bar.o: relocation R_386_GOTOFF against protected data `a' can
>> not be used when making a shared object
>
> I presume this isn't gcc-5.  So for code generated by older versions
> of gcc you now emit an error when linking shared libraries that access
> protected visibility variables?  And the reason you emit an error when
> building the shared library is that you'd like everyone to use gcc-5?
>
> I really dislike the way this whole saga is playing out.  Especially
> since you know that there is absolutely nothing wrong with using
> R_386_GOTOFF against protected visibility data.  In fact, code using a
> relative access is more efficient, and I wouldn't be surprised if
> someone raises a gcc-5 regression over *not* using R_386_GOTOFF in a
> shared library making heavy use of protected visibility.
>
> If you want to go ahead with this patch, you need to make it x86
> specific.  -z extern-protected-data is not appropriate as a general
> ld option.
>

This is what I checked in.

-- 
H.J.

[-- Attachment #2: 0001-Add-z-noextern-protected-data-to-ld-for-ELF-x86.patch --]
[-- Type: text/x-patch, Size: 14896 bytes --]

From 889c2a67967f7047c245779a0a0fd8ba8796846e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 14 Apr 2015 04:12:55 -0700
Subject: [PATCH] Add -z noextern-protected-data to ld for ELF/x86

Address of protected data defined in the shared library may be external,
i.e., due to copy relocation.  By default, linker backend checks if
relocations against protected data symbols are valid for building shared
library and issues an error if relocation isn't allowed.  The new option
override linker backend default.  When -z noextern-protected-data is used,
updates on protected data symbols by another module won't be visibile
to the resulting shared library.  This option is specific to ELF/i386
and ELF/x86-64.

bfd/

	PR ld/pr17709
	* elflink.c (_bfd_elf_adjust_dynamic_copy): Check
	info->extern_protected_data when warning copy relocs against
	protected symbols.
	(_bfd_elf_symbol_refs_local_p): Check info->extern_protected_data
	when checking protected non-function symbols.

include/

	PR ld/pr17709
	* bfdlink.h (bfd_link_info): Add extern_protected_data.

ld/

	PR ld/pr17709
	* ld.texinfo: Document "-z noextern-protected-data".
	* ldmain.c (main): Initialize link_info.extern_protected_data
	to -1.
	* lexsup.c (elf_shlib_list_options): Add
	"-z [no]extern-protected-data".
	* emulparams/elf32_x86_64.sh: Source extern_protected_data.sh.
	* emulparams/elf_i386.sh: Likewise.
	* emulparams/elf_i386_be.sh: Likewise.
	* emulparams/elf_i386_chaos.sh: Likewise.
	* emulparams/elf_i386_ldso.sh: Likewise.
	* emulparams/elf_i386_vxworks.sh: Likewise.
	* emulparams/elf_k1om.sh: Likewise.
	* emulparams/elf_l1om.sh: Likewise.
	* emulparams/elf_x86_64.sh: Source extern_protected_data.sh.
	(PARSE_AND_LIST_OPTIONS): Renamed to ...
	(PARSE_AND_LIST_OPTIONS_BNDPLT): This.
	(PARSE_AND_LIST_ARGS_CASE_Z): Renamed to ...
	(PARSE_AND_LIST_ARGS_CASE_Z_BNDPLT): This.
	(PARSE_AND_LIST_OPTIONS): Append $PARSE_AND_LIST_OPTIONS_BNDPLT.
	(PARSE_AND_LIST_ARGS_CASE_Z): Append
	$PARSE_AND_LIST_ARGS_CASE_Z_BNDPLT.
	* emulparams/extern_protected_data.sh: New file.

ld/testsuite/

	PR ld/pr17709
	* ld-i386/i386.exp: Run protected6b.
	* ld-i386/protected6b.d: New file.
	* ld-x86-64/protected6b.d: Likewise.
	* ld-x86-64/x86-64.exp:  Run protected6b.
---
 bfd/ChangeLog                          |  9 +++++++++
 bfd/elflink.c                          |  9 +++++++--
 include/ChangeLog                      |  5 +++++
 include/bfdlink.h                      |  5 +++++
 ld/ChangeLog                           | 26 ++++++++++++++++++++++++++
 ld/emulparams/elf32_x86_64.sh          |  1 +
 ld/emulparams/elf_i386.sh              |  1 +
 ld/emulparams/elf_i386_be.sh           |  1 +
 ld/emulparams/elf_i386_chaos.sh        |  1 +
 ld/emulparams/elf_i386_ldso.sh         |  1 +
 ld/emulparams/elf_i386_vxworks.sh      |  1 +
 ld/emulparams/elf_k1om.sh              |  1 +
 ld/emulparams/elf_l1om.sh              |  1 +
 ld/emulparams/elf_x86_64.sh            |  7 +++++--
 ld/emulparams/extern_protected_data.sh |  9 +++++++++
 ld/ld.texinfo                          |  8 ++++++++
 ld/ldmain.c                            |  1 +
 ld/testsuite/ChangeLog                 |  8 ++++++++
 ld/testsuite/ld-i386/i386.exp          |  1 +
 ld/testsuite/ld-i386/protected6b.d     |  6 ++++++
 ld/testsuite/ld-x86-64/protected6b.d   |  6 ++++++
 ld/testsuite/ld-x86-64/x86-64.exp      |  1 +
 22 files changed, 105 insertions(+), 4 deletions(-)
 create mode 100644 ld/emulparams/extern_protected_data.sh
 create mode 100644 ld/testsuite/ld-i386/protected6b.d
 create mode 100644 ld/testsuite/ld-x86-64/protected6b.d

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 55c37f0..056833e 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,12 @@
+2015-04-14  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/pr17709
+	* elflink.c (_bfd_elf_adjust_dynamic_copy): Check
+	info->extern_protected_data when warning copy relocs against
+	protected symbols.
+	(_bfd_elf_symbol_refs_local_p): Check info->extern_protected_data
+	when checking protected non-function symbols.
+
 2015-04-13  John Baldwin  <jhb@FreeBSD.org>
 
 	* elf.c (elfcore_grok_note): Recognize NT_X86_XSTATE on
diff --git a/bfd/elflink.c b/bfd/elflink.c
index e3d1abe..ea9246b 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2675,7 +2675,9 @@ _bfd_elf_adjust_dynamic_copy (struct bfd_link_info *info,
 
   /* No error if extern_protected_data is true.  */
   if (h->protected_def
-      && !get_elf_backend_data (dynbss->owner)->extern_protected_data)
+      && (!info->extern_protected_data
+	  || (info->extern_protected_data < 0
+	      && !get_elf_backend_data (dynbss->owner)->extern_protected_data)))
     info->callbacks->einfo
       (_("%P: copy reloc against protected `%T' is dangerous\n"),
        h->root.root.string);
@@ -2837,7 +2839,10 @@ _bfd_elf_symbol_refs_local_p (struct elf_link_hash_entry *h,
 
   /* If extern_protected_data is false, STV_PROTECTED non-function
      symbols are local.  */
-  if (!bed->extern_protected_data && !bed->is_function_type (h->type))
+  if ((!info->extern_protected_data
+       || (info->extern_protected_data < 0
+	   && !bed->extern_protected_data))
+      && !bed->is_function_type (h->type))
     return TRUE;
 
   /* Function pointer equality tests may require that STV_PROTECTED
diff --git a/include/ChangeLog b/include/ChangeLog
index 35da015..7d4919d 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2015-04-14  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/pr17709
+	* bfdlink.h (bfd_link_info): Add extern_protected_data.
+
 2015-03-10  Matthew Wahab  <matthew.wahab@arm.com>
 
 	PR ld/16572
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 6a02a3c..1b15826 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -517,6 +517,11 @@ struct bfd_link_info
      relaxation returning true in *AGAIN.  */
   int relax_trip;
 
+  /* > 0 to treat protected data defined in the shared library as
+     reference external.  0 to treat it as internal.  -1 to let
+     backend to decide.  */
+  int extern_protected_data;
+
   /* Non-zero if auto-import thunks for DATA items in pei386 DLLs
      should be generated/linked against.  Set to 1 if this feature
      is explicitly requested by the user, -1 if enabled by default.  */
diff --git a/ld/ChangeLog b/ld/ChangeLog
index bcad3f9..3e944fe 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,29 @@
+2015-04-14  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/pr17709
+	* ld.texinfo: Document "-z noextern-protected-data".
+	* ldmain.c (main): Initialize link_info.extern_protected_data
+	to -1.
+	* lexsup.c (elf_shlib_list_options): Add
+	"-z [no]extern-protected-data".
+	* emulparams/elf32_x86_64.sh: Source extern_protected_data.sh.
+	* emulparams/elf_i386.sh: Likewise.
+	* emulparams/elf_i386_be.sh: Likewise.
+	* emulparams/elf_i386_chaos.sh: Likewise.
+	* emulparams/elf_i386_ldso.sh: Likewise.
+	* emulparams/elf_i386_vxworks.sh: Likewise.
+	* emulparams/elf_k1om.sh: Likewise.
+	* emulparams/elf_l1om.sh: Likewise.
+	* emulparams/elf_x86_64.sh: Source extern_protected_data.sh.
+	(PARSE_AND_LIST_OPTIONS): Renamed to ...
+	(PARSE_AND_LIST_OPTIONS_BNDPLT): This.
+	(PARSE_AND_LIST_ARGS_CASE_Z): Renamed to ...
+	(PARSE_AND_LIST_ARGS_CASE_Z_BNDPLT): This.
+	(PARSE_AND_LIST_OPTIONS): Append $PARSE_AND_LIST_OPTIONS_BNDPLT.
+	(PARSE_AND_LIST_ARGS_CASE_Z): Append
+	$PARSE_AND_LIST_ARGS_CASE_Z_BNDPLT.
+	* emulparams/extern_protected_data.sh: New file.
+
 2015-04-11  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* plugin.c (plugin_load_plugins): Removed an extra ';'.
diff --git a/ld/emulparams/elf32_x86_64.sh b/ld/emulparams/elf32_x86_64.sh
index 11d17ad..8fd96fb 100644
--- a/ld/emulparams/elf32_x86_64.sh
+++ b/ld/emulparams/elf32_x86_64.sh
@@ -1,4 +1,5 @@
 . ${srcdir}/emulparams/plt_unwind.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf
 ELFSIZE=32
 OUTPUT_FORMAT="elf32-x86-64"
diff --git a/ld/emulparams/elf_i386.sh b/ld/emulparams/elf_i386.sh
index 2ebfaac..ae87f6b 100644
--- a/ld/emulparams/elf_i386.sh
+++ b/ld/emulparams/elf_i386.sh
@@ -1,4 +1,5 @@
 . ${srcdir}/emulparams/plt_unwind.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
 NO_RELA_RELOCS=yes
diff --git a/ld/emulparams/elf_i386_be.sh b/ld/emulparams/elf_i386_be.sh
index 1e27faa..06a80c7 100644
--- a/ld/emulparams/elf_i386_be.sh
+++ b/ld/emulparams/elf_i386_be.sh
@@ -1,3 +1,4 @@
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
 NO_RELA_RELOCS=yes
diff --git a/ld/emulparams/elf_i386_chaos.sh b/ld/emulparams/elf_i386_chaos.sh
index b3005e1..c59dbce 100644
--- a/ld/emulparams/elf_i386_chaos.sh
+++ b/ld/emulparams/elf_i386_chaos.sh
@@ -1,4 +1,5 @@
 . ${srcdir}/emulparams/plt_unwind.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf_chaos
 OUTPUT_FORMAT="elf32-i386"
 TEXT_START_ADDR=0x40000000
diff --git a/ld/emulparams/elf_i386_ldso.sh b/ld/emulparams/elf_i386_ldso.sh
index e1a2cb7..7fd08fe 100644
--- a/ld/emulparams/elf_i386_ldso.sh
+++ b/ld/emulparams/elf_i386_ldso.sh
@@ -1,4 +1,5 @@
 . ${srcdir}/emulparams/plt_unwind.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
 NO_RELA_RELOCS=yes
diff --git a/ld/emulparams/elf_i386_vxworks.sh b/ld/emulparams/elf_i386_vxworks.sh
index 61839c8..306e8d3 100644
--- a/ld/emulparams/elf_i386_vxworks.sh
+++ b/ld/emulparams/elf_i386_vxworks.sh
@@ -11,3 +11,4 @@ GENERATE_SHLIB_SCRIPT=yes
 GENERATE_PIE_SCRIPT=yes
 NO_SMALL_DATA=yes
 . ${srcdir}/emulparams/vxworks.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
diff --git a/ld/emulparams/elf_k1om.sh b/ld/emulparams/elf_k1om.sh
index 00bf2ca..0cd606a 100644
--- a/ld/emulparams/elf_k1om.sh
+++ b/ld/emulparams/elf_k1om.sh
@@ -1,4 +1,5 @@
 . ${srcdir}/emulparams/plt_unwind.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf
 ELFSIZE=64
 OUTPUT_FORMAT="elf64-k1om"
diff --git a/ld/emulparams/elf_l1om.sh b/ld/emulparams/elf_l1om.sh
index abf59f1..1964e85 100644
--- a/ld/emulparams/elf_l1om.sh
+++ b/ld/emulparams/elf_l1om.sh
@@ -1,4 +1,5 @@
 . ${srcdir}/emulparams/plt_unwind.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf
 ELFSIZE=64
 OUTPUT_FORMAT="elf64-l1om"
diff --git a/ld/emulparams/elf_x86_64.sh b/ld/emulparams/elf_x86_64.sh
index 984e5e9..a304771 100644
--- a/ld/emulparams/elf_x86_64.sh
+++ b/ld/emulparams/elf_x86_64.sh
@@ -1,4 +1,5 @@
 . ${srcdir}/emulparams/plt_unwind.sh
+. ${srcdir}/emulparams/extern_protected_data.sh
 SCRIPT_NAME=elf
 ELFSIZE=64
 OUTPUT_FORMAT="elf64-x86-64"
@@ -36,14 +37,16 @@ case "$target" in
     case "$EMULATION_NAME" in
       *64*)
         LIBPATH_SUFFIX=64
-	PARSE_AND_LIST_OPTIONS='
+	PARSE_AND_LIST_OPTIONS_BNDPLT='
   fprintf (file, _("\
   -z bndplt                   Always generate BND prefix in PLT entries\n"));
 '
-	PARSE_AND_LIST_ARGS_CASE_Z='
+	PARSE_AND_LIST_ARGS_CASE_Z_BNDPLT='
       else if (strcmp (optarg, "bndplt") == 0)
 	link_info.bndplt = TRUE;
 '
+	PARSE_AND_LIST_OPTIONS="$PARSE_AND_LIST_OPTIONS $PARSE_AND_LIST_OPTIONS_BNDPLT"
+	PARSE_AND_LIST_ARGS_CASE_Z="$PARSE_AND_LIST_ARGS_CASE_Z $PARSE_AND_LIST_ARGS_CASE_Z_BNDPLT"
         ;;
     esac
     ;;
diff --git a/ld/emulparams/extern_protected_data.sh b/ld/emulparams/extern_protected_data.sh
new file mode 100644
index 0000000..fd4bd3b
--- /dev/null
+++ b/ld/emulparams/extern_protected_data.sh
@@ -0,0 +1,9 @@
+PARSE_AND_LIST_OPTIONS='
+  fprintf (file, _("\
+  -z noextern-protected-data  Do not treat protected data symbol as external\n"));
+'
+
+PARSE_AND_LIST_ARGS_CASE_Z='
+      else if (strcmp (optarg, "noextern-protected-data") == 0)
+	link_info.extern_protected_data = FALSE;
+'
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 5384c98..4348c88 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -1146,6 +1146,14 @@ Specifying zero will override any default non-zero sized
 @item bndplt
 Always generate BND prefix in PLT entries. Supported for Linux/x86_64.
 
+@item noextern-protected-data
+Don't treat protected data symbol as external when building shared
+library.  This option overrides linker backend default.  It can be used
+to workaround incorrect relocations against protected data symbols
+generated by compiler.  Updates on protected data symbols by another
+module aren't visibile to the resulting shared library.  Supported for
+i386 and x86-64.
+
 @end table
 
 Other keywords are ignored for Solaris compatibility.
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 6674a80..2ecb92d 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -285,6 +285,7 @@ main (int argc, char **argv)
   link_info.init_function = "_init";
   link_info.fini_function = "_fini";
   link_info.relax_pass = 1;
+  link_info.extern_protected_data = -1;
   link_info.pei386_auto_import = -1;
   link_info.spare_dynamic_tags = 5;
   link_info.path_separator = ':';
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 8b45279..b35eb83 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2015-04-14  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/pr17709
+	* ld-i386/i386.exp: Run protected6b.
+	* ld-i386/protected6b.d: New file.
+	* ld-x86-64/protected6b.d: Likewise.
+	* ld-x86-64/x86-64.exp:  Run protected6b.
+
 2015-04-11  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* ld-i386/i386.exp: Run protected6a.
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 0c0fd96..f214d89 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -237,6 +237,7 @@ run_dump_test "protected3"
 run_dump_test "protected4"
 run_dump_test "protected5"
 run_dump_test "protected6a"
+run_dump_test "protected6b"
 run_dump_test "tlspie1"
 run_dump_test "tlspie2"
 run_dump_test "nogot1"
diff --git a/ld/testsuite/ld-i386/protected6b.d b/ld/testsuite/ld-i386/protected6b.d
new file mode 100644
index 0000000..5642c60
--- /dev/null
+++ b/ld/testsuite/ld-i386/protected6b.d
@@ -0,0 +1,6 @@
+#source: protected6.s
+#as: --32
+#ld: -shared -melf_i386 -z noextern-protected-data
+#readelf: -r
+
+There are no relocations in this file.
diff --git a/ld/testsuite/ld-x86-64/protected6b.d b/ld/testsuite/ld-x86-64/protected6b.d
new file mode 100644
index 0000000..8b44331
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/protected6b.d
@@ -0,0 +1,6 @@
+#source: protected6.s
+#as: --64
+#ld: -shared -melf_x86_64 -z noextern-protected-data
+#readelf: -r
+
+There are no relocations in this file.
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 213a4c0..8352ad9 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -221,6 +221,7 @@ run_dump_test "protected3-l1om"
 run_dump_test "protected4"
 run_dump_test "protected5"
 run_dump_test "protected6a"
+run_dump_test "protected6b"
 run_dump_test "protected7a"
 run_dump_test "protected7b"
 run_dump_test "tlsle1"
-- 
2.1.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-04-14 11:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-10  9:46 Downgrade linker error on protected symbols in .dynbss to a warning Alan Modra
2015-04-10 10:49 ` H.J. Lu
2015-04-10 12:25   ` Alan Modra
2015-04-10 12:49     ` H.J. Lu
2015-04-10 13:21       ` Alan Modra
2015-04-10 13:39         ` H.J. Lu
2015-04-10 14:59           ` Alan Modra
2015-04-10 18:00             ` H.J. Lu
2015-04-10 23:53               ` H.J. Lu
2015-04-11 15:28                 ` H.J. Lu
2015-04-13  3:33                   ` Alan Modra
2015-04-13 11:55                     ` H.J. Lu
2015-04-14  2:09                       ` Alan Modra
2015-04-14 11:16                         ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).