Replace __attribute__((visibility("protected")))
Commit Message
With copy relocation, address of protected data defined in the shared
library may be external. Compiler shouldn't asssume protected data will
be local. But due to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65248
__attribute__((visibility("protected"))) doesn't work correctly, we need
to use asm (".protected xxx") instead.
OK for master?
Thanks.
H.J.
---
* elf/ifuncdep2.c (global): Replace
__attribute__((visibility("protected"))) with
asm (".protected xxx").
* elf/ifuncmod1.c (global): Likewise.
* elf/ifuncmod5.c (global): Likewise.
---
elf/ifuncdep2.c | 3 ++-
elf/ifuncmod1.c | 3 ++-
elf/ifuncmod5.c | 3 ++-
3 files changed, 6 insertions(+), 3 deletions(-)
Comments
"H.J. Lu" <hongjiu.lu@intel.com> writes:
> diff --git a/elf/ifuncdep2.c b/elf/ifuncdep2.c
> index 99d1926..599eba7 100644
> --- a/elf/ifuncdep2.c
> +++ b/elf/ifuncdep2.c
> @@ -2,7 +2,8 @@
>
> #include "ifunc-sel.h"
>
> -int global __attribute__ ((visibility ("protected"))) = -1;
> +int global = -1;
> +asm (".protected global");
Some architectures use a symbol prefix.
Andreas.
On Fri, 6 Mar 2015, Andreas Schwab wrote:
> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>
> > diff --git a/elf/ifuncdep2.c b/elf/ifuncdep2.c
> > index 99d1926..599eba7 100644
> > --- a/elf/ifuncdep2.c
> > +++ b/elf/ifuncdep2.c
> > @@ -2,7 +2,8 @@
> >
> > #include "ifunc-sel.h"
> >
> > -int global __attribute__ ((visibility ("protected"))) = -1;
> > +int global = -1;
> > +asm (".protected global");
>
> Some architectures use a symbol prefix.
No such architectures are currently supported by glibc, and I expect lots
of places would need fixing if support were to be added for any such
architecture. (Using symbol prefixes with ELF also violates the gABI,
which says "External C symbols have the same names in C and object files'
symbol tables.".)
On Fri, Mar 6, 2015 at 2:28 PM, Roland McGrath <roland@hack.frob.com> wrote:
> It needs comments.
Did you mean duplicate my commit log to each
__attribute__ ((visibility ("protected"))) change?
What about existing
asm (".protected xxx");
Do they also need comments? I don't think it is necessary
since the commit log explains why a change is made.
> On Fri, Mar 6, 2015 at 2:28 PM, Roland McGrath <roland@hack.frob.com> wrote:
> > It needs comments.
>
> Did you mean duplicate my commit log to each
> __attribute__ ((visibility ("protected"))) change?
> What about existing
>
> asm (".protected xxx");
>
> Do they also need comments? I don't think it is necessary
> since the commit log explains why a change is made.
What's necessary is that comments next to the actual code explain why it's
any nonobvious way it is. In this case, you need a short comment
mentioning the GCC bug in each place that you're avoiding using the obvious
compiler feature because of a bug.
@@ -2,7 +2,8 @@
#include "ifunc-sel.h"
-int global __attribute__ ((visibility ("protected"))) = -1;
+int global = -1;
+asm (".protected global");
static int
one (void)
@@ -6,7 +6,8 @@
*/
#include "ifunc-sel.h"
-int global __attribute__ ((visibility ("protected"))) = -1;
+int global = -1;
+asm (".protected global");
static int
one (void)
@@ -1,7 +1,8 @@
/* Test STT_GNU_IFUNC symbols without direct function call. */
#include "ifunc-sel.h"
-int global __attribute__ ((visibility ("protected"))) = -1;
+int global = -1;
+asm (".protected global");
static int
one (void)