Re: [edk2-devel] [PATCH 07/10] OvmfPkg/AcpiPlatformDxe: suppress invalid "deref of undef pointer" warning

2019-04-15 Thread Philippe Mathieu-Daudé 
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 
> Cc: Jordan Justen 
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
> Issue: scan-0991.txt
> Signed-off-by: Laszlo Ersek 
> ---
>  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 ();
>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 

-=-=-=-=-=-=-=-=-=-=-=-
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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 07/10] OvmfPkg/AcpiPlatformDxe: suppress invalid "deref of undef pointer" warning

2019-04-12 Thread Laszlo Ersek 
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 
Cc: Jordan Justen 
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710
Issue: scan-0991.txt
Signed-off-by: Laszlo Ersek 
---
 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 ();
   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),
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#38992): https://edk2.groups.io/g/devel/message/38992
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]
-=-=-=-=-=-=-=-=-=-=-=-