[PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility for module entry points


Liming Gao
 

Ard:
This update works. It is better. For my question, I get the answer from your previous patch. Please ignore it.

Patches 3&4&6&8 are good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks
Liming

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
Gao, Liming
Sent: Tuesday, August 02, 2016 10:39 AM
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Justen, Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org;
leif.lindholm@linaro.org; lersek@redhat.com
Subject: Re: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility
for module entry points

Ard:
I will verify it. And, I would ask why only X64 requires it? IA32, ARM and
AARCH64 doesn't specially handle it?

Thanks
Liming
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Tuesday, August 02, 2016 12:12 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
<yonghong.zhu@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
edk2-devel@lists.01.org; lersek@redhat.com; leif.lindholm@linaro.org
Subject: Re: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden'
visibility
for module entry points

On 1 August 2016 at 17:51, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:56, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:49, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED
and
apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It
has
been specified in LINK_FLAGS in tools_def.txt. Could we also specify its
attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds
b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.
Hmm, looks like I spoke too soon. I don't know what I did wrong, but
this does not actually work.
The only alternative I can think of is to add a static non-lto object
to the tree that refers to _ModuleEntryPoint, similar to the way I
handle the ARM intrinsics in patch #5
As it turns out, the LTO linker does not need to visibility pragma to
prevent it from emitting GOT based relocations. This makes sense,
considering that the LTO linker can see that no symbol references are
ever satisfied across dynamic object boundaries. That means I could
work around this in the following way:

diff --git a/BaseTools/Conf/tools_def.template
b/BaseTools/Conf/tools_def.template
index 314adaf6bfa8..983e2fea7390 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4462,7 +4462,7 @@ DEFINE GCC49_ARM_ASLDLINK_FLAGS =
DEF(GCC48_ARM_ASLDLINK_FLAGS)
DEFINE GCC49_AARCH64_ASLDLINK_FLAGS =
DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -flto
-fno-builtin
-DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin
+DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin -DUSING_LTO
DEFINE GCC5_IA32_X64_DLINK_COMMON =
DEF(GCC49_IA32_X64_DLINK_COMMON)
DEFINE GCC5_IA32_X64_ASLDLINK_FLAGS =
DEF(GCC49_IA32_X64_ASLDLINK_FLAGS)
DEFINE GCC5_IA32_X64_DLINK_FLAGS =
DEF(GCC49_IA32_X64_DLINK_FLAGS) -flto
diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..77fab7055afc 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -27,12 +27,15 @@
#pragma pack()
#endif

-#if defined(__GNUC__) && defined(__pic__)
+#if defined(__GNUC__) && defined(__pic__) && !defined(USING_LTO)
//
// 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.
+// The LTO linker will not emit GOT based relocations anyway, so there is
no
+// need to set the pragma in that case (and doing so will cause issues of its
+// own)
//
#pragma GCC visibility push (hidden)
#endif

and I can drop this patch.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Liming Gao
 

Ard:
I will verify it. And, I would ask why only X64 requires it? IA32, ARM and AARCH64 doesn't specially handle it?

Thanks
Liming

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Tuesday, August 02, 2016 12:12 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
<yonghong.zhu@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
edk2-devel@lists.01.org; lersek@redhat.com; leif.lindholm@linaro.org
Subject: Re: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility
for module entry points

On 1 August 2016 at 17:51, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:56, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:49, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED and
apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It has
been specified in LINK_FLAGS in tools_def.txt. Could we also specify its
attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds
b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.
Hmm, looks like I spoke too soon. I don't know what I did wrong, but
this does not actually work.
The only alternative I can think of is to add a static non-lto object
to the tree that refers to _ModuleEntryPoint, similar to the way I
handle the ARM intrinsics in patch #5
As it turns out, the LTO linker does not need to visibility pragma to
prevent it from emitting GOT based relocations. This makes sense,
considering that the LTO linker can see that no symbol references are
ever satisfied across dynamic object boundaries. That means I could
work around this in the following way:

diff --git a/BaseTools/Conf/tools_def.template
b/BaseTools/Conf/tools_def.template
index 314adaf6bfa8..983e2fea7390 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4462,7 +4462,7 @@ DEFINE GCC49_ARM_ASLDLINK_FLAGS =
DEF(GCC48_ARM_ASLDLINK_FLAGS)
DEFINE GCC49_AARCH64_ASLDLINK_FLAGS =
DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -flto
-fno-builtin
-DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin
+DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin -DUSING_LTO
DEFINE GCC5_IA32_X64_DLINK_COMMON =
DEF(GCC49_IA32_X64_DLINK_COMMON)
DEFINE GCC5_IA32_X64_ASLDLINK_FLAGS =
DEF(GCC49_IA32_X64_ASLDLINK_FLAGS)
DEFINE GCC5_IA32_X64_DLINK_FLAGS =
DEF(GCC49_IA32_X64_DLINK_FLAGS) -flto
diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..77fab7055afc 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -27,12 +27,15 @@
#pragma pack()
#endif

-#if defined(__GNUC__) && defined(__pic__)
+#if defined(__GNUC__) && defined(__pic__) && !defined(USING_LTO)
//
// 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.
+// The LTO linker will not emit GOT based relocations anyway, so there is no
+// need to set the pragma in that case (and doing so will cause issues of its
+// own)
//
#pragma GCC visibility push (hidden)
#endif

and I can drop this patch.


Ard Biesheuvel
 

On 1 August 2016 at 17:51, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
On 1 August 2016 at 16:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
On 1 August 2016 at 16:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED and apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It has been specified in LINK_FLAGS in tools_def.txt. Could we also specify its attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.
Hmm, looks like I spoke too soon. I don't know what I did wrong, but
this does not actually work.
The only alternative I can think of is to add a static non-lto object
to the tree that refers to _ModuleEntryPoint, similar to the way I
handle the ARM intrinsics in patch #5
As it turns out, the LTO linker does not need to visibility pragma to
prevent it from emitting GOT based relocations. This makes sense,
considering that the LTO linker can see that no symbol references are
ever satisfied across dynamic object boundaries. That means I could
work around this in the following way:

diff --git a/BaseTools/Conf/tools_def.template
b/BaseTools/Conf/tools_def.template
index 314adaf6bfa8..983e2fea7390 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4462,7 +4462,7 @@ DEFINE GCC49_ARM_ASLDLINK_FLAGS =
DEF(GCC48_ARM_ASLDLINK_FLAGS)
DEFINE GCC49_AARCH64_ASLDLINK_FLAGS = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -flto
-fno-builtin
-DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin
+DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin -DUSING_LTO
DEFINE GCC5_IA32_X64_DLINK_COMMON = DEF(GCC49_IA32_X64_DLINK_COMMON)
DEFINE GCC5_IA32_X64_ASLDLINK_FLAGS = DEF(GCC49_IA32_X64_ASLDLINK_FLAGS)
DEFINE GCC5_IA32_X64_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_FLAGS) -flto
diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..77fab7055afc 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -27,12 +27,15 @@
#pragma pack()
#endif

-#if defined(__GNUC__) && defined(__pic__)
+#if defined(__GNUC__) && defined(__pic__) && !defined(USING_LTO)
//
// 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.
+// The LTO linker will not emit GOT based relocations anyway, so there is no
+// need to set the pragma in that case (and doing so will cause issues of its
+// own)
//
#pragma GCC visibility push (hidden)
#endif

and I can drop this patch.


Ard Biesheuvel
 

On 1 August 2016 at 16:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
On 1 August 2016 at 16:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED and apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It has been specified in LINK_FLAGS in tools_def.txt. Could we also specify its attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.
Hmm, looks like I spoke too soon. I don't know what I did wrong, but
this does not actually work.
The only alternative I can think of is to add a static non-lto object
to the tree that refers to _ModuleEntryPoint, similar to the way I
handle the ARM intrinsics in patch #5

Which one do you hate the least? :-)


Ard Biesheuvel
 

On 1 August 2016 at 16:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED and apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It has been specified in LINK_FLAGS in tools_def.txt. Could we also specify its attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.
Hmm, looks like I spoke too soon. I don't know what I did wrong, but
this does not actually work.

--
Ard.


Ard Biesheuvel
 

On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED and apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It has been specified in LINK_FLAGS in tools_def.txt. Could we also specify its attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.

I will drop this patch, and add this hunk to GccBase.lds instead.

--
Ard.


Thanks
Liming
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
Ard Biesheuvel
Sent: Monday, August 01, 2016 4:02 PM
To: Shi, Steven ; Zhu, Yonghong
; Gao, Liming ; Justen,
Jordan L ; edk2-devel@lists.01.org
Cc: lersek@redhat.com; leif.lindholm@linaro.org; Ard Biesheuvel

Subject: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility for
module entry points

When building with LTO enabled, the LTO routines infer from the 'hidden'
visibility of the _ModuleEntryPoint symbol that its code only needs to be
retained if it is referenced internally, and disregards the fact that
it is referenced by the entry point field in the ELF metadata.

This is arguably a bug in LTO, but we can work around it by ensuring that
the _ModuleEntryPoint symbol is emitted with protected visibility instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel
---
MdePkg/Include/X64/ProcessorBind.h | 9 ++++++++-
MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf | 2 ++
MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf | 2 ++
MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf | 2 ++
MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf | 2
++
MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf | 2 ++
6 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..e45b9cba2bb3 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -29,13 +29,20 @@

#if defined(__GNUC__) && defined(__pic__)
//
-// Mark all symbol declarations and references as hidden, 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.
//
+// * Under LTO, the entry point of a module must have protected or default
+// visibility to prevent it from being pruned.
+//
+#ifdef GCC_VISIBILITY_PROTECTED
+#pragma GCC visibility push (protected)
+#else
#pragma GCC visibility push (hidden)
#endif
+#endif

#if defined(__INTEL_COMPILER)
//
diff --git a/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
b/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
index 01f64c34c7a1..2d6f87ed062e 100644
--- a/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
+++ b/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
@@ -39,3 +39,5 @@ [LibraryClasses]
BaseLib
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
b/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
index d920306713c5..4e61783b3bd5 100644
--- a/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
+++ b/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
@@ -37,3 +37,5 @@ [LibraryClasses]
BaseLib
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
b/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
index a2db9e058bbe..adfd91bdc57e 100644
--- a/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
+++ b/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
@@ -37,3 +37,5 @@ [Packages]
[LibraryClasses]
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git
a/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
b/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
index be92b3dc0760..9525c55c2051 100644
---
a/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
+++
b/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
@@ -38,3 +38,5 @@ [LibraryClasses]
DebugLib
BaseLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
b/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
index 7a9dcbcd4df2..8d30b1197850 100644
--- a/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
+++ b/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
@@ -65,3 +65,5 @@ [Depex.common.UEFI_DRIVER]
gEfiVariableArchProtocolGuid AND
gEfiWatchdogTimerArchProtocolGuid

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Liming Gao
 

Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED and apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It has been specified in LINK_FLAGS in tools_def.txt. Could we also specify its attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?

Thanks
Liming

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
Ard Biesheuvel
Sent: Monday, August 01, 2016 4:02 PM
To: Shi, Steven ; Zhu, Yonghong
; Gao, Liming ; Justen,
Jordan L ; edk2-devel@lists.01.org
Cc: lersek@redhat.com; leif.lindholm@linaro.org; Ard Biesheuvel

Subject: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility for
module entry points

When building with LTO enabled, the LTO routines infer from the 'hidden'
visibility of the _ModuleEntryPoint symbol that its code only needs to be
retained if it is referenced internally, and disregards the fact that
it is referenced by the entry point field in the ELF metadata.

This is arguably a bug in LTO, but we can work around it by ensuring that
the _ModuleEntryPoint symbol is emitted with protected visibility instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel
---
MdePkg/Include/X64/ProcessorBind.h | 9 ++++++++-
MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf | 2 ++
MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf | 2 ++
MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf | 2 ++
MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf | 2
++
MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf | 2 ++
6 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..e45b9cba2bb3 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -29,13 +29,20 @@

#if defined(__GNUC__) && defined(__pic__)
//
-// Mark all symbol declarations and references as hidden, 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.
//
+// * Under LTO, the entry point of a module must have protected or default
+// visibility to prevent it from being pruned.
+//
+#ifdef GCC_VISIBILITY_PROTECTED
+#pragma GCC visibility push (protected)
+#else
#pragma GCC visibility push (hidden)
#endif
+#endif

#if defined(__INTEL_COMPILER)
//
diff --git a/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
b/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
index 01f64c34c7a1..2d6f87ed062e 100644
--- a/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
+++ b/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
@@ -39,3 +39,5 @@ [LibraryClasses]
BaseLib
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
b/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
index d920306713c5..4e61783b3bd5 100644
--- a/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
+++ b/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
@@ -37,3 +37,5 @@ [LibraryClasses]
BaseLib
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
b/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
index a2db9e058bbe..adfd91bdc57e 100644
--- a/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
+++ b/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
@@ -37,3 +37,5 @@ [Packages]
[LibraryClasses]
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git
a/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
b/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
index be92b3dc0760..9525c55c2051 100644
---
a/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
+++
b/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
@@ -38,3 +38,5 @@ [LibraryClasses]
DebugLib
BaseLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
b/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
index 7a9dcbcd4df2..8d30b1197850 100644
--- a/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
+++ b/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
@@ -65,3 +65,5 @@ [Depex.common.UEFI_DRIVER]
gEfiVariableArchProtocolGuid AND
gEfiWatchdogTimerArchProtocolGuid

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Ard Biesheuvel
 

When building with LTO enabled, the LTO routines infer from the 'hidden'
visibility of the _ModuleEntryPoint symbol that its code only needs to be
retained if it is referenced internally, and disregards the fact that
it is referenced by the entry point field in the ELF metadata.

This is arguably a bug in LTO, but we can work around it by ensuring that
the _ModuleEntryPoint symbol is emitted with protected visibility instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdePkg/Include/X64/ProcessorBind.h | 9 ++++++++-
MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf | 2 ++
MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf | 2 ++
MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf | 2 ++
MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf | 2 ++
MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf | 2 ++
6 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/X64/ProcessorBind.h b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..e45b9cba2bb3 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -29,13 +29,20 @@

#if defined(__GNUC__) && defined(__pic__)
//
-// Mark all symbol declarations and references as hidden, 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.
//
+// * Under LTO, the entry point of a module must have protected or default
+// visibility to prevent it from being pruned.
+//
+#ifdef GCC_VISIBILITY_PROTECTED
+#pragma GCC visibility push (protected)
+#else
#pragma GCC visibility push (hidden)
#endif
+#endif

#if defined(__INTEL_COMPILER)
//
diff --git a/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf b/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
index 01f64c34c7a1..2d6f87ed062e 100644
--- a/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
+++ b/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
@@ -39,3 +39,5 @@ [LibraryClasses]
BaseLib
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf b/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
index d920306713c5..4e61783b3bd5 100644
--- a/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
+++ b/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
@@ -37,3 +37,5 @@ [LibraryClasses]
BaseLib
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf b/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
index a2db9e058bbe..adfd91bdc57e 100644
--- a/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
+++ b/MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
@@ -37,3 +37,5 @@ [Packages]
[LibraryClasses]
DebugLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf b/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
index be92b3dc0760..9525c55c2051 100644
--- a/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
+++ b/MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
@@ -38,3 +38,5 @@ [LibraryClasses]
DebugLib
BaseLib

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
diff --git a/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf b/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
index 7a9dcbcd4df2..8d30b1197850 100644
--- a/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
+++ b/MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
@@ -65,3 +65,5 @@ [Depex.common.UEFI_DRIVER]
gEfiVariableArchProtocolGuid AND
gEfiWatchdogTimerArchProtocolGuid

+[BuildOptions]
+ GCC:*_*_X64_CC_FLAGS = -DGCC_VISIBILITY_PROTECTED
--
2.7.4