From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web10.6313.1634198735452041525 for ; Thu, 14 Oct 2021 01:05:36 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=dotutZzX; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id C0C94240104 for ; Thu, 14 Oct 2021 10:05:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1634198731; bh=q0fK8QgpVfejpSAjsbAVbMs+5rW7H2lWYSVSvJuRrfU=; h=Date:Subject:To:Cc:From:From; b=dotutZzXISEIU653ATE43kTWVvBkPM3UvVCiLvxjA41ctg8lxe5BwvTCU6pX5bZK6 HjGZfNi/H1jaaj6gsepTLDMO/qvLEcseXAOtmztdiHk5RHD3Sm90mdmExi1uF0sYSz yvr4xXd9JSF+AX9Eq1mBf/kuShnecJJzp4UTJXkocIi70c97S8bn/ugvbr6XS4Gfyt cehG92CoyFjXWEpi+VFYW2OwYRLs+0fI4IaT+oKcZG67poIJzgLy+6GE8HepLZhDv0 aezVkIgdjonGoPeWBBD+VcVbHz0bVZ6Cb47+mrH6paqyauCPZdLQgF6hxnBNDMYjKy VezDubrHupEvA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4HVMQd4fSYz6tmD; Thu, 14 Oct 2021 10:05:29 +0200 (CEST) Message-ID: <71754204-38e7-09c5-3d83-ba090d393552@posteo.de> Date: Thu, 14 Oct 2021 08:05:29 +0000 MIME-Version: 1.0 Subject: Re: [PATCH v1 1/1] StandaloneMmPkg: To support CLANGPDB build To: Jiyang Yang , devel@edk2.groups.io Cc: Ard Biesheuvel , Sami Mujawar , Jiewen Yao , Supreeth Venkatesh , Vitaly Cheptsov , Steven Shi References: <20211014031235.1839-1-jiyangx.yang@intel.com> <20211014031235.1839-2-jiyangx.yang@intel.com> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= In-Reply-To: <20211014031235.1839-2-jiyangx.yang@intel.com> Content-Language: en-GB Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hey Jiyang, NO! Please do not. :) Yes, this fixes build, but the AARCH64 core (I did not check ARM)=20 depends on self-relocation as it is loaded in-place at a location=20 unknown at compile-time. PIE helps ensure there are no relocations in=20 .text among other things. I know CLANGPDB does not support ARM/AARCH64=20 yet, but if it is added, this may generate binaries with more dangerous=20 relocations, which means the chance of executing an instruction that=20 requires relocation without relocating first (relocation is done in C=20 code now!) is significantly higher. We do not need PIE for IA32 or X64=20 at all (or more specifically, we only need it for ARM-based=20 architectures as of now), so I prefer my patch which makes that=20 explicit. Though we can theoretically use your solution when limited to=20 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 > Cc: Sami Mujawar > Cc: Jiewen Yao > Cc: Supreeth Venkatesh > Cc: Vitaly Cheptsov > Cc: Marvin H=C3=A4user > Cc: Steven Shi > Signed-off-by: Jiyang Yang > --- > StandaloneMmPkg/Core/StandaloneMmCore.inf = | 2 ++ > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEn= tryPoint.inf | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPk= g/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 =3D -fpie > GCC:*_*_*_DLINK_FLAGS =3D -Wl,-z,text,-Bsymbolic,-pie > + CLANGPDB:*_*_*_CC_FLAGS =3D -fno-pie > + CLANGPDB:*_*_*_DLINK_FLAGS =3D > diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Standal= oneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPo= int/StandaloneMmCoreEntryPoint.inf > index 1762586cfa02..ef69e07d2c07 100644 > --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo= reEntryPoint.inf > +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo= reEntryPoint.inf > @@ -56,3 +56,4 @@ > =20 > [BuildOptions] > GCC:*_*_*_CC_FLAGS =3D -fpie > + CLANGPDB:*_*_*_CC_FLAGS =3D -fno-pie