Re: [OE-core] [PATCH] ovmf: Fix native build with gcc-12

[richard.purdie@linuxfoundation.org]

On Sat, 2022-05-14 at 14:42 -0700, Khem Raj wrote:
> 
> 
> On Sat, May 14, 2022 at 2:31 PM <richard.purdie@linuxfoundation.org>
> wrote:
> > On Sat, 2022-05-14 at 13:15 +0100, Richard Purdie via
> > lists.openembedded.org wrote:
> > > On Fri, 2022-05-13 at 09:22 -0700, Khem Raj wrote:
> > > > Host distros have started deploying gcc12 as well e.g.
> > > > archlinux
> > > > this build failure shows up which has been fixed upstream
> > > > 
> > > > In function ‘GetAlignmentFromFile’,
> > > >      inlined from ‘main’ at GenFfs.c:816:20:
> > > > GenFfs.c:545:5: error: pointer ‘InFileHandle’ used after
> > > > ‘fclose’ [-Werror=use-after-free]
> > > >    545 |     Error(NULL, 0, 4001, "Resource", "memory cannot be
> > > > allocated  of %s", InFileHandle);
> > > >        |   
> > > >  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > ~~~~~~~~~~~~~~~~~~~~~
> > > > 
> > > > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > > ---
> > > >   .../0001-BaseTools-fix-gcc12-warning-1.patch  | 51
> > > > +++++++++++++++++++
> > > >   .../0001-BaseTools-fix-gcc12-warning.patch    | 49
> > > > ++++++++++++++++++
> > > >   meta/recipes-core/ovmf/ovmf_git.bb            |  6 +++
> > > >   3 files changed, 106 insertions(+)
> > > >   create mode 100644 meta/recipes-core/ovmf/ovmf/0001-
> > > > BaseTools-fix-gcc12-warning-1.patch
> > > >   create mode 100644 meta/recipes-core/ovmf/ovmf/0001-
> > > > BaseTools-fix-gcc12-warning.patch
> > > > 
> > > > diff --git a/meta/recipes-core/ovmf/ovmf/0001-BaseTools-fix-
> > > > gcc12-warning-1.patch b/meta/recipes-core/ovmf/ovmf/0001-
> > > > BaseTools-fix-gcc12-warning-1.patch
> > > > new file mode 100644
> > > > index 00000000000..23366b4d078
> > > > --- /dev/null
> > > > +++ b/meta/recipes-core/ovmf/ovmf/0001-BaseTools-fix-gcc12-
> > > > warning-1.patch
> > > > @@ -0,0 +1,51 @@
> > > > +From b0a0b7b605aeb8106e7b50d1efeb746f401498cc Mon Sep 17
> > > > 00:00:00 2001
> > > > +From: Gerd Hoffmann <kraxel@redhat.com>
> > > > +Date: Thu, 24 Mar 2022 20:04:35 +0800
> > > > +Subject: [PATCH] BaseTools: fix gcc12 warning
> > > > +
> > > > +Sdk/C/LzmaEnc.c: In function ?LzmaEnc_CodeOneMemBlock?:
> > > > +Sdk/C/LzmaEnc.c:2828:19: error: storing the address of local
> > > > variable ?outStream? in ?*p.rc.outStream? [-Werror=dangling-
> > > > pointer=]
> > > > + 2828 |   p->rc.outStream = &outStream.vt;
> > > > +      |   ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
> > > > +Sdk/C/LzmaEnc.c:2811:28: note: ?outStream? declared here
> > > > + 2811 |   CLzmaEnc_SeqOutStreamBuf outStream;
> > > > +      |                            ^~~~~~~~~
> > > > +Sdk/C/LzmaEnc.c:2811:28: note: ?pp? declared here
> > > > +Sdk/C/LzmaEnc.c:2828:19: error: storing the address of local
> > > > variable ?outStream? in ?*(CLzmaEnc *)pp.rc.outStream? [-
> > > > Werror=dangling-pointer=]
> > > > + 2828 |   p->rc.outStream = &outStream.vt;
> > > > +      |   ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
> > > > +Sdk/C/LzmaEnc.c:2811:28: note: ?outStream? declared here
> > > > + 2811 |   CLzmaEnc_SeqOutStreamBuf outStream;
> > > > +      |                            ^~~~~~~~~
> > > > +Sdk/C/LzmaEnc.c:2811:28: note: ?pp? declared here
> > > > +cc1: all warnings being treated as errors
> > > > +
> > > > +Upstream-Status: Backport
> > > > [https://github.com/tianocore/edk2/commit/85021f8cf22d1bd411480
> > > > 3c6c610dea5ef0059f1]
> > > > +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > > +Reviewed-by: Bob Feng <bob.c.feng@intel.com>
> > > > +---
> > > > + BaseTools/Source/C/LzmaCompress/Sdk/C/LzmaEnc.c | 3 ++-
> > > > + 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > +
> > > > +diff --git a/BaseTools/Source/C/LzmaCompress/Sdk/C/LzmaEnc.c
> > > > b/BaseTools/Source/C/LzmaCompress/Sdk/C/LzmaEnc.c
> > > > +index 4e9b499f8d..4b9f5fa692 100644
> > > > +--- a/BaseTools/Source/C/LzmaCompress/Sdk/C/LzmaEnc.c
> > > > ++++ b/BaseTools/Source/C/LzmaCompress/Sdk/C/LzmaEnc.c
> > > > +@@ -2825,12 +2825,13 @@ SRes
> > > > LzmaEnc_CodeOneMemBlock(CLzmaEncHandle pp, BoolInt reInit,
> > > > + 
> > > > +   nowPos64 = p->nowPos64;
> > > > +   RangeEnc_Init(&p->rc);
> > > > +-  p->rc.outStream = &outStream.vt;
> > > > + 
> > > > +   if (desiredPackSize == 0)
> > > > +     return SZ_ERROR_OUTPUT_EOF;
> > > > + 
> > > > ++  p->rc.outStream = &outStream.vt;
> > > > +   res = LzmaEnc_CodeOneBlock(p, desiredPackSize,
> > > > *unpackSize);
> > > > ++  p->rc.outStream = NULL;
> > > > + 
> > > > +   *unpackSize = (UInt32)(p->nowPos64 - nowPos64);
> > > > +   *destLen -= outStream.rem;
> > > > +-- 
> > > > +2.36.1
> > > > +
> > > > diff --git a/meta/recipes-core/ovmf/ovmf/0001-BaseTools-fix-
> > > > gcc12-warning.patch b/meta/recipes-core/ovmf/ovmf/0001-
> > > > BaseTools-fix-gcc12-warning.patch
> > > > new file mode 100644
> > > > index 00000000000..91c01647db2
> > > > --- /dev/null
> > > > +++ b/meta/recipes-core/ovmf/ovmf/0001-BaseTools-fix-gcc12-
> > > > warning.patch
> > > > @@ -0,0 +1,49 @@
> > > > +From 2435c17e56652479315853cec5b91fb0ea0911a3 Mon Sep 17
> > > > 00:00:00 2001
> > > > +From: Gerd Hoffmann <kraxel@redhat.com>
> > > > +Date: Thu, 24 Mar 2022 20:04:34 +0800
> > > > +Subject: [PATCH] BaseTools: fix gcc12 warning
> > > > +
> > > > +GenFfs.c:545:5: error: pointer ?InFileHandle? used after
> > > > ?fclose? [-Werror=use-after-free]
> > > > +  545 |     Error(NULL, 0, 4001, "Resource", "memory cannot be
> > > > allocated  of %s", InFileHandle);
> > > > +      |   
> > > >  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > ~~~~~~~~~~~~~~~~~~~~~
> > > > +GenFfs.c:544:5: note: call to ?fclose? here
> > > > +  544 |     fclose (InFileHandle);
> > > > +      |     ^~~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +Upstream-Status: Backport
> > > > [https://github.com/tianocore/edk2/commit/7b005f344e533cd913c3c
> > > > a05b266f9872df886d1]
> > > > +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > > +Reviewed-by: Bob Feng <bob.c.feng@intel.com>
> > > > +---
> > > > + BaseTools/Source/C/GenFfs/GenFfs.c | 2 +-
> > > > + BaseTools/Source/C/GenSec/GenSec.c | 2 +-
> > > > + 2 files changed, 2 insertions(+), 2 deletions(-)
> > > > +
> > > > +diff --git a/BaseTools/Source/C/GenFfs/GenFfs.c
> > > > b/BaseTools/Source/C/GenFfs/GenFfs.c
> > > > +index 949025c333..d78d62ab36 100644
> > > > +--- a/BaseTools/Source/C/GenFfs/GenFfs.c
> > > > ++++ b/BaseTools/Source/C/GenFfs/GenFfs.c
> > > > +@@ -542,7 +542,7 @@ GetAlignmentFromFile(char *InFile, UINT32
> > > > *Alignment)
> > > > +   PeFileBuffer = (UINT8 *) malloc (PeFileSize);
> > > > +   if (PeFileBuffer == NULL) {
> > > > +     fclose (InFileHandle);
> > > > +-    Error(NULL, 0, 4001, "Resource", "memory cannot be
> > > > allocated  of %s", InFileHandle);
> > > > ++    Error(NULL, 0, 4001, "Resource", "memory cannot be
> > > > allocated for %s", InFile);
> > > > +     return EFI_OUT_OF_RESOURCES;
> > > > +   }
> > > > +   fread (PeFileBuffer, sizeof (UINT8), PeFileSize,
> > > > InFileHandle);
> > > > +diff --git a/BaseTools/Source/C/GenSec/GenSec.c
> > > > b/BaseTools/Source/C/GenSec/GenSec.c
> > > > +index d54a4f9e0a..b1d05367ec 100644
> > > > +--- a/BaseTools/Source/C/GenSec/GenSec.c
> > > > ++++ b/BaseTools/Source/C/GenSec/GenSec.c
> > > > +@@ -1062,7 +1062,7 @@ GetAlignmentFromFile(char *InFile,
> > > > UINT32 *Alignment)
> > > > +   PeFileBuffer = (UINT8 *) malloc (PeFileSize);
> > > > +   if (PeFileBuffer == NULL) {
> > > > +     fclose (InFileHandle);
> > > > +-    Error(NULL, 0, 4001, "Resource", "memory cannot be
> > > > allocated  of %s", InFileHandle);
> > > > ++    Error(NULL, 0, 4001, "Resource", "memory cannot be
> > > > allocated for %s", InFile);
> > > > +     return EFI_OUT_OF_RESOURCES;
> > > > +   }
> > > > +   fread (PeFileBuffer, sizeof (UINT8), PeFileSize,
> > > > InFileHandle);
> > > > +-- 
> > > > +2.36.1
> > > > +
> > > > diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-
> > > > core/ovmf/ovmf_git.bb
> > > > index b15d40eac83..84e3360a3af 100644
> > > > --- a/meta/recipes-core/ovmf/ovmf_git.bb
> > > > +++ b/meta/recipes-core/ovmf/ovmf_git.bb
> > > > @@ -15,12 +15,18 @@ PACKAGECONFIG +=
> > > > "${@bb.utils.contains('MACHINE_FEATURES', 'tpm2', 'tpm', '', d)
> > > >   PACKAGECONFIG[secureboot] = ",,,"
> > > >   PACKAGECONFIG[tpm] = "-D TPM_ENABLE=TRUE,-D
> > > > TPM_ENABLE=FALSE,,"
> > > >   
> > > > +# GCC12 trips on it
> > > > +#see
> > > > https://src.fedoraproject.org/rpms/edk2/blob/rawhide/f/0032-Basetools-turn-off-gcc12-warning.patch
> > > > +BUILD_CFLAGS += "-Wno-error=stringop-overflow"
> > > > +
> > > >   SRC_URI =
> > > > "gitsm://github.com/tianocore/edk2.git;branch=master;protocol=h
> > > > ttps \
> > > >            
> > > >  file://0001-ovmf-update-path-to-native-BaseTools.patch \
> > > >            
> > > >  file://0002-BaseTools-makefile-adjust-to-build-in-under-bitbak
> > > > e.patch \
> > > >              file://0003-ovmf-Update-to-latest.patch \
> > > >              file://0005-debug-prefix-map.patch \
> > > >              file://0006-reproducible.patch \
> > > > +           file://0001-BaseTools-fix-gcc12-warning.patch \
> > > > +           file://0001-BaseTools-fix-gcc12-warning-1.patch \
> > > >              "
> > > >   
> > > >   PV = "edk2-stable202202"
> > > 
> > > I think this may cause the efi failures in:
> > > 
> > > https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/3583/steps/14/logs/stdio
> > > 
> > > (and all the other similar oe-selftest runs)
> > 
> > I'm going to retract that, further testing shows it is something
> > else,
> > still not sure what.
> > 
> 
> 
> Right I was a little confused as well and was trying to reproduce
> locally here with and without this patch and the results were same 

It definitely wasn't this patch, sorry. It can sometimes be really hard
to tell which failure is from which patch, it has taken me a few tries
to isolate this one!

Cheers,

Richard