Skip to content

Commit

Permalink
MdePkg: Fix null macros for XCODE5 and CLANG
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Mike Beaton <[email protected]>
  • Loading branch information
2 people authored and mergify[bot] committed Oct 18, 2024
1 parent 6e197a8 commit ae83c6b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ DescribeExceptionSyndrome (
DEBUG ((DEBUG_ERROR, "\n %a \n", Message));
}

#ifndef MDEPKG_NDEBUG
STATIC
CONST CHAR8 *
BaseName (
Expand All @@ -177,8 +176,6 @@ BaseName (
return Str;
}

#endif

/**
This is the default action to take on an unexpected exception
Expand Down
35 changes: 31 additions & 4 deletions MdePkg/Include/Library/DebugLib.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
Expand Down Expand Up @@ -403,7 +410,12 @@ UnitTestDebugAssert (
} \
} while (FALSE)
#else
#define ASSERT(Expression)
#define ASSERT(Expression) \
do { \
if (FALSE) { \
(VOID) (Expression); \
} \
} while (FALSE)
#endif

/**
Expand All @@ -426,7 +438,12 @@ UnitTestDebugAssert (
} \
} while (FALSE)
#else
#define DEBUG(Expression)
#define DEBUG(Expression) \
do { \
if (FALSE) { \
_DEBUGLIB_DEBUG (Expression); \
} \
} while (FALSE)
#endif

/**
Expand All @@ -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

/**
Expand All @@ -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

/**
Expand Down

0 comments on commit ae83c6b

Please sign in to comment.