From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from zrleap.intel-email.com (zrleap.intel-email.com [114.80.218.36]) by mx.groups.io with SMTP id smtpd.web11.44228.1683186592215991289 for ; Thu, 04 May 2023 00:49:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@byosoft.com.cn header.s=cloud-union header.b=IcnVqNHU; spf=pass (domain: byosoft.com.cn, ip: 114.80.218.36, mailfrom: gaoliming@byosoft.com.cn) Received: from zrleap.intel-email.com (localhost [127.0.0.1]) by zrleap.intel-email.com (Postfix) with ESMTP id 0108CA32E133 for ; Thu, 4 May 2023 15:49:46 +0800 (CST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=byosoft.com.cn; s=cloud-union; t=1683186587; bh=NXwZvgvGXp0x8wh7eOa04gROvlbHa6Mcm0VvWdB5sWM=; h=From:To:Cc:References:In-Reply-To:Subject:Date; b=IcnVqNHUw1aaG4rJ8E+XnaFSPtmEawnYTZciOgifYcmRtCYnJqpTD2ZJvn95p6MeU x2PIMHWTFJc7i3Bh+eC0s6LvyntAGEmrpPNbXGC9ndDFM/w7ISXxdNUhTidUI/N/uU ybvgcWcjhZzLIBtJqC7vrsIr6Fex4Kwf0hlqr/hg= Received: from localhost (localhost [127.0.0.1]) by zrleap.intel-email.com (Postfix) with ESMTP id A7EE6A32E11A for ; Thu, 4 May 2023 15:49:46 +0800 (CST) Received: from zrleap.intel-email.com (localhost [127.0.0.1]) by zrleap.intel-email.com (Postfix) with ESMTP id 4E5CAA32E027 for ; Thu, 4 May 2023 15:49:44 +0800 (CST) Authentication-Results: zrleap.intel-email.com; none Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by zrleap.intel-email.com (Postfix) with SMTP id 0A5CDA32E15C for ; Thu, 4 May 2023 15:49:41 +0800 (CST) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Thu, 04 May 2023 15:49:37 +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: "'Michael Kubacki'" References: <20230427061714.510-1-gaoliming@byosoft.com.cn> <001501d97997$07241c90$156c55b0$@byosoft.com.cn> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIXSBNZGVQa2cgQmFzZVBlQ29mZkxpYjogSWdub3JlIHRoZSBkZWJ1ZyBlbnRyeSByZWFkIGVycm9yIGFmdGVyIGl0IGlzIGZvdW5k?= Date: Thu, 4 May 2023 15:49:40 +0800 Message-ID: <033f01d97e5c$fb81f770$f285e650$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHB0gnMqs3hkL1bR+LlNFo1/DXHmQJyV916AYkB7CcCYnjRya9GJkCg Sender: "gaoliming" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Ard: > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Ard > Biesheuvel > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2023=E5=B9=B44=E6=9C=8828=E6=97=A5 = 17:21 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; gaoliming@byosoft.com.= cn > =E6=8A=84=E9=80=81: Michael Kubacki > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore= the debug > entry read error after it is found >=20 > On Fri, 28 Apr 2023 at 07:02, gaoliming via groups.io > wrote: > > > > Ard: > > > > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io =E4=BB=A3=E8=A1=A8 Ard > > > Biesheuvel > > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2023=E5=B9=B44=E6=9C=8827=E6=97= =A5 22:47 > > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; gaoliming@byosoft.= com.cn > > > =E6=8A=84=E9=80=81: Michael Kubacki > > > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ig= nore the debug > > > entry read error after it is found > > > > > > On Thu, 27 Apr 2023 at 07:17, gaoliming via groups.io > > > wrote: > > > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4425 > > > > > > > > This change is to support the pre-built EFI image on the old Edk2 c= ode. > > > > Old Edk2 GenFw tool generates the wrong debug entry in EFI image. > > > > Those pre-built images can be loaded before d6457b309. > > > > > > > > Signed-off-by: Liming Gao > > > > Cc: Ard Biesheuvel > > > > Cc: Michael Kubacki > > > > --- > > > > MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > > b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > > > index 4b71176a0c..4636842eeb 100644 > > > > --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > > > +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > > > @@ -735,7 +735,7 @@ PeCoffLoaderGetImageInfo ( > > > > &Size, > > > > &DebugEntry > > > > ); > > > > - if (RETURN_ERROR (Status) || (Size !=3D ReadSize)) { > > > > + if ((ImageContext->DebugDirectoryEntryRva =3D=3D 0) && > > > (RETURN_ERROR (Status) || (Size !=3D ReadSize))) { > > > > > > I'm not sure I understand how this fixes anything. Why is the > > > condition 'RVA =3D=3D 0' significant here? > > > > > > Would something like the below perhaps work as well? > > > > > > > DebugDirectoryEntryRva will set to the real value if > DEBUG_TYPE_CODEVIEW is found. > > If its value is not zero, it means DebugEntry has been found. This is n= ot an > error, and don't need to report error here. > > > > > --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > > +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > > @@ -755,6 +755,17 @@ PeCoffLoaderGetImageInfo ( > > > ImageContext->ImageSize +=3D DebugEntry.SizeOfData; > > > } > > > > > > + // > > > + // Implementations of GenFw before commit > > > 60e85a39fe49071 will > > > + // concatenate the debug directory entry and the > codeview > > > entry, > > > + // and erroneously put the combined size into the debug > > > directory > > > + // entry's size field. If this is the case, no other rel= evant > > > + // directory entries can exist, and we can terminate her= e. > > > + // > > > + if (DebugEntry.FileOffset =3D=3D > > > (DebugDirectoryEntryFileOffset + Index)) { > > > + break; > > > + } > > > + > > d6457b309 adds DEBUG_TYPE_EX_DLLCHARACTERISTICS support. So, > there may be two DEBUG entries in one EFI image. > > One is DEBUG_TYPE_CODEVIEW, another is > DEBUG_TYPE_EX_DLLCHARACTERISTICS. > > > > Here break after DEBUG_TYPE_CODEVIEW is found. So, is there the chance > to search DEBUG_TYPE_EX_DLLCHARACTERISTICS? > > >=20 > No. But if the codeview payload follows the codeview entry directly, > there cannot be any other entries, so there is no reason to proceed. >=20 >=20 I verified this change. It doesn't work, because=20 DebugEntry.FileOffset value is DebugDirectoryEntryFileOffset + Index + size= of (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) I review the change 60e85a39fe49071 in GenFw. It places DebugEntry as the l= ast debug directory.=20 So, I think the simple fix is to directly break once DebugEntry is found.= =20 Thanks Liming >=20 >=20