From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.45]) by mx.groups.io with SMTP id smtpd.web11.3115.1584560174479541412 for ; Wed, 18 Mar 2020 12:36:15 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: ispras.ru, ip: 83.149.199.45, mailfrom: cheptsov@ispras.ru) Received: from [127.0.0.1] (unknown [77.232.9.83]) by mail.ispras.ru (Postfix) with ESMTPSA id B9E1A64F; Wed, 18 Mar 2020 22:36:11 +0300 (MSK) From: "Vitaly Cheptsov" Message-Id: Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) Subject: Re: Disabling safe string constraint assertions Date: Wed, 18 Mar 2020 22:36:10 +0300 In-Reply-To: <38e4a6e4-ca31-3c2f-5df5-fb4b67e05f33@redhat.com> Cc: devel@edk2.groups.io To: Laszlo Ersek , Andrew Fish , Mike Kinney , =?utf-8?Q?Marvin_H=C3=A4user?= , "Gao, Liming" , "Gao, Zhichao" References: <5F8859C7-5776-4DF0-99F5-1D0BC56B50C4@apple.com> <318F875F-3C9A-4236-841B-5BB9908CC139@ispras.ru> <38e4a6e4-ca31-3c2f-5df5-fb4b67e05f33@redhat.com> X-Mailer: Apple Mail (2.3608.60.0.2.5) X-Groupsio-MsgNum: 55961 Content-Type: multipart/signed; boundary="Apple-Mail=_09089207-DDB0-4EB2-8DA7-35CEDC1005E1"; protocol="application/pgp-signature"; micalg=pgp-sha512 --Apple-Mail=_09089207-DDB0-4EB2-8DA7-35CEDC1005E1 Content-Type: multipart/alternative; boundary="Apple-Mail=_C41DDFE7-698E-4E75-A0A8-510D0C9DFF0F" --Apple-Mail=_C41DDFE7-698E-4E75-A0A8-510D0C9DFF0F Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 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. =46rom 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/BaseOrderedCollectionR= edBlackTreeLib.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 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2020 =D0=B3., =D0=B2 16:14, Laszlo = Ersek =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0= ): >=20 > On 03/11/20 14:09, Vitaly Cheptsov wrote: >> Hi everyone, >>=20 >> So, I believe that by now we mostly agreed to let the original >> proposition land as a short-term solution. We end up with: >>=20 >> 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. >>=20 >> 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: >>=20 >> 1. Adding an extra bit to PcdDebugPropertyMask is cleaner. >> 2. Extending DebugLib interface with a new function is not a good = idea. >>=20 >> Therefore I suggest: >>=20 >> 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. >>=20 >> I will submit the new version of the patch soon unless there is an >> immediate opposing opinion. >=20 > Not sure about any particular deprecation timeline, but to me the = above > certainly sounds worth submitting for review. >=20 > (NB I don't plan to review in detail -- I just meant to comment on the > design, since I was asked to.) >=20 > Thanks! > Laszlo --Apple-Mail=_C41DDFE7-698E-4E75-A0A8-510D0C9DFF0F Content-Type: multipart/mixed; boundary="Apple-Mail=_658134D3-16F9-4316-BF4A-CFC053835748" --Apple-Mail=_658134D3-16F9-4316-BF4A-CFC053835748 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii
Hello!=

I have a prototy= pe of the patch, but there currently is an issue with the current EDK II bu= ild system.
I attached the patch to this e-mail, howev= er, it will not compile for reasonably obscure causes.

From what I understand:
- DebugLib header now directly uses PCDs from Debu= gLib, like PcdDebugPropertyMask.
- Any library impleme= nting 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 Debug= Lib to get its PCDs (that already looks fine).

However, for some reason DebugLib PCDs are no= t included in Autogen.h header for other libraries some reason, and we get = errors like:
MdePkg/Library/BaseOrderedCollectionRedBl= ackTreeLib/BaseOrderedCollectionRedBlackTreeLib.c:1151:9: error: use of und= eclared identifier '_PCD_GET_MODE_8_PcdDebugPropertyMask'

I am not familiar with the buil= d 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 droppin= g DebugAssertEnabled-like functions, so I suggest us to focus on resol= ving the build system limitation rather than trying a new approach.

Best regards,
Vitaly




11 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2020 =D0=B3., =D0= =B2 16:14, Laszlo Ersek <lersek@redhat.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0):

= On 03/11/20 14:09, Vitaly Cheptsov wrote:
Hi everyone,

S= o, I believe that by now we mostly agreed to let the original
proposition land as a short-term solution. We end up with:
<= br class=3D"">1. A PCD condition within SAFE_STRING_COSTRAINT_CHECK macro.<= br class=3D"">2. Make this condition evaluate to TRUE by default (i.e. ASSE= RT).
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 al= most agreed on
two things:

1. Ad= ding an extra bit to PcdDebugPropertyMask is cleaner.
2. Exte= nding DebugLib interface with a new function is not a good idea.

Therefore I suggest:

1.A= dd #define DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED 0x40.
2. = Create header-only macros to replace functions like
DebugAsse= rtEnabled. We can then use these macros in new code and
depre= cate the original functions.
3. Enable DEBUG_PROPERTY_AS= SERT_CONSTRAINT_ENABLED bit in MdePkg by default.

I will submit the new version of the patch soon unless there is animmediate opposing opinion.

Not sure about any particular deprec= ation 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

--Apple-Mail=_658134D3-16F9-4316-BF4A-CFC053835748 Content-Disposition: attachment; filename=constraint-r3.diff Content-Type: application/octet-stream; x-unix-mode=0644; name="constraint-r3.diff" Content-Transfer-Encoding: quoted-printable 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.
# BIT4 - Enable BreakPoint as ASSERT.
# BIT5 - Enable DeadLoop as ASSERT.
+ # BIT6 - Treat constrait violations as ASSERT.
# @Prompt Debug Property. # @Expression 0x80000002 | (gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMa= sk & 0xC0) =3D=3D 0 - gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0|UINT8|0x00000005 + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0|UINT8|0x00000045 =20 ## This flag is used to control the print out Debug message.

# BIT0 - Initialization message.
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 =20 + +[LibraryClasses] + PcdLib + + +[Pcd] + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES + diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/Base= Lib.h index ecadff8b23..08beaa8c23 100644 --- a/MdePkg/Include/Library/BaseLib.h +++ b/MdePkg/Include/Library/BaseLib.h @@ -189,7 +189,7 @@ StrnSizeS ( =20 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_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -225,7 +225,7 @@ StrCpyS ( =20 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 ASSER= T(). - If an error would be returned, then the function will also ASSERT(). + If an error would be returned, then the function will also ASSERT_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -263,7 +263,7 @@ StrnCpyS ( =20 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_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -303,7 +303,7 @@ StrCatS ( =20 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_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -350,12 +350,12 @@ StrnCatS ( be ignored. Then, the function stops at the first character that is a no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 t= han PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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 no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 t= han PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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. =20 - 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 t= han PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 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. =20 - 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 t= han PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 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 ( =20 This function is similar as strcpy_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). + If an error would be returned, then the function will also ASSERT_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -656,7 +656,7 @@ AsciiStrCpyS ( =20 This function is similar as strncpy_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). + If an error would be returned, then the function will also ASSERT_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -692,7 +692,7 @@ AsciiStrnCpyS ( =20 This function is similar as strcat_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). + If an error would be returned, then the function will also ASSERT_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -730,7 +730,7 @@ AsciiStrCatS ( =20 This function is similar as strncat_s defined in C11. =20 - If an error would be returned, then the function will also ASSERT(). + If an error would be returned, then the function will also ASSERT_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -777,11 +777,11 @@ AsciiStrnCatS ( be ignored. Then, the function stops at the first character that is a no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 tha= n PcdMaximumAsciiStringLength Ascii characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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 no= t a valid decimal character or a Null-terminator, whichever one comes first. =20 - 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 tha= n PcdMaximumAsciiStringLength Ascii characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 If String has no valid decimal digits in the above format, then 0 is sto= red 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. =20 - 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 tha= n PcdMaximumAsciiStringLength Ascii characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 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. =20 - 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 tha= n PcdMaximumAsciiStringLength Ascii characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 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 onl= y 0. The "::" can only appear once in the String. =20 - If String is NULL, then ASSERT(). + If String is NULL, then ASSERT_CONSTRAINT(). =20 - If Address is NULL, then ASSERT(). + If Address is NULL, then ASSERT_CONSTRAINT(). =20 If String is not aligned in a 16-bit boundary, then ASSERT(). =20 If PcdMaximumUnicodeStringLength is not zero, and String contains more t= han PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 If EndPointer is not NULL and Address is translated from String, a point= er 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. =20 - If String is NULL, then ASSERT(). + If String is NULL, then ASSERT_CONSTRAINT(). =20 - If Address is NULL, then ASSERT(). + If Address is NULL, then ASSERT_CONSTRAINT(). =20 If String is not aligned in a 16-bit boundary, then ASSERT(). =20 If PcdMaximumUnicodeStringLength is not zero, and String contains more t= han PcdMaximumUnicodeStringLength Unicode characters, not including the - Null-terminator, then ASSERT(). + Null-terminator, then ASSERT_CONSTRAINT(). =20 If EndPointer is not NULL and Address is translated from String, a point= er 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] =20 - 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(). =20 @param String Pointer to a Null-terminated Unicode st= ring. @@ -1676,16 +1676,16 @@ StrToGuid ( =20 If String is not aligned in a 16-bit boundary, then ASSERT(). =20 - If String is NULL, then ASSERT(). + If String is NULL, then ASSERT_CONSTRAINT(). =20 - If Buffer is NULL, then ASSERT(). + If Buffer is NULL, then ASSERT_CONSTRAINT(). =20 - If Length is not multiple of 2, then ASSERT(). + If Length is not multiple of 2, then ASSERT_CONSTRAINT(). =20 If PcdMaximumUnicodeStringLength is not zero and Length is greater than - PcdMaximumUnicodeStringLength, then ASSERT(). + PcdMaximumUnicodeStringLength, then ASSERT_CONSTRAINT(). =20 - If MaxBufferSize is less than (Length / 2), then ASSERT(). + If MaxBufferSize is less than (Length / 2), then ASSERT_CONSTRAINT(). =20 @param String Pointer to a Null-terminated Unicode st= ring. @param Length The number of Unicode characters to dec= ode. @@ -1777,7 +1777,7 @@ UnicodeStrToAsciiStr ( the upper 8 bits, then ASSERT(). =20 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_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -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_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -2388,9 +2388,9 @@ AsciiStrHexToUint64 ( "::" can be used to compress one or more groups of X when X contains onl= y 0. The "::" can only appear once in the String. =20 - If String is NULL, then ASSERT(). + If String is NULL, then ASSERT_CONSTRAINT(). =20 - If Address is NULL, then ASSERT(). + If Address is NULL, then ASSERT_CONSTRAINT(). =20 If EndPointer is not NULL and Address is translated from String, a point= er 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. =20 - If String is NULL, then ASSERT(). + If String is NULL, then ASSERT_CONSTRAINT(). =20 - If Address is NULL, then ASSERT(). + If Address is NULL, then ASSERT_CONSTRAINT(). =20 If EndPointer is not NULL and Address is translated from String, a point= er 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] =20 - 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(). =20 @param String Pointer to a Null-terminated ASCII stri= ng. @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. =20 - If String is NULL, then ASSERT(). + If String is NULL, then ASSERT_CONSTRAINT(). =20 - If Buffer is NULL, then ASSERT(). + If Buffer is NULL, then ASSERT_CONSTRAINT(). =20 - If Length is not multiple of 2, then ASSERT(). + If Length is not multiple of 2, then ASSERT_CONSTRAINT(). =20 If PcdMaximumAsciiStringLength is not zero and Length is greater than - PcdMaximumAsciiStringLength, then ASSERT(). + PcdMaximumAsciiStringLength, then ASSERT_CONSTRAINT(). =20 - If MaxBufferSize is less than (Length / 2), then ASSERT(). + If MaxBufferSize is less than (Length / 2), then ASSERT_CONSTRAINT(). =20 @param String Pointer to a Null-terminated ASCII stri= ng. @param Length The number of ASCII characters to decod= e. @@ -2632,7 +2632,7 @@ AsciiStrToUnicodeStr ( equal or greater than ((AsciiStrLen (Source) + 1) * sizeof (CHAR16)) in = bytes. =20 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_CONSTR= AINT(). =20 If an error is returned, then the Destination is unmodified. =20 @@ -2678,7 +2678,7 @@ AsciiStrToUnicodeStrS ( ((MIN(AsciiStrLen(Source), Length) + 1) * sizeof (CHAR8)) in bytes. =20 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_CONSTR= AINT(). =20 If an error is returned, then Destination and DestinationLength are unmodified. diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/Deb= ugLib.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__ =20 +#include + // // 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 =20 // // Declare bits for PcdDebugPrintErrorLevel and the ErrorLevel parameter o= f DebugPrint() @@ -71,6 +74,92 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define EFI_D_VERBOSE DEBUG_VERBOSE #define EFI_D_ERROR DEBUG_ERROR =20 + +/** + 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 PcdDebug= ProperyMask is set. + @retval FALSE The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of PcdDebug= ProperyMask is clear. + +**/ +#define DEBUG_ASSERT_ENABLED() \ + ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_ASSER= T_ENABLED) !=3D 0)) + + +/** + Returns TRUE if DEBUG() macros are enabled. + + This macro evaluates to TRUE if the DEBUG_PROPERTY_DEBUG_PRINT_ENABLED b= it of + PcdDebugProperyMask is set. Otherwise, FALSE is returned. + + @retval TRUE The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of PcdDebugP= roperyMask is set. + @retval FALSE The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of PcdDebugP= roperyMask is clear. + +**/ +#define DEBUG_PRINT_ENABLED() \ + ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_PRINT= _ENABLED) !=3D 0)) + + +/** + Returns TRUE if DEBUG_CODE() macros are enabled. + + This macro evaluates to TRUE if the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bi= t of + PcdDebugProperyMask is set. Otherwise, FALSE is returned. + + @retval TRUE The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of PcdDebugPr= operyMask is set. + @retval FALSE The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of PcdDebugPr= operyMask is clear. + +**/ +#define DEBUG_CODE_ENABLED() \ + ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_CODE_= ENABLED) !=3D 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 PcdDebug= ProperyMask is set. + @retval FALSE The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of PcdDebug= ProperyMask is clear. + +**/ +#define DEBUG_CLEAR_MEMORY_ENABLED() \ + ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_CLEAR_MEMOR= Y_ENABLED) !=3D 0)) + + +/** + Returns TRUE if ASSERT_CONSTRAINT() macros are enabled. + + This macro evaluates to TRUE if the DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENA= BLED bit of + PcdDebugProperyMask is set. Otherwise, FALSE is returned. + + @retval TRUE The DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of Pcd= DebugProperyMask is set. + @retval FALSE The DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of Pcd= DebugProperyMask is clear. + +**/ +#define DEBUG_ASSERT_CONSTRAINT_ENABLED() \ + ((BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_CONS= TRAINT_ENABLED) !=3D 0)) + + +/** + Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixe= dDebugPrintErrorLevel. + + This macro compares the bit mask of ErrorLevel and PcdFixedDebugPrintErr= orLevel. + + @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)) != =3D 0)) + + /** Prints a debug message to the debug output device if the specified error= level is enabled. =20 @@ -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 ( =20 **/ #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 =20 + +/** + Macro that calls ASSERT when constrain assertions are enabled. + + If DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED bit of PcdDebugProperyMask i= s set, + then this macro evaluates to an ASSERT macro passing in the original Exp= ression. + + @param Expression Boolean expression. + +**/ +#define ASSERT_CONSTRAINT(Expression) \ + do { \ + if (DEBUG_ASSERT_CONSTRAINT_ENABLED ()) { \ + ASSERT (Expression); \ + } \ + } while (FALSE) + + /** Macro that calls DebugPrint(). =20 @@ -356,11 +463,11 @@ DebugPrintLevelEnabled ( =20 **/ #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 =3D %r)\n", Sta= tusParameter)); \ _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 =3D %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 !=3D NULL); = \ if (Handle =3D=3D NULL) { = \ @@ -471,7 +578,7 @@ DebugPrintLevelEnabled ( are not included in a module. =20 **/ -#define DEBUG_CODE_BEGIN() do { if (DebugCodeEnabled ()) { UINT8 __Debug= CodeLocal +#define DEBUG_CODE_BEGIN() do { if (DEBUG_CODE_ENABLED ()) { UINT8 __Deb= ugCodeLocal =20 =20 /** @@ -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 ( =20 **/ #if !defined(MDEPKG_NDEBUG) - #define CR(Record, TYPE, Field, TestSignature) = \ - (DebugAssertEnabled () && (BASE_CR (Record, TYPE, Field)->Signature != =3D TestSignature)) ? \ - (TYPE *) (_ASSERT (CR has Bad Signature), Record) : = \ + #define CR(Record, TYPE, Field, TestSignature) = \ + (DEBUG_ASSERT_ENABLED () && (BASE_CR (Record, TYPE, Field)->Signature = !=3D 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 =20 diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/Uefi= Lib.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 !=3D NULL); \ if ((LockParameter)->Lock !=3D EfiLockAcquired) { \ _ASSERT (LockParameter not locked); \ diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/S= afeString.c index 7dc03d2caa..f6cdd76c82 100644 --- a/MdePkg/Library/BaseLib/SafeString.c +++ b/MdePkg/Library/BaseLib/SafeString.c @@ -14,7 +14,7 @@ =20 #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.
\n" = "BIT3 - Enable Clear Memory.
\n" = "BIT4 - Enable BreakPoint as ASSERT.
\n" - = "BIT5 - Enable DeadLoop as ASSERT.
" + = "BIT5 - Enable DeadLoop as ASSERT.
\n" + = "BIT6 - Treat constrait violations as ASSERT.
" =20 #string STR_gEfiMdePkgTokenSpaceGuid_ERR_80000002 #language en-US "Reserve= d bits must be set to zero." =20 --Apple-Mail=_658134D3-16F9-4316-BF4A-CFC053835748-- --Apple-Mail=_C41DDFE7-698E-4E75-A0A8-510D0C9DFF0F-- --Apple-Mail=_09089207-DDB0-4EB2-8DA7-35CEDC1005E1 Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEsLABAI5Y5VbvBdmpL8K2O86Eyz4FAl5yeCoACgkQL8K2O86E yz5ZfxAAlyraT0IJzcMqajxEGOgHDyJGlnjcslRouoikO5jVzgpgOPwi0x2qPbPT FfISH+CxSbuWsfMZwecBJR0xqXjpqRO7huJSMuC66S3/hpULsxrJ0Lfv1C16rIab EYXoYgbqjpgD+kok/A1Xqwt7nH/G/WFM9H3Xf1PW3di3G8u3hhJJx7uhAoH1rvc2 gWsK2OM/MqF3x7Ul2tck7qujzUfzJYa8hTNm6OGmrfnASYTnw7oQO3O6nTKtbHNA ulmv0pqmbmADqqDjUWT9spTr/wXRL4WSQ4DW87VBqBlzSvWy/iwhRfD8T8tEVeL/ Vw3p6ey/9kp6y3KT1w2HQpUAIkeyAJOs7JRBusIdPl0iuIHhA0fbpwcq6o15zmKm A5oSwcXZtGh2fgjLhBSYBI3r4AzOvF0Vyq4zy44guxloYAgTAKPrE95lRzSKhL8t 2vo89E8Z47J3U3jqN2IFzHBr+DsmwqMDoDs4fJGgUIOazf4x4A/IVdc9BEdnLRJE IwQkcsRCN4GtPvbo6oObKCP04jmSOBuqQArPTLpdBClBVp5J4WY2rfI72vroW1sj nEmcr00/KVNgpTjmhTFXviiiwois01avB93m7QZbFmb9/CgrJ5+AeeM/g/fzd6dK aTDQvBAPQ95z9KGIm2ca7lHr2KUuoy2dTyFz8nHMRoUpDE/BfTI= =8qwJ -----END PGP SIGNATURE----- --Apple-Mail=_09089207-DDB0-4EB2-8DA7-35CEDC1005E1--