public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Vitaly Cheptsov" <cheptsov@ispras.ru>
To: "Laszlo Ersek" <lersek@redhat.com>,
	"Andrew Fish" <afish@apple.com>,
	"Mike Kinney" <michael.d.kinney@intel.com>,
	"Marvin Häuser" <mhaeuser@outlook.de>,
	"Gao, Liming" <liming.gao@intel.com>,
	"Gao, Zhichao" <zhichao.gao@intel.com>
Cc: devel@edk2.groups.io
Subject: Re: Disabling safe string constraint assertions
Date: Wed, 18 Mar 2020 22:36:10 +0300	[thread overview]
Message-ID: <C621C342-5C83-45D1-A501-891A4D1EE34D@ispras.ru> (raw)
In-Reply-To: <38e4a6e4-ca31-3c2f-5df5-fb4b67e05f33@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 2891 bytes --]

Hello!

I have a prototype of the patch, but there currently is an issue with the current EDK II build system.
I attached the patch to this e-mail, however, it will not compile for reasonably obscure causes.

From what I understand:
- DebugLib header now directly uses PCDs from DebugLib, like PcdDebugPropertyMask.
- Any library implementing DebugLib should now depend on these PCDs, which seems fairly natural (and I fixed that in BaseDebugLibNull).
- Any library using DebugLib header should depend on DebugLib, which also depend on DebugLib to get its PCDs (that already looks fine).

However, for some reason DebugLib PCDs are not included in Autogen.h header for other libraries some reason, and we get errors like:
MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.c:1151:9: error: use of undeclared identifier '_PCD_GET_MODE_8_PcdDebugPropertyMask'

I am not familiar with the build system well enough to resolve this, so I either need guidance on where to look first or it will be great if somebody else handles that.
I do not believe it is a great idea to abandon the idea of dropping DebugAssertEnabled-like functions, so I suggest us to focus on resolving the build system limitation rather than trying a new approach.

Best regards,
Vitaly




> 11 марта 2020 г., в 16:14, Laszlo Ersek <lersek@redhat.com> написал(а):
> 
> On 03/11/20 14:09, Vitaly Cheptsov wrote:
>> Hi everyone,
>> 
>> So, I believe that by now we mostly agreed to let the original
>> proposition land as a short-term solution. We end up with:
>> 
>> 1. A PCD condition within SAFE_STRING_COSTRAINT_CHECK macro.
>> 2. Make this condition evaluate to TRUE by default (i.e. ASSERT).
>> 3. Update documentation for BaseLib functions to include the information
>> about this behaviour.
>> 
>> The only thing in question is whether this should be a separate PCD or
>> an extra bit in PcdDebugPropertyMask. I believe that we almost agreed on
>> two things:
>> 
>> 1. Adding an extra bit to PcdDebugPropertyMask is cleaner.
>> 2. Extending DebugLib interface with a new function is not a good idea.
>> 
>> Therefore I suggest:
>> 
>> 1.Add #define DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED 0x40.
>> 2. Create header-only macros to replace functions like
>> DebugAssertEnabled. We can then use these macros in new code and
>> deprecate the original functions.
>> 3. Enable DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit in MdePkg by default.
>> 
>> I will submit the new version of the patch soon unless there is an
>> immediate opposing opinion.
> 
> Not sure about any particular deprecation timeline, but to me the above
> certainly sounds worth submitting for review.
> 
> (NB I don't plan to review in detail -- I just meant to comment on the
> design, since I was asked to.)
> 
> Thanks!
> Laszlo


[-- Attachment #1.2.1: Type: text/html, Size: 10353 bytes --]

[-- Attachment #1.2.2: constraint-r3.diff --]
[-- Type: application/octet-stream, Size: 30115 bytes --]

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index ac1f5339af..b060ac2ac9 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2133,9 +2133,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>
diff --git a/MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf b/MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
index 81a63a5074..1173ac30b5 100644
--- a/MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+++ b/MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
@@ -29,3 +29,12 @@ [Sources]
 [Packages]
   MdePkg/MdePkg.dec
 
+
+[LibraryClasses]
+  PcdLib
+
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask         ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES
+
diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index ecadff8b23..08beaa8c23 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -189,7 +189,7 @@ StrnSizeS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -225,7 +225,7 @@ StrCpyS (
 
   If Length > 0 and Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Length > 0 and Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -263,7 +263,7 @@ StrnCpyS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -303,7 +303,7 @@ StrCatS (
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -350,12 +350,12 @@ StrnCatS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
   If PcdMaximumUnicodeStringLength is not zero, and String contains more than
   PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -406,12 +406,12 @@ StrDecimalToUintnS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
   If PcdMaximumUnicodeStringLength is not zero, and String contains more than
   PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -467,12 +467,12 @@ StrDecimalToUint64S (
   the first character that is a not a valid hexadecimal character or NULL,
   whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
   If PcdMaximumUnicodeStringLength is not zero, and String contains more than
   PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -528,12 +528,12 @@ StrHexToUintnS (
   the first character that is a not a valid hexadecimal character or NULL,
   whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
   If PcdMaximumUnicodeStringLength is not zero, and String contains more than
   PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -622,7 +622,7 @@ AsciiStrnSizeS (
 
   This function is similar as strcpy_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -656,7 +656,7 @@ AsciiStrCpyS (
 
   This function is similar as strncpy_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -692,7 +692,7 @@ AsciiStrnCpyS (
 
   This function is similar as strcat_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -730,7 +730,7 @@ AsciiStrCatS (
 
   This function is similar as strncat_s defined in C11.
 
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -777,11 +777,11 @@ AsciiStrnCatS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If PcdMaximumAsciiStringLength is not zero, and String contains more than
   PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -832,11 +832,11 @@ AsciiStrDecimalToUintnS (
   be ignored. Then, the function stops at the first character that is a not a
   valid decimal character or a Null-terminator, whichever one comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If PcdMaximumAsciiStringLength is not zero, and String contains more than
   PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid decimal digits in the above format, then 0 is stored
   at the location pointed to by Data.
@@ -891,11 +891,11 @@ AsciiStrDecimalToUint64S (
   character that is a not a valid hexadecimal character or Null-terminator,
   whichever on comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If PcdMaximumAsciiStringLength is not zero, and String contains more than
   PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -950,11 +950,11 @@ AsciiStrHexToUintnS (
   character that is a not a valid hexadecimal character or Null-terminator,
   whichever on comes first.
 
-  If String is NULL, then ASSERT().
-  If Data is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Data is NULL, then ASSERT_CONSTRAINT().
   If PcdMaximumAsciiStringLength is not zero, and String contains more than
   PcdMaximumAsciiStringLength Ascii characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
@@ -1506,15 +1506,15 @@ StrHexToUint64 (
   "::" can be used to compress one or more groups of X when X contains only 0.
   The "::" can only appear once in the String.
 
-  If String is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
 
-  If Address is NULL, then ASSERT().
+  If Address is NULL, then ASSERT_CONSTRAINT().
 
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
   If PcdMaximumUnicodeStringLength is not zero, and String contains more than
   PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
@@ -1567,15 +1567,15 @@ StrToIpv6Address (
   When /P is in the String, the function stops at the first character that is not
   a valid decimal digit character after P is converted.
 
-  If String is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
 
-  If Address is NULL, then ASSERT().
+  If Address is NULL, then ASSERT_CONSTRAINT().
 
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
   If PcdMaximumUnicodeStringLength is not zero, and String contains more than
   PcdMaximumUnicodeStringLength Unicode characters, not including the
-  Null-terminator, then ASSERT().
+  Null-terminator, then ASSERT_CONSTRAINT().
 
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
@@ -1640,8 +1640,8 @@ StrToIpv4Address (
                   oo          Data4[48:55]
                   pp          Data4[56:63]
 
-  If String is NULL, then ASSERT().
-  If Guid is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Guid is NULL, then ASSERT_CONSTRAINT().
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
   @param  String                   Pointer to a Null-terminated Unicode string.
@@ -1676,16 +1676,16 @@ StrToGuid (
 
   If String is not aligned in a 16-bit boundary, then ASSERT().
 
-  If String is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
 
-  If Buffer is NULL, then ASSERT().
+  If Buffer is NULL, then ASSERT_CONSTRAINT().
 
-  If Length is not multiple of 2, then ASSERT().
+  If Length is not multiple of 2, then ASSERT_CONSTRAINT().
 
   If PcdMaximumUnicodeStringLength is not zero and Length is greater than
-  PcdMaximumUnicodeStringLength, then ASSERT().
+  PcdMaximumUnicodeStringLength, then ASSERT_CONSTRAINT().
 
-  If MaxBufferSize is less than (Length / 2), then ASSERT().
+  If MaxBufferSize is less than (Length / 2), then ASSERT_CONSTRAINT().
 
   @param  String                   Pointer to a Null-terminated Unicode string.
   @param  Length                   The number of Unicode characters to decode.
@@ -1777,7 +1777,7 @@ UnicodeStrToAsciiStr (
   the upper 8 bits, then ASSERT().
 
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -1824,7 +1824,7 @@ UnicodeStrToAsciiStrS (
   If any Unicode characters in Source contain non-zero value in the upper 8
   bits, then ASSERT().
   If Source is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -2388,9 +2388,9 @@ AsciiStrHexToUint64 (
   "::" can be used to compress one or more groups of X when X contains only 0.
   The "::" can only appear once in the String.
 
-  If String is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
 
-  If Address is NULL, then ASSERT().
+  If Address is NULL, then ASSERT_CONSTRAINT().
 
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
@@ -2443,9 +2443,9 @@ AsciiStrToIpv6Address (
   When /P is in the String, the function stops at the first character that is not
   a valid decimal digit character after P is converted.
 
-  If String is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
 
-  If Address is NULL, then ASSERT().
+  If Address is NULL, then ASSERT_CONSTRAINT().
 
   If EndPointer is not NULL and Address is translated from String, a pointer
   to the character that stopped the scan is stored at the location pointed to
@@ -2508,8 +2508,8 @@ AsciiStrToIpv4Address (
                   oo          Data4[48:55]
                   pp          Data4[56:63]
 
-  If String is NULL, then ASSERT().
-  If Guid is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
+  If Guid is NULL, then ASSERT_CONSTRAINT().
 
   @param  String                   Pointer to a Null-terminated ASCII string.
   @param  Guid                     Pointer to the converted GUID.
@@ -2541,16 +2541,16 @@ AsciiStrToGuid (
   decoding stops after Length of characters and outputs Buffer containing
   (Length / 2) bytes.
 
-  If String is NULL, then ASSERT().
+  If String is NULL, then ASSERT_CONSTRAINT().
 
-  If Buffer is NULL, then ASSERT().
+  If Buffer is NULL, then ASSERT_CONSTRAINT().
 
-  If Length is not multiple of 2, then ASSERT().
+  If Length is not multiple of 2, then ASSERT_CONSTRAINT().
 
   If PcdMaximumAsciiStringLength is not zero and Length is greater than
-  PcdMaximumAsciiStringLength, then ASSERT().
+  PcdMaximumAsciiStringLength, then ASSERT_CONSTRAINT().
 
-  If MaxBufferSize is less than (Length / 2), then ASSERT().
+  If MaxBufferSize is less than (Length / 2), then ASSERT_CONSTRAINT().
 
   @param  String                   Pointer to a Null-terminated ASCII string.
   @param  Length                   The number of ASCII characters to decode.
@@ -2632,7 +2632,7 @@ AsciiStrToUnicodeStr (
   equal or greater than ((AsciiStrLen (Source) + 1) * sizeof (CHAR16)) in bytes.
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then the Destination is unmodified.
 
@@ -2678,7 +2678,7 @@ AsciiStrToUnicodeStrS (
   ((MIN(AsciiStrLen(Source), Length) + 1) * sizeof (CHAR8)) in bytes.
 
   If Destination is not aligned on a 16-bit boundary, then ASSERT().
-  If an error would be returned, then the function will also ASSERT().
+  If an error would be returned, then the function will also ASSERT_CONSTRAINT().
 
   If an error is returned, then Destination and DestinationLength are
   unmodified.
diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
index f1d55cf62b..38aebd1701 100644
--- a/MdePkg/Include/Library/DebugLib.h
+++ b/MdePkg/Include/Library/DebugLib.h
@@ -16,6 +16,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #ifndef __DEBUG_LIB_H__
 #define __DEBUG_LIB_H__
 
+#include <Library/PcdLib.h>
+
 //
 // Declare bits for PcdDebugPropertyMask
 //
@@ -25,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #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()
@@ -71,6 +74,92 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define EFI_D_VERBOSE   DEBUG_VERBOSE
 #define EFI_D_ERROR     DEBUG_ERROR
 
+
+/**
+  Returns TRUE if ASSERT() macros are enabled.
+
+  This macro evaluates to TRUE if the DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise, FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of PcdDebugProperyMask is clear.
+
+**/
+#define DEBUG_ASSERT_ENABLED() \
+  ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED) != 0))
+
+
+/**
+  Returns TRUE if DEBUG() macros are enabled.
+
+  This macro evaluates to TRUE if the DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise, FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of PcdDebugProperyMask is clear.
+
+**/
+#define DEBUG_PRINT_ENABLED() \
+  ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0))
+
+
+/**
+  Returns TRUE if DEBUG_CODE() macros are enabled.
+
+  This macro evaluates to TRUE if the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise, FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of PcdDebugProperyMask is clear.
+
+**/
+#define DEBUG_CODE_ENABLED() \
+  ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_CODE_ENABLED) != 0))
+
+
+/**
+  Returns TRUE if DEBUG_CLEAR_MEMORY() macro is enabled.
+
+  This macro evaluates to TRUE if the DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise, FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of PcdDebugProperyMask is clear.
+
+**/
+#define DEBUG_CLEAR_MEMORY_ENABLED() \
+  ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0))
+
+
+/**
+  Returns TRUE if ASSERT_CONSTRAINT() macros are enabled.
+
+  This macro evaluates to 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.
+
+**/
+#define DEBUG_ASSERT_CONSTRAINT_ENABLED() \
+  ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED) != 0))
+
+
+/**
+  Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel.
+
+  This macro compares the bit mask of ErrorLevel and PcdFixedDebugPrintErrorLevel.
+
+  @param  ErrorLevel  The error level to compare.
+
+  @retval  TRUE    Current ErrorLevel is supported.
+  @retval  FALSE   Current ErrorLevel is not supported.
+
+**/
+#define DEBUG_PRINT_LEVEL_ENABLED(ErrorLevel) \
+  ((BOOLEAN) (((ErrorLevel) & PcdGet32 (PcdFixedDebugPrintErrorLevel)) != 0))
+
+
 /**
   Prints a debug message to the debug output device if the specified error level is enabled.
 
@@ -308,7 +397,7 @@ DebugPrintLevelEnabled (
 #if !defined(MDE_CPU_EBC) && (!defined (_MSC_VER) || _MSC_VER > 1400)
   #define _DEBUG_PRINT(PrintLevel, ...)              \
     do {                                             \
-      if (DebugPrintLevelEnabled (PrintLevel)) {     \
+      if (DEBUG_PRINT_LEVEL_ENABLED (PrintLevel)) {  \
         DebugPrint (PrintLevel, ##__VA_ARGS__);      \
       }                                              \
     } while (FALSE)
@@ -330,19 +419,37 @@ DebugPrintLevelEnabled (
 
 **/
 #if !defined(MDEPKG_NDEBUG)
-  #define ASSERT(Expression)        \
-    do {                            \
-      if (DebugAssertEnabled ()) {  \
-        if (!(Expression)) {        \
-          _ASSERT (Expression);     \
-          ANALYZER_UNREACHABLE ();  \
-        }                           \
-      }                             \
+  #define ASSERT(Expression)          \
+    do {                              \
+      if (DEBUG_ASSERT_ENABLED ()) {  \
+        if (!(Expression)) {          \
+          _ASSERT (Expression);       \
+          ANALYZER_UNREACHABLE ();    \
+        }                             \
+      }                               \
     } while (FALSE)
 #else
   #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 (DEBUG_ASSERT_CONSTRAINT_ENABLED ()) {  \
+      ASSERT (Expression);                     \
+    }                                          \
+  } while (FALSE)
+
+
 /**
   Macro that calls DebugPrint().
 
@@ -356,11 +463,11 @@ DebugPrintLevelEnabled (
 
 **/
 #if !defined(MDEPKG_NDEBUG)
-  #define DEBUG(Expression)        \
-    do {                           \
-      if (DebugPrintEnabled ()) {  \
-        _DEBUG (Expression);       \
-      }                            \
+  #define DEBUG(Expression)          \
+    do {                             \
+      if (DEBUG_PRINT_ENABLED ()) {  \
+        _DEBUG (Expression);         \
+      }                              \
     } while (FALSE)
 #else
   #define DEBUG(Expression)
@@ -381,7 +488,7 @@ DebugPrintLevelEnabled (
 #if !defined(MDEPKG_NDEBUG)
   #define ASSERT_EFI_ERROR(StatusParameter)                                              \
     do {                                                                                 \
-      if (DebugAssertEnabled ()) {                                                       \
+      if (DEBUG_ASSERT_ENABLED ()) {                                                     \
         if (EFI_ERROR (StatusParameter)) {                                               \
           DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", StatusParameter));  \
           _ASSERT (!EFI_ERROR (StatusParameter));                                        \
@@ -407,7 +514,7 @@ DebugPrintLevelEnabled (
 #if !defined(MDEPKG_NDEBUG)
   #define ASSERT_RETURN_ERROR(StatusParameter)                          \
     do {                                                                \
-      if (DebugAssertEnabled ()) {                                      \
+      if (DEBUG_ASSERT_ENABLED ()) {                                    \
         if (RETURN_ERROR (StatusParameter)) {                           \
           DEBUG ((DEBUG_ERROR, "\nASSERT_RETURN_ERROR (Status = %r)\n", \
             StatusParameter));                                          \
@@ -444,7 +551,7 @@ DebugPrintLevelEnabled (
 #if !defined(MDEPKG_NDEBUG)
   #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid)                               \
     do {                                                                                \
-      if (DebugAssertEnabled ()) {                                                      \
+      if (DEBUG_ASSERT_ENABLED ()) {                                                    \
         VOID  *Instance;                                                                \
         ASSERT (Guid != NULL);                                                          \
         if (Handle == NULL) {                                                           \
@@ -471,7 +578,7 @@ DebugPrintLevelEnabled (
   are not included in a module.
 
 **/
-#define DEBUG_CODE_BEGIN()  do { if (DebugCodeEnabled ()) { UINT8  __DebugCodeLocal
+#define DEBUG_CODE_BEGIN()  do { if (DEBUG_CODE_ENABLED ()) { UINT8  __DebugCodeLocal
 
 
 /**
@@ -512,7 +619,7 @@ DebugPrintLevelEnabled (
 **/
 #define DEBUG_CLEAR_MEMORY(Address, Length)  \
   do {                                       \
-    if (DebugClearMemoryEnabled ()) {        \
+    if (DEBUG_CLEAR_MEMORY_ENABLED ()) {     \
       DebugClearMemory (Address, Length);    \
     }                                        \
   } while (FALSE)
@@ -561,12 +668,12 @@ DebugPrintLevelEnabled (
 
 **/
 #if !defined(MDEPKG_NDEBUG)
-  #define CR(Record, TYPE, Field, TestSignature)                                              \
-    (DebugAssertEnabled () && (BASE_CR (Record, TYPE, Field)->Signature != TestSignature)) ?  \
-    (TYPE *) (_ASSERT (CR has Bad Signature), Record) :                                       \
+  #define CR(Record, TYPE, Field, TestSignature)                                                \
+    (DEBUG_ASSERT_ENABLED () && (BASE_CR (Record, TYPE, Field)->Signature != TestSignature)) ?  \
+    (TYPE *) (_ASSERT (CR has Bad Signature), Record) :                                         \
     BASE_CR (Record, TYPE, Field)
 #else
-  #define CR(Record, TYPE, Field, TestSignature)                                              \
+  #define CR(Record, TYPE, Field, TestSignature)                                                \
     BASE_CR (Record, TYPE, Field)
 #endif
 
diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 0abb40d6ec..b18e76bb87 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -331,7 +331,7 @@ EfiInitializeLock (
 #if !defined(MDEPKG_NDEBUG)
   #define ASSERT_LOCKED(LockParameter)                  \
     do {                                                \
-      if (DebugAssertEnabled ()) {                      \
+      if (DEBUG_ASSERT_ENABLED ()) {                    \
         ASSERT (LockParameter != NULL);                 \
         if ((LockParameter)->Lock != EfiLockAcquired) { \
           _ASSERT (LockParameter not locked);           \
diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
index 7dc03d2caa..f6cdd76c82 100644
--- a/MdePkg/Library/BaseLib/SafeString.c
+++ b/MdePkg/Library/BaseLib/SafeString.c
@@ -14,7 +14,7 @@
 
 #define SAFE_STRING_CONSTRAINT_CHECK(Expression, Status)  \
   do { \
-    ASSERT (Expression); \
+    ASSERT_CONSTRAINT (Expression); \
     if (!(Expression)) { \
       return Status; \
     } \
diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
index 5c1fa24065..5addb0eaba 100644
--- a/MdePkg/MdePkg.uni
+++ b/MdePkg/MdePkg.uni
@@ -189,7 +189,8 @@
                                                                                 "BIT2 - Enable Debug Code.<BR>\n"
                                                                                 "BIT3 - Enable Clear Memory.<BR>\n"
                                                                                 "BIT4 - Enable BreakPoint as ASSERT.<BR>\n"
-                                                                                "BIT5 - Enable DeadLoop as ASSERT.<BR>"
+                                                                                "BIT5 - Enable DeadLoop as ASSERT.<BR>\n"
+                                                                                "BIT6 - Treat constrait violations as ASSERT.<BR>"
 
 #string STR_gEfiMdePkgTokenSpaceGuid_ERR_80000002 #language en-US "Reserved bits must be set to zero."
 

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-03-18 19:36 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-03 21:12 Disabling safe string constraint assertions Vitaly Cheptsov
2020-03-03 22:32 ` [edk2-devel] " Andrew Fish
2020-03-04 13:33 ` Laszlo Ersek
2020-03-04 17:53   ` Andrew Fish
2020-03-04 18:18     ` Laszlo Ersek
2020-03-04 18:56       ` Andrew Fish
2020-03-11 13:09         ` cheptsov
2020-03-11 13:14           ` Laszlo Ersek
2020-03-18 19:36             ` Vitaly Cheptsov [this message]
2020-03-18 20:35               ` [edk2-devel] " Michael D Kinney
2020-03-18 20:43                 ` Vitaly Cheptsov
2020-03-18 20:55                   ` Michael D Kinney
2020-03-18 21:31                     ` Vitaly Cheptsov
2020-03-18 21:53                       ` Andrew Fish
2020-03-19  0:04                         ` Vitaly Cheptsov
2020-05-11 12:03                           ` Vitaly Cheptsov
2020-05-11 15:19                             ` Laszlo Ersek
2020-05-11 15:35                               ` Vitaly Cheptsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C621C342-5C83-45D1-A501-891A4D1EE34D@ispras.ru \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox