diff mbox

Replace __attribute__((visibility("protected")))

Message ID CAMe9rOrySMqGRCxXw-utwZWLx8ZV0NcLQU5_AHbwBts8jRH3uw@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu March 6, 2015, 11:35 p.m. UTC 
On Fri, Mar 6, 2015 at 2:59 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> 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.

Here is the updated patch.  OK to install?

Thanks.

Comments

Roland McGrath March 7, 2015, 12:28 a.m. UTC | #1 
Looks OK to me.
diff mbox

Patch

From d314b0004e9a9d18c30dce4befb7367d6dd8f2bd Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 6 Mar 2015 04:55:56 -0800
Subject: [PATCH] Replace __attribute__((visibility("protected")))

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.

	* elf/ifuncdep2.c (global): Replace
	__attribute__((visibility("protected"))) with
	asm (".protected global").
	* elf/ifuncmod1.c (global): Likewise.
	* elf/ifuncmod5.c (global): Likewise.
---
 elf/ifuncdep2.c | 8 +++++++-
 elf/ifuncmod1.c | 8 +++++++-
 elf/ifuncmod5.c | 8 +++++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/elf/ifuncdep2.c b/elf/ifuncdep2.c
index 99d1926..6e66d31 100644
--- a/elf/ifuncdep2.c
+++ b/elf/ifuncdep2.c
@@ -2,7 +2,13 @@ 
 
 #include "ifunc-sel.h"
 
-int global __attribute__ ((visibility ("protected"))) = -1;
+int global = -1;
+/* Can't use __attribute__((visibility("protected"))) until the GCC bug:
+
+   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65248
+
+   is fixed.  */
+asm (".protected global");
 
 static int
 one (void)
diff --git a/elf/ifuncmod1.c b/elf/ifuncmod1.c
index 2b8195c..0b61380 100644
--- a/elf/ifuncmod1.c
+++ b/elf/ifuncmod1.c
@@ -6,7 +6,13 @@ 
  */
 #include "ifunc-sel.h"
 
-int global __attribute__ ((visibility ("protected"))) = -1;
+int global = -1;
+/* Can't use __attribute__((visibility("protected"))) until the GCC bug:
+
+   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65248
+
+   is fixed.  */
+asm (".protected global");
 
 static int
 one (void)
diff --git a/elf/ifuncmod5.c b/elf/ifuncmod5.c
index 9a08e8c..0e65a63 100644
--- a/elf/ifuncmod5.c
+++ b/elf/ifuncmod5.c
@@ -1,7 +1,13 @@ 
 /* Test STT_GNU_IFUNC symbols without direct function call.  */
 #include "ifunc-sel.h"
 
-int global __attribute__ ((visibility ("protected"))) = -1;
+int global = -1;
+/* Can't use __attribute__((visibility("protected"))) until the GCC bug:
+
+   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65248
+
+   is fixed.  */
+asm (".protected global");
 
 static int
 one (void)
-- 
1.9.3