On Tue, 12 Dec 2023 at 22:46, Mike Beaton <mjsbea...@gmail.com> wrote:
>
> From: Mikhail Krichanov <mikhailkricha...@gmail.com>
>
> Provides a variant of the DEBUG macro for clang when MDEPKG_NDEBUG is
> defined, which uses but discards the contained expression. This means
> clang can tell that it has optimised away variable usage as part of
> valid debug patterns (such as a STATIC variable which is only used in
> DEBUG statements) - rather than just seeing such variables as
> completely unused - therefore we can keep
> -Wunneeded-internal-declaration (as part of -Wall) to warn about
> mistakenly genuinely unused variables elsewhere.
>
> Note that the _Pragma in the clang/gcc variant is to temporarily
> suppress the warning about `(VOID) Expression;`, whilst the presence
> of `(VOID) Expression;` (once allowed) is what prevents the unwanted
> warning about unneeded variables.
>
> Signed-off-by: Mikhail Krichanov <mikhailkricha...@gmail.com>
> Signed-off-by: Mike Beaton <mjsbea...@gmail.com>
This patch breaks the build on ARM (undeclared functions and
variables). Nothing we couldn't fix, but that needs to be done before
we can consider this change.
> ---
> BaseTools/Conf/tools_def.template | 2 +-
> MdePkg/Include/Library/DebugLib.h | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> 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..7368004f46 100644
> --- a/MdePkg/Include/Library/DebugLib.h
> +++ b/MdePkg/Include/Library/DebugLib.h
> @@ -425,6 +425,16 @@ UnitTestDebugAssert (
> _DEBUG (Expression); \
> } \
> } while (FALSE)
> +#elif defined (__GNUC__) || defined (__clang__)
This is now being enabled on GCC too - why?
> +#define DEBUG(Expression) \
> + do { \
> + _Pragma("GCC diagnostic push") \
> + _Pragma("GCC diagnostic ignored \"-Wunused-value\"") \
> + if ((FALSE)) { \
> + (VOID) Expression; \
So what is the point of the VOID cast here? Usually, these are added
to avoid unused value warnings, but these are disabled explicitly via
the pragma.
Why can't we just use
--- 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
/**
here?
> + } \
> + _Pragma("GCC diagnostic pop") \
> + } while (FALSE)
> #else
> #define DEBUG(Expression)
> #endif
> --
> 2.39.2
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112459): https://edk2.groups.io/g/devel/message/112459
Mute This Topic: https://groups.io/mt/103138778/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
