From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::342; helo=mail-wm1-x342.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com [IPv6:2a00:1450:4864:20::342]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4A8F821198CD1 for ; Tue, 11 Dec 2018 04:57:29 -0800 (PST) Received: by mail-wm1-x342.google.com with SMTP id y185so8954463wmd.1 for ; Tue, 11 Dec 2018 04:57:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=rDlNdAW68RbIC6D+OY0sduzDUC+OMOy7YVCOIDSyW6c=; b=Jy+P3E+jTQTtunePBaUvnLWjTOkWlln7GASGQjG2yyntbFA+KNqMxhn4WX++mpUnRl 9kAZeZ3iwrR0n7qlYnVuUp9cxeRu7VXrMBKzpoI73Pwrdnu5ftAxIzFoU77kBoZ1o8vg 9DYwiw8HgAU0TZ8vaLgbCLlDNvxmEz3q4jCW4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=rDlNdAW68RbIC6D+OY0sduzDUC+OMOy7YVCOIDSyW6c=; b=caUyUsf3zdblcbYbQJ9tmRXUsd1OoZI2zaQMPUscQ9vJo4T9Ki1ldT2KT2qhC8AwYC M5fO+1WqKI/ruIWqM1nbvBuofDa8vzmcup7L3rM23Xfrf1A/Tb04xleo2S7JslXzIZCc i2betkKXWScSR+Wn5cHYLg0Ie46vprA2+6GtQJvsIE3LCaMak3YNU6YYyEHe2TEqQx3v ppoA4cN6cuAgcG3BlDGNZ+8NjCIg4p2A40glrClj6PEH5cSPdCsM/XNpL6ZHil2P5tJe CN189mFkxjLTlMjHSgwVfJyDi8Xjj8DDsF6pYsqNPZx8+GLPvTSUYyU+3hDQ4byMvbt8 HtFw== X-Gm-Message-State: AA+aEWaXqNb5ywRuwL7hDbl2MpZ171ueu4J/vmhmfrM/KiKbcXEveu7c YPVIDmf9HSvbx3Rpa+d5YQBkmg== X-Google-Smtp-Source: AFSGD/WaBuTFhqRBX7zW5xDZOlQbSGNPmXhjk2XnpZY1j2Vw7Lb9GyQOO6uKNVZFxTPSyofuQJh9aw== X-Received: by 2002:a1c:2d4b:: with SMTP id t72mr2017677wmt.99.1544533048325; Tue, 11 Dec 2018 04:57:28 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id l20sm37114514wrb.93.2018.12.11.04.57.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 11 Dec 2018 04:57:27 -0800 (PST) Date: Tue, 11 Dec 2018 12:57:26 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, lersek@redhat.com, philmd@redhat.com Message-ID: <20181211125726.4dhmwuquemu3bkhg@bivouac.eciton.net> References: <20181211121936.3599-1-ard.biesheuvel@linaro.org> <20181211121936.3599-3-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20181211121936.3599-3-ard.biesheuvel@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Dec 2018 12:57:30 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > --- > 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? > +// > +// 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 when those cirquits may already be in use.) / Leif > +// GOT indirected symbol references are emitted, we need to use the pragma, or > +// GOT based cross object references could be emitted, e.g., in libraries, and > +// these cannot be relaxed to ordinary symbol references at link time. > +// > +#pragma GCC visibility push (hidden) > -- > 2.19.2 >