Re: [edk2-devel] [PATCH V4 26/27] MdePkg: Introduce assertion on constraint debug mask bit

["Laszlo Ersek" <lersek@redhat.com>]

On 05/11/20 22:58, Laszlo Ersek wrote:
> On 05/11/20 17:41, Vitaly Cheptsov wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2054
>>
>> This bit allows to control assertion on constrain violation.

(1) Ugh, sorry, my point (1) was going to be about out two instances of
the same typo:

s/constrain/constraint/

in (a) the commit message, and (b) in the ASSERT_CONSTRAINT macro's
documentation, below.

Thanks & sorry for the mixup
Laszlo

>>
>> Signed-off-by: Vitaly Cheptsov <vit9696@protonmail.com>
>> ---
>>  MdePkg/Include/Library/DebugCommonLib.h            | 18 ++++++++++++++++++
>>  MdePkg/Include/Library/DebugLib.h                  | 18 ++++++++++++++++++
>>  MdePkg/Library/BaseDebugCommonLib/DebugCommonLib.c | 20 ++++++++++++++++++++
>>  MdePkg/MdePkg.dec                                  |  3 ++-
>>  4 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdePkg/Include/Library/DebugCommonLib.h b/MdePkg/Include/Library/DebugCommonLib.h
>> index 31249f91fa..3dc906d02a 100644
>> --- a/MdePkg/Include/Library/DebugCommonLib.h
>> +++ b/MdePkg/Include/Library/DebugCommonLib.h
>> @@ -21,6 +21,7 @@
>>  #define DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED       0x08
>>  #define DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED  0x10
>>  #define DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED    0x20
>> +#define DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED  0x40
>>  
>>  //
>>  // Declare bits for PcdDebugPrintErrorLevel and the ErrorLevel parameter of DebugPrint()
>> @@ -135,6 +136,23 @@ DebugClearMemoryEnabled (
>>    );
>>  
>>  
>> +/**
>> +  Returns TRUE if ASSERT_CONSTRAINT() macro is enabled.
>> +
>> +  This function returns TRUE if the DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of
>> +  PcdDebugProperyMask is set.  Otherwise, FALSE is returned.
>> +
>> +  @retval  TRUE    The DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of PcdDebugProperyMask is set.
>> +  @retval  FALSE   The DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of PcdDebugProperyMask is clear.
>> +
>> +**/
>> +BOOLEAN
>> +EFIAPI
>> +DebugAssertConstraintEnabled (
>> +  VOID
>> +  );
>> +
>> +
>>  /**
>>    Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel.
>>  
>> diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
>> index a38e0e2904..ea38522ae3 100644
>> --- a/MdePkg/Include/Library/DebugLib.h
>> +++ b/MdePkg/Include/Library/DebugLib.h
>> @@ -212,6 +212,24 @@ DebugClearMemory (
>>    #define ASSERT(Expression)
>>  #endif
>>  
>> +
>> +/**
>> +  Macro that calls ASSERT when constrain assertions are enabled.
>> +
>> +  If DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of PcdDebugProperyMask is set,
>> +  then this macro evaluates to an ASSERT macro passing in the original Expression.
>> +
>> +  @param  Expression  Boolean expression.
>> +
>> +**/
>> +#define ASSERT_CONSTRAINT(Expression)      \
>> +  do {                                     \
>> +    if (DebugAssertConstraintEnabled ()) { \
>> +      ASSERT (Expression);                 \
>> +    }                                      \
>> +  } while (FALSE)
>> +
>> +
>>  /**
>>    Macro that calls DebugPrint().
>>  
>> diff --git a/MdePkg/Library/BaseDebugCommonLib/DebugCommonLib.c b/MdePkg/Library/BaseDebugCommonLib/DebugCommonLib.c
>> index 2291fb5382..870f9ceec5 100644
>> --- a/MdePkg/Library/BaseDebugCommonLib/DebugCommonLib.c
>> +++ b/MdePkg/Library/BaseDebugCommonLib/DebugCommonLib.c
>> @@ -93,6 +93,26 @@ DebugClearMemoryEnabled (
>>  }
>>  
>>  
>> +/**
>> +  Returns TRUE if ASSERT_CONSTRAINT() macro is enabled.
>> +
>> +  This function returns TRUE if the DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of
>> +  PcdDebugProperyMask is set.  Otherwise, FALSE is returned.
>> +
>> +  @retval  TRUE    The DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of PcdDebugProperyMask is set.
>> +  @retval  FALSE   The DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of PcdDebugProperyMask is clear.
>> +
>> +**/
>> +BOOLEAN
>> +EFIAPI
>> +DebugAssertConstraintEnabled (
>> +  VOID
>> +  )
>> +{
>> +  return (BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED) != 0);
>> +}
>> +
>> +
>>  /**
>>    Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel.
>>  
>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index a121b9ecab..63003357d6 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -2136,9 +2136,10 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
>>    #  BIT3 - Enable Clear Memory.<BR>
>>    #  BIT4 - Enable BreakPoint as ASSERT.<BR>
>>    #  BIT5 - Enable DeadLoop as ASSERT.<BR>
>> +  #  BIT6 - Treat constrait violations as ASSERT.<BR>
>>    # @Prompt Debug Property.
>>    # @Expression  0x80000002 | (gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask & 0xC0) == 0
>> -  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0|UINT8|0x00000005
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0|UINT8|0x00000045
>>  
>>    ## This flag is used to control the print out Debug message.<BR><BR>
>>    #  BIT0  - Initialization message.<BR>
>>
> 
> (2) The "MdePkg/MdePkg.uni" file update from patch#1 belongs here, IMO.
> 
> Otherwise, the patch looks OK to me.
> 
> Thanks
> Laszlo
>