From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp02.apple.com (ma1-aaemail-dr-lapp02.apple.com [17.171.2.68]) by mx.groups.io with SMTP id smtpd.web10.3352.1616644077008389419 for ; Wed, 24 Mar 2021 20:47:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=oW0g6LVZ; spf=pass (domain: apple.com, ip: 17.171.2.68, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp02.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp02.apple.com (8.16.0.42/8.16.0.42) with SMTP id 12P3Y0Lk004636; Wed, 24 Mar 2021 20:47:47 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=op1LBGtGAFdK+zPl0spyjEQx6+RrWkFLNVa9bmhl6V4=; b=oW0g6LVZRvptZUR1FEtFhFTl/0HRqBY8D76YSHumW2gq3LFmy/wpE8I/p8uwnyEsTfLA 37pSzLVsiFxEu9xOzBJkAWxGi2NNophdFIHFIAO/eToDXbVtePvUZS+Qlahs2eqqV4ef Ub1aHy7QmFnpBxAWlHHyRBlq0lllkZPhxypgJ4bI2/4Htb3xb8RD39yxoIym/QC7O3M0 OUMov5VWaFceDT+u0NyTBOW2UFGnoLXhWFIgNSd2cXeul7EFACn6uEx47pKg9/15A0Rm qQ/0oSuWj/e9VkK/EtO7Rhbcxlx5mF8n/7O2bitUxq61gDN15JGEUu4XCL6Qi04SizpR 9g== Received: from rn-mailsvcp-mta-lapp04.rno.apple.com (rn-mailsvcp-mta-lapp04.rno.apple.com [10.225.203.152]) by ma1-aaemail-dr-lapp02.apple.com with ESMTP id 37ddqt1e1p-4 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 24 Mar 2021 20:47:47 -0700 Received: from rn-mailsvcp-mmp-lapp03.rno.apple.com (rn-mailsvcp-mmp-lapp03.rno.apple.com [17.179.253.16]) by rn-mailsvcp-mta-lapp04.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) with ESMTPS id <0QQI0033UAJNVM40@rn-mailsvcp-mta-lapp04.rno.apple.com>; Wed, 24 Mar 2021 20:47:47 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-mmp-lapp03.rno.apple.com by rn-mailsvcp-mmp-lapp03.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) id <0QQI00P009ZPPD00@rn-mailsvcp-mmp-lapp03.rno.apple.com>; Wed, 24 Mar 2021 20:47:47 -0700 (PDT) X-Va-A: X-Va-T-CD: 36e7a95c5a6048d9c36308b131fa87c1 X-Va-E-CD: 9a4ee0c196a465cee3151bf532b18c49 X-Va-R-CD: 323ef448fbae37932f95489e7d7da54b X-Va-CD: 0 X-Va-ID: 5eb5bc56-96a8-49cc-bbac-12f3a5b73126 X-V-A: X-V-T-CD: 36e7a95c5a6048d9c36308b131fa87c1 X-V-E-CD: 9a4ee0c196a465cee3151bf532b18c49 X-V-R-CD: 323ef448fbae37932f95489e7d7da54b X-V-CD: 0 X-V-ID: efe74438-ea2d-42f3-b14c-a1c2d47653a4 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369,18.0.761 definitions=2021-03-24_14:2021-03-24,2021-03-24 signatures=0 Received: from [17.235.46.127] (unknown [17.235.46.127]) by rn-mailsvcp-mmp-lapp03.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) with ESMTPSA id <0QQI00VGPAJGB800@rn-mailsvcp-mmp-lapp03.rno.apple.com>; Wed, 24 Mar 2021 20:47:44 -0700 (PDT) From: "Andrew Fish" Message-id: <42AA9483-4473-47FF-95B5-93E570430B9E@apple.com> MIME-version: 1.0 (Mac OS X Mail 14.0 \(3654.20.0.2.1\)) Subject: Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds Date: Wed, 24 Mar 2021 20:47:40 -0700 In-reply-to: Cc: devel@edk2.groups.io, ross@burtonini.com, lersek@redhat.com To: gaoliming References: <20210324115819.605436-1-ross.burton@arm.com> <00b301d7211d$fc148390$f43d8ab0$@byosoft.com.cn> X-Mailer: Apple Mail (2.3654.20.0.2.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369,18.0.761 definitions=2021-03-24_14:2021-03-24,2021-03-24 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_26946B27-A83B-49D9-AA21-23EFA1D1D7E3" --Apple-Mail=_26946B27-A83B-49D9-AA21-23EFA1D1D7E3 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 While we are talking about what toolchain targets should do I don=E2=80=99t= understand this [1]? Why does OVMF need to turn on =E2=80=94keepexceptiont= able? If these toolchains require it why don=E2=80=99t they turn it on? I s= ee Mike fixed it[2]. Is this another case of a global GENFW_FLAG breaking t= hings? Looks like the CLANGPDB toolchain does what I would expect, and the = other toolchains do not? Is there some history here I=E2=80=99m missing? [1] $ git grep "keepexceptiontable" BaseTools/Conf/tools_def.template:2872:DEBUG_CLANGPDB_X64_GENFW_FLAGS = =3D --keepexceptiontable BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GENFW_FLAGS = =3D --keepexceptiontable =E2=80=A6 OvmfPkg/OvmfPkgIa32X64.dsc:80: MSFT:*_*_X64_GENFW_FLAGS =3D --keepexcept= iontable OvmfPkg/OvmfPkgIa32X64.dsc:81: GCC:*_*_X64_GENFW_FLAGS =3D --keepexcept= iontable OvmfPkg/OvmfPkgIa32X64.dsc:82: INTEL:*_*_X64_GENFW_FLAGS =3D --keepexcept= iontable OvmfPkg/OvmfPkgX64.dsc:80: MSFT:*_*_X64_GENFW_FLAGS =3D --keepexceptiont= able OvmfPkg/OvmfPkgX64.dsc:81: GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiont= able OvmfPkg/OvmfPkgX64.dsc:82: INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiont= able OvmfPkg/OvmfXen.dsc:71: MSFT:*_*_X64_GENFW_FLAGS =3D --keepexceptiontabl= e OvmfPkg/OvmfXen.dsc:72: GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontabl= e OvmfPkg/OvmfXen.dsc:73: INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontabl= e [2] $ git show bbcafc442b2db commit bbcafc442b2db91322dd3ba04e166236a41b111d Author: mdkinney Date: Wed Oct 3 21:00:26 2012 +0000 The exception table information in X64 PE/COFF images is being strippe= d by default in the OvmfPkg. This patch preserves this information when SOURCE_DEBUG_ENABLE is set. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Michael Kinney Reviewed-by: Laszlo Ersek git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@1= 3780 6f19259b-4bc3-4df7-8a09-765794883524 diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 9b2ba8626ab3..40fd5e97b24e 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -41,7 +41,12 @@ [BuildOptions] INTEL:RELEASE_*_*_CC_FLAGS =3D /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS =3D /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS =3D -mno-mmx -mno-sse - +!ifdef $(SOURCE_DEBUG_ENABLE) + MSFT:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable + GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable + INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable +!endif + #########################################################################= ####### # # SKU Identification section - list of all SKU IDs supported by this Plat= form. diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 9ff4a5de1005..c61d41dccbbe 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -41,6 +41,11 @@ [BuildOptions] INTEL:RELEASE_*_*_CC_FLAGS =3D /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS =3D /D MDEPKG_NDEBUG GCC:*_*_*_CC_FLAGS =3D -mno-mmx -mno-sse +!ifdef $(SOURCE_DEBUG_ENABLE) + MSFT:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable + GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable + INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable +!endif =20 #########################################################################= ####### # Thanks, Andrew Fish > On Mar 24, 2021, at 8:38 PM, Andrew Fish wrote: >=20 > Liming, >=20 > I understand how the tool works, but that is not how it is used. To make= it work like other toolchains *_*_*_GENFW_FLAGS for ELF targets should pas= s =E2=80=94zero for builds that do not contain symbols. This is how it work= s today [1]. >=20 > The proposed fix is a global hammer ` RELEASE_*_*_GENFW_FLAGS =3D --zero= ` and it prevents some one from overriding the toolchain definition to turn= on source level debugging for Release builds. In all the other places this= is a tool chain choice. So my complaint is solving this globally vs. on a = per toolchain basis. All the other toolchains don=E2=80=99t produce. >=20 > If you look at the XCODE5 toolchian that we override the edk2 default = DEBUG_XCODE5_X64_CC_FLAGS has -g and RELEASE_XCODE5_X64_CC_FLAGS does not. = So the source level debugging or not is part of the compiler target choice.= = =20 > DEBUG_XCODE5_*_MTOC_FLAGS =3D -align 0x20 -d $(DEBUG_DIR)/$(MODULE_NAM= E).dll > RELEASE_XCODE5_*_MTOC_FLAGS =3D -align 0x20 >=20 > For our override we pass -g to RELEASE builds CC_FLAGS and do `RELEASE_X= CODE5_*_MTOC_FLAGS =3D -align 0x20 -d $(MODULE_GUID)` so the override to fo= rce --zero on every call to GenFw will break this.=20 >=20 > So my point is if the toolchain is generating a Debug Directory entry in= a RELEASE target that does not support debugging then that target should s= etting `RELEASE__*_GENFW_FLAGS =3D =E2=80=94zero` to undo the unwan= ted work. I understand why the debug info gets added as it reduces the comp= lexity of the ELF to PE/COFF conversion, so I=E2=80=99m not complaining abo= ut that just that the ELF targets don=E2=80=99t handle this themselves. So = to me this fix is working the bug of the ELF targets not passing =E2=80=94z= ero on RELEASE builds to GenFw. >=20 > [1] $ git grep _GENFW_FLAGS -- *.template > BaseTools/Conf/tools_def.template:2872:DEBUG_CLANGPDB_X64_GENFW_FLAGS = =3D --keepexceptiontable > BaseTools/Conf/tools_def.template:2877:RELEASE_CLANGPDB_X64_GENFW_FLAGS = =3D > BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GENFW_FLAGS = =3D --keepexceptiontable > BaseTools/Conf/tools_def.template:3124:*_*_*_GENFW_FLAGS = =3D >=20 > Thanks, >=20 > Andrew Fish >=20 >> On Mar 24, 2021, at 7:24 PM, gaoliming > wrote: >>=20 >> Andrew: >> GenFw generates debug entry, and zero debug entry. Its functionality i= s same to the image without debug entry. So, new option is not introduced t= o control whether GenFw generates debug entry when it converts ELF image to= EFI image.=20 >> >> Thanks >> Liming >> =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > =E4=BB=A3=E8= =A1=A8 Andrew Fish via groups.io >> =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B43=E6=9C=8825=E6=97= =A5 7:26 >> =E6=94=B6=E4=BB=B6=E4=BA=BA: edk2-devel-groups-io >; ross@burtonini.com >> =E6=8A=84=E9=80=81: lersek@redhat.com >> =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build pa= ths in release builds >> >> This breaks some usage we have have in our fork. We have symbols turned= on for Release builds, so this change would break that.=20 >> >> It looks to me that the root cause of this issue might be that the GenF= w is blindly writing the debug directory entry into the PE/COFF? For native= PE/COFF I think this is controlled by linker flags? For Xcode/clang it is = controlled by the *_XCODE5_*_MTOC_FLAGS. So at this point it is kind of up = to each toolchain how they want to deal with symbols on release builds.=20 >> >> >> It seems kind of strange to insert a section and then zero it. Almost s= eems like the intent of =E2=80=94zero was to post process compare images?= =20 >>=20 >>=20 >> -z, --zero Zero the Debug Data Fields in the PE input imag= e file. >> It also zeros the time stamp fields. >> This option can be used to compare the binary e= fi image. >> It can't be combined with other action options >> except for -o, -r option. It is a action option= . >> If it is combined with other action options, th= e later >> input action option will override the previous = one. >>=20 >>=20 >> And in case you are going to ask our fork uses relative paths from the = Build directory and/or a UUID string for the Debug Directory entry file nam= e so it is a constant value and does not impact build reproducibility.=20 >>=20 >>=20 >> From a feature stand point this change will break any hope of source le= vel debugging with RELEASE builds. I also think it changes the exception ha= ndler code output in OVMF [1] for ELF toolchains. You are going to get the = (No PDB) vs. the file and path you were getting today. I assume if you had = tools that natively produce PE/COFF and did not have a Debug Directory entr= y the same thing would happen prior to this change.=20 >>=20 >>=20 >> Status =3D PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &EntryPoin= t); >> if (EFI_ERROR (Status)) { >> EntryPoint =3D NULL; >> } >> InternalPrintMessage ("!!!! Find image based on IP(0x%x) ", Current= Eip); >> PdbPointer =3D PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data); >> if (PdbPointer !=3D NULL) { >> InternalPrintMessage ("%a", PdbPointer); >> } else { >> InternalPrintMessage ("(No PDB) " ); >> } >> InternalPrintMessage ( >> " (ImageBase=3D%016lp, EntryPoint=3D%016p) !!!!\n", >> (VOID *) Pe32Data, >> EntryPoint >> ); >>=20 >>=20 >> Not saying we have to "stop the presses", but just trying to point out = the side effects of this change. It is not so much that this change is bad,= but that we have no way to turn off the Debug Directory Entry for ELF conv= ersion, so we seem to be working around that issue with a bigger hammer? >> >> [1] https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/Cp= uExceptionHandlerLib/CpuExceptionCommon.c#L117 >> >> Thanks, >>=20 >>=20 >> Andrew Fish >>=20 >>=20 >>> On Mar 24, 2021, at 4:58 AM, Ross Burton > wrote: >>> >>> GenFw will embed a NB10 section which contains the path to the input f= ile, >>> which means the output files have build paths embedded in them. To re= duce >>> information leakage and ensure reproducible builds, pass --zero in rel= ease >>> builds to remove this information. >>>=20 >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3256 >>> Signed-off-by: Ross Burton > >>> --- >>> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 + >>> OvmfPkg/Bhyve/BhyveX64.dsc | 1 + >>> OvmfPkg/OvmfPkgIa32.dsc | 1 + >>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >>> OvmfPkg/OvmfPkgX64.dsc | 1 + >>> OvmfPkg/OvmfXen.dsc | 1 + >>> 6 files changed, 6 insertions(+) >>>=20 >>> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.d= sc >>> index 65c42284d9..69a05feea9 100644 >>> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc >>> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc >>> @@ -78,6 +78,7 @@ >>> GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> !endif >>> + RELEASE_*_*_GENFW_FLAGS =3D --zero >>>=20 >>> # >>> # Disable deprecated APIs. >>> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc >>> index 4a1cdf5aca..132f55cf69 100644 >>> --- a/OvmfPkg/Bhyve/BhyveX64.dsc >>> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc >>> @@ -76,6 +76,7 @@ >>> GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> !endif >>> + RELEASE_*_*_GENFW_FLAGS =3D --zero >>>=20 >>> # >>> # Disable deprecated APIs. >>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >>> index 1eaf3e99c6..93c209950c 100644 >>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>> @@ -80,6 +80,7 @@ >>> !if $(TOOL_CHAIN_TAG) !=3D "XCODE5" && $(TOOL_CHAIN_TAG) !=3D "CLANGPD= B" >>> GCC:*_*_*_CC_FLAGS =3D -mno-mmx -mno-sse >>> !endif >>> + RELEASE_*_*_GENFW_FLAGS =3D --zero >>>=20 >>> # >>> # Disable deprecated APIs. >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>> index 4a5a430147..97cc438250 100644 >>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>> @@ -84,6 +84,7 @@ >>> GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> !endif >>> + RELEASE_*_*_GENFW_FLAGS =3D --zero >>>=20 >>> # >>> # Disable deprecated APIs. >>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>> index d4d601b444..f544fb04bf 100644 >>> --- a/OvmfPkg/OvmfPkgX64.dsc >>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>> @@ -84,6 +84,7 @@ >>> GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> !endif >>> + RELEASE_*_*_GENFW_FLAGS =3D --zero >>>=20 >>> # >>> # Disable deprecated APIs. >>> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc >>> index 507029404f..fcaa35acf1 100644 >>> --- a/OvmfPkg/OvmfXen.dsc >>> +++ b/OvmfPkg/OvmfXen.dsc >>> @@ -74,6 +74,7 @@ >>> GCC:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable >>> !endif >>> + RELEASE_*_*_GENFW_FLAGS =3D --zero >>>=20 >>> # >>> # Disable deprecated APIs. >>> --=20 >>> 2.25.1 >>>=20 >>>=20 >>>=20 >>>=20 >>>=20 >>>=20 >>=20 >> >>=20 >=20 --Apple-Mail=_26946B27-A83B-49D9-AA21-23EFA1D1D7E3 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 While we are talking abou= t what toolchain targets should do I don=E2=80=99t understand this [1]? Why= does OVMF need to turn on =E2=80=94keepexceptiontable? If these toolc= hains require it why don=E2=80=99t they turn it on? I see Mike fixed it[2].= Is this another case of a global GENFW_FLAG breaking things? Looks like th= e CLANGPDB toolchain does what I would expect, and the other toolchains do = not? Is there some history here I=E2=80=99m missing?

[1]  $ git grep "keepexceptiontable"
BaseTools/Conf/tools_def.template= :2872:DEBUG_CLANGPDB_X64_GENFW_= FLAGS      =3D --keepexcept= iontable
BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GENFW_FLAGS  =     =3D --keepexceptiontable=
=E2=80=A6
OvmfPkg/OvmfPkgIa32X64.dsc:80:  MS= FT:*_*_X64_GENFW_FLAGS  =3D --keepexceptiontable
OvmfPkg/OvmfPkgIa32X64.= dsc:81:  GCC:*_*_X64_GENFW= _FLAGS   =3D --keepexceptiontable=
OvmfPkg/OvmfPkgIa32X64.dsc:82<= span style=3D"font-variant-ligatures: no-common-ligatures; color: #2eaebb" = class=3D"">:  INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:80:  MSFT:*_*_X64_GENFW_FLAG= S  =3D --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:81:  GCC:*_*_X64_GENFW_FLAGS   =3D --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:82:  INTEL:*_*_X64_GENFW_FLAGS = = =3D --keepexceptiontable
=
OvmfPkg/OvmfXen.dsc:71:<= span style=3D"font-variant-ligatures: no-common-ligatures" class=3D""> = ; MSFT:*_*_X64_GENFW_FLAGS  =3D --kee= pexceptiontable
OvmfPkg/OvmfXen.dsc:<= /span>72:  GCC:*_*_X64_GENFW_FLAGS   =3D --keepexceptiontable
OvmfPk= g/OvmfXen.dsc:73:  INTEL:*= _*_X64_GENFW_FLAGS =3D --keepexceptiontabl= e

=
[2] $ git show bbcafc442b2db
commit bbcafc442b2db91322dd3ba04e166236a41b111d=
Author: mdkinney <mdkinney@6f19259b-4bc3-4df7-8a09-765794883524= >
Date:   Wed Oct 3 21:00:26 2012 +0000
=

    The excep= tion table information in X64 PE/COFF images is being stripped by default i= n the OvmfPkg.

    

    This patch preserv= es this information when SOURCE_DEBUG_ENABLE is set.

    

    Contributed-under: TianoCore Contribution Agreement = 1.0
    Signed-off-by: Michael Kinney <michael.d.kinney@intel.c= om>
    Reviewed-by: Laszlo Ersek

    

    git-svn-id: https://edk2.svn.so= urceforge.net/svnroot/edk2/trunk/edk2@13780 6f19259b-4bc3-4df7-8a09-765= 794883524

<= b class=3D"">diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X= 64.dsc
index 9b2ba8626ab3..40fd5e97b24e 10= 0644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc<= /span>
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -41,7 +41,12 @@ [Buil= dOptions]
   INTEL:RELEASE_*_*_CC_FLAGS   &nb= sp;       =3D /D MDEPKG_NDEBUG
   M= SFT:RELEASE_*_*_CC_FLAGS            =3D /D MD= EPKG_NDEBUG
   GCC:*_*_*_CC_FLAGS     &n= bsp;             =3D -mno-mmx -mno-sse=
-
+!ifdef $(SOURCE_DEBUG_ENABLE)
+=   MSFT:*_*_X64_GENFW_FLAGS  =3D --keepexceptiontable
=
+  GCC:*_*_X64_GENFW_FLAGS   =3D --ke= epexceptiontable
+  INTEL:*_*_= X64_GENFW_FLAGS =3D --keepexceptiontable
+!endif
+  
 ############################################= ####################################
 #
 # SKU Identification section - list of all SKU IDs supported by th= is Platform.
diff --git a/OvmfPkg/OvmfPkgX64.d= sc b/OvmfPkg/OvmfPkgX64.dsc
index 9ff4a5de= 1005..c61d41dccbbe 100644
--- a/OvmfPkg/Ov= mfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc<= /b>
@@ -41,6 +41,11 @@ [BuildOptions]
   INTEL:RELEASE_*_*_CC_FLA= GS           =3D /D MDEPKG_NDEBUG
&= nbsp;  MSFT:RELEASE_*_*_CC_FLAGS          &nb= sp; =3D /D MDEPKG_NDEBUG
   GCC:*_*_*_CC_FLAGS &nb= sp;                 =3D -mno-mmx -m= no-sse
+!ifdef $(SOURCE_DEBUG_ENABL= E)
+  MSFT:*_*_X64_GENFW_FLAGS=   =3D --keepexceptiontable
+&n= bsp; GCC:*_*_X64_GENFW_FLAGS   =3D --keepexceptiontable
+  INTEL:*_*_X64_GENFW_FLAGS =3D --keepexcep= tiontable
+!endif

 

&n= bsp;#######################################################################= #########
 #


Thanks,

Andrew Fish

On Mar 24, 2021, at 8:38 PM, Andrew Fish &= lt;afish@apple.com> wr= ote:

=
Liming,

=
I understand how the tool works, but that is not how it is = used. To make it work like other toolchains *_*_*_GENFW_FLAGS for ELF = targets should pass =E2=80=94zero for builds that do not contain symbols. T= his is how it works today [1].

The proposed fix is a global hammer ` RELEASE_*_*_GENFW_= FLAGS =3D --zero` and it prevents some one from overriding the toolchain de= finition to turn on source level debugging for Release builds. In all the o= ther places this is a tool chain choice. So my complaint is solving this gl= obally vs. on a per toolchain basis. All the other toolchains don=E2=80=99t= produce.

If you = look at the XCODE5 toolchian that we override the edk2 default   = DEBUG_XCODE5_X64_CC_FLAGS has -g and RELEASE_XCODE5_X64_CC_FLAGS does = not. So the source level debugging or not is part of the compiler target ch= oice. 
  DEBUG_XCODE5_*_MTOC= _FLAGS =3D -align 0x20 -d $(DEBUG_DIR)/$(MODULE_NAME).dll
RELEASE_XCODE5_*_MTOC_FLAGS =3D -align 0x20
For our override we pass -g to RELEASE bu= ilds CC_FLAGS and do `RELEASE_XCODE5_*_MTOC_FLAGS =3D -align 0x20 -d $(MODU= LE_GUID)` so the override to force --zero on every call to GenFw will break= this. 

So m= y point is if the toolchain is generating a Debug Directory entry in a RELE= ASE target that does not support debugging then that target should setting = `RELEASE_<target>_*_GENFW_FLAGS =3D =E2=80=94zero` to undo the unwant= ed work. I understand why the debug info gets added as it reduces the compl= exity of the ELF to PE/COFF conversion, so I=E2=80=99m not complaining abou= t that just that the ELF targets don=E2=80=99t handle this themselves. So t= o me this fix is working the bug of the ELF targets not passing =E2=80=94ze= ro on RELEASE builds to GenFw.

[1] $ git grep _GENFW_FLAGS -- *.template
BaseTools/Conf= /tools_def.template:2872:DEBUG_= CLANGPDB_X64_GENFW_FLAGS  &nbs= p;   =3D --keepexceptiontable
BaseTools/Conf/tools_def.template:2877:RELEASE_CLANGPDB_X64_G= ENFW_FLAGS    =3D
BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GEN= FW_FLAGS      =3D --keepexceptiontable
BaseTools/Conf/tools_def.template:3124:*_*_*_GENFW_FLAGS             = ;     =3D

Thanks,

=
Andrew Fish

On Mar 24, 2021, at 7:24 PM, gaol= iming <gaoliming@= byosoft.com.cn> wrote:

=
Andrew:
 GenFw generates debug entry, and zer= o debug entry. Its functionality is same to the image without debug entry. = So, new option is not introduced to control whether GenFw generates debug e= ntry when it converts ELF image to EFI image. 
 
Thanks
=
Liming
=E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups= .io <devel@edk2.groups.io> =E4=BB=A3=E8=A1=A8 Andrew Fish = via grou= ps.io
=E5=8F=91=E9=80=81=E6=97=B6= =E9=97=B4: 2021=E5=B9= =B43=E6=9C=8825=E6=97=A5 7:26
=E6=94=B6=E4=BB=B6=E4=BA=BA: edk2-devel-groups-io <devel@edk2.groups= .io>; ross@burtonini.com
=E6= =8A=84=E9=80=81: lersek@redhat.com
= = =E4=B8=BB=E9=A2=98: R= e: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds
 
This breaks some usag= e we have have in our fork. We have symbols turned on for Release builds, s= o this change would break that. 
 
It looks = to me that the root cause of this issue might be that the GenFw is blindly = writing the debug directory entry into the PE/COFF? For native PE/COFF I th= ink this is controlled by linker flags? For Xcode/clang it is controlled by= the *_XCODE5_*_MTOC_FLAGS. So at this point it is kind of up to each = toolchain how they want to deal with symbols on release builds. 
=
 <= /span>
<= div style=3D"margin: 0cm; font-size: 12pt; font-family: =E5=AE=8B=E4=BD=93;= " class=3D""> 
It seems kind of strange to insert a section and then zero it. Almos= t seems like the intent of =E2=80=94zero was to post process compare images= ? 


  -z, --zero          &nbs= p; Zero the Debug Data Fields in the PE input image file.
 = ;                     &nb= sp; It also zeros the time stamp fields.<= /div>
      &= nbsp;                 This opt= ion can be used to compare the binary efi image.
     = ;                   It ca= n't be combined with other action options
       = ;                 except for -= o, -r option. It is a action option.
        &n= bsp;               If it is combine= d with other action options, the later
        &= nbsp;               input action op= tion will override the previous one.


And in= case you are going to ask our fork uses relative paths from the Build dire= ctory and/or a UUID string for the Debug Directory entry file name so it is= a constant value and does not impact build reproducibility. 


From a feature stand point this change will break any hope of source leve= l debugging with RELEASE builds. I also think it changes the exception hand= ler code output in OVMF [1] for ELF toolchains. You are going to get the (N= o PDB) vs. the file and path you were getting today. I assume if you had to= ols that natively produce PE/COFF and did not have a Debug Directory entry = the same thing would happen prior to this change. 


  &n= bsp; }= <= td width=3D"50" nowrap=3D"" valign=3D"top" id=3D"L139" style=3D"width: 37.5= pt; padding: 0cm 7.5pt; box-sizing: border-box; min-width: 50px; color: var= (--color-diff-blob-num-text); cursor: pointer; -webkit-user-select: none;" = class=3D"">= <= /table>
=

Not saying we ha= ve to "stop the presses", but just trying to point out the side effects of = this change. It is not so much that this change is bad, but that we have no= way to turn off the Debug Directory Entry for ELF conversion, so we seem t= o be working around that issue with a bigger hammer?<= /span>
 
[1] = https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExcepti= onHandlerLib/CpuExceptionCommon.c#L117
 
Thanks,=
<= br class=3D"">
Andrew Fish


=
On Mar 24, 2021, at 4:58 AM, Ross Burton <ross@burtonini.com> wrote:
=
 =

GenFw will embed a NB10 section whic= h contains the path to the input file,
which means the output= files have build paths embedded in them.  To reduce
inf= ormation leakage and ensure reproducible builds, pass --zero in release
builds to remove this information.

= Ref: https://bugzilla.tianocore.org/show_bug.cg= i?id=3D3256
Signed-off-by: Ross Burton <ross.burton@arm.com>
---
Ovm= fPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/Bhyve/BhyveX64.dsc &n= bsp; | 1 +
OvmfPkg/OvmfPkgIa32.dsc    &nb= sp; | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc   | 1 +OvmfPkg/OvmfPkgX64.dsc       | 1= +
OvmfPkg/OvmfXen.dsc       &n= bsp;  | 1 +
6 files changed, 6 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/= AmdSev/AmdSevX64.dsc
index 65c42284d9..69a05feea9 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/Am= dSev/AmdSevX64.dsc
@@ -78,6 +78,7 @@
 &nbs= p;GCC:*_*_X64_GENFW_FLAGS   =3D --keepexceptiontable
  INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable
!endif
+  RELEASE_*_*_GENFW_FLAGS =3D --zero

  #
  # Disable = deprecated APIs.
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/Ov= mfPkg/Bhyve/BhyveX64.dsc
index 4a1cdf5aca..132f55cf69 100644<= br class=3D"">--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/= Bhyve/BhyveX64.dsc
@@ -76,6 +76,7 @@
 &nbs= p;GCC:*_*_X64_GENFW_FLAGS   =3D --keepexceptiontable
  INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable
!endif
+  RELEASE_*_*_GENFW_FLAGS =3D --zero

  #
  # Disable = deprecated APIs.
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfP= kg/OvmfPkgIa32.dsc
index 1eaf3e99c6..93c209950c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa= 32.dsc
@@ -80,6 +80,7 @@
!if $(TOOL_CHAIN_TAG) = !=3D "XCODE5" && $(TOOL_CHAIN_TAG) !=3D "CLANGPDB"
&n= bsp; GCC:*_*_*_CC_FLAGS        &nbs= p;          =3D -mno-mmx = -mno-sse
!endif
+  RELEASE_*_*_GENFW_FLAGS= =3D --zero

  #
 =  # Disable deprecated APIs.
diff --git a/OvmfPkg/OvmfPkg= Ia32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 4a5a430147..97= cc438250 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -84,6 +84,7 @@
  GCC:*_*_X64_GENFW_FLAGS   =3D --keepexceptiont= able
  INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptio= ntable
!endif
+  RELEASE_*_*_GENFW_FLAGS = =3D --zero

  #
 =  # Disable deprecated APIs.
diff --git a/OvmfPkg/OvmfPkg= X64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d4d601b444..f544fb04bf= 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/Ovmf= Pkg/OvmfPkgX64.dsc
@@ -84,6 +84,7 @@
 &nbs= p;GCC:*_*_X64_GENFW_FLAGS   =3D --keepexceptiontable
  INTEL:*_*_X64_GENFW_FLAGS =3D --keepexceptiontable
!endif
+  RELEASE_*_*_GENFW_FLAGS =3D --zero

  #
  # Disable = deprecated APIs.
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/O= vmfXen.dsc
index 507029404f..fcaa35acf1 100644
= --- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -74,6 +74,7 @@
  GCC:*_*_X64_GENFW_FLAGS =   =3D --keepexceptiontable
  INTEL:*_*_X6= 4_GENFW_FLAGS =3D --keepexceptiontable
!endif
+=  RELEASE_*_*_GENFW_FLAGS =3D --zero

&nbs= p; #
  # Disable deprecated APIs.
-- 
2.25.= 1





<= /div>
 =


--Apple-Mail=_26946B27-A83B-49D9-AA21-23EFA1D1D7E3--
    Statu= s =3D PeCoffLoaderGetEntryPoint&n= bsp;((VOID *) Pe32Data, &EntryPoint);
    if (EFI_ERROR (Status)) {<= /span>
     =  EntryPoint =3D NULL;
    InternalPrintMessage=  (= "!!!! Find image based on IP(0x%x) ", CurrentEip);
    PdbPointer =3D PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);
    if (PdbPointer !=3D NULL) {
      InternalPr= intMessage ("%a", PdbPointer);
    } else {
   &nb= sp;  InternalPrintMessage ("(No PDB) "&n= bsp;);
    }
   &nbs= p;InternalPrintMessage (
   &nb= sp;  " (ImageBase=3D%016lp, EntryPoint=3D%016p) !!!!\n",
      = (VOID *) Pe32Data,
    &n= bsp; EntryPoint
      );