From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp03.apple.com (ma1-aaemail-dr-lapp03.apple.com [17.171.2.72]) by mx.groups.io with SMTP id smtpd.web10.139.1584568395137375607 for ; Wed, 18 Mar 2020 14:53:15 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=nLw5lb2L; spf=pass (domain: apple.com, ip: 17.171.2.72, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp03.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp03.apple.com (8.16.0.27/8.16.0.27) with SMTP id 02ILqgnp049319; Wed, 18 Mar 2020 14:53:13 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=8n+D/xWUA998380NU9G4XXeuBK4PMb32i9iVRf6isXk=; b=nLw5lb2LkRn2H/86xfdztzccz1hD9FZwuWhMlJr69bX6hgpa2Sy51yaoG+bTe/Xq6JMw Zr7cEL4F6qgZys0whtqnhudaQFnLt+2/2TKQphrEADh1XUJaoWocqqJOQ7uQiam0X+jU 8kKD2VwhgHMvFFDNRFoZkqFjL1lT6NaeA+EdVHqFrek7pB7FtIXjIocllzvBB3Iwa7r+ /UQ7xUpWz1mwX6YHWtO3zJFk6OVHEZ2TKUZEqbVnUiE4I4l2SNtG8BVX7SEN82avGt6v 9GqQwtwwo3IUWiFwXovUUOmNsnkZD9eCOtSF3tZsS5Mv2Kl3wuzz6MckaFZLZI/lvzsP CA== Received: from rn-mailsvcp-mta-lapp02.rno.apple.com (rn-mailsvcp-mta-lapp02.rno.apple.com [10.225.203.150]) by ma1-aaemail-dr-lapp03.apple.com with ESMTP id 2yrx8wn2hh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 18 Mar 2020 14:53:13 -0700 Received: from nwk-mmpp-sz13.apple.com (nwk-mmpp-sz13.apple.com [17.128.115.216]) by rn-mailsvcp-mta-lapp02.rno.apple.com (Oracle Communications Messaging Server 8.1.0.5.20200312 64bit (built Mar 12 2020)) with ESMTPS id <0Q7E00WF3SSONL20@rn-mailsvcp-mta-lapp02.rno.apple.com>; Wed, 18 Mar 2020 14:53:12 -0700 (PDT) Received: from process_milters-daemon.nwk-mmpp-sz13.apple.com by nwk-mmpp-sz13.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) id <0Q7E00H00SMNFC00@nwk-mmpp-sz13.apple.com>; Wed, 18 Mar 2020 14:53:12 -0700 (PDT) X-Va-A: X-Va-T-CD: 2c323b19c9bf43c24bf8f3f2bb9cbf14 X-Va-E-CD: 5fe8742df1f24b2a63953342b8aec60f X-Va-R-CD: a28cdce158fb57d7593fd497a2f3bfd9 X-Va-CD: 0 X-Va-ID: c3247d20-fee6-4744-a8e1-09f750e19b41 X-V-A: X-V-T-CD: 2c323b19c9bf43c24bf8f3f2bb9cbf14 X-V-E-CD: 5fe8742df1f24b2a63953342b8aec60f X-V-R-CD: a28cdce158fb57d7593fd497a2f3bfd9 X-V-CD: 0 X-V-ID: f92caa8a-4031-4b33-99db-f080da3b949b X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.645 definitions=2020-03-18_07:2020-03-18,2020-03-18 signatures=0 Received: from [17.235.2.62] by nwk-mmpp-sz13.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPSA id <0Q7E00KFISSLOY80@nwk-mmpp-sz13.apple.com>; Wed, 18 Mar 2020 14:53:11 -0700 (PDT) Sender: afish@apple.com From: "Andrew Fish" Message-id: <4219F456-E97B-403F-80D1-07EA07CE8549@apple.com> MIME-version: 1.0 (Mac OS X Mail 13.0 \(3594.4.17\)) Subject: Re: [edk2-devel] Disabling safe string constraint assertions Date: Wed, 18 Mar 2020 14:53:09 -0700 In-reply-to: <7867AB09-6EB0-4EC6-93B4-0874DD3B1EB3@ispras.ru> Cc: Mike Kinney , Laszlo Ersek , =?utf-8?Q?Marvin_H=C3=A4user?= , "Gao, Liming" , "Gao, Zhichao" To: devel@edk2.groups.io, cheptsov@ispras.ru References: <7867AB09-6EB0-4EC6-93B4-0874DD3B1EB3@ispras.ru> X-Mailer: Apple Mail (2.3594.4.17) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.645 definitions=2020-03-18_07:2020-03-18,2020-03-18 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_DAAAC4AA-C299-4504-AC29-C90895DAF60C" --Apple-Mail=_DAAAC4AA-C299-4504-AC29-C90895DAF60C Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Vitaly, The library object files can be shared between modules. If is possible to = override PCD settings per module in the DSC file. So libraries need to eith= er derive their PCD value from the driver/app they are linking with, or we = would need to build different instances of the library with the different P= CD defaults and link the correct one. The build system does not support bui= lding extra copies of the libraries so we have the restriction Mike mention= ed.=20 https://github.com/tianocore/edk2/blob/master/OvmfPkg/OvmfPkgX64.dsc#L856 ShellPkg/Application/Shell/Shell.inf { ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellComman= dLib.inf NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comm= andsLib.inf NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Comm= andsLib.inf NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Comm= andsLib.inf NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Co= mmandsLib.inf NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comm= andsLib.inf NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1= CommandsLib.inf NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1= CommandsLib.inf !if $(NETWORK_IP6_ENABLE) =3D=3D TRUE NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2= CommandsLib.inf !endif HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandlePar= singLib.inf PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcf= gCommandLib.inf gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000 } Thanks, Andrew Fish > On Mar 18, 2020, at 2:31 PM, Vitaly Cheptsov wrote: >=20 > Mike, >=20 > That explains the current behaviour, but makes me even more confused. >=20 > I do not really understand how DEC format is responsible for this. Libra= ries, described with INF files, consume PCDs and potentially override their= values. DEC files produce PCDs, which libraries or modules (drivers, appic= ations) can consume. Header-only libraries have no INF files, and thus are = not really libraries one can depend on, and thus can have no PCDs. I cannot= make a connection of how a library consuming a PCD could influence on a DE= C file. >=20 > BaseTools' AutoGen implements DependentLibraryList and LibraryPcdList pr= operties, which effectively gather all library PCDs for a module. So they a= lready have all the information about the PCDs used and needed to be added = to AutoGen.c and AutoGen.h. >=20 > I expected them to add library PCD definitions to AutoGen.h for modules,= but for some reason it does not happen. They also explicitly skip PCD depe= ndency walk for libraries, which I assumed to be some questionable performa= nce optimisation before I realised that they are not exposed for the former= case as well. >=20 > It is very possible that I miss something, but to me it looks like the f= act that we cannot see library PCDs in modules and higher level libraries i= s just an artificial limitation, which should be possible to lift with reas= onably few changes in BaseTools for a person that is well aware of their co= debase. Could you give a better insight on this or perhaps ask somebody who= knows BaseTools internals? >=20 > If you believe it is much worse than I see, I can just trust you for the= time being and focus on implementing an alternative approach by separating= a common DebugCommonLib. >=20 > Thanks! >=20 > Best regards, > Vitaly >=20 >> On 18 Mar 2020, at 23:55, Kinney, Michael D wrote: >>=20 >> =EF=BB=BF >> Vitaly, >> >> It has to do with where PCDs are declared in INF files. >> >> If you access a PCD from a macro like you have added to a library class= , the module using that library class does not know there is a macro that u= ses a PCD. So the PCD declaration in the Module INF is missing. By only u= sing the PCDs from the library implementation, the library implementation I= NF declares the PCDs it uses and the module inherits the PCDs from the libr= ary instances. We do not have a feature that allows a library class (which= only has a .h file and a one line declaration in a DEC file) to provide ex= tra information such as PCDs that the library class uses. We would need a = significant extension to the DEC file format and build tools for a library = class declaration to provide more information. >> >> Mike >> >> From: Vitaly Cheptsov =20 >> Sent: Wednesday, March 18, 2020 1:43 PM >> To: Kinney, Michael D >> Cc: devel@edk2.groups.io; Laszlo Ersek ; Andrew Fish= ; Marvin H=C3=A4user ; Gao, Liming <= liming.gao@intel.com>; Gao, Zhichao >> Subject: Re: [edk2-devel] Disabling safe string constraint assertions >> >> Mike, >> >> Thanks for the clarification. I failed to find it in the specs, but the= code of the BaseTools kind of gave me such a suspect. >> Is there any particular reason why this limitation was added? At the mo= ment I do not see a good reason why this is done. >> >> If there is one, I guess we could consider some other approach, for exa= mple, we can factor out these functions to a separate DebugHelperLib/DebugB= aseLib/DebugCommonLib, which every DebugLib will depend on. This will make = sense to me as a workaround of such limitation, as neither us, nor Andrew, = as he mentioned previously, are happy of having to duplicate code in DebugL= ib implementations and update them for a minor Pcd change. >> >> If there is no good reason, to be honest, it feels like we should just = fix this. After reading the spec I do not see what kind of compiler issue c= ould arise here with normal PCDs. >> >> Best regards, >> Vitaly >>=20 >>=20 >> 18 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2020 =D0=B3., =D0=B2 23:35, Kinney, M= ichael D > = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >> >> Vitaly, >> >> The break you are seeing is because you are not using functions to eval= the PCD. This is a known restriction in how PCDs work between libs and mo= dules and is why the current design uses the XxxEnabled() functions. >> >> I have not reviewed this issue in a very long time, so I do not know if= there are any attributes of newer compilers that would allow a different d= esign now. >> >> Best regards, >> >> Mike >> >> From: devel@edk2.groups.io > On Behalf Of Vitaly Cheptsov >> Sent: Wednesday, March 18, 2020 12:36 PM >> To: Laszlo Ersek >; Andrew= Fish >; Kinney, Michael D >; Marvin H=C3=A4us= er >; Gao, Liming >; Gao, Zhichao > >> Cc: devel@edk2.groups.io >> Subject: Re: [edk2-devel] Disabling safe string constraint assertions >> >> Hello! >> >> I have a prototype of the patch, but there currently is an issue with t= he current EDK II build system. >> I attached the patch to this e-mail, however, it will not compile for r= easonably obscure causes. >> >> From what I understand: >> - DebugLib header now directly uses PCDs from DebugLib, like PcdDebugPr= opertyMask. >> - Any library implementing DebugLib should now depend on these PCDs, wh= ich seems fairly natural (and I fixed that in BaseDebugLibNull). >> - Any library using DebugLib header should depend on DebugLib, which al= so depend on DebugLib to get its PCDs (that already looks fine). >> >> However, for some reason DebugLib PCDs are not included in Autogen.h he= ader for other libraries some reason, and we get errors like: >> MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollecti= onRedBlackTreeLib.c:1151:9: error: use of undeclared identifier '_PCD_GET_M= ODE_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 someb= ody else handles that. >> I do not believe it is a great idea to abandon the idea of dropping Deb= ugAssertEnabled-like functions, so I suggest us to focus on resolving the b= uild system limitation rather than trying a new approach. >> >> Best regards, >> Vitaly >> >> >> >>=20 >>=20 >>=20 >> 11 =C3=90=C2=BC=C3=90=C2=B0=C3=91=E2=82=AC=C3=91=E2=80=9A=C3=90=C2=B0 2= 020 =C3=90=C2=B3., =C3=90=C2=B2 16:14, Laszlo Ersek > =C3=90=C2=BD=C3=90=C2=B0=C3=90=C2=BF=C3=90=C2=B8=C3= = =91=C2=81=C3=90=C2=B0=C3=90=C2=BB(=C3=90=C2=B0): >> >> On 03/11/20 14:09, Vitaly Cheptsov wrote: >>=20 >>=20 >> 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 informatio= n >> 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 o= n >> 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 def= ault. >>=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 >> >> >=20 --Apple-Mail=_DAAAC4AA-C299-4504-AC29-C90895DAF60C Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Vitaly,
The library object files can be shared b= etween modules. If is possible to override PCD settings per module in the D= SC file. So libraries need to either derive their PCD value from the driver= /app they are linking with, or we would need to build different instances o= f the library with the different PCD defaults and link the correct one. The= build system does not support building extra copies of the libraries so we= have the restriction Mike mentioned. 

https://github.com/tiano= core/edk2/blob/master/OvmfPkg/OvmfPkgX64.dsc#L856
= =
ShellPkg/Application/Shell/Shell.i= nf {
= <LibraryClasses>
= ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLi= b.inf
= NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command= sLib.inf
= NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Command= sLib.inf
= NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Command= sLib.inf
= NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Comma= ndsLib.inf
= NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Command= sLib.inf
= NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1Com= mandsLib.inf
= NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com= mandsLib.inf
!i= f $(NETWORK_IP6_ENABLE) =3D=3D TRUE
= NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Com= mandsLib.inf
!e= ndif
= HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsin= gLib.inf
= PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
= BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCo= mmandLib.inf
= <PcdsFixedAtBuild>
= gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
= gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
= gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
= }


Thanks,

Andrew Fish

On Ma= r 18, 2020, at 2:31 PM, Vitaly Cheptsov <cheptsov@ispras.ru> wrote:

Mike,

That explains the current behaviour, but makes me even more confused.

<= div dir=3D"ltr" style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica;= font-size: 12px; font-style: normal; font-variant-caps: normal; font-weigh= t: normal; letter-spacing: normal; text-align: start; text-indent: 0px; tex= t-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-str= oke-width: 0px; text-decoration: none;" class=3D"">I do not really understa= nd how DEC format is responsible for this. Libraries, described with INF fi= les, consume PCDs and potentially override their values. DEC files produce = PCDs, which libraries or modules (drivers, appications) can consume. Header= -only libraries have no INF files, and thus are not really libraries one ca= n depend on, and thus can have no PCDs. I cannot make a connection of how a= library consuming a PCD could influence on a DEC file.

BaseTools' AutoGen implements Dependent= LibraryList and LibraryPcdList properties, which effectively gather all lib= rary PCDs for a module. So they already have all the information about the = PCDs used and needed to be added to AutoGen.c and AutoGen.h.

I expected them to add library= PCD definitions to AutoGen.h for modules, but for some reason it does not = happen. They also explicitly skip PCD dependency walk for libraries, which = I assumed to be some questionable performance optimisation before I realise= d that they are not exposed for the former case as well.

It is very possible that I miss someth= ing, but to me it looks like the fact that we cannot see library PCDs in mo= dules and higher level libraries is just an artificial limitation, which sh= ould be possible to lift with reasonably few changes in BaseTools for a per= son that is well aware of their codebase. Could you give a better insight o= n this or perhaps ask somebody who knows BaseTools internals?

If you believe it is much wors= e than I see, I can just trust you for the time being and focus on implemen= ting an alternative approach by separating a common DebugCommonLib.

Thanks!

Best regards,
Vitaly

On= 18 Mar 2020, at 23:55, Kinney, Michael D <michael.d.kinney@intel.com> wrote:

=EF=BB=BF
Vitaly,
 
It has to do with where PCDs are declared in INF files.
 
If you access a PCD from a macro like you have added to a library class,= the module using that library class does not know there is a macro that us= es a PCD.  = ;So the PCD declaration in the Module INF is missing.  By= only using the PCDs from the library implementation, the library implement= ation INF declares the PCDs it uses and the module inherits the PCDs from t= he library instances.  We do not have a feature that allows a library c= lass (which only has a .h file and a one line declaration in a DEC file) to= provide extra information such as PCDs that the library class uses.  W= e would need a significant extension to the DEC file format and build tools= for a library class declaration to provide more information.
 
Mike
 
= From: Vitaly Cheptsov <cheptsov@ispras.ru> 
Sent:=  Wednesday, March 18, 202= 0 1:43 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>
<= b class=3D"">Cc: devel@edk2.groups.io; La= szlo Ersek <lersek@redha= t.com>; Andrew Fish <afish@apple.com>; Marvin H=C3=A4user <mhaeuser@outlook.de>; Gao, Liming <liming.gao@intel.com>;= Gao, Zhichao <zhich= ao.gao@intel.com>
Subject: Re: [edk2-devel] Disabling safe st= ring constraint assertions
 
Mike,
&nb= sp;
Thanks for the clarification. I failed to find it in the spec= s, but the code of the BaseTools kind of gave me such a suspect.
Is there any particular reason why this limitation was ad= ded? At the moment I do not see a good reason why this is done.
 
If there is one, I gues= s we could consider some other approach, for example, we can factor out the= se functions to a separate DebugHelperLib/DebugBaseLib/DebugCommonLib, whic= h every DebugLib will depend on. This will make sense to me as a workaround= of such limitation, as neither us, nor Andrew, as he mentioned previously,= are happy of having to duplicate code in DebugLib implementations and upda= te them for a minor Pcd change.
&= nbsp;
<= span class=3D"">If there is no good reason, to be honest, it feels like we = should just fix this. After reading the spec I do not see what kind of comp= iler issue could arise here with normal PCDs.<= /div>
 
Best regards,
Vital= y


18 = =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2020 =D0=B3., =D0=B2 23:35, Kinney, Michael= D <michael.d.kinney@intel.com>= ; =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):=
 
Vitaly,
 = ;
The break you are seeing is because you are not= using functions to eval the PCD.  This is a known restriction in how PCDs work between libs an= d modules and is why the current design uses the XxxEnabled() function= s.
 
I have not re= viewed this issue in a very long time, so I do not know if there are any at= tributes of newer compilers that would allow a different design now.
 
Best regards,
 
Mike
 
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly Cheptsov
Sent: Wednesd= ay, March 18, 2020 12:36 PM
To: Laszlo Ersek <lersek@redhat.com= >; Andrew Fish <afish@apple.com>; Kinney, Michael D <mi= chael.d.kinney@intel.com>; Marvin H=C3=A4user <mhaeuser@outlook= .de>; Gao, Liming <liming.gao@intel.com>; Gao, Z= hichao <zhichao.gao@intel.com>
Cc= : devel@edk2.groups.io
Subject: Re: [edk2-devel] Disabling safe string constraint = assertions
 <= /o:p>
Hello!
 
I have a prot= otype of the patch, but there currently is an issue with the current EDK II= build system.
I attached t= he patch to this e-mail, however, it will not compile for reasonably obscur= e causes.
 
From what I understand:
- DebugLib header now directly=  uses PCDs from DebugLib, like PcdDebugPropertyMask.
- Any library implementing DebugLib should n= ow depend on these PCDs, which seems fairly natural (and I fixed that in Ba= seDebugLibNull).
- Any l= ibrary using DebugLib header should depend on DebugLib, which also depend o= n DebugLib to get its PCDs (that already looks fine).=
 
<= /div>
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/BaseOrderedCollectio= nRedBlackTreeLib/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 b= e great if somebody else handles that.
I do not believe it is a great idea to abandon the idea of drop= ping DebugAssertEnabled-like functions, so I suggest us to focus on re= solving the build system limitation rather than trying a new approach.
<= div style=3D"margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibr= i, sans-serif;" class=3D""> 
Best regards,
Vitaly
 <= o:p class=3D"">
 
 
=



11 =C3= = =90=C2=BC=C3=90=C2=B0=C3=91=E2=82=AC=C3=91=E2=80=9A=C3=90=C2=B0 2020 =C3= =90=C2=B3., =C3=90=C2=B2 16:14, Laszlo Ersek <lersek@redhat.com>= =C3=90=C2=BD=C3=90=C2=B0=C3=90=C2=BF=C3=90=C2=B8=C3=91=C2=81=C3=90=C2=B0= =C3=90=C2=BB(=C3=90=C2=B0):
=
 
On 03/11/20 14:09, Vitaly Cheptsov wrote:


Hi ev= eryone,

So, I believe that by now we mostly ag= reed to let the original
proposition land as a short-term sol= ution. We end up with:

1. A PCD condition with= in SAFE_STRING_COSTRAINT_CHECK macro.
2. Make this condition = evaluate to TRUE by default (i.e. ASSERT).
3. Update document= ation 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 PcdD= ebugPropertyMask. I believe that we almost agreed on
two thin= gs:

1. Adding an extra bit to PcdDebugProperty= Mask is cleaner.
2. Extending DebugLib interface with a new f= unction is not a good idea.

Therefore I sugges= t:

1.Add #define DEBUG_PROPERTY_ASSERT_CONSTRA= INT_ENABLED 0x40.
2. Create header-only macros to replace fun= ctions 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 abo= ut any particular deprecation timeline, but to me the above
c= ertainly 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=_DAAAC4AA-C299-4504-AC29-C90895DAF60C--