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 <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Jordan Justen <[email protected]>
> Cc: Julien Grall <[email protected]>
> 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 <[email protected]>
> ---
> 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: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
