[edk2-devel] [PATCH V4 1/2] DebugLib: Update DEBUG macro used when MDEPKG_NDEBUG is defined

[edk2-devel] [PATCH V4 1/2] DebugLib: Update DEBUG macro used when MDEPKG_NDEBUG is defined

[Mike Beaton]

From: Mike Beaton <mjsbeaton@gmail.com>

The variant provided when MDEPKG_NDEBUG is defined will be optimised
away in RELEASE builds, but by referencing the argument list, avoids
unused variable errors from valid debug code, for example when STATIC
variables are used only in DEBUG statements.

Variables EventNames in OvmfPkg/VirtioSerialDxe/VirtioSerial.c
was causing this issue in CLANGPDB X64 RELEASE build, prior to this
commit. It is also necessary to remove manual work-arounds which had
been applied to some similar cases in ArmPkg (see following commit).

Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
---
 BaseTools/Conf/tools_def.template | 2 +-
 MdePkg/Include/Library/DebugLib.h | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index c34ecfd557..eaccf0b698 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -1859,7 +1859,7 @@ DEFINE CLANGDWARF_X64_DLINK2_FLAGS        = -Wl,--defsym=PECOFF_HEADER_SIZE=0x22
 DEFINE CLANGDWARF_IA32_TARGET             = -target i686-pc-linux-gnu
 DEFINE CLANGDWARF_X64_TARGET              = -target x86_64-pc-linux-gnu
 
-DEFINE CLANGDWARF_WARNING_OVERRIDES    = -Wno-parentheses-equality -Wno-empty-body -Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-unaligned-access -Wno-unneeded-internal-declaration
+DEFINE CLANGDWARF_WARNING_OVERRIDES    = -Wno-parentheses-equality -Wno-empty-body -Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-unaligned-access
 DEFINE CLANGDWARF_ALL_CC_FLAGS         = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANGDWARF_WARNING_OVERRIDES) -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -msoft-float -mno-implicit-float  -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference
 
 ###########################
diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
index 40772f2e0f..bc7789f01c 100644
--- a/MdePkg/Include/Library/DebugLib.h
+++ b/MdePkg/Include/Library/DebugLib.h
@@ -426,7 +426,12 @@ UnitTestDebugAssert (
       }                            \
     } while (FALSE)
 #else
-#define DEBUG(Expression)
+#define DEBUG(Expression)        \
+    do {                           \
+      if (FALSE) {                 \
+        _DEBUG (Expression);       \
+      }                            \
+    } while (FALSE)
 #endif
 
 /**
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112500): https://edk2.groups.io/g/devel/message/112500
Mute This Topic: https://groups.io/mt/103166252/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH V4 2/2] ArmPkg: Remove manual exclusion of debug vars when MDEPKG_NDEBUG is not defined

[Mike Beaton]

From: Mike Beaton <mjsbeaton@gmail.com>

This is no longer required since the revised DEBUG macro automatically
compiles away unused var accesses when MDEPKG_NDEBUG is defined;
keeping these lines is incompatible with the updated DEBUG macro, as
there has to be a variable, access to which to discard.

Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
---
 .../DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c     | 4 ----
 .../AArch64/DefaultExceptionHandler.c                         | 3 ---
 2 files changed, 7 deletions(-)

diff --git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
index 432112354f..dc49e8eba7 100644
--- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
+++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
@@ -71,9 +71,7 @@ PeCoffLoaderRelocateImageExtraAction (
   IN OUT PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
   )
 {
- #if !defined (MDEPKG_NDEBUG)
   CHAR8  Temp[512];
- #endif
 
   if (ImageContext->PdbPointer) {
  #ifdef __CC_ARM
@@ -106,9 +104,7 @@ PeCoffLoaderUnloadImageExtraAction (
   IN OUT PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
   )
 {
- #if !defined (MDEPKG_NDEBUG)
   CHAR8  Temp[512];
- #endif
 
   if (ImageContext->PdbPointer) {
  #ifdef __CC_ARM
diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
index a39896d576..1d3ea61311 100644
--- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
+++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
@@ -157,7 +157,6 @@ DescribeExceptionSyndrome (
   DEBUG ((DEBUG_ERROR, "\n %a \n", Message));
 }
 
-#ifndef MDEPKG_NDEBUG
 STATIC
 CONST CHAR8 *
 BaseName (
@@ -177,8 +176,6 @@ BaseName (
   return Str;
 }
 
-#endif
-
 /**
   This is the default action to take on an unexpected exception
 
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112501): https://edk2.groups.io/g/devel/message/112501
Mute This Topic: https://groups.io/mt/103166254/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V4 2/2] ArmPkg: Remove manual exclusion of debug vars when MDEPKG_NDEBUG is not defined

[Mike Beaton]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

Hi, I'm not fully used to email-based git commits. In this case, I've got two commits with different messages which are meant to make one patch set. I was under the impression that the first line of each commit is taken from the subject, but in that case these cease to appear as one thread in edk2-devel. Not yet sure how to handle? Apologies!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112503): https://edk2.groups.io/g/devel/message/112503
Mute This Topic: https://groups.io/mt/103166254/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 1124 bytes --]

Re: [edk2-devel] [PATCH V4 2/2] ArmPkg: Remove manual exclusion of debug vars when MDEPKG_NDEBUG is not defined

[Ard Biesheuvel]

On Thu, 14 Dec 2023 at 08:28, Mike Beaton <mjsbeaton@gmail.com> wrote:
>
> From: Mike Beaton <mjsbeaton@gmail.com>
>
> This is no longer required since the revised DEBUG macro automatically
> compiles away unused var accesses when MDEPKG_NDEBUG is defined;
> keeping these lines is incompatible with the updated DEBUG macro, as
> there has to be a variable, access to which to discard.
>
> Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>

I take it this will trigger a build warning if it is not merged at the
same time as the other change? How about GCC?

> ---
>  .../DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c     | 4 ----
>  .../AArch64/DefaultExceptionHandler.c                         | 3 ---
>  2 files changed, 7 deletions(-)
>
> diff --git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> index 432112354f..dc49e8eba7 100644
> --- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> +++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> @@ -71,9 +71,7 @@ PeCoffLoaderRelocateImageExtraAction (
>    IN OUT PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
>    )
>  {
> - #if !defined (MDEPKG_NDEBUG)
>    CHAR8  Temp[512];
> - #endif
>
>    if (ImageContext->PdbPointer) {
>   #ifdef __CC_ARM
> @@ -106,9 +104,7 @@ PeCoffLoaderUnloadImageExtraAction (
>    IN OUT PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
>    )
>  {
> - #if !defined (MDEPKG_NDEBUG)
>    CHAR8  Temp[512];
> - #endif
>
>    if (ImageContext->PdbPointer) {
>   #ifdef __CC_ARM
> diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
> index a39896d576..1d3ea61311 100644
> --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
> +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
> @@ -157,7 +157,6 @@ DescribeExceptionSyndrome (
>    DEBUG ((DEBUG_ERROR, "\n %a \n", Message));
>  }
>
> -#ifndef MDEPKG_NDEBUG
>  STATIC
>  CONST CHAR8 *
>  BaseName (
> @@ -177,8 +176,6 @@ BaseName (
>    return Str;
>  }
>
> -#endif
> -
>  /**
>    This is the default action to take on an unexpected exception
>
> --
> 2.39.2
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112504): https://edk2.groups.io/g/devel/message/112504
Mute This Topic: https://groups.io/mt/103166254/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V4 2/2] ArmPkg: Remove manual exclusion of debug vars when MDEPKG_NDEBUG is not defined

[Ard Biesheuvel]

On Thu, 14 Dec 2023 at 08:40, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 14 Dec 2023 at 08:28, Mike Beaton <mjsbeaton@gmail.com> wrote:
> >
> > From: Mike Beaton <mjsbeaton@gmail.com>
> >
> > This is no longer required since the revised DEBUG macro automatically
> > compiles away unused var accesses when MDEPKG_NDEBUG is defined;
> > keeping these lines is incompatible with the updated DEBUG macro, as
> > there has to be a variable, access to which to discard.
> >
> > Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
>
> I take it this will trigger a build warning if it is not merged at the
> same time as the other change? How about GCC?
>

Looking at this more closely, we should probably just drop
DeCygwinPathIfNeeded() - I don't think it is still needed.

For the exception handler case, we can just drop the #Ifdefs around
the definition of BaseName () entirely given that it will now always
be referenced. But that does depend a lot on how other toolchains deal
with this (VS201x primarily)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112509): https://edk2.groups.io/g/devel/message/112509
Mute This Topic: https://groups.io/mt/103166254/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V4 2/2] ArmPkg: Remove manual exclusion of debug vars when MDEPKG_NDEBUG is not defined

[Mike Beaton]

> For the exception handler case, we can just drop the #Ifdefs around
> the definition of BaseName () entirely given that it will now always
> be referenced. But that does depend a lot on how other toolchains deal
> with this (VS201x primarily)

The V5/V6 version of the patch builds on VS2019


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112602): https://edk2.groups.io/g/devel/message/112602
Mute This Topic: https://groups.io/mt/103166254/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V4 2/2] ArmPkg: Remove manual exclusion of debug vars when MDEPKG_NDEBUG is not defined

[Ard Biesheuvel]

On Fri, Dec 15, 2023 at 12:56 PM Mike Beaton <mjsbeaton@gmail.com> wrote:
>
> > For the exception handler case, we can just drop the #Ifdefs around
> > the definition of BaseName () entirely given that it will now always
> > be referenced. But that does depend a lot on how other toolchains deal
> > with this (VS201x primarily)
>
> The V5/V6 version of the patch builds on VS2019

Excellent, thanks for checking.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112714): https://edk2.groups.io/g/devel/message/112714
Mute This Topic: https://groups.io/mt/103166254/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-