Hey Steven,
As I said, I prefer my patch, but this would work too of course.
I talked about the PIE stuff with Ard before, so maybe he has an opinion
on this? :)
(Small correction for my last e-mail, of course we are not *guaranteed*
there are *no* relocations in .text, but they'd all point to GOT (or
whatever else the target uses for PIE), and references will probably be
relative; for ARM architectures I remember Ard talking about specific
kinds of relocations being avoided entirely).
Best regards,
Marvin
On 14.10.21 11:02, Shi, Steven wrote:
Hi Marvin,
How about we limit the -fno-pie option only apply on IA32 and X64 like
below?
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf
b/StandaloneMmPkg/Core/StandaloneMmCore.inf
[BuildOptions]
GCC:*_*_*_CC_FLAGS = -fpie
GCC:*_*_*_DLINK_FLAGS = -Wl,-z,text,-Bsymbolic,-pie
+ CLANGPDB:*_*_ *IA32*_CC_FLAGS= -fno-pie
+ CLANGPDB:*_*_ *X64*_CC_FLAGS= -fno-pie
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
[BuildOptions]
GCC:*_*_*_CC_FLAGS = -fpie
+ CLANGPDB:*_*_ *IA32*_CC_FLAGS= -fno-pie
+ CLANGPDB:*_*_ *X64*_CC_FLAGS= -fno-pie
Thanks
Steven Shi
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin
> Häuser
> Sent: Thursday, October 14, 2021 4:05 PM
> To: Yang, JiyangX <jiyangx.y...@intel.com>; devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>; Sami Mujawar
> <sami.muja...@arm.com>; Yao, Jiewen <jiewen....@intel.com>; Supreeth
> Venkatesh <supreeth.venkat...@arm.com>; Vitaly Cheptsov
> <vit9...@protonmail.com>; Shi, Steven <steven....@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: To support
> CLANGPDB build
>
> 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+tianoc...@kernel.org
<mailto:ardb+tianoc...@kernel.org>>
> > Cc: Sami Mujawar <sami.muja...@arm.com <mailto:sami.muja...@arm.com>>
> > Cc: Jiewen Yao <jiewen....@intel.com <mailto:jiewen....@intel.com>>
> > Cc: Supreeth Venkatesh <supreeth.venkat...@arm.com
<mailto:supreeth.venkat...@arm.com>>
> > Cc: Vitaly Cheptsov <vit9...@protonmail.com
<mailto:vit9...@protonmail.com>>
> > Cc: Marvin Häuser <mhaeu...@posteo.de <mailto:mhaeu...@posteo.de>>
> > Cc: Steven Shi <steven....@intel.com <mailto:steven....@intel.com>>
> > Signed-off-by: Jiyang Yang <jiyangx.y...@intel.com
<mailto:jiyangx.y...@intel.com>>
> > ---
> > StandaloneMmPkg/Core/StandaloneMmCore.inf | 2
> ++
> >
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmC
> oreEntryPoint.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/StandaloneMm
> CoreEntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> > index 1762586cfa02..ef69e07d2c07 100644
> > ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.inf
> > +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.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 (#82008): https://edk2.groups.io/g/devel/message/82008
Mute This Topic: https://groups.io/mt/86305054/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
