On 03/19/16 00:27, Andrew Fish wrote: > >> On Mar 18, 2016, at 3:46 PM, Michael Brown <[email protected]> wrote: >> >> On 18/03/16 22:30, Laszlo Ersek wrote: >>> Unfortunately, I've run into a big issue: the DEBUG(()) macro. (To a >>> smaller extent, the ASSERT_EFI_ERROR() macro as well.) These macros >>> expand to the null replacement string when MDEPKG_NDEBUG is defined >>> (which is occasionally the case for RELEASE builds). If those macro >>> invocations constitute the only read accesses to some local variables, >>> then the MDEPKG_NDEBUG build triggers "-Wunused-but-set-variable". >>> >>> For ASSERT_EFI_ERROR(), I could work it around like this: >>> >>> ((VOID)(TRUE ? 0 : (StatusParameter))) >>> >>> but I have no clue how to do the same with DEBUG(), which can receive >>> any number of arguments (of any type too). >> >> With the caveat that I haven't looked at how DEBUG() is implemented in EDK2: >> >> Could this rely upon dead code elimination? At least gcc will happily >> accept something which expands to >> >> if ( 0 ) { >> do_something_with ( a_variable ); >> } >> >> and treat that as being a "use" of a_variable (for the purposes of >> -Wunused-but-set-variable). >> > > All the DEBUG macros work this way all ready. MDEPKG_NDEBUG was a > hack-a-round for compilers that don't do link time optimization.
Like, all versions of gcc that are currently supported? :) > So you don't need to redefine anything, just don't use MDEPKG_NDEBUG. That sounds like good advice, but reality would have to catch up: $ git grep MDEPKG_NDEBUG -- '*.dsc' ArmPkg/ArmPkg.dsc: RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc: GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc: GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc: INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc: MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc: GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc: GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc: INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc: MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32.dsc: GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32.dsc: GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32.dsc: INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32.dsc: MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32X64.dsc: GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32X64.dsc: GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32X64.dsc: INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG OvmfPkg/OvmfPkgIa32X64.dsc: MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG OvmfPkg/OvmfPkgX64.dsc: GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG OvmfPkg/OvmfPkgX64.dsc: GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG OvmfPkg/OvmfPkgX64.dsc: INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG OvmfPkg/OvmfPkgX64.dsc: MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc: ICC:*_*_*_CC_FLAGS = -D MDEPKG_NDEBUG Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc: GCC:*_*_*_CC_FLAGS = -D MDEPKG_NDEBUG Vlv2TbltDevicePkg/PlatformPkgIA32.dsc: ICC:*_*_*_CC_FLAGS = /D MDEPKG_NDEBUG Vlv2TbltDevicePkg/PlatformPkgIA32.dsc: GCC:*_*_*_CC_FLAGS = -D MDEPKG_NDEBUG Vlv2TbltDevicePkg/PlatformPkgX64.dsc: ICC:*_*_*_CC_FLAGS = /D MDEPKG_NDEBUG Vlv2TbltDevicePkg/PlatformPkgX64.dsc: GCC:*_*_*_CC_FLAGS = -D MDEPKG_NDEBUG > That will give you the same path as VC++ and clang (assuming LTO is enabled). Well, LTO is unusable in edk2 with the supported GCC toolchains, to my knowledge. > https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/DebugLib.h > > #if !defined(MDEPKG_NDEBUG) > #define DEBUG(Expression) \ > do { \ > if (DebugPrintEnabled ()) { \ > _DEBUG (Expression); \ > } \ > } while (FALSE) > #else > #define DEBUG(Expression) > #endif > > > If you have code that only exists in DEBUG builds you can use these macros to > isolate it. > > /** > Macro that marks the beginning of debug source code. > If the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of PcdDebugProperyMask is set, > then this macro marks the beginning of source code that is included in a > module. > Otherwise, the source lines between DEBUG_CODE_BEGIN() and DEBUG_CODE_END() > are not included in a module. > **/ > #define DEBUG_CODE_BEGIN() do { if (DebugCodeEnabled ()) { UINT8 > __DebugCodeLocal > > > /** > The macro that marks the end of debug source code. > If the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of PcdDebugProperyMask is set, > then this macro marks the end of source code that is included in a module. > Otherwise, the source lines between DEBUG_CODE_BEGIN() and DEBUG_CODE_END() > are not included in a module. > **/ > #define DEBUG_CODE_END() __DebugCodeLocal = 0; __DebugCodeLocal++; } } > while (FALSE) Right, but the code that triggers the problem is more like: CHAR8 *Message; Message = ""; if (whatever) { // do some real logic Message = "blah\n"; // do some more real logic } DEBUG ((EFI_D_INFO, "%a\n", Message)); I.e., it's hard to find a common, tight scope for all uses of the variable in question. Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
