From ae83c6b7fd83a5906e016a32027c1bcd792a624e Mon Sep 17 00:00:00 2001 From: Mike Beaton Date: Sat, 28 Sep 2024 02:43:15 +0100 Subject: MdePkg: Fix null macros for XCODE5 and CLANG When building OvmfPkg in RELEASE mode in the XCODE5 toolchain, the ASSERT_EFI_ERROR change prevents this error: .../MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c:141:15: error: variable 'Status' set but not used [-Werror,-Wunused-but-set-variable] EFI_STATUS Status; ^ which is currently stopping the build. When building in RELEASE mode in the CLANGPDB toolchain,the DEBUG macro change prevents this error: .../edk2/OvmfPkg/VirtioSerialDxe/VirtioSerial.c:28:22: error: variable 'EventNames' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] STATIC CONST CHAR8 *EventNames[] = { ^ which is currently stopping the build. CLANGDWARF produces the same error as CLANGPDB above, if -Wno-unneeded-internal-declaration is removed from its build flags. With the null DEBUG macro change, this warning suppression can be removed from CLANGDWARF, which is considered a benefit as it has the potential to catch real coding errors. This is done in a subsequent commit. This commit has the desirable side effect that we no longer require (and cannot use) explicit `#ifndef MDEPKG_NDEBUG` around items only used in DEBUG macros. This requires the ArmPkg change made here to be in the same commit as the MdePkg changes. Note: In common with existing macros in EDK II, including the pre-existing and unchanged DEBUG/NOOPT versions of the macros which are modified here, we use the standard approach of adding `do { ... } while (FALSE)` wrapping to ensure that the macros behave correctly with surrounding code (e.g. require a following ';' and do not combine in unexpected ways with nearby conditionals). Continuous-integration-options: PatchCheck.ignore-multi-package Co-authored-by: Mikhail Krichanov Signed-off-by: Mike Beaton --- MdePkg/Include/Library/DebugLib.h | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) (limited to 'MdePkg') diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h index 033c4e1a66..b48e08a7e4 100644 --- a/MdePkg/Include/Library/DebugLib.h +++ b/MdePkg/Include/Library/DebugLib.h @@ -8,6 +8,13 @@ of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is defined, then debug and assert related macros wrapped by it are the NULL implementations. + The implementations of the macros used when MDEPKG_NDEBUG is defined rely on the fact that + directly unreachable code is pruned, even with compiler optimization disabled (which has + been confirmed by generated code size tests on supported compilers). The advantage of + implementations which consume their arguments within directly unreachable code is that + compilers understand this, and stop warning about variables which would become unused when + MDEPKG_NDEBUG is defined if the macros had completely empty definitions. + Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent @@ -403,7 +410,12 @@ UnitTestDebugAssert ( } \ } while (FALSE) #else -#define ASSERT(Expression) +#define ASSERT(Expression) \ + do { \ + if (FALSE) { \ + (VOID) (Expression); \ + } \ + } while (FALSE) #endif /** @@ -426,7 +438,12 @@ UnitTestDebugAssert ( } \ } while (FALSE) #else -#define DEBUG(Expression) +#define DEBUG(Expression) \ + do { \ + if (FALSE) { \ + _DEBUGLIB_DEBUG (Expression); \ + } \ + } while (FALSE) #endif /** @@ -452,7 +469,12 @@ UnitTestDebugAssert ( } \ } while (FALSE) #else -#define ASSERT_EFI_ERROR(StatusParameter) +#define ASSERT_EFI_ERROR(StatusParameter) \ + do { \ + if (FALSE) { \ + (VOID) (StatusParameter); \ + } \ + } while (FALSE) #endif /** @@ -479,7 +501,12 @@ UnitTestDebugAssert ( } \ } while (FALSE) #else -#define ASSERT_RETURN_ERROR(StatusParameter) +#define ASSERT_RETURN_ERROR(StatusParameter) \ + do { \ + if (FALSE) { \ + (VOID) (StatusParameter); \ + } \ + } while (FALSE) #endif /** -- cgit v1.2.3