On Tue, 11 Dec 2018 at 13:57, Leif Lindholm <leif.lindh...@linaro.org> wrote: > > Not my package, but a couple of minor style/language comments below > (because this is awkward enough any future readers/users will need all > the help they can get). > > On Tue, Dec 11, 2018 at 01:19:36PM +0100, Ard Biesheuvel wrote: > > The self-relocating PrePi module that is used by the ArmVirtQemuKernel > > and ArmVirtXen targets runs the linker in PIE mode so that it emits > > dynamic relocations into the final image in a way that permits the > > module to relocate itself into place before calling into the C code. > > > > When building these targets using the CLANG38 toolchain, we switch > > from the BFD to the GOLD linker, which behaves a bit differently when > > building PIE executables, and insists on emitting GOT indirected symbol > > references throughout, which means a) that we end up with absolute > > addresses (which need to be fixed up at load time) for no good reason, > > and b) we have to add support for handling GOT entries to GenFw if we > > want to convert them into PE/COFF. > > > > So instead, let's emit a shared library. Since the ELF image only serves > > as the input to GenFw, this does not lead to any loss of functionality, > > although it does require the -Bsymbolic linker option to be added to > > ensure that no symbol based dynamic relocations are emitted (which > > would, e.g., permit lazy binding for shared libraries). So for all > > other toolchains, the linker option changes are a no-op. > > > > Then, we have to convince CLANG38/GOLD that there is no need to refer > > to symbols via a GOT entry. This is done by forcing hidden visibility > > for all symbols in all components that make up the PrePi SEC module: > > this informs the linker that a symbol is never exported or preempted, > > making it safe to refer to it directly from anywhere in the code, > > rather than indirectly via a GOT entry. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > > --- > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 12 ++++++++-- > > ArmVirtPkg/ArmVirtXen.dsc | 12 ++++++++-- > > ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 2 +- > > ArmVirtPkg/Include/Platform/Hidden.h | 23 > > ++++++++++++++++++++ > > 4 files changed, 44 insertions(+), 5 deletions(-) > > > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > index 9928919bf5b0..c4324a9e264b 100644 > > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > @@ -68,11 +68,19 @@ [LibraryClasses.common] > > [LibraryClasses.common.UEFI_DRIVER] > > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > > > -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] > > +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > > # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE > > # executable we build for the relocatable PrePi. They are not runtime > > # relocatable in ELF. > > - *_CLANG35_*_CC_FLAGS = -mno-movt > > + *_CLANG35_ARM_CC_FLAGS = -mno-movt > > + > > + # > > + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which > > insists > > + # on emitting GOT based symbol references when running in shared mode, > > unless > > + # we override visibility to 'hidden' in all modules that make up the > > PrePi > > + # build. > > + # > > + GCC:*_CLANG38_*_CC_FLAGS = -include > > $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > > > > > > ################################################################################ > > # > > diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc > > index 20fae9e675bb..e083666f54ea 100644 > > --- a/ArmVirtPkg/ArmVirtXen.dsc > > +++ b/ArmVirtPkg/ArmVirtXen.dsc > > @@ -57,11 +57,19 @@ [LibraryClasses] > > [LibraryClasses.common.UEFI_DRIVER] > > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > > > -[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE] > > +[BuildOptions.common.EDKII.SEC, BuildOptions.common.EDKII.BASE] > > # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE > > # executable we build for the relocatable PrePi. They are not runtime > > # relocatable in ELF. > > - *_CLANG35_*_CC_FLAGS = -mno-movt > > + *_CLANG35_ARM_CC_FLAGS = -mno-movt > > + > > + # > > + # CLANG38 with LTO support enabled uses the GNU GOLD linker, which > > insists > > + # on emitting GOT based symbol references when running in shared mode, > > unless > > + # we override visibility to 'hidden' in all modules that make up the > > PrePi > > + # build. > > + # > > + GCC:*_CLANG38_*_CC_FLAGS = -include > > $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h > > > > > > ################################################################################ > > # > > diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > > b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > > index 034ddb41cb48..5fe6cd8eb481 100755 > > --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > > +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > > @@ -105,4 +105,4 @@ [Pcd] > > gArmTokenSpaceGuid.PcdFvBaseAddress > > > > [BuildOptions] > > - GCC:*_*_*_DLINK_FLAGS = -pie -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds > > + GCC:*_*_*_DLINK_FLAGS = -shared -Wl,-Bsymbolic > > -Wl,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds > > diff --git a/ArmVirtPkg/Include/Platform/Hidden.h > > b/ArmVirtPkg/Include/Platform/Hidden.h > > new file mode 100644 > > index 000000000000..20e9f5d896b2 > > --- /dev/null > > +++ b/ArmVirtPkg/Include/Platform/Hidden.h > > @@ -0,0 +1,23 @@ > > +/** @file > > + > > + Copyright (c) 2018, Linaro Limited. All rights reserved. > > + > > + This program and the accompanying materials > > + are licensed and made available under the terms and conditions of the > > BSD License > > + which accompanies this distribution. The full text of the license may > > be found at > > + http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > > IMPLIED. > > + > > +**/ > > + > > I know this isn't intended to be explicitly included, but could we > still have in include guard? >
Sure. > > +// > > +// Setting the GCC -fvisibility=hidden command line option is not quite > > the same > > +// as setting the pragma below: the former only affects definitions, > > whereas the > > +// latter affects extern declarations as well. So if we want to ensure > > that no > > s/latter/pragma/? (Just saves on recursive natural language parsing > Fair enough. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
