On 4/13/19 1:31 AM, Laszlo Ersek wrote: > RH covscan emits the following false positive: > >> Error: CLANG_WARNING: >> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:182:14: >> warning: Dereference of undefined pointer value >> # Status = FwVol->ReadSection ( >> # ^~~~~~~~~~~~~~~~~~ >> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:164:7: note: >> Assuming the condition is false >> # if (QemuDetected ()) { >> # ^~~~~~~~~~~~~~~ >> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:164:3: note: >> Taking false branch >> # if (QemuDetected ()) { >> # ^ >> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:174:3: note: >> Taking false branch >> # if (EFI_ERROR (Status)) { >> # ^ >> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:180:10: note: >> Assuming 'Status' is equal to EFI_SUCCESS >> # while (Status == EFI_SUCCESS) { >> # ^~~~~~~~~~~~~~~~~~~~~ >> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:180:3: note: >> Loop condition is true. Entering loop body >> # while (Status == EFI_SUCCESS) { >> # ^ >> edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c:182:14: note: >> Dereference of undefined pointer value >> # Status = FwVol->ReadSection ( >> # ^~~~~~~~~~~~~~~~~~ >> # 180| while (Status == EFI_SUCCESS) { >> # 181| >> # 182|-> Status = FwVol->ReadSection ( >> # 183| FwVol, >> # 184| (EFI_GUID*)PcdGetPtr >> (PcdAcpiTableStorageFile), > > This is invalid because LocateFvInstanceWithTables() sets FwVol on > success. > > Suppress the message by: > - assigning FwVol NULL first (this would replace the original report with > "nullptr deref"), > - asserting that FwVol is no longer NULL, on success. > > What's important here is that ASSERT() ends with ANALYZER_UNREACHABLE() on > failure. > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 > Issue: scan-0991.txt > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > index f2c49953950b..2b529d58a15c 100644 > --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > @@ -156,23 +156,30 @@ InstallOvmfFvTables ( > TableHandle = 0; > > if (QemuDetected ()) { > TableInstallFunction = QemuInstallAcpiTable; > } else { > TableInstallFunction = InstallAcpiTable; > } > > + // > + // set FwVol (and use an ASSERT() below) to suppress incorrect > + // compiler/analyzer warnings > + // > + FwVol = NULL; > // > // Locate the firmware volume protocol > // > Status = LocateFvInstanceWithTables (&FwVol); > if (EFI_ERROR (Status)) { > return EFI_ABORTED; > } > + ASSERT (FwVol != NULL); > + > // > // Read tables from the storage file. > // > while (Status == EFI_SUCCESS) { > > Status = FwVol->ReadSection ( > FwVol, > (EFI_GUID*)PcdGetPtr (PcdAcpiTableStorageFile), >
Reviewed-by: Philippe Mathieu-Daude <phi...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39107): https://edk2.groups.io/g/devel/message/39107 Mute This Topic: https://groups.io/mt/31070307/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-