* Re: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data
2021-06-05 1:18 [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data gaoliming
@ 2021-06-05 1:30 ` Ni, Ray
2021-06-05 5:05 ` Bob Feng
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Ni, Ray @ 2021-06-05 1:30 UTC (permalink / raw)
To: Leif Lindholm, gaoliming@byosoft.com.cn; +Cc: Feng, Bob C, devel@edk2.groups.io
Cool! Thank you so much!!
It answers my question that why CI cannot detect such failure because it's using GCC5+.
Leif,
Can you verify that this can resolve your GCC49 issue?
Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> gaoliming
> Sent: Saturday, June 5, 2021 9:19 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Leif Lindholm <leif@nuviainc.com>; Feng,
> Bob C <bob.c.feng@intel.com>
> Subject: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to
> convert the image to ACPI data
>
> With GCC49 tool chain, aslc file is compiled into elf image.
> But, its text section has no CODE attribute. So, it can't be detected
> by new GenFw tool.For this type file. its text section is not required.
> Its data section will be converted to acpi table.
>
> This fix is to remove assert check when the generated image is ACPI data.
>
> Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> ---
> This patch has been verified on GCC49 for AARCH64 platform.
>
> BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
> BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c
> b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index 7c8a065678ac..314f8233234d 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -435,7 +435,7 @@ ScanSections32 (
> }
> }
>
> - if (!FoundSection) {
> + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> assert (FALSE);
> }
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 8fe672e98448..8b09db7b690b 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -714,7 +714,7 @@ ScanSections64 (
> }
> }
>
> - if (!FoundSection) {
> + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> assert (FALSE);
> }
> --
> 2.27.0.windows.1
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data
2021-06-05 1:18 [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data gaoliming
2021-06-05 1:30 ` [edk2-devel] " Ni, Ray
@ 2021-06-05 5:05 ` Bob Feng
2021-06-05 18:52 ` Leif Lindholm
2021-06-07 15:18 ` Laszlo Ersek
3 siblings, 0 replies; 7+ messages in thread
From: Bob Feng @ 2021-06-05 5:05 UTC (permalink / raw)
To: devel@edk2.groups.io, gaoliming@byosoft.com.cn; +Cc: Ni, Ray, Leif Lindholm
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Saturday, June 5, 2021 9:19 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Leif Lindholm <leif@nuviainc.com>; Feng, Bob C <bob.c.feng@intel.com>
Subject: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data
With GCC49 tool chain, aslc file is compiled into elf image.
But, its text section has no CODE attribute. So, it can't be detected by new GenFw tool.For this type file. its text section is not required.
Its data section will be converted to acpi table.
This fix is to remove assert check when the generated image is ACPI data.
Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Bob Feng <bob.c.feng@intel.com>
---
This patch has been verified on GCC49 for AARCH64 platform.
BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +- BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
index 7c8a065678ac..314f8233234d 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -435,7 +435,7 @@ ScanSections32 (
}
}
- if (!FoundSection) {
+ if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
assert (FALSE);
}
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 8fe672e98448..8b09db7b690b 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -714,7 +714,7 @@ ScanSections64 (
}
}
- if (!FoundSection) {
+ if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
assert (FALSE);
}
--
2.27.0.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data
2021-06-05 1:18 [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data gaoliming
2021-06-05 1:30 ` [edk2-devel] " Ni, Ray
2021-06-05 5:05 ` Bob Feng
@ 2021-06-05 18:52 ` Leif Lindholm
2021-06-06 7:40 ` 回复: " gaoliming
2021-06-07 15:18 ` Laszlo Ersek
3 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2021-06-05 18:52 UTC (permalink / raw)
To: devel, gaoliming; +Cc: Ray Ni, Bob Feng
Hi Liming,
On Sat, Jun 05, 2021 at 09:18:54 +0800, gaoliming wrote:
> With GCC49 tool chain, aslc file is compiled into elf image.
> But, its text section has no CODE attribute. So, it can't be detected
> by new GenFw tool.For this type file. its text section is not required.
> Its data section will be converted to acpi table.
>
> This fix is to remove assert check when the generated image is ACPI data.
Note - this does not only affect GCC49: it also affects at least
GCC5 and CLANG38. This patch resolves the issue.
Tested-by: Leif Lindholm <leif@nuviainc.com> (GCC49, GCC5, CLANG38)
Thanks!
/
Leif
> Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> ---
> This patch has been verified on GCC49 for AARCH64 platform.
>
> BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
> BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index 7c8a065678ac..314f8233234d 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -435,7 +435,7 @@ ScanSections32 (
> }
> }
>
> - if (!FoundSection) {
> + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> assert (FALSE);
> }
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 8fe672e98448..8b09db7b690b 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -714,7 +714,7 @@ ScanSections64 (
> }
> }
>
> - if (!FoundSection) {
> + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> assert (FALSE);
> }
> --
> 2.27.0.windows.1
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* 回复: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data
2021-06-05 18:52 ` Leif Lindholm
@ 2021-06-06 7:40 ` gaoliming
0 siblings, 0 replies; 7+ messages in thread
From: gaoliming @ 2021-06-06 7:40 UTC (permalink / raw)
To: 'Leif Lindholm', devel; +Cc: 'Ray Ni', 'Bob Feng'
Leif and Bob:
Thanks for your test and review. I create PR
https://github.com/tianocore/edk2/pull/1690 for this patch.
Thanks
Liming
> -----邮件原件-----
> 发件人: Leif Lindholm <leif@nuviainc.com>
> 发送时间: 2021年6月6日 2:53
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: Ray Ni <ray.ni@intel.com>; Bob Feng <bob.c.feng@intel.com>
> 主题: Re: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue
to
> convert the image to ACPI data
>
> Hi Liming,
>
> On Sat, Jun 05, 2021 at 09:18:54 +0800, gaoliming wrote:
> > With GCC49 tool chain, aslc file is compiled into elf image.
> > But, its text section has no CODE attribute. So, it can't be detected
> > by new GenFw tool.For this type file. its text section is not required.
> > Its data section will be converted to acpi table.
> >
> > This fix is to remove assert check when the generated image is ACPI
data.
>
> Note - this does not only affect GCC49: it also affects at least
> GCC5 and CLANG38. This patch resolves the issue.
>
> Tested-by: Leif Lindholm <leif@nuviainc.com> (GCC49, GCC5, CLANG38)
>
> Thanks!
>
> /
> Leif
>
> > Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > ---
> > This patch has been verified on GCC49 for AARCH64 platform.
> >
> > BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
> > BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c
> b/BaseTools/Source/C/GenFw/Elf32Convert.c
> > index 7c8a065678ac..314f8233234d 100644
> > --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> > +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> > @@ -435,7 +435,7 @@ ScanSections32 (
> > }
> > }
> >
> > - if (!FoundSection) {
> > + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> > Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.
");
> > assert (FALSE);
> > }
> > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > index 8fe672e98448..8b09db7b690b 100644
> > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > @@ -714,7 +714,7 @@ ScanSections64 (
> > }
> > }
> >
> > - if (!FoundSection) {
> > + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> > Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.
");
> > assert (FALSE);
> > }
> > --
> > 2.27.0.windows.1
> >
> >
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data
2021-06-05 1:18 [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data gaoliming
` (2 preceding siblings ...)
2021-06-05 18:52 ` Leif Lindholm
@ 2021-06-07 15:18 ` Laszlo Ersek
2021-06-08 1:19 ` 回复: " gaoliming
3 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2021-06-07 15:18 UTC (permalink / raw)
To: devel, gaoliming; +Cc: Ray Ni, Leif Lindholm, Bob Feng
On 06/05/21 03:18, gaoliming wrote:
> With GCC49 tool chain, aslc file is compiled into elf image.
> But, its text section has no CODE attribute. So, it can't be detected
> by new GenFw tool.For this type file. its text section is not required.
> Its data section will be converted to acpi table.
>
> This fix is to remove assert check when the generated image is ACPI data.
>
> Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> ---
> This patch has been verified on GCC49 for AARCH64 platform.
>
> BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
> BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
If this is a regression, then the commit message should please identify
the commit that introduced the regression. We usually do this with:
Fixes: ....
Thanks
Laszlo
>
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index 7c8a065678ac..314f8233234d 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -435,7 +435,7 @@ ScanSections32 (
> }
> }
>
> - if (!FoundSection) {
> + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> assert (FALSE);
> }
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 8fe672e98448..8b09db7b690b 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -714,7 +714,7 @@ ScanSections64 (
> }
> }
>
> - if (!FoundSection) {
> + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> assert (FALSE);
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread* 回复: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to convert the image to ACPI data
2021-06-07 15:18 ` Laszlo Ersek
@ 2021-06-08 1:19 ` gaoliming
0 siblings, 0 replies; 7+ messages in thread
From: gaoliming @ 2021-06-08 1:19 UTC (permalink / raw)
To: 'Laszlo Ersek', devel
Cc: 'Ray Ni', 'Leif Lindholm', 'Bob Feng'
Laszlo:
Thanks for your reminder. When I merge this change, I add the regression commit info in the commit message.
This change is merged at ddb3fdbef30de5a2946f9bd51060e8d5b1987aef.
Thanks
Liming
> -----邮件原件-----
> 发件人: Laszlo Ersek <lersek@redhat.com>
> 发送时间: 2021年6月7日 23:19
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: Ray Ni <ray.ni@intel.com>; Leif Lindholm <leif@nuviainc.com>; Bob
> Feng <bob.c.feng@intel.com>
> 主题: Re: [edk2-devel] [PATCH 1/1] BaseTools GenFw: Fix regression issue to
> convert the image to ACPI data
>
> On 06/05/21 03:18, gaoliming wrote:
> > With GCC49 tool chain, aslc file is compiled into elf image.
> > But, its text section has no CODE attribute. So, it can't be detected
> > by new GenFw tool.For this type file. its text section is not required.
> > Its data section will be converted to acpi table.
> >
> > This fix is to remove assert check when the generated image is ACPI data.
> >
> > Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > ---
> > This patch has been verified on GCC49 for AARCH64 platform.
> >
> > BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
> > BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
>
> If this is a regression, then the commit message should please identify
> the commit that introduced the regression. We usually do this with:
>
> Fixes: ....
>
> Thanks
> Laszlo
>
> >
> > diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c
> b/BaseTools/Source/C/GenFw/Elf32Convert.c
> > index 7c8a065678ac..314f8233234d 100644
> > --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> > +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> > @@ -435,7 +435,7 @@ ScanSections32 (
> > }
> > }
> >
> > - if (!FoundSection) {
> > + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> > Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> > assert (FALSE);
> > }
> > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > index 8fe672e98448..8b09db7b690b 100644
> > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > @@ -714,7 +714,7 @@ ScanSections64 (
> > }
> > }
> >
> > - if (!FoundSection) {
> > + if (!FoundSection && mOutImageType != FW_ACPI_IMAGE) {
> > Error (NULL, 0, 3000, "Invalid", "Did not find any '.text' section.");
> > assert (FALSE);
> > }
> >
^ permalink raw reply [flat|nested] 7+ messages in thread