From nobody Sat Apr 20 08:31:57 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+73248+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+73248+1787277+3901457@groups.io ARC-Seal: i=1; a=rsa-sha256; t=1616639080; cv=none; d=zohomail.com; s=zohoarc; b=QLWUB2y1Cr1d4ZwUmb7lz//lpDTkdfB6A60DCPJULXdY6/WOSZCD873ZY6JailipV0eKb3pk+fvFpZCLu1M6CCXtx6MMPh3Z5B+3fLGg0fThmm1IlHPGNx6lnAxKimxUREiIIKlyPxSP+6Yl5+NovAxJ3RkWI7Vh6shooQFeyQQ= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1616639080; h=Content-Type:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=2+uTEKIIppIXic5gtO6Jqy/YhcWciliyN9wyriTOiLg=; b=hR6xQmL0Vvja5A48uTMMfbCGgw9Rtjx4xzHCxDV3o7Xq3elX0moi8EvwkzOqnMuZKJr1eaJOmN6BcGLPZ4FMboLQCRzT7jjWyqnRRwfGEup+IYFC9lvL0ygRyWcgchOb6Bkj7riuNypccssfDH2K5bcodqf9muS2Q8ali4uU43k= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+73248+1787277+3901457@groups.io Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1616639080356130.58303818155946; Wed, 24 Mar 2021 19:24:40 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id xeI3YY1788612xumFlDsoo3g; Wed, 24 Mar 2021 19:24:39 -0700 X-Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web11.2658.1616639078018172181 for ; Wed, 24 Mar 2021 19:24:39 -0700 X-Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Thu, 25 Mar 2021 10:24:29 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , , Cc: References: <20210324115819.605436-1-ross.burton@arm.com> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHYyXSBPdm1mUGtnOiBzdHJpcCBidWlsZCBwYXRocyBpbiByZWxlYXNlIGJ1aWxkcw==?= Date: Thu, 25 Mar 2021 10:24:29 +0800 Message-ID: <00b301d7211d$fc148390$f43d8ab0$@byosoft.com.cn> MIME-Version: 1.0 Thread-Index: AQK0MBI6XptBEhua6czXWhDjt1q4iAJ4fWmsqMYNpAA= Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,gaoliming@byosoft.com.cn X-Gm-Message-State: IgkENfUanucZgygIfgewFdKFx1787277AA= Content-Type: multipart/alternative; boundary="----=_NextPart_000_00B4_01D72161.0A39BF60" Content-Language: zh-cn DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1616639079; bh=Wapihzm5Cn4rEGitUJ8yrvRS403uE5/YQH+Ojj2+fnM=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=amGXR1eu93JpqwdKfuUNpjauFXGeqz4GpzWfobbqzm7OBtr6T+zXQ+fHUkh0MZgVIcZ +a2SNPsKiBCdwuR7/XZF0Rw4DX5vD4RsxzOumpttRShUy6mSDtui3XV6no3DOOnHS+AfQ E3a2V1P05CUHGUktEIYDvBI3vFbM7z9H6oc= X-ZohoMail-DKIM: pass (identity @groups.io) ------=_NextPart_000_00B4_01D72161.0A39BF60 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Andrew: GenFw generates debug entry, and zero debug entry. Its functionality is sa= me to the image without debug entry. So, new option is not introduced to co= ntrol whether GenFw generates debug entry when it converts ELF image to EFI= image.=20 =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 ; r= oss@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 paths = in release builds =20 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 =20 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 think this is controlled by linker flags? For Xcode/clang it is cont= rolled by the *_XCODE5_*_MTOC_FLAGS. So at this point it is kind of up to e= ach toolchain how they want to deal with symbols on release builds.=20 =20 =20 It seems kind of strange to insert a section and then zero it. Almost seems= like the intent of =E2=80=94zero was to post process compare images?=20 -z, --zero Zero the Debug Data Fields in the PE input image fi= le. It also zeros the time stamp fields. This option can be used to compare the binary efi i= mage. 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, the la= ter input action option will override the previous one. And in case you are going to ask our fork uses relative paths from the Buil= d directory and/or a UUID string for the Debug Directory entry file name so= it is a constant value and does not impact build reproducibility.=20 From a feature stand point this change will break any hope of source level = debugging with RELEASE builds. I also think it changes the exception handle= r 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 tool= s that natively produce PE/COFF and did not have a Debug Directory entry th= e same thing would happen prior to this change.=20 =09 Status =3D PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &EntryPoint); =09 if (EFI_ERROR (Status)) { =09 EntryPoint =3D NULL; =09 } =09 InternalPrintMessage ("!!!! Find image based on IP(0x%x) ", CurrentEip); =09 PdbPointer =3D PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data); =09 if (PdbPointer !=3D NULL) { =09 InternalPrintMessage ("%a", PdbPointer); =09 } else { =09 InternalPrintMessage ("(No PDB) " ); =09 } =09 InternalPrintMessage ( =09 " (ImageBase=3D%016lp, EntryPoint=3D%016p) !!!!\n", =09 (VOID *) Pe32Data, =09 EntryPoint =09 ); =09 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 conversi= on, so we seem to be working around that issue with a bigger hammer? =20 [1] https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExc= eptionHandlerLib/CpuExceptionCommon.c#L117 =20 Thanks, Andrew Fish On Mar 24, 2021, at 4:58 AM, Ross Burton > wrote: =20 GenFw will embed a NB10 section which contains the path to the input file, which means the output files have build paths embedded in them. To reduce information leakage and ensure reproducible builds, pass --zero in release builds to remove this information. 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(+) diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc 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 # # 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 # # 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 "CLANGPDB" GCC:*_*_*_CC_FLAGS =3D -mno-mmx -mno-sse !endif + RELEASE_*_*_GENFW_FLAGS =3D --zero # # 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 # # 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 # # 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 # # Disable deprecated APIs. --=20 2.25.1 =20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73248): https://edk2.groups.io/g/devel/message/73248 Mute This Topic: https://groups.io/mt/81593771/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- ------=_NextPart_000_00B4_01D72161.0A39BF60 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable

Andrew:

= =C2=A0GenFw generates debug entry, and zero debug entry. Its functionality= is same to the image without debug entry. So, new option is not introduced= to control whether GenFw generates debug entry when it converts ELF image = to EFI image.

 

Thanks

Liming

= =E5=8F= =91=E4=BB=B6=E4=BA=BA: devel@edk2.g= roups.io <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: ed= k2-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: Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build p= aths in release builds

 

This breaks some usage we have have in our fork. We ha= ve symbols turned on for Release builds, so this change would break that.&n= bsp;

 

= 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 think this is controlled by linker flags? For Xcode/clang it is cont= rolled 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.&nb= sp;

 

 

It seems kind of strange to insert a section and then zero it= . Almost seems like the intent of =E2=80=94zero was to post process compare= images? 



=   -z, --zero            Zero the De= bug Data Fields in the PE input image file.

                &= nbsp;       It also zeros the time stamp fields.

        &nbs= p;               This option can be= used to compare the binary efi image.

                 =       It can't be combined with other action options

       =                 except for -o= , -r option. It is a action option.

                  &n= bsp;     If it is combined with other action options, the la= ter

      &= nbsp;                 input ac= tion option will override the previous one.



And in case you a= re going to ask our fork uses relative paths from the Build directory and/o= r 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 s= tand point this change will break any hope of source level debugging with R= ELEASE builds. I also think it changes the exception handler code output in= OVMF [1] for ELF toolchains. You are going to get the (No PDB) vs. the fil= e and path you were getting today. I assume if you had tools that natively = produce PE/COFF and did not have a Debug Directory entry the same thing wou= ld happen prior to this change. 



<= td valign=3Dtop style=3D'padding:0cm 7.5pt 0cm 7.5pt;box-sizing: border-box= ;color:var(--color-text-primary);overflow:visible' id=3DLC133>

=C2=A0=C2=A0=C2=A0 Status =3D PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &= EntryPoint);

= = = =

=C2=A0=C2=A0 =C2=A0if (EFI_ERROR (Status)) {

=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EntryPoint =3D NULL;

=C2=A0=C2=A0=C2= =A0 }

=C2=A0=C2=A0 =C2=A0InternalPrintMessage ("!!!! Find image based on IP(0x%x= ) ", CurrentEip)= ;

=C2=A0=C2=A0=C2=A0 PdbPointer =3D PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);

=C2=A0=C2=A0 =C2=A0if (PdbP= ointer !=3D NULL) {

=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0InternalPrintMessage ("%a", PdbPointer);

= =C2=A0=C2=A0=C2=A0 } else {

=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0InternalPrintM= essage ("(No PDB= ) " );

=C2=A0=C2=A0=C2=A0 }

=C2=A0=C2=A0 =C2=A0InternalPrintMessage (

=C2=A0=C2=A0=C2=A0=C2=A0 =C2=A0" (ImageBase=3D%016lp, EntryPoint=3D%016p) !!!!\n&q= uot;,

=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (= VOID *) Pe32Data,

=

=C2=A0=C2=A0=C2=A0=C2=A0=C2= = =A0 EntryPoint

=C2=A0=C2=A0=C2=A0=C2=A0=C2= = =A0 );



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 conversion, so we seem to be working around that i= ssue with a bigger hammer?

 

 

Thanks,



Andrew Fish=


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

 

GenFw will embed a NB10 secti= on which contains the path to the input file,
which means the output fil= es have build paths embedded in them.  To reduce
information leakag= e and ensure reproducible builds, pass --zero in release
builds to remov= e this information.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D325= 6
Signed-off-by: Ross Burton <ross.burton@arm.com>
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 = +
OvmfPkg/Bhyve/BhyveX64.dsc   | 1 +
OvmfPkg/OvmfPkgIa32.ds= c      | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc  &= nbsp;| 1 +
OvmfPkg/OvmfPkgX64.dsc       | = 1 +
OvmfPkg/OvmfXen.dsc         =  | 1 +
6 files changed, 6 insertions(+)

diff --git a/OvmfPkg= /AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 65c42284d9..6= 9a05feea9 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/Amd= Sev/AmdSevX64.dsc
@@ -78,6 +78,7 @@
  GCC:*_*_X64_GENFW_FLA= GS   =3D --keepexceptiontable
  INTEL:*_*_X64_GENFW_= FLAGS =3D --keepexceptiontable
!endif
+  RELEASE_*_*_GENFW_FLAGS= =3D --zero

  #
  # Disable deprecated APIs.<= br>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

  #
  # Disable de= precated APIs.
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa3= 2.dsc
index 1eaf3e99c6..93c209950c 100644
--- a/OvmfPkg/OvmfPkgIa32.d= sc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -80,6 +80,7 @@
!if $(TOOL_CHAI= N_TAG) !=3D "XCODE5" && $(TOOL_CHAIN_TAG) !=3D "CLAN= GPDB"
  GCC:*_*_*_CC_FLAGS      =             &nb= sp;=3D -mno-mmx -mno-sse
!endif
+  RELEASE_*_*_GENFW_FLAGS =3D -= -zero

  #
  # Disable deprecated APIs.
dif= f --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:*_*_X= 64_GENFW_FLAGS =3D --keepexceptiontable
!endif
+  RELEASE_*_*_GE= NFW_FLAGS =3D --zero

  #
  # Disable deprecat= ed APIs.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dscindex d4d601b444..f544fb04bf 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++= b/OvmfPkg/OvmfPkgX64.dsc
@@ -84,6 +84,7 @@
  GCC:*_*_X64_G= ENFW_FLAGS   =3D --keepexceptiontable
  INTEL:*_*_X6= 4_GENFW_FLAGS =3D --keepexceptiontable
!endif
+  RELEASE_*_*_GEN= FW_FLAGS =3D --zero

  #
  # Disable deprecate= d 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 &n= bsp; =3D --keepexceptiontable
  INTEL:*_*_X64_GENFW_FLAGS= =3D --keepexceptiontable
!endif
+  RELEASE_*_*_GENFW_FLAGS =3D = --zero

  #
  # Disable deprecated APIs.
--=
2.25.1





 

<= /html>
_._,_._,_
Gr= oups.io Links:

You receive all messages sent to this group.

Vie= w/Reply Online (#73248) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [importer@patchew.org]

_._,_._,_
=20 ------=_NextPart_000_00B4_01D72161.0A39BF60--