From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::141; helo=mail-it1-x141.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x141.google.com (mail-it1-x141.google.com [IPv6:2607:f8b0:4864:20::141]) (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 0975721198CD1 for ; Tue, 11 Dec 2018 04:58:54 -0800 (PST) Received: by mail-it1-x141.google.com with SMTP id h193so3570252ita.5 for ; Tue, 11 Dec 2018 04:58:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=5LkSqN3VL9q9REoMK/PWu4E6jRU8kP0d9bZO1io23Kk=; b=GM0OTOOIv5PtGLGUs0+RzKN/644cbPyFHfdzLtmkn952gQ0fSSNGgbvKzTqsNjOmtg Qq0Qkw+s+rKPQNSpvrGGUYN9iWZrMCfYelORcQOT11tL5329Zw+yvnaE4d3NSpJDVE3h gFirh7Utfjn4YBBYJxcatJqyh84b+e6un+DkU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5LkSqN3VL9q9REoMK/PWu4E6jRU8kP0d9bZO1io23Kk=; b=XyCzWqfy/WSinamZkV0n9FIznKjK8U4kkgt6Dn58SqVdc+uRQlFI8gwfh/EDW3Rum3 XPcyjU0UU8PveUO/enQseD+MSBc1PY6feiVGqgGFxrHa53JSaC/t0bVrDbDblP5RJlIs 5UxJBuF8kHH9Dyeh7TiUloaoML4yJIf/mpVImUzAufSSwO0VCudepGFU0a2BlkDMrEY5 /shhOBlA+cLrFdflxneUbj5/OzxnDhadAOzJVv53IUcsXUNxn8nBNnCRIK1hNN5B6uDm xH3xre0yZf06MTPQr0rSPkNNfoWlVoHj4f20qlcxImLBB74c4KReSEP0UNh5ai4VmeXY lAcA== X-Gm-Message-State: AA+aEWZwi1W/nzhYvNsvIIxtZpoqaZy0a8IbvXH0415H8rv5zNOc8Moi Q2itMlH500x/UmtNU5r0nreHWHeVSbM+gzzrmoD5Vg== X-Google-Smtp-Source: AFSGD/UbtOibvUXM9ATcc547LeHOScifBcGCcWvx0CqXKeJZw65SnRJm4i/TNobVpuVCcH/dZVx/elZfBJU2jeqBDTY= X-Received: by 2002:a24:710:: with SMTP id f16mr1803522itf.121.1544533134057; Tue, 11 Dec 2018 04:58:54 -0800 (PST) MIME-Version: 1.0 References: <20181211121936.3599-1-ard.biesheuvel@linaro.org> <20181211121936.3599-3-ard.biesheuvel@linaro.org> <20181211125726.4dhmwuquemu3bkhg@bivouac.eciton.net> In-Reply-To: <20181211125726.4dhmwuquemu3bkhg@bivouac.eciton.net> From: Ard Biesheuvel Date: Tue, 11 Dec 2018 13:58:42 +0100 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= 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:58:55 -0000 Content-Type: text/plain; charset="UTF-8" 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.