Hi Mike I remember we have discussed it before. The general concern is that: how many additional PCD we need introduce, to control different ASSERT in different modules ?
We may want to enable *some* assert in some modules, but disable *some other* assert. E.g. the assert for linkedlist in base lib is another example... What is your thought? Thank you Yao Jiewen > -----Original Message----- > From: Gao, Liming <liming....@intel.com> > Sent: Monday, October 21, 2019 11:17 AM > To: devel@edk2.groups.io; vit9...@protonmail.com > Cc: Yao, Jiewen <jiewen....@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; > Gao, Liming <liming....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com> > Subject: RE: [edk2-devel] [PATCH v1 1/1] MdePkg: Add PCD to disable safe > string > constraint assertions > > Include more people. > > Basically, to keep the compatible behavior, PcdAssertOnSafeStringConstraints > default value should be TRUE. > The different platform can configure it. > > Thanks > Liming > >-----Original Message----- > >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > >Vitaly Cheptsov via Groups.Io > >Sent: Sunday, October 20, 2019 9:06 PM > >To: devel@edk2.groups.io > >Subject: [edk2-devel] [PATCH v1 1/1] MdePkg: Add PCD to disable safe string > >constraint assertions > > > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2054 > > > >Runtime data checks are not meant to cause debug assertions > >unless explicitly needed by some debug code (thus the PCD) > >as this breaks debug builds validating data with BaseLib. > > > >Signed-off-by: Vitaly Cheptsov <vit9...@protonmail.com>> > >--- > > MdePkg/MdePkg.dec | 6 ++++++ > > MdePkg/Library/BaseLib/BaseLib.inf | 11 ++++++----- > > MdePkg/Library/BaseLib/SafeString.c | 4 +++- > > MdePkg/MdePkg.uni | 6 ++++++ > > 4 files changed, 21 insertions(+), 6 deletions(-) > > > >diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec > >index 3fd7d1634c..dda2cdf401 100644 > >--- a/MdePkg/MdePkg.dec > >+++ b/MdePkg/MdePkg.dec > >@@ -2221,6 +2221,12 @@ [PcdsFixedAtBuild,PcdsPatchableInModule] > > # @Prompt Memory Address of GuidedExtractHandler Table. > > > >gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x10000 > >00|UINT64|0x30001015 > > > >+ ## Indicates if safe string constraint violation should assert.<BR><BR> > >+ # TRUE - Safe string constraint violation causes assertion.<BR> > >+ # FALSE - Safe string constraint violation does not cause assertion.<BR> > >+ # @Prompt Enable safe string constraint violation assertions. > >+ > >gEfiMdePkgTokenSpaceGuid.PcdAssertOnSafeStringConstraints|FALSE|BOOL > >EAN|0x0000002e > >+ > > [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > > ## This value is used to set the base address of PCI express hierarchy. > > # @Prompt PCI Express Base Address. > >diff --git a/MdePkg/Library/BaseLib/BaseLib.inf > >b/MdePkg/Library/BaseLib/BaseLib.inf > >index 3586beb0ab..bc98bc6134 100644 > >--- a/MdePkg/Library/BaseLib/BaseLib.inf > >+++ b/MdePkg/Library/BaseLib/BaseLib.inf > >@@ -390,11 +390,12 @@ [LibraryClasses] > > BaseMemoryLib > > > > [Pcd] > >- gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength ## > >SOMETIMES_CONSUMES > >- gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ## > >SOMETIMES_CONSUMES > >- gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## > >SOMETIMES_CONSUMES > >- gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask > >## SOMETIMES_CONSUMES > >- gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ## > >SOMETIMES_CONSUMES > >+ gEfiMdePkgTokenSpaceGuid.PcdAssertOnSafeStringConstraints ## > >SOMETIMES_CONSUMES > >+ gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength ## > >SOMETIMES_CONSUMES > >+ gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ## > >SOMETIMES_CONSUMES > >+ gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## > >SOMETIMES_CONSUMES > >+ gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask > >## SOMETIMES_CONSUMES > >+ gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ## > >SOMETIMES_CONSUMES > > > > [FeaturePcd] > > gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList ## CONSUMES > >diff --git a/MdePkg/Library/BaseLib/SafeString.c > >b/MdePkg/Library/BaseLib/SafeString.c > >index 7dc03d2caa..56b5e34a8d 100644 > >--- a/MdePkg/Library/BaseLib/SafeString.c > >+++ b/MdePkg/Library/BaseLib/SafeString.c > >@@ -14,7 +14,9 @@ > > > > #define SAFE_STRING_CONSTRAINT_CHECK(Expression, Status) \ > > do { \ > >- ASSERT (Expression); \ > >+ if (PcdGetBool (PcdAssertOnSafeStringConstraints)) { \ > >+ ASSERT (Expression); \ > >+ } \ > > if (!(Expression)) { \ > > return Status; \ > > } \ > >diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni > >index 5c1fa24065..425b66bb43 100644 > >--- a/MdePkg/MdePkg.uni > >+++ b/MdePkg/MdePkg.uni > >@@ -287,6 +287,12 @@ > > > > #string > >STR_gEfiMdePkgTokenSpaceGuid_PcdGuidedExtractHandlerTableAddress_H > >ELP #language en-US "This value is used to set the available memory address > >to store Guided Extract Handlers. The required memory space is decided by > >the value of PcdMaximumGuidedExtractHandler." > > > >+#string > >STR_gEfiMdePkgTokenSpaceGuid_PcdAssertOnSafeStringConstraints_PROM > >PT #language en-US "Enable safe string constraint violation assertions" > >+ > >+#string > >STR_gEfiMdePkgTokenSpaceGuid_PcdAssertOnSafeStringConstraints_HELP > >#language en-US "Indicates if safe string constraint violation should > >assert.<BR><BR>\n" > >+ > > "TRUE - Safe string constraint > >violation causes assertion.<BR>\n" > >+ > > "FALSE - Safe string constraint > >violation does not cause assertion.<BR>" > >+ > > #string > >STR_gEfiMdePkgTokenSpaceGuid_PcdPciExpressBaseAddress_PROMPT > >#language en-US "PCI Express Base Address" > > > > #string STR_gEfiMdePkgTokenSpaceGuid_PcdPciExpressBaseAddress_HELP > >#language en-US "This value is used to set the base address of PCI express > >hierarchy." > >-- > >2.21.0 (Apple Git-122) > > > > > >-=-=-=-=-=-= > >Groups.io Links: You receive all messages sent to this group. > > > >View/Reply Online (#49255): https://edk2.groups.io/g/devel/message/49255 > >Mute This Topic: https://groups.io/mt/35943317/1759384 > >Group Owner: devel+ow...@edk2.groups.io > >Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming....@intel.com] > >-=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49275): https://edk2.groups.io/g/devel/message/49275 Mute This Topic: https://groups.io/mt/35943317/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
