[edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: To support CLANGPDB build

[Marvin Häuser]

Hey Jiyang,

NO! Please do not. :)
Yes, this fixes build, but the AARCH64 core (I did not check ARM) 
depends on self-relocation as it is loaded in-place at a location 
unknown at compile-time. PIE helps ensure there are no relocations in 
.text among other things. I know CLANGPDB does not support ARM/AARCH64 
yet, but if it is added, this may generate binaries with more dangerous 
relocations, which means the chance of executing an instruction that 
requires relocation without relocating first (relocation is done in C 
code now!) is significantly higher. We do not need PIE for IA32 or X64 
at all (or more specifically, we only need it for ARM-based 
architectures as of now), so I prefer my patch which makes that 
explicit. Though we can theoretically use your solution when limited to 
non-ARM architectures if you really dislike my patch that much.

I'd prefer to hear from the ARM core maintainers before making any move.

Best regards,
Marvin

On 14.10.21 05:12, Jiyang Yang wrote:
> the flag "-fpie" is passed for all builds with a GCC family toolchain,
> including CLANGPDB, but CLANGPDB does not support this flag, it will
> report "clang: error: unsupported option '-fpie' for target
> 'x86_64-unknown-windows-gnu'". So we add the CLANGPDB option "-fno-pie"
> later to overwrite it.
>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Sami Mujawar <sami.mujawar at arm.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Supreeth Venkatesh <supreeth.venkatesh at arm.com>
> Cc: Vitaly Cheptsov <vit9696 at protonmail.com>
> Cc: Marvin Häuser <mhaeuser at posteo.de>
> Cc: Steven Shi <steven.shi at intel.com>
> Signed-off-by: Jiyang Yang <jiyangx.yang at intel.com>
> ---
>   StandaloneMmPkg/Core/StandaloneMmCore.inf                                         | 2 ++
>   StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
>   2 files changed, 3 insertions(+)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> index 56042b7b39f4..3213142523f4 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> @@ -79,3 +79,5 @@
>   [BuildOptions]
>     GCC:*_*_*_CC_FLAGS = -fpie
>     GCC:*_*_*_DLINK_FLAGS = -Wl,-z,text,-Bsymbolic,-pie
> +  CLANGPDB:*_*_*_CC_FLAGS = -fno-pie
> +  CLANGPDB:*_*_*_DLINK_FLAGS =
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> index 1762586cfa02..ef69e07d2c07 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> @@ -56,3 +56,4 @@
>   
>   [BuildOptions]
>     GCC:*_*_*_CC_FLAGS = -fpie
> +  CLANGPDB:*_*_*_CC_FLAGS = -fno-pie



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81986): https://edk2.groups.io/g/devel/message/81986
Mute This Topic: https://groups.io/mt/86305054/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-