From: Laszlo Ersek <lersek@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Leif Lindholm <leif.lindholm@linaro.org>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues
Date: Tue, 11 Dec 2018 17:10:13 +0100 [thread overview]
Message-ID: <2cdf4fc0-3b85-d82e-2683-8ba9052e8e47@redhat.com> (raw)
In-Reply-To: <CAKv+Gu8wYYNEb45PpKAH5DxGLR7BffruAfQ2hcYWbJ1TWv+GfA@mail.gmail.com>
On 12/11/18 13:58, Ard Biesheuvel wrote:
> On Tue, 11 Dec 2018 at 13:57, Leif Lindholm <leif.lindholm@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.biesheuvel@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.
>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
next prev parent reply other threads:[~2018-12-11 16:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-11 12:19 [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel
2018-12-11 12:19 ` [PATCH 1/2] ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM Ard Biesheuvel
2018-12-11 15:59 ` Laszlo Ersek
2018-12-11 12:19 ` [PATCH 2/2] ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues Ard Biesheuvel
2018-12-11 12:57 ` Leif Lindholm
2018-12-11 12:58 ` Ard Biesheuvel
2018-12-11 16:10 ` Laszlo Ersek [this message]
2018-12-11 16:50 ` [PATCH 0/2] ArmVirtPkg: fixes for ARM and CLANG38 builds Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2cdf4fc0-3b85-d82e-2683-8ba9052e8e47@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox