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

["Gao, Liming" <liming.gao@intel.com>]

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