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 <[email protected]>
Thanks Liming > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Gao, Liming > Sent: Tuesday, August 02, 2016 10:39 AM > To: Ard Biesheuvel <[email protected]> > Cc: Justen, Jordan L <[email protected]>; [email protected]; > [email protected]; [email protected] > 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:[email protected]] > > Sent: Tuesday, August 02, 2016 12:12 AM > > To: Gao, Liming <[email protected]> > > Cc: Shi, Steven <[email protected]>; Zhu, Yonghong > > <[email protected]>; Justen, Jordan L <[email protected]>; > > [email protected]; [email protected]; [email protected] > > 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 <[email protected]> > > wrote: > > > On 1 August 2016 at 16:56, Ard Biesheuvel <[email protected]> > > wrote: > > >> On 1 August 2016 at 16:49, Ard Biesheuvel <[email protected]> > > wrote: > > >>> On 1 August 2016 at 16:18, Gao, Liming <[email protected]> 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 > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
