> On Mar 18, 2016, at 5:13 PM, Laszlo Ersek <[email protected]> wrote: > > 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: >
Well you got advice to reinvent the wheel..... If you followed that path you would end up reinventing the non MDEPKG_NDEBUG path. > $ git grep MDEPKG_NDEBUG -- '*.dsc' > I did not realize that some packages had used MDEPKG_NDEBUG, but it makes sense I guess so the coding errors would be the same for all the supported compilers. > 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. > I feel bad saying this, but make that variable volatile and the compiler would probably think you know what you are doing and stop warning. Thanks, Andrew Fish > Thanks > Laszlo > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
