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

["Steven Shi" <steven.shi@intel.com>]

Marvin,
It's fine to use your below patch to fix this issue. Please help to check-in it.
https://edk2.groups.io/g/devel/message/78894?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2CStandaloneMmPkg%3A+Support+CLANGPDB+X64+builds%2C20%2C2%2C0%2C84754068


Thanks
Steven Shi



> -----Original Message-----
> From: Marvin Häuser <mhaeuser@posteo.de>
> Sent: Thursday, October 14, 2021 5:08 PM
> To: Shi, Steven <steven.shi@intel.com>; devel@edk2.groups.io; Yang,
> JiyangX <jiyangx.yang@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> <sami.mujawar@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Supreeth
> Venkatesh <supreeth.venkatesh@arm.com>; Vitaly Cheptsov
> <vit9696@protonmail.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg: To support
> CLANGPDB build
> 
> 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/StandaloneMm
> CoreEntryPoint.inf
> >
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
> CoreEntryPoint.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.yang@intel.com>; devel@edk2.groups.io
> >
> > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar
> >
> > > <sami.mujawar@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Supreeth
> >
> > > Venkatesh <supreeth.venkatesh@arm.com>; Vitaly Cheptsov
> >
> > > <vit9696@protonmail.com>; Shi, Steven <steven.shi@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+tianocore@kernel.org
> > <mailto:ardb+tianocore@kernel.org>>
> >
> > > > Cc: Sami Mujawar <sami.mujawar@arm.com
> <mailto:sami.mujawar@arm.com>>
> >
> > > > Cc: Jiewen Yao <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com>>
> >
> > > > Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com
> > <mailto:supreeth.venkatesh@arm.com>>
> >
> > > > Cc: Vitaly Cheptsov <vit9696@protonmail.com
> > <mailto:vit9696@protonmail.com>>
> >
> > > > Cc: Marvin Häuser <mhaeuser@posteo.de
> <mailto:mhaeuser@posteo.de>>
> >
> > > > Cc: Steven Shi <steven.shi@intel.com <mailto:steven.shi@intel.com>>
> >
> > > > Signed-off-by: Jiyang Yang <jiyangx.yang@intel.com
> > <mailto:jiyangx.yang@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
> >
> > >
> >
> > >
> >
> > >
> >
> > > 
> >
> > >
> >