Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Add PCD to disable safe string constraint assertions

["Yao, Jiewen" <jiewen.yao@intel.com>]

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.gao@intel.com>
> Sent: Monday, October 21, 2019 11:17 AM
> To: devel@edk2.groups.io; vit9696@protonmail.com
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@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 <vit9696@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+owner@edk2.groups.io
> >Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com]
> >-=-=-=-=-=-=