From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 69F0721199528 for ; Tue, 11 Dec 2018 08:10:16 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D1B3E89AD3; Tue, 11 Dec 2018 16:10:15 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-134.rdu2.redhat.com [10.10.120.134]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F69619747; Tue, 11 Dec 2018 16:10:14 +0000 (UTC) To: Ard Biesheuvel , Leif Lindholm Cc: "edk2-devel@lists.01.org" , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= References: <20181211121936.3599-1-ard.biesheuvel@linaro.org> <20181211121936.3599-3-ard.biesheuvel@linaro.org> <20181211125726.4dhmwuquemu3bkhg@bivouac.eciton.net> From: Laszlo Ersek Message-ID: <2cdf4fc0-3b85-d82e-2683-8ba9052e8e47@redhat.com> Date: Tue, 11 Dec 2018 17:10:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 11 Dec 2018 16:10:15 +0000 (UTC) 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 16:10:16 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/11/18 13:58, Ard Biesheuvel wrote: > On Tue, 11 Dec 2018 at 13:57, Leif Lindholm 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 >>> --- >>> 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. > Acked-by: Laszlo Ersek Thanks Laszlo