public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdePkg: move to 'hidden' visibility for all symbols under GCC/X64
@ 2016-08-01  6:57 Ard Biesheuvel
  2016-08-02  2:49 ` Gao, Liming
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2016-08-01  6:57 UTC (permalink / raw)
  To: steven.shi, yonghong.zhu, liming.gao, jordan.l.justen, edk2-devel
  Cc: Ard Biesheuvel

When using GCC to build for X64, we switched to the position independent
small code model, which is much more efficient in terms of code generation
and runtime relocation footprint, and produces binaries that can execute
correctly from any offset.

However, the PIC routines are by default geared towards hosted binaries
containing symbol references that may resolve to definitions in other
dynamic objects, and for this reason, external symbol references are
indirected via a GOT entry by default (which also results in a .reloc fixup
entry) unless we annotate them.

For this reason, we introduced the 'protected' visibility annotation for
all symbol definitions and references, by setting the GCC visibility
pragma. However, as it turns out, this is not sufficient for all versions
of GCC, and in some cases (GCC 5.x using the GCC49 toolchain tag), may
still result in GOT based relocations.

So switch to 'hidden' visibility instead, which is slightly stronger, and
fixes this issue for the versions of GCC that exhibit the problem.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Include/X64/ProcessorBind.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/X64/ProcessorBind.h b/MdePkg/Include/X64/ProcessorBind.h
index a4aad3e524e8..666cc8e8bd16 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -29,12 +29,12 @@
 
 #if defined(__GNUC__) && defined(__pic__)
 //
-// Mark all symbol declarations and references as protected, meaning they will
+// Mark all symbol declarations and references as hidden, meaning they will
 // not be subject to symbol preemption. This allows the compiler to refer to
 // symbols directly using relative references rather than via the GOT, which
 // contains absolute symbol addresses that are subject to runtime relocation.
 //
-#pragma GCC visibility push (protected)
+#pragma GCC visibility push (hidden)
 #endif
 
 #if defined(__INTEL_COMPILER)
-- 
2.7.4



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

* Re: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under GCC/X64
  2016-08-01  6:57 [PATCH] MdePkg: move to 'hidden' visibility for all symbols under GCC/X64 Ard Biesheuvel
@ 2016-08-02  2:49 ` Gao, Liming
  2016-08-02  6:21   ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Gao, Liming @ 2016-08-02  2:49 UTC (permalink / raw)
  To: Ard Biesheuvel, Shi, Steven, Zhu, Yonghong, Justen, Jordan L,
	edk2-devel@lists.01.org

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, August 01, 2016 2:57 PM
> To: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
> <yonghong.zhu@intel.com>; Gao, Liming <liming.gao@intel.com>; Justen,
> Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under
> GCC/X64
> 
> When using GCC to build for X64, we switched to the position independent
> small code model, which is much more efficient in terms of code generation
> and runtime relocation footprint, and produces binaries that can execute
> correctly from any offset.
> 
> However, the PIC routines are by default geared towards hosted binaries
> containing symbol references that may resolve to definitions in other
> dynamic objects, and for this reason, external symbol references are
> indirected via a GOT entry by default (which also results in a .reloc fixup
> entry) unless we annotate them.
> 
> For this reason, we introduced the 'protected' visibility annotation for
> all symbol definitions and references, by setting the GCC visibility
> pragma. However, as it turns out, this is not sufficient for all versions
> of GCC, and in some cases (GCC 5.x using the GCC49 toolchain tag), may
> still result in GOT based relocations.
> 
> So switch to 'hidden' visibility instead, which is slightly stronger, and
> fixes this issue for the versions of GCC that exhibit the problem.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Include/X64/ProcessorBind.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/X64/ProcessorBind.h
> b/MdePkg/Include/X64/ProcessorBind.h
> index a4aad3e524e8..666cc8e8bd16 100644
> --- a/MdePkg/Include/X64/ProcessorBind.h
> +++ b/MdePkg/Include/X64/ProcessorBind.h
> @@ -29,12 +29,12 @@
> 
>  #if defined(__GNUC__) && defined(__pic__)
>  //
> -// Mark all symbol declarations and references as protected, meaning they
> will
> +// Mark all symbol declarations and references as hidden, meaning they will
>  // not be subject to symbol preemption. This allows the compiler to refer to
>  // symbols directly using relative references rather than via the GOT, which
>  // contains absolute symbol addresses that are subject to runtime relocation.
>  //
> -#pragma GCC visibility push (protected)
> +#pragma GCC visibility push (hidden)
>  #endif
> 
>  #if defined(__INTEL_COMPILER)
> --
> 2.7.4



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

* Re: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under GCC/X64
  2016-08-02  2:49 ` Gao, Liming
@ 2016-08-02  6:21   ` Ard Biesheuvel
  0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2016-08-02  6:21 UTC (permalink / raw)
  To: Gao, Liming
  Cc: Shi, Steven, Zhu, Yonghong, Justen, Jordan L,
	edk2-devel@lists.01.org

On 2 August 2016 at 04:49, Gao, Liming <liming.gao@intel.com> wrote:
> Reviewed-by: Liming Gao <liming.gao@intel.com>
>

Thanks
Pushed as 28ade7b802e0

-- 
Ard.

>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Monday, August 01, 2016 2:57 PM
>> To: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
>> <yonghong.zhu@intel.com>; Gao, Liming <liming.gao@intel.com>; Justen,
>> Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under
>> GCC/X64
>>
>> When using GCC to build for X64, we switched to the position independent
>> small code model, which is much more efficient in terms of code generation
>> and runtime relocation footprint, and produces binaries that can execute
>> correctly from any offset.
>>
>> However, the PIC routines are by default geared towards hosted binaries
>> containing symbol references that may resolve to definitions in other
>> dynamic objects, and for this reason, external symbol references are
>> indirected via a GOT entry by default (which also results in a .reloc fixup
>> entry) unless we annotate them.
>>
>> For this reason, we introduced the 'protected' visibility annotation for
>> all symbol definitions and references, by setting the GCC visibility
>> pragma. However, as it turns out, this is not sufficient for all versions
>> of GCC, and in some cases (GCC 5.x using the GCC49 toolchain tag), may
>> still result in GOT based relocations.
>>
>> So switch to 'hidden' visibility instead, which is slightly stronger, and
>> fixes this issue for the versions of GCC that exhibit the problem.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  MdePkg/Include/X64/ProcessorBind.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/MdePkg/Include/X64/ProcessorBind.h
>> b/MdePkg/Include/X64/ProcessorBind.h
>> index a4aad3e524e8..666cc8e8bd16 100644
>> --- a/MdePkg/Include/X64/ProcessorBind.h
>> +++ b/MdePkg/Include/X64/ProcessorBind.h
>> @@ -29,12 +29,12 @@
>>
>>  #if defined(__GNUC__) && defined(__pic__)
>>  //
>> -// Mark all symbol declarations and references as protected, meaning they
>> will
>> +// Mark all symbol declarations and references as hidden, meaning they will
>>  // not be subject to symbol preemption. This allows the compiler to refer to
>>  // symbols directly using relative references rather than via the GOT, which
>>  // contains absolute symbol addresses that are subject to runtime relocation.
>>  //
>> -#pragma GCC visibility push (protected)
>> +#pragma GCC visibility push (hidden)
>>  #endif
>>
>>  #if defined(__INTEL_COMPILER)
>> --
>> 2.7.4
>


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

end of thread, other threads:[~2016-08-02  6:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-01  6:57 [PATCH] MdePkg: move to 'hidden' visibility for all symbols under GCC/X64 Ard Biesheuvel
2016-08-02  2:49 ` Gao, Liming
2016-08-02  6:21   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox