On 07/08/15 21:26, Andrew Fish wrote:
> 
>> On Jul 8, 2015, at 12:02 PM, Laszlo Ersek <ler...@redhat.com
>> <mailto:ler...@redhat.com>> wrote:
>>
>>>
>>> It should be noted that once in a while I run into this sort of thing in 
>>> VxWorks as well, only it's often the other way around. We typically have 
>>> DEBUG()-style messages turned off by default, and sometimes there are
>>> debug 
>>> messages that refer to variables which no longer exist in the code. (The 
>>> variables were removed but the debug messages were never updated to
>>> match.)
>>
>> SeaBIOS employs the following trick to avoid this: all CONFIG_XXXX flags
>> are checked in regular if() statements, not in #if macros. That is, any
>> elmination happens during compilation / optimization / linking, not in
>> preprocessing. This allows the compiler to catch such errors even when a
>> CONFIG_XXXX option is predominantly false (or predominantly true).
> 
> This is how the edk2 works. Your platform has to “opt-in” to the #if.
> The default is to use PCDs, and that is what happens on VC++.
> The MDEPKG_NDEBUG
> #define was added for compilers, like gcc, that could not dead strip the
> code. 
> 
> https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Include/Library/DebugLib.h
> 
> #if !defined(MDEPKG_NDEBUG)       
>   #define ASSERT(Expression)        \
>     do {                            \
>       if (DebugAssertEnabled ()) {  \
>         if (!(Expression)) {        \
>           _ASSERT (Expression);     \
>         }                           \
>       }                             \
>     } while (FALSE)
> #else
>   #define ASSERT(Expression)
> #endif
> 
> If you are not using the optional MDEPKG_NDEBUG flag then DEBUG and
> ASSERT macros just become policy for a release build based on PCD. 

Ah! That's very useful background info.

Perhaps it's time for dropping MDEPKG_NDEBUG? We have:

[BuildOptions]
  GCC:*_UNIXGCC_*_CC_FLAGS             = -DMDEPKG_NDEBUG
  GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
  INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
  MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG

I wonder why MDEPKG_NDEBUG was then added for MSFT in the first place
(assuming MSVC had always been able to remove dead code), but in any
case, perhaps we should restrict MDEPKG_NDEBUG to RELEASE builds with
older (4.4, 4.5, ... ?) gcc.

... I experimented a bit now. I removed MDEPKG_NDEBUG, and watched how
the size of DXEFV changed when I flipped the DEBUG_VERBOSE bit on and
off. Summary: no matter what I tried in place of MDEPKG_NDEBUG (-O0,
-O1, -O1 -flto), given one specific CC_FLAGS (and DLINK_FLAGS) setting,
inverting the DEBUG_VERBOSE bit in the PCD mask had no effect.

I think it would make a difference if -flto *actually* worked, but from
a quick google search, I think it either doesn't work with gcc-4.8 at
all, *or* the edk2 build system would have to use FLTO-aware binutils
and linker wrappers (or parameters). I have no clue how to set that up.
So for now we'll have to stick with MDEPKG_NDEBUG I guess.

Thanks!
Laszlo

> 
> Also in most of the DebugLib instances what do on the ASSERT is a policy
> choice (breakpoint, dead loop, no-op). 
> https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> 
> VOID
> EFIAPI
> DebugAssert (
>   IN CONST CHAR8  *FileName,
>   IN UINTN        LineNumber,
>   IN CONST CHAR8  *Description
>   )
> {
>   CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> 
>   //
>   // Generate the ASSERT() message in Ascii format
>   //
>   AsciiSPrint (Buffer, sizeof (Buffer), "ASSERT %a(%d): %a\n", FileName, 
> LineNumber, Description);
> 
>   //
>   // Send the print string to the Console Output device
>   //
>   SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
> 
>   //
>   // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
>   //
>   if ((PcdGet8(PcdDebugPropertyMask) & 
> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
>     CpuBreakpoint ();
>   } else if ((PcdGet8(PcdDebugPropertyMask) & 
> DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
>     CpuDeadLoop ();
>   }
> }
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> 
> 
> 
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> 
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
> 


------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to