[edk2-staging/edk2-test Patch V2 0/2] TestFrameworkPkg/TestProfile: Fix overlapping buffer ASSERT()

[edk2-staging/edk2-test Patch V2 0/2] TestFrameworkPkg/TestProfile: Fix overlapping buffer ASSERT()

[Michael D Kinney]

New in V2
* Update the DSC files to improve debug of the TestFrameworkPkg
  and TestCasePkg.

The patch is a bug fix for TestFrameworkPkg/Support/TestProfile
driver. AsciiStrCpy() function was used to copy overlapping
strings, which triggered an ASSERT().

Cc: Andrew Fish <afish@apple.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Felix Polyudov <felixp@ami.com>

Michael D Kinney (2):
  Test*Pkg: Update DSC files to improve debug
  TestFrameworkPkg/TestProfile: Fix overlapping buffer ASSERT()

 TestCasePkg/TestCasePkg.dsc                        | 65 ++++++++++++++++++++++
 TestFrameworkPkg/Support/TestProfile/TestProfile.c |  4 +-
 TestFrameworkPkg/TestFrameworkPkg.dsc              | 62 +++++++++++++++++++++
 3 files changed, 129 insertions(+), 2 deletions(-)

-- 
2.13.1.windows.2

[edk2-staging/edk2-test Patch V2 1/2] Test*Pkg: Update DSC files to improve debug

[Michael D Kinney]

Update the DSC files to improve debug of the TestFrameworkPkg
and TestCasePkg.

* If -b DEBUG build, then enable DEBUG() messages on the standard
  error console device with messages of type DEBUG_ERROR,
  DEBUG_VERBOSE, DEBUG_INFO, DEBUG_WARN, and DEBUG_INIT enabled.
* If -b DEBUG build, then enable ASSERT() macros and configure
  ASSERT() macros to generate a break point when triggered.
* If -b DEBUG build, then generated mixed C/ASM output for MSFT
  builds to help find and fix source code that causes memcpy() and
  memset() instrinsics to be used.
* If -b RELEASE build, then disable all DEBUG() messages and
  ASSERT() macros and do not generated mixed C/ASM output files.
* Add NT32 define that builds the TestFrameworkPkg and TestCasePkg
  components so they are compatible with NT32 environment debuggers.

Cc: Felix Polyudov <felixp@ami.com>
Cc: Andrew Fish <afish@apple.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Kinney <michael.d.kinney@intel.com>
---
 TestCasePkg/TestCasePkg.dsc           | 65 +++++++++++++++++++++++++++++++++++
 TestFrameworkPkg/TestFrameworkPkg.dsc | 62 +++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)

diff --git a/TestCasePkg/TestCasePkg.dsc b/TestCasePkg/TestCasePkg.dsc
index c2f1832ae..e7e32d4e0 100644
--- a/TestCasePkg/TestCasePkg.dsc
+++ b/TestCasePkg/TestCasePkg.dsc
@@ -24,6 +24,18 @@
   SKUID_IDENTIFIER               = DEFAULT
   POSTBUILD                      = TestCasePkg/GenFramework.cmd
 
+  #
+  # Platform On/Off features are defined here
+  #
+  DEFINE LOGGING              = FALSE
+  DEFINE SOURCE_DEBUG_ENABLE  = FALSE
+  DEFINE NT32                 = FALSE
+
+  !if $(TARGET) == "DEBUG"
+    DEFINE LOGGING             = TRUE
+    DEFINE SOURCE_DEBUG_ENABLE = TRUE
+  !endif
+
 [LibraryClasses]
   UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
@@ -39,11 +51,64 @@
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
+!if $(LOGGING)
   DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
+!else
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+!endif
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
   PostCodeLib|MdePkg/Library/BasePostCodeLibPort80/BasePostCodeLibPort80.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   EfiTestLib|TestFrameworkPkg/Library/EfiTestLib/EfiTestLib.inf
 
+[PcdsFixedAtBuild]
+!if $(LOGGING)
+  !if $(SOURCE_DEBUG_ENABLE)
+    #
+    # Enabled ASSERT(), DEBUG(), and DEBUG_CODE() and configure ASSERT() to
+    # generate a breakpoint.
+    #
+    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x17
+  !else
+    #
+    # Enabled ASSERT(), DEBUG(), and DEBUG_CODE() and configure ASSERT() to
+    # generate a deadloop.
+    #
+    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x27
+  !endif
+!else
+  #
+  # Disable ASSERT(), DEBUG(), DEBUG_CODE(), and DEBUG_CLEAR_MEMORY()
+  #
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0
+!endif
+
+[PcdsPatchableInModule]
+!if $(LOGGING)
+  #
+  # Enable DEBUG() messages of type DEBUG_ERROR, DEBUG_VERBOSE, DEBUG_INFO,
+  # DEBUG_WARN, and DEBUG_INIT
+  #
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80400043
+!endif
+
 [Components]
   TestCasePkg/Timer/ArchTimerBBTest.inf
+
+[BuildOptions]
+!if $(TARGET) == "DEBUG"
+  #
+  # Generate mixed C/ASM files for debug builds
+  #
+  MSFT:*_*_*_CC_FLAGS = /FAsc
+!endif
+
+!if $(NT32)
+  #
+  # If -D NT32 is set on command line to build, then build all components to be
+  # compatible with NT32 environment debuggers.
+  #
+  DEBUG_*_*_DLINK_FLAGS   = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000 /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
+  NOOPT_*_*_DLINK_FLAGS   = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000 /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
+  RELEASE_*_*_DLINK_FLAGS = /ALIGN:4096 /FILEALIGN:4096
+!endif
diff --git a/TestFrameworkPkg/TestFrameworkPkg.dsc b/TestFrameworkPkg/TestFrameworkPkg.dsc
index 93c8886af..ebf866e39 100644
--- a/TestFrameworkPkg/TestFrameworkPkg.dsc
+++ b/TestFrameworkPkg/TestFrameworkPkg.dsc
@@ -24,6 +24,18 @@
   SKUID_IDENTIFIER               = DEFAULT
   POSTBUILD                      = TestFrameworkPkg/GenFramework.cmd
 
+  #
+  # Platform On/Off features are defined here
+  #
+  DEFINE LOGGING              = FALSE
+  DEFINE SOURCE_DEBUG_ENABLE  = FALSE
+  DEFINE NT32                 = FALSE
+
+  !if $(TARGET) == "DEBUG"
+    DEFINE LOGGING             = TRUE
+    DEFINE SOURCE_DEBUG_ENABLE = TRUE
+  !endif
+
 [LibraryClasses]
   UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
@@ -39,7 +51,11 @@
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
+!if $(LOGGING)
   DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
+!else
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+!endif
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
   PostCodeLib|MdePkg/Library/BasePostCodeLibPort80/BasePostCodeLibPort80.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
@@ -56,6 +72,37 @@
   EntsLib|TestFrameworkPkg/Library/EasLib/EntsLib.inf
   EfiTestLib|TestFrameworkPkg/Library/EfiTestLib/EfiTestLib.inf
 
+[PcdsFixedAtBuild]
+!if $(LOGGING)
+  !if $(SOURCE_DEBUG_ENABLE)
+    #
+    # Enabled ASSERT(), DEBUG(), and DEBUG_CODE() and configure ASSERT() to
+    # generate a breakpoint.
+    #
+    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x17
+  !else
+    #
+    # Enabled ASSERT(), DEBUG(), and DEBUG_CODE() and configure ASSERT() to
+    # generate a deadloop.
+    #
+    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x27
+  !endif
+!else
+  #
+  # Disable ASSERT(), DEBUG(), DEBUG_CODE(), and DEBUG_CLEAR_MEMORY()
+  #
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0
+!endif
+
+[PcdsPatchableInModule]
+!if $(LOGGING)
+  #
+  # Enable DEBUG() messages of type DEBUG_ERROR, DEBUG_VERBOSE, DEBUG_INFO,
+  # DEBUG_WARN, and DEBUG_INIT
+  #
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80400043
+!endif
+
 [Components]
   TestFrameworkPkg/Library/EasLib/EntsLib.inf
   TestFrameworkPkg/Library/EfiTestLib/EfiTestLib.inf
@@ -79,4 +126,19 @@
   TestFrameworkPkg/PeiSctManageModule/PeiSctManageModule.inf
 
 [BuildOptions]
+!if $(TARGET) == "DEBUG"
+  #
+  # Generate mixed C/ASM files for debug builds
+  #
   MSFT:*_*_*_CC_FLAGS = /FAsc
+!endif
+
+!if $(NT32)
+  #
+  # If -D NT32 is set on command line to build, then build all components to be
+  # compatible with NT32 environment debuggers.
+  #
+  DEBUG_*_*_DLINK_FLAGS   = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000 /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
+  NOOPT_*_*_DLINK_FLAGS   = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000 /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
+  RELEASE_*_*_DLINK_FLAGS = /ALIGN:4096 /FILEALIGN:4096
+!endif
-- 
2.13.1.windows.2

[edk2-staging/edk2-test Patch V2 2/2] TestFrameworkPkg/TestProfile: Fix overlapping buffer ASSERT()

[Michael D Kinney]

The patch is a bug fix for TestFrameworkPkg/Support/TestProfile
driver. AsciiStrCpy() function was used to copy overlapping
strings, which triggered an ASSERT().

The function was used on overlapping strings in a couple of places:
1. _alltrim(): AsciiStrCpy() is replaced with CopyMem(), which can
   handle overlapping buffers.
2. _prosessLine(): AsciiStrCpy() at the beginning of the function
   is removed. The call was redundant. The function was trying to
   move the data that have already been moved by _alltrim().

Cc: Andrew Fish <afish@apple.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Felix Polyudov <felixp@ami.com>
---
 TestFrameworkPkg/Support/TestProfile/TestProfile.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/TestFrameworkPkg/Support/TestProfile/TestProfile.c b/TestFrameworkPkg/Support/TestProfile/TestProfile.c
index f02e9dfef..2476c5740 100644
--- a/TestFrameworkPkg/Support/TestProfile/TestProfile.c
+++ b/TestFrameworkPkg/Support/TestProfile/TestProfile.c
@@ -269,7 +269,7 @@ Routine Description:
   }
 
   tmp[Index] = '\0';
-  AsciiStrCpy (ptrStr, tmp);
+  CopyMem(ptrStr, tmp, Index + 1);
 
   return ptrStr;
 }
@@ -419,7 +419,7 @@ Routine Description:
   CHAR8 ptrValue[MAX_STRING_LEN + 1];
   INI   *ptrItem;
 
-  AsciiStrCpy (ptrLine, _alltrim (ptrLine));
+  _alltrim (ptrLine);
 
   if (*ptrLine == '#') {
     // it's a comment line
-- 
2.13.1.windows.2

Re: [edk2-staging/edk2-test Patch V2 0/2] TestFrameworkPkg/TestProfile: Fix overlapping buffer ASSERT()

[Felix Poludov]

The patch looks fine.
Reviewed-by: Felix Polyudov <felixp@ami.com>

-----Original Message-----
From: Michael D Kinney [mailto:michael.d.kinney@intel.com]
Sent: Monday, August 14, 2017 11:57 PM
To: edk2-devel@lists.01.org
Cc: Andrew Fish; Michael Kinney; Felix Poludov
Subject: [edk2-staging/edk2-test Patch V2 0/2] TestFrameworkPkg/TestProfile: Fix overlapping buffer ASSERT()

New in V2
* Update the DSC files to improve debug of the TestFrameworkPkg
  and TestCasePkg.

The patch is a bug fix for TestFrameworkPkg/Support/TestProfile
driver. AsciiStrCpy() function was used to copy overlapping strings, which triggered an ASSERT().

Cc: Andrew Fish <afish@apple.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Felix Polyudov <felixp@ami.com>

Michael D Kinney (2):
  Test*Pkg: Update DSC files to improve debug
  TestFrameworkPkg/TestProfile: Fix overlapping buffer ASSERT()

 TestCasePkg/TestCasePkg.dsc                        | 65 ++++++++++++++++++++++
 TestFrameworkPkg/Support/TestProfile/TestProfile.c |  4 +-
 TestFrameworkPkg/TestFrameworkPkg.dsc              | 62 +++++++++++++++++++++
 3 files changed, 129 insertions(+), 2 deletions(-)

--
2.13.1.windows.2


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends, Inc.  This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited.  Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.