[edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug entry read error after it is found

gaoliming via groups.io posted 1 patch 1 year ago
Failed in applying to current master (apply log)
MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug entry read error after it is found
Posted by gaoliming via groups.io 1 year ago
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4425

This change is to support the pre-built EFI image on the old Edk2 code.
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 <gaoliming@byosoft.com.cn>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
---
 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 != ReadSize)) {
+          if ((ImageContext->DebugDirectoryEntryRva == 0) && (RETURN_ERROR (Status) || (Size != ReadSize))) {
             ImageContext->ImageError = IMAGE_ERROR_IMAGE_READ;
             if (Size != ReadSize) {
               Status = RETURN_UNSUPPORTED;
-- 
2.37.3.windows.1




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103698): https://edk2.groups.io/g/devel/message/103698
Mute This Topic: https://groups.io/mt/98531785/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug entry read error after it is found
Posted by Ard Biesheuvel 1 year ago
On Thu, 27 Apr 2023 at 07:17, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4425
>
> This change is to support the pre-built EFI image on the old Edk2 code.
> 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 <gaoliming@byosoft.com.cn>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  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 != ReadSize)) {
> +          if ((ImageContext->DebugDirectoryEntryRva == 0) && (RETURN_ERROR (Status) || (Size != ReadSize))) {

I'm not sure I understand how this fixes anything. Why is the
condition 'RVA == 0' significant here?

Would something like the below perhaps work as well?

--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -755,6 +755,17 @@ PeCoffLoaderGetImageInfo (
               ImageContext->ImageSize += 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 relevant
+            // directory entries can exist, and we can terminate here.
+            //
+            if (DebugEntry.FileOffset ==
(DebugDirectoryEntryFileOffset + Index)) {
+              break;
+            }
+
             continue;
           }


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103717): https://edk2.groups.io/g/devel/message/103717
Mute This Topic: https://groups.io/mt/98531785/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug entry read error after it is found
Posted by gaoliming via groups.io 1 year ago
Ard:

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2023年4月27日 22:47
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: Michael Kubacki <michael.kubacki@microsoft.com>
> 主题: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug
> entry read error after it is found
> 
> On Thu, 27 Apr 2023 at 07:17, gaoliming via groups.io
> <gaoliming=byosoft.com.cn@groups.io> wrote:
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4425
> >
> > This change is to support the pre-built EFI image on the old Edk2 code.
> > 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 <gaoliming@byosoft.com.cn>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > ---
> >  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 != ReadSize)) {
> > +          if ((ImageContext->DebugDirectoryEntryRva == 0) &&
> (RETURN_ERROR (Status) || (Size != ReadSize))) {
> 
> I'm not sure I understand how this fixes anything. Why is the
> condition 'RVA == 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 not 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 += 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 relevant
> +            // directory entries can exist, and we can terminate here.
> +            //
> +            if (DebugEntry.FileOffset ==
> (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?

Thanks
Liming
>              continue;
>            }
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103754): https://edk2.groups.io/g/devel/message/103754
Mute This Topic: https://groups.io/mt/98553124/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug entry read error after it is found
Posted by Ard Biesheuvel 1 year ago
On Fri, 28 Apr 2023 at 07:02, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> Ard:
>
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> > Biesheuvel
> > 发送时间: 2023年4月27日 22:47
> > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > 抄送: Michael Kubacki <michael.kubacki@microsoft.com>
> > 主题: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug
> > entry read error after it is found
> >
> > On Thu, 27 Apr 2023 at 07:17, gaoliming via groups.io
> > <gaoliming=byosoft.com.cn@groups.io> wrote:
> > >
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4425
> > >
> > > This change is to support the pre-built EFI image on the old Edk2 code.
> > > 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 <gaoliming@byosoft.com.cn>
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > > ---
> > >  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 != ReadSize)) {
> > > +          if ((ImageContext->DebugDirectoryEntryRva == 0) &&
> > (RETURN_ERROR (Status) || (Size != ReadSize))) {
> >
> > I'm not sure I understand how this fixes anything. Why is the
> > condition 'RVA == 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 not 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 += 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 relevant
> > +            // directory entries can exist, and we can terminate here.
> > +            //
> > +            if (DebugEntry.FileOffset ==
> > (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?
>

No. But if the codeview payload follows the codeview entry directly,
there cannot be any other entries, so there is no reason to proceed.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103773): https://edk2.groups.io/g/devel/message/103773
Mute This Topic: https://groups.io/mt/98554964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug entry read error after it is found
Posted by gaoliming via groups.io 1 year ago
Ard:

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2023年4月28日 17:21
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: Michael Kubacki <michael.kubacki@microsoft.com>
> 主题: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug
> entry read error after it is found
> 
> On Fri, 28 Apr 2023 at 07:02, gaoliming via groups.io
> <gaoliming=byosoft.com.cn@groups.io> wrote:
> >
> > Ard:
> >
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> > > Biesheuvel
> > > 发送时间: 2023年4月27日 22:47
> > > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > > 抄送: Michael Kubacki <michael.kubacki@microsoft.com>
> > > 主题: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug
> > > entry read error after it is found
> > >
> > > On Thu, 27 Apr 2023 at 07:17, gaoliming via groups.io
> > > <gaoliming=byosoft.com.cn@groups.io> wrote:
> > > >
> > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4425
> > > >
> > > > This change is to support the pre-built EFI image on the old Edk2 code.
> > > > 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 <gaoliming@byosoft.com.cn>
> > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > > > ---
> > > >  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 != ReadSize)) {
> > > > +          if ((ImageContext->DebugDirectoryEntryRva == 0) &&
> > > (RETURN_ERROR (Status) || (Size != ReadSize))) {
> > >
> > > I'm not sure I understand how this fixes anything. Why is the
> > > condition 'RVA == 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 not 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 += 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 relevant
> > > +            // directory entries can exist, and we can terminate here.
> > > +            //
> > > +            if (DebugEntry.FileOffset ==
> > > (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?
> >
> 
> No. But if the codeview payload follows the codeview entry directly,
> there cannot be any other entries, so there is no reason to proceed.
> 
> 
I verified this change. It doesn't work, because 
DebugEntry.FileOffset value is DebugDirectoryEntryFileOffset + Index + sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)

I review the change 60e85a39fe49071 in GenFw. It places DebugEntry as the last debug directory. 
So, I think the simple fix is to directly break once DebugEntry is found. 

Thanks
Liming
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103973): https://edk2.groups.io/g/devel/message/103973
Mute This Topic: https://groups.io/mt/98679630/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug entry read error after it is found
Posted by Ard Biesheuvel 1 year ago
On Thu, 4 May 2023 at 09:49, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> Ard:
>
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> > Biesheuvel
> > 发送时间: 2023年4月28日 17:21
> > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > 抄送: Michael Kubacki <michael.kubacki@microsoft.com>
> > 主题: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug
> > entry read error after it is found
> >
> > On Fri, 28 Apr 2023 at 07:02, gaoliming via groups.io
> > <gaoliming=byosoft.com.cn@groups.io> wrote:
> > >
> > > Ard:
> > >
> > > > -----邮件原件-----
> > > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> > > > Biesheuvel
> > > > 发送时间: 2023年4月27日 22:47
> > > > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > > > 抄送: Michael Kubacki <michael.kubacki@microsoft.com>
> > > > 主题: Re: [edk2-devel] [PATCH] MdePkg BasePeCoffLib: Ignore the debug
> > > > entry read error after it is found
> > > >
> > > > On Thu, 27 Apr 2023 at 07:17, gaoliming via groups.io
> > > > <gaoliming=byosoft.com.cn@groups.io> wrote:
> > > > >
> > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4425
> > > > >
> > > > > This change is to support the pre-built EFI image on the old Edk2 code.
> > > > > 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 <gaoliming@byosoft.com.cn>
> > > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > > > > ---
> > > > >  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 != ReadSize)) {
> > > > > +          if ((ImageContext->DebugDirectoryEntryRva == 0) &&
> > > > (RETURN_ERROR (Status) || (Size != ReadSize))) {
> > > >
> > > > I'm not sure I understand how this fixes anything. Why is the
> > > > condition 'RVA == 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 not 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 += 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 relevant
> > > > +            // directory entries can exist, and we can terminate here.
> > > > +            //
> > > > +            if (DebugEntry.FileOffset ==
> > > > (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?
> > >
> >
> > No. But if the codeview payload follows the codeview entry directly,
> > there cannot be any other entries, so there is no reason to proceed.
> >
> >
> I verified this change. It doesn't work, because
> DebugEntry.FileOffset value is DebugDirectoryEntryFileOffset + Index + sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)
>
> I review the change 60e85a39fe49071 in GenFw. It places DebugEntry as the last debug directory.
> So, I think the simple fix is to directly break once DebugEntry is found.
>

I'm confused. Doesn't that mean we will ignore the
DEBUG_TYPE_EX_DLLCHARACTERISTICS entry?

Given that the old, broken GenFw puts the NB10 directory entry and the
NB10 data structure contiguous in memory, we should be able to detect
that, right? And in this case, because the NB10 directory entry is
followed by something we /know/ is not another directory entry, we can
exit the loop early.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104008): https://edk2.groups.io/g/devel/message/104008
Mute This Topic: https://groups.io/mt/98684582/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-