[PATCH] Enable hidden visibility in libc.a compiled with PIE

[PATCH] Enable hidden visibility in libc.a compiled with PIE

[H.J. Lu]

When building libc.a with PIE, enable hidden visibility to allow direct
access to definitions within libc.a without using GOT.

Size comparison of elf/ldconfig when the compiler defaults to -fPIE:

On x86-64:
        text	   data	    bss	    dec	    hex
Before: 619206	  20132	   5488	 644826	  9d6da
After : 619062	  20132	   5488	 644682	  9d64a
On i686:
        text	   data	    bss	    dec	    hex
Before: 556305	  10816	   3056	 570177	  8b341
After : 553688	  10756	   3056	 567500	  8a8cc

OK for master?

H.J.
---
	* include/libc-symbols.h (attribute_hidden): Enable hidden
	visibility in libc.a compiled with PIE.
---
 include/libc-symbols.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index fe3ab81c51..d6a1c260f6 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -363,7 +363,8 @@ for linking")
   strong_alias(real, name)
 #endif
 
-#if defined SHARED || defined LIBC_NONSHARED
+#if defined SHARED || defined LIBC_NONSHARED \
+  || (BUILD_PIE_DEFAULT && IS_IN (libc))
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
-- 
2.13.4

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[H.J. Lu]

On Tue, Aug 8, 2017 at 9:40 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> When building libc.a with PIE, enable hidden visibility to allow direct
> access to definitions within libc.a without using GOT.
>
> Size comparison of elf/ldconfig when the compiler defaults to -fPIE:
>
> On x86-64:
>         text       data     bss     dec     hex
> Before: 619206    20132    5488  644826   9d6da
> After : 619062    20132    5488  644682   9d64a
> On i686:
>         text       data     bss     dec     hex
> Before: 556305    10816    3056  570177   8b341
> After : 553688    10756    3056  567500   8a8cc
>
> OK for master?
>
> H.J.
> ---
>         * include/libc-symbols.h (attribute_hidden): Enable hidden
>         visibility in libc.a compiled with PIE.
> ---
>  include/libc-symbols.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index fe3ab81c51..d6a1c260f6 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -363,7 +363,8 @@ for linking")
>    strong_alias(real, name)
>  #endif
>
> -#if defined SHARED || defined LIBC_NONSHARED
> +#if defined SHARED || defined LIBC_NONSHARED \
> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>  #else
>  # define attribute_hidden
> --
> 2.13.4
>

Any comments or objections?

-- 
H.J.

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[H.J. Lu]

On Mon, Aug 14, 2017 at 5:51 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Aug 8, 2017 at 9:40 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> When building libc.a with PIE, enable hidden visibility to allow direct
>> access to definitions within libc.a without using GOT.
>>
>> Size comparison of elf/ldconfig when the compiler defaults to -fPIE:
>>
>> On x86-64:
>>         text       data     bss     dec     hex
>> Before: 619206    20132    5488  644826   9d6da
>> After : 619062    20132    5488  644682   9d64a
>> On i686:
>>         text       data     bss     dec     hex
>> Before: 556305    10816    3056  570177   8b341
>> After : 553688    10756    3056  567500   8a8cc
>>
>> OK for master?
>>
>> H.J.
>> ---
>>         * include/libc-symbols.h (attribute_hidden): Enable hidden
>>         visibility in libc.a compiled with PIE.
>> ---
>>  include/libc-symbols.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
>> index fe3ab81c51..d6a1c260f6 100644
>> --- a/include/libc-symbols.h
>> +++ b/include/libc-symbols.h
>> @@ -363,7 +363,8 @@ for linking")
>>    strong_alias(real, name)
>>  #endif
>>
>> -#if defined SHARED || defined LIBC_NONSHARED
>> +#if defined SHARED || defined LIBC_NONSHARED \
>> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>>  #else
>>  # define attribute_hidden
>> --
>> 2.13.4
>>
>
> Any comments or objections?
>

I will check it in tomorrow.


-- 
H.J.

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[Florian Weimer]

On 08/08/2017 06:40 PM, H.J. Lu wrote:
> -#if defined SHARED || defined LIBC_NONSHARED
> +#if defined SHARED || defined LIBC_NONSHARED \
> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>  #else
>  # define attribute_hidden

I think this broke “make subdirs=benchtests bench-build” because
json-lib.c is erroneously compiled with -DMODULE_NAME=libc.

Thanks,
Florian

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[Florian Weimer]

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

On 08/21/2017 05:48 PM, Florian Weimer wrote:
> On 08/08/2017 06:40 PM, H.J. Lu wrote:
>> -#if defined SHARED || defined LIBC_NONSHARED
>> +#if defined SHARED || defined LIBC_NONSHARED \
>> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>>  #else
>>  # define attribute_hidden
> 
> I think this broke “make subdirs=benchtests bench-build” because
> json-lib.c is erroneously compiled with -DMODULE_NAME=libc.

I think this fixes it.

Florian

[-- Attachment #2: benchtests.patch --]
[-- Type: text/x-patch, Size: 1144 bytes --]

benchtests: Do not compile benchmark helper objects as libc modules

Otherwise, this will lead to link failures due to hidden symbol
references.

2017-08-21  Florian Weimer  <fweimer@redhat.com>

	Do not compile benchmark helper objects with -DMODULE_NAME=libc.
	* benchtests/Makefile (others-extras): Set to $(bench-extra-objs).
	Move before inclusion of ../Rules.

diff --git a/benchtests/Makefile b/benchtests/Makefile
index 37788e8c31..a0c3470398 100644
--- a/benchtests/Makefile
+++ b/benchtests/Makefile
@@ -93,6 +93,11 @@ $(objpfx)bench-malloc-thread: $(shared-thread-library)
 # affect their performance.
 .NOTPARALLEL:
 
+bench-extra-objs = json-lib.o
+
+extra-objs += $(bench-extra-objs)
+others-extras = $(bench-extra-objs)
+
 include ../Rules
 
 binaries-bench := $(addprefix $(objpfx)bench-,$(bench))
@@ -125,10 +130,6 @@ cpp-srcs-left := $(binaries-benchset:=.c) $(binaries-bench:=.c) \
 lib := nonlib
 include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
 
-bench-extra-objs = json-lib.o
-
-extra-objs += $(bench-extra-objs)
-
 bench-deps := bench-skeleton.c bench-timing.h Makefile
 
 run-bench = $(test-wrapper-env) \

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[H.J. Lu]

On Mon, Aug 21, 2017 at 8:52 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/21/2017 05:48 PM, Florian Weimer wrote:
>> On 08/08/2017 06:40 PM, H.J. Lu wrote:
>>> -#if defined SHARED || defined LIBC_NONSHARED
>>> +#if defined SHARED || defined LIBC_NONSHARED \
>>> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>>>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>>>  #else
>>>  # define attribute_hidden
>>
>> I think this broke “make subdirs=benchtests bench-build” because
>> json-lib.c is erroneously compiled with -DMODULE_NAME=libc.

Yes, this is

https://sourceware.org/bugzilla/show_bug.cgi?id=21864

> I think this fixes it.
>
> Florian

LGTM.  Please add "[BZ #21864]" in ChangeLog entry.

Thanks.

-- 
H.J.

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[Szabolcs Nagy]

On 08/08/17 17:40, H.J. Lu wrote:
> When building libc.a with PIE, enable hidden visibility to allow direct
> access to definitions within libc.a without using GOT.
...
> -#if defined SHARED || defined LIBC_NONSHARED
> +#if defined SHARED || defined LIBC_NONSHARED \
> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>  # define attribute_hidden __attribute__ ((visibility ("hidden")))

btw what happens if this is enabled for non-pie libc.a ?
i'd expect that to work too (it prevents some symbol
interpositions?)

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[H.J. Lu]

On Mon, Aug 21, 2017 at 9:31 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> On 08/08/17 17:40, H.J. Lu wrote:
>> When building libc.a with PIE, enable hidden visibility to allow direct
>> access to definitions within libc.a without using GOT.
> ...
>> -#if defined SHARED || defined LIBC_NONSHARED
>> +#if defined SHARED || defined LIBC_NONSHARED \
>> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>
> btw what happens if this is enabled for non-pie libc.a ?

It makes those symbols in libc.a hidden.

> i'd expect that to work too (it prevents some symbol
> interpositions?)
>

You can't interpose any symbols in libc.a, PIE or non-PIE.
What my patch does is to avoid GOT/PLT in PIE libc.a when
accessing them within libc.a.


-- 
H.J.

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[Szabolcs Nagy]

On 21/08/17 17:44, H.J. Lu wrote:
> On Mon, Aug 21, 2017 at 9:31 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>> On 08/08/17 17:40, H.J. Lu wrote:
>>> When building libc.a with PIE, enable hidden visibility to allow direct
>>> access to definitions within libc.a without using GOT.
>> ...
>>> -#if defined SHARED || defined LIBC_NONSHARED
>>> +#if defined SHARED || defined LIBC_NONSHARED \
>>> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>>>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>>
>> btw what happens if this is enabled for non-pie libc.a ?
> 
> It makes those symbols in libc.a hidden.
> 
>> i'd expect that to work too (it prevents some symbol
>> interpositions?)
>>
> 
> You can't interpose any symbols in libc.a, PIE or non-PIE.
> What my patch does is to avoid GOT/PLT in PIE libc.a when
> accessing them within libc.a.
> 

yes, but my point is that the ifdef can be simplified
if it's valid for the non-pie case too.

Re: [PATCH] Enable hidden visibility in libc.a compiled with PIE

[H.J. Lu]

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

On Mon, Aug 21, 2017 at 9:50 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> On 21/08/17 17:44, H.J. Lu wrote:
>> On Mon, Aug 21, 2017 at 9:31 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>>> On 08/08/17 17:40, H.J. Lu wrote:
>>>> When building libc.a with PIE, enable hidden visibility to allow direct
>>>> access to definitions within libc.a without using GOT.
>>> ...
>>>> -#if defined SHARED || defined LIBC_NONSHARED
>>>> +#if defined SHARED || defined LIBC_NONSHARED \
>>>> +  || (BUILD_PIE_DEFAULT && IS_IN (libc))
>>>>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>>>
>>> btw what happens if this is enabled for non-pie libc.a ?
>>
>> It makes those symbols in libc.a hidden.
>>
>>> i'd expect that to work too (it prevents some symbol
>>> interpositions?)
>>>
>>
>> You can't interpose any symbols in libc.a, PIE or non-PIE.
>> What my patch does is to avoid GOT/PLT in PIE libc.a when
>> accessing them within libc.a.
>>
>
> yes, but my point is that the ifdef can be simplified
> if it's valid for the non-pie case too.

Here is the patch.   Tested with PIE and non-PIE libc.a.

Any comments?

-- 
H.J.

[-- Attachment #2: 0001-Always-hide-symbols-referenced-internally-within-lib.patch --]
[-- Type: text/x-patch, Size: 1023 bytes --]

From d301c3c02e70ee9ff50d173da7d4d9626767bc4c Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 21 Aug 2017 10:27:57 -0700
Subject: [PATCH] Always hide symbols referenced internally within libc.a

Since none of internal symbols within libc.a, PIE or non-PIE, can be
interposed, we can always hide symbols referenced internally within
libc.a.

	* include/libc-symbols.h (attribute_hidden): Remove
	BUILD_PIE_DEFAULT check.
---
 include/libc-symbols.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index fe3571af52..d11844b97d 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -363,8 +363,7 @@ for linking")
   strong_alias(real, name)
 #endif
 
-#if defined SHARED || defined LIBC_NONSHARED \
-  || (BUILD_PIE_DEFAULT && IS_IN (libc))
+#if defined SHARED || defined LIBC_NONSHARED || IS_IN (libc)
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
-- 
2.13.5