On 2019-04-12 16:31:26, Laszlo Ersek wrote: > RH covscan warns about assignments that it can determine are never > "consumed" later ("dead stores"). The idea behind the warning is > presumably that the programmer forgot to implement a dependent check > somewhere. > > For each such warning that has been emitted for OvmfPkg, > the case is one of the following however: > > - We assign a variable a value for (re-)initialization's sake, in > preparation for further or future uses. This practice is safe (sometimes > even recommended!), hence we should suppress these warnings. > > - We capture a result or a computation in a variable, following a general > programming pattern, but then decide to ignore the value in that > particular case. This is again safe, and we should suppress these > warnings too. > > According to the Clang documentation at > > https://clang-analyzer.llvm.org/faq.html#dead_store > > we should use > > (void)x; > > See the logs below (produced originally for edk2-stable201903). > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:156:3: warning: Value > > stored to 'NumberOfTableEntries' is never read > > # NumberOfTableEntries = 0; > > # ^ ~ > > edk2-89910a39dcfd/OvmfPkg/AcpiPlatformDxe/Xen.c:156:3: note: Value > > stored to 'NumberOfTableEntries' is never read > > # NumberOfTableEntries = 0; > > # ^ ~ > > # 154| DsdtTable = NULL; > > # 155| TableHandle = 0; > > # 156|-> NumberOfTableEntries = 0; > > # 157| > > # 158| // > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c:43:3: > > warning: Value stored to 'SetupSize' is never read > > # SetupSize = 0; > > # ^ ~ > > edk2-89910a39dcfd/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c:43:3: > > note: Value stored to 'SetupSize' is never read > > # SetupSize = 0; > > # ^ ~ > > # 41| > > # 42| SetupBuf = NULL; > > # 43|-> SetupSize = 0; > > # 44| KernelBuf = NULL; > > # 45| KernelInitialSize = 0; > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c:609:9: > > warning: Value stored to 'VariableDataBufferSize' is never read > > # VariableDataBufferSize = 0; > > # ^ ~ > > edk2-89910a39dcfd/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c:609:9: > > note: Value stored to 'VariableDataBufferSize' is never read > > # VariableDataBufferSize = 0; > > # ^ ~ > > # 607| FreePool (VariableData); > > # 608| VariableData = NULL; > > # 609|-> VariableDataBufferSize = 0; > > # 610| } > > # 611| VariableData = AllocatePool (VariableDataSize); > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/Library/VirtioLib/VirtioLib.c:125:3: warning: > > Value stored to 'RingPagesPtr' is never read > > # RingPagesPtr += sizeof *Ring->Used.AvailEvent; > > # ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > edk2-89910a39dcfd/OvmfPkg/Library/VirtioLib/VirtioLib.c:125:3: note: > > Value stored to 'RingPagesPtr' is never read > > # RingPagesPtr += sizeof *Ring->Used.AvailEvent; > > # ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > # 123| > > # 124| Ring->Used.AvailEvent = (volatile VOID *) RingPagesPtr; > > # 125|-> RingPagesPtr += sizeof *Ring->Used.AvailEvent; > > # 126| > > # 127| Ring->QueueSize = QueueSize; > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c:1079:3: > > warning: Value stored to 'FwhInstance' is never read > > # FwhInstance = (EFI_FW_VOL_INSTANCE *) > > # ^ ~~~~~~~~~~~~~~~~~~~~~~~ > > edk2-89910a39dcfd/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c:1079:3: > > note: Value stored to 'FwhInstance' is never read > > # FwhInstance = (EFI_FW_VOL_INSTANCE *) > > # ^ ~~~~~~~~~~~~~~~~~~~~~~~ > > # 1077| ASSERT_RETURN_ERROR (PcdStatus); > > # 1078| > > # 1079|-> FwhInstance = (EFI_FW_VOL_INSTANCE *) > > # 1080| ( > > # 1081| (UINTN) ((UINT8 *) FwhInstance) + FwVolHeader->HeaderLength > > + > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c:173:3: > > warning: Value stored to 'Status' is never read > > # Status = gDS->RemoveMemorySpace ( > > # ^ ~~~~~~~~~~~~~~~~~~~~~~~~ > > edk2-89910a39dcfd/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c:173:3: > > note: Value stored to 'Status' is never read > > # Status = gDS->RemoveMemorySpace ( > > # ^ ~~~~~~~~~~~~~~~~~~~~~~~~ > > # 171| // Mark flash region as runtime memory > > # 172| // > > # 173|-> Status = gDS->RemoveMemorySpace ( > > # 174| BaseAddress, > > # 175| Length > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/SataControllerDxe/SataController.c:231:3: > > warning: Value stored to 'DeviceUDmaMode' is never read > > # DeviceUDmaMode = 0; > > # ^ ~ > > edk2-89910a39dcfd/OvmfPkg/SataControllerDxe/SataController.c:231:3: > > note: Value stored to 'DeviceUDmaMode' is never read > > # DeviceUDmaMode = 0; > > # ^ ~ > > # 229| UINT16 DeviceUDmaMode; > > # 230| > > # 231|-> DeviceUDmaMode = 0; > > # 232| > > # 233| // > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/VirtioGpuDxe/Gop.c:65:5: warning: Value stored > > to 'Status' is never read > > # Status = VirtioGpuSetScanout ( > > # ^ ~~~~~~~~~~~~~~~~~~~~~ > > edk2-89910a39dcfd/OvmfPkg/VirtioGpuDxe/Gop.c:65:5: note: Value stored to > > 'Status' is never read > > # Status = VirtioGpuSetScanout ( > > # ^ ~~~~~~~~~~~~~~~~~~~~~ > > # 63| // by setting ResourceId=0 for it. > > # 64| // > > # 65|-> Status = VirtioGpuSetScanout ( > > # 66| VgpuGop->ParentBus, // VgpuDev > > # 67| 0, 0, 0, 0, // X, Y, Width, Height > > > > Error: CLANG_WARNING: > > edk2-89910a39dcfd/OvmfPkg/VirtioNetDxe/SnpReceive.c:165:3: warning: > > Value stored to 'RxPtr' is never read > > # RxPtr += sizeof (UINT16); > > # ^ ~~~~~~~~~~~~~~~ > > edk2-89910a39dcfd/OvmfPkg/VirtioNetDxe/SnpReceive.c:165:3: note: Value > > stored to 'RxPtr' is never read > > # RxPtr += sizeof (UINT16); > > # ^ ~~~~~~~~~~~~~~~ > > # 163| *Protocol = (UINT16) ((RxPtr[0] << 8) | RxPtr[1]); > > # 164| } > > # 165|-> RxPtr += sizeof (UINT16); > > # 166| > > # 167| Status = EFI_SUCCESS; > > Cc: Anthony Perard <anthony.per...@citrix.com> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Julien Grall <julien.gr...@arm.com> > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 > Issue: scan-0992.txt > Issue: scan-0996.txt > Issue: scan-0997.txt > Issue: scan-0998.txt > Issue: scan-1000.txt > Issue: scan-1001.txt > Issue: scan-1006.txt > Issue: scan-1011.txt > Issue: scan-1012.txt > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > OvmfPkg/AcpiPlatformDxe/Xen.c | 5 +++++ > OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c | 5 +++++ > OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c | 4 ++++ > OvmfPkg/Library/VirtioLib/VirtioLib.c | 5 +++++ > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c | 5 +++++ > OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c | 5 +++++ > OvmfPkg/SataControllerDxe/SataController.c | 5 +++++ > OvmfPkg/VirtioGpuDxe/Gop.c | 6 ++++++ > OvmfPkg/VirtioNetDxe/SnpReceive.c | 5 +++++ > 9 files changed, 45 insertions(+) > > diff --git a/OvmfPkg/AcpiPlatformDxe/Xen.c b/OvmfPkg/AcpiPlatformDxe/Xen.c > index 357c60d23f4e..c8f275a8ee84 100644 > --- a/OvmfPkg/AcpiPlatformDxe/Xen.c > +++ b/OvmfPkg/AcpiPlatformDxe/Xen.c > @@ -144,16 +144,21 @@ InstallXenTables ( > Fadt2Table = NULL; > Fadt1Table = NULL; > Facs2Table = NULL; > Facs1Table = NULL; > DsdtTable = NULL; > TableHandle = 0; > NumberOfTableEntries = 0; > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)NumberOfTableEntries; > +
I've seen this solution to shut up the compiler for similar reasons, and it kind of bugs me. It looks like both paths of the if & else initialize NumberOfTableEntries, so maybe we can just drop setting it to 0? I looked at FwhInstance too, and it doesn't look like it is used after being set. So, maybe that should just be dropped? I guess both of your bullet points give arguments allowing the unused set. Rather than adding "Suppress..." comments everywhere, maybe a global macro could be defined for similar uses in EDK II? // // Suppress "Value stored to ... is never read" analyzer warnings. // #define IGNORE_UNUSED_ASSIGNMENT(var) ((VOID)var) Then again, I guess we decided to suppress this in EDK II compiler warnings with -Wno-unused-but-set-variable. So, can you adjust the RH covscan tool settings to similarly ignore these warnings? -Jordan > // > // Try to find Xen ACPI tables > // > Status = GetXenAcpiRsdp (&XenAcpiRsdpStructurePtr); > if (EFI_ERROR (Status)) { > return Status; > } > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c > b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c > index ddfef925edd3..fde8c40218a4 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c > +++ b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c > @@ -37,16 +37,21 @@ TryRunningQemuKernel ( > SetupSize = 0; > KernelBuf = NULL; > KernelInitialSize = 0; > CommandLine = NULL; > CommandLineSize = 0; > InitrdData = NULL; > InitrdSize = 0; > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)SetupSize; > + > if (!QemuFwCfgIsAvailable ()) { > return EFI_NOT_FOUND; > } > > QemuFwCfgSelectItem (QemuFwCfgItemKernelSize); > KernelSize = (UINTN) QemuFwCfgRead64 (); > > QemuFwCfgSelectItem (QemuFwCfgItemKernelSetupSize); > diff --git a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > index a6cebcb54f6b..64fe9d9be543 100644 > --- a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > +++ b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > @@ -596,16 +596,20 @@ SerializeVariablesIterateSystemVariables ( > // > // The currently allocated VariableData buffer is too small, > // so we allocate a larger buffer. > // > if (VariableDataBufferSize != 0) { > FreePool (VariableData); > VariableData = NULL; > VariableDataBufferSize = 0; > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)VariableDataBufferSize; > } > VariableData = AllocatePool (VariableDataSize); > if (VariableData == NULL) { > Status = EFI_OUT_OF_RESOURCES; > break; > } > VariableDataBufferSize = VariableDataSize; > > diff --git a/OvmfPkg/Library/VirtioLib/VirtioLib.c > b/OvmfPkg/Library/VirtioLib/VirtioLib.c > index 555d2a5ce7c9..0e4b8b6b265e 100644 > --- a/OvmfPkg/Library/VirtioLib/VirtioLib.c > +++ b/OvmfPkg/Library/VirtioLib/VirtioLib.c > @@ -113,16 +113,21 @@ VirtioRingInit ( > RingPagesPtr += sizeof *Ring->Used.Idx; > > Ring->Used.UsedElem = (volatile VOID *) RingPagesPtr; > RingPagesPtr += sizeof *Ring->Used.UsedElem * QueueSize; > > Ring->Used.AvailEvent = (volatile VOID *) RingPagesPtr; > RingPagesPtr += sizeof *Ring->Used.AvailEvent; > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)RingPagesPtr; > + > Ring->QueueSize = QueueSize; > return EFI_SUCCESS; > } > > > /** > > Tear down the internal resources of a configured virtio ring. > diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > index edf438a422fa..ff6565865846 100644 > --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > @@ -1071,16 +1071,21 @@ FvbInitialize ( > ASSERT_RETURN_ERROR (PcdStatus); > > FwhInstance = (EFI_FW_VOL_INSTANCE *) > ( > (UINTN) ((UINT8 *) FwhInstance) + FwVolHeader->HeaderLength + > (sizeof (EFI_FW_VOL_INSTANCE) - sizeof (EFI_FIRMWARE_VOLUME_HEADER)) > ); > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)FwhInstance; > + > // > // Module type specific hook. > // > InstallVirtualAddressChangeHandler (); > > PcdStatus = PcdSetBoolS (PcdOvmfFlashVariablesEnable, TRUE); > ASSERT_RETURN_ERROR (PcdStatus); > return EFI_SUCCESS; > diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c > b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c > index 69b20916bc7c..979122f2e2fd 100644 > --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c > +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c > @@ -164,16 +164,21 @@ MarkIoMemoryRangeForRuntimeAccess ( > // > // Mark flash region as runtime memory > // > Status = gDS->RemoveMemorySpace ( > BaseAddress, > Length > ); > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)Status; > + > Status = gDS->AddMemorySpace ( > EfiGcdMemoryTypeMemoryMappedIo, > BaseAddress, > Length, > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > ); > ASSERT_EFI_ERROR (Status); > > diff --git a/OvmfPkg/SataControllerDxe/SataController.c > b/OvmfPkg/SataControllerDxe/SataController.c > index 8d6a6bbb2286..0cf6b42c886a 100644 > --- a/OvmfPkg/SataControllerDxe/SataController.c > +++ b/OvmfPkg/SataControllerDxe/SataController.c > @@ -219,16 +219,21 @@ CalculateBestUdmaMode ( > OUT UINT16 *SelectedMode > ) > { > UINT16 TempMode; > UINT16 DeviceUDmaMode; > > DeviceUDmaMode = 0; > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)DeviceUDmaMode; > + > // > // Check whether the WORD 88 (supported UltraDMA by drive) is valid > // > if ((IdentifyData->AtaData.field_validity & 0x04) == 0x00) { > return EFI_UNSUPPORTED; > } > > DeviceUDmaMode = IdentifyData->AtaData.ultra_dma_mode; > diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c > index 0b2659d1d2b9..d0f81c349f73 100644 > --- a/OvmfPkg/VirtioGpuDxe/Gop.c > +++ b/OvmfPkg/VirtioGpuDxe/Gop.c > @@ -57,16 +57,22 @@ ReleaseGopResources ( > // by setting ResourceId=0 for it. > // > Status = VirtioGpuSetScanout ( > VgpuGop->ParentBus, // VgpuDev > 0, 0, 0, 0, // X, Y, Width, Height > 0, // ScanoutId > 0 // ResourceId > ); > + > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)Status; > + > // > // HACK BEGINS HERE > // > // According to the GPU Device section of the VirtIo specification, the > // above operation is valid: > // > // "The driver can use resource_id = 0 to disable a scanout." > // > diff --git a/OvmfPkg/VirtioNetDxe/SnpReceive.c > b/OvmfPkg/VirtioNetDxe/SnpReceive.c > index cdee9a2aee47..51b2f8a4e370 100644 > --- a/OvmfPkg/VirtioNetDxe/SnpReceive.c > +++ b/OvmfPkg/VirtioNetDxe/SnpReceive.c > @@ -153,16 +153,21 @@ VirtioNetReceive ( > } > RxPtr += SIZE_OF_VNET (Mac); > > if (Protocol != NULL) { > *Protocol = (UINT16) ((RxPtr[0] << 8) | RxPtr[1]); > } > RxPtr += sizeof (UINT16); > > + // > + // Suppress "Value stored to ... is never read" analyzer warnings. > + // > + (VOID)RxPtr; > + > Status = EFI_SUCCESS; > > RecycleDesc: > ++Dev->RxLastUsed; > > // > // virtio-0.9.5, 2.4.1 Supplying Buffers to The Device > // > -- > 2.19.1.3.g30247aa5d201 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39003): https://edk2.groups.io/g/devel/message/39003 Mute This Topic: https://groups.io/mt/31070308/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-