[edk2-devel] Disabling safe string constraint assertions

[Vitaly Cheptsov]

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 at 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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55961): https://edk2.groups.io/g/devel/message/55961
Mute This Topic: https://groups.io/mt/71711587/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200318/9dd59ffc/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: constraint-r3.diff
Type: application/octet-stream
Size: 29430 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200318/9dd59ffc/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Message signed with OpenPGP
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200318/9dd59ffc/attachment.sig>