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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

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.