public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config
@ 2019-08-16  2:14 Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 01/10] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni

New in V4 (Resolve XCODE5 issues)
==================================
* Disable inline if SecGdbScriptBreak() for XCODE5 issues
* Disable XCODE5 compiler optimizations of Unix/Host
* Fix Host output location for XCODE5 X64 builds
* Update lldb scripts for XCODE5 symbiolic debugging
* Clean up BrekelyPlacketFilter.c for uninitialized variable and old debug code.
* Remove TftpDynamicCommand and LogoDxe modules from XCODE5 builds.  These 
  modules use HII section in a PE/COFF resource sections that is not currently
  supported by XCODE5 builds.
* EmulatorPkg/Sec - Move declaration of PpiArray[10] to beginning of function
  so storage is for entire lifetime of the function.  Delaractrion in the 
  middle of the function in {} cause corruption and exceptions in XCODE5 builds.
* Add -gdwarf flag to tols_def.template for XCODE5 X64 for symbolic debug.

New in V3
==========
* Fix size value used in call to AsciiStrCpyS() in PosixFileSystem.c
* Fix XCODE5 safe string function build failure in BerkleyPacketFilter.c
* Add NOOPT build target to DSC file.

New in V2
=========
* Fix size values in safe string function calls.
* Update POSIX sources to use AsciiStrCpyS() and AsciiStrCatS().
* Verify that no exceptions occur if EMU_MAGIC_PAGE() can not be mapped.  An
  error message is generated and the host app exits normally.
* Update EmulatorPkg DEC file with a new PcdPeiServicesTablePage default value
  that works for Windows/POSIX hosts for both IA32 and X64.

https://bugzilla.tianocore.org/show_bug.cgi?id=162
https://bugzilla.tianocore.org/show_bug.cgi?id=2055
https://bugzilla.tianocore.org/show_bug.cgi?id=2056

* Fix VS20xx IA32 boot failure
* Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
* Add -D DISABLE_NEW_DEPRECATED_INTERFACES

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>

Michael D Kinney (10):
  EmulatorPkg: Fix VS20xx IA32 boot failure
  EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES
  EmulatorPkg: Add support for NOOPT target
  EmulatorPkg/Unix/Host: Disable inline/optimizations
  EmulatorPkg: Fix XCODE5 lldb issues
  EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues
  EmulatorPkg: Disable TftpDynamicCommand and LogoDxe for XCODE5
  EmulatorPkg/Sec: Change local variable scope for XCODE5
  BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64

 BaseTools/Conf/tools_def.template             |   4 +-
 EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c |   9 +-
 EmulatorPkg/EmulatorPkg.dec                   |   4 +-
 EmulatorPkg/EmulatorPkg.dsc                   |  38 +++---
 EmulatorPkg/EmulatorPkg.fdf                   |   4 +
 EmulatorPkg/FlashMapPei/FlashMapPei.c         |   8 +-
 EmulatorPkg/Library/SmbiosLib/SmbiosLib.c     |   4 +-
 .../ThunkProtocolList/ThunkProtocolList.c     |  11 +-
 EmulatorPkg/Readme.md                         |   8 +-
 EmulatorPkg/Sec/Sec.c                         |  16 ++-
 EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c  |  15 +--
 EmulatorPkg/Unix/Host/Host.c                  |   3 +
 EmulatorPkg/Unix/Host/Host.inf                |   4 +-
 EmulatorPkg/Unix/Host/PosixFileSystem.c       |  80 ++++++++----
 EmulatorPkg/Unix/Host/X11GraphicsWindow.c     |   4 +-
 EmulatorPkg/Unix/lldbefi.py                   |   8 +-
 EmulatorPkg/Win/Host/WinFileSystem.c          | 116 ++++++++++++------
 EmulatorPkg/Win/VS2017/BuildVS.bat            |   2 +-
 EmulatorPkg/build.sh                          |  25 +---
 19 files changed, 227 insertions(+), 136 deletions(-)

-- 
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Patch V4 01/10] EmulatorPkg: Fix VS20xx IA32 boot failure
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 02/10] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Michael D Kinney
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni, Hao A Wu

https://bugzilla.tianocore.org/show_bug.cgi?id=2056

The IA32 build of the EmulatorPkg for VS20xx does not boot
because the default value of PCD PcdPeiServicesTablePage
rarely succeeds to be mapped on IA32 Windows Host Environments.
Change the DEC default value for this PCD to a value that
is compatible with Windows and POSIX host environments for
IA32 and X64.  For IA32 builds, this 64-bit PCD is truncated
to a 32-bit value.

PcdPeiServicesTablePage is changed from 0x1003000000 to
0x1013000000.  With this new value, no boot failures are
observed.  However, the use of this hard coded value can
potentially cause a boot failure if this address specified
by the PCD is already allocated in the user process.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
---
 EmulatorPkg/EmulatorPkg.dec | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/EmulatorPkg.dec b/EmulatorPkg/EmulatorPkg.dec
index c36f2c4186..99250d9fe5 100644
--- a/EmulatorPkg/EmulatorPkg.dec
+++ b/EmulatorPkg/EmulatorPkg.dec
@@ -2,7 +2,7 @@
 #
 # This is the Emu Emulation Environment Platform
 #
-# Copyright (c) 2008 - 2011, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
 # Portions copyright (c) 2011, Apple Inc. All rights reserved.
 #
 #    SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -66,7 +66,7 @@ [PcdsFixedAtBuild]
   gEmulatorPkgTokenSpaceGuid.PcdEmuApCount|L"0"|VOID*|0x00001019
 
   ## Magic page to implement PEI Services Table Pointer Lib
-  gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x1003000000|UINT64|0x0000101b
+  gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x1013000000|UINT64|0x0000101b
 
   ## Size of the packet filter
   gEmulatorPkgTokenSpaceGuid.PcdNetworkPacketFilterSize|524288|UINT32|0x0000101c
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 02/10] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 01/10] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 03/10] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Michael D Kinney
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni, Hao A Wu

https://bugzilla.tianocore.org/show_bug.cgi?id=2055

Remove the use of the defines UNIX_SEC_BUILD and
WIN_SEC_BUILD.  This simplifies the build command
for the EmulatorPkg.  Instead, use !if statements
in the DSC file using $(ARCH) and $(FAMILY) to
determine if the build is for a Windows or POSIX
environment.

The Readme.md, BAT, and sh files are also updated
to remove the use of these defines.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
---
 EmulatorPkg/EmulatorPkg.dsc        | 26 +++++++++++++-------------
 EmulatorPkg/Readme.md              |  8 ++++----
 EmulatorPkg/Win/VS2017/BuildVS.bat |  2 +-
 EmulatorPkg/build.sh               |  8 ++++----
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index ea8b6ce76e..153da464f1 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -4,7 +4,7 @@
 # The Emulation Platform can be used to debug individual modules, prior to creating
 # a real platform. This also provides an example for how an DSC is created.
 #
-# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 # Portions copyright (c) 2010 - 2011, Apple Inc. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -236,18 +236,18 @@ [PcdsDynamicHii.common.DEFAULT]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10
 
 [Components]
-!ifdef $(UNIX_SEC_BUILD)
-  ##
-  #  Emulator, OS POSIX application
-  ##
-  EmulatorPkg/Unix/Host/Host.inf
-!endif
-
-!ifdef $(WIN_SEC_BUILD)
-  ##
-  #  Emulator, OS WIN application
-  ##
-  EmulatorPkg/Win/Host/WinHost.inf
+!if "IA32" in $(ARCH) || "X64" in $(ARCH)
+  !if "MSFT" in $(FAMILY)
+    ##
+    #  Emulator, OS WIN application
+    ##
+    EmulatorPkg/Win/Host/WinHost.inf
+  !else
+    ##
+    #  Emulator, OS POSIX application
+    ##
+    EmulatorPkg/Unix/Host/Host.inf
+  !endif
 !endif
 
 !ifndef $(SKIP_MAIN_BUILD)
diff --git a/EmulatorPkg/Readme.md b/EmulatorPkg/Readme.md
index 461975e859..5ea61ca7ab 100644
--- a/EmulatorPkg/Readme.md
+++ b/EmulatorPkg/Readme.md
@@ -21,19 +21,19 @@ https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg
 **You can use the following command to build.**
   * 32bit emulator in Windows:
 
-    `build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 -D WIN_SEC_BUILD -a IA32`
+    `build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 -a IA32`
 
   * 64bit emulator in Windows:
 
-    `build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 -D WIN_SEC_BUILD -a X64`
+    `build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 -a X64`
 
   * 32bit emulator in Linux:
 
-    `build -p EmulatorPkg\EmulatorPkg.dsc -t GCC5 -D UNIX_SEC_BUILD -a IA32`
+    `build -p EmulatorPkg\EmulatorPkg.dsc -t GCC5 -a IA32`
 
   * 64bit emulator in Linux:
 
-    `build -p EmulatorPkg\EmulatorPkg.dsc -t GCC5 -D UNIX_SEC_BUILD -a X64`
+    `build -p EmulatorPkg\EmulatorPkg.dsc -t GCC5 -a X64`
 
 **You can start/run the emulator using the following command:**
   * 32bit emulator in Windows:
diff --git a/EmulatorPkg/Win/VS2017/BuildVS.bat b/EmulatorPkg/Win/VS2017/BuildVS.bat
index 83aebc77dc..6fcf40cc0a 100644
--- a/EmulatorPkg/Win/VS2017/BuildVS.bat
+++ b/EmulatorPkg/Win/VS2017/BuildVS.bat
@@ -1,3 +1,3 @@
 cd ../../../
 @call edksetup.bat
-build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 -D WIN_SEC_BUILD %*
+build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 %*
diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
index 2dab035ed5..60056e1b6c 100755
--- a/EmulatorPkg/build.sh
+++ b/EmulatorPkg/build.sh
@@ -233,13 +233,13 @@ fi
 
 case $CLEAN_TYPE in
   clean)
-    build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc -a $PROCESSOR -b $BUILDTARGET -t $HOST_TOOLS -D UNIX_SEC_BUILD -n 3 clean
+    build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc -a $PROCESSOR -b $BUILDTARGET -t $HOST_TOOLS -n 3 clean
     build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc -a $PROCESSOR -b $BUILDTARGET -t $TARGET_TOOLS -n 3 clean
     exit $?
     ;;
   cleanall)
     make -C $WORKSPACE/BaseTools clean
-    build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc -a $PROCESSOR -b $BUILDTARGET -t $HOST_TOOLS -D UNIX_SEC_BUILD -n 3 clean
+    build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc -a $PROCESSOR -b $BUILDTARGET -t $HOST_TOOLS -n 3 clean
     build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc -a $PROCESSOR -b $BUILDTARGET -t $TARGET_TOOLS -n 3 clean
     build -p $WORKSPACE/ShellPkg/ShellPkg.dsc -a IA32 -b $BUILDTARGET -t $TARGET_TOOLS -n 3 clean
     exit $?
@@ -251,9 +251,9 @@ esac
 # Build the edk2 EmulatorPkg
 #
 if [[ $HOST_TOOLS == $TARGET_TOOLS ]]; then
-  build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a $PROCESSOR -b $BUILDTARGET -t $TARGET_TOOLS -D BUILD_$ARCH_SIZE -D UNIX_SEC_BUILD $NETWORK_SUPPORT $BUILD_NEW_SHELL $BUILD_FAT -n 3
+  build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a $PROCESSOR -b $BUILDTARGET -t $TARGET_TOOLS -D BUILD_$ARCH_SIZE $NETWORK_SUPPORT $BUILD_NEW_SHELL $BUILD_FAT -n 3
 else
-  build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a $PROCESSOR -b $BUILDTARGET -t $HOST_TOOLS  -D BUILD_$ARCH_SIZE -D UNIX_SEC_BUILD -D SKIP_MAIN_BUILD -n 3 modules
+  build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a $PROCESSOR -b $BUILDTARGET -t $HOST_TOOLS  -D BUILD_$ARCH_SIZE -D SKIP_MAIN_BUILD -n 3 modules
   build -p $WORKSPACE/EmulatorPkg/EmulatorPkg.dsc $BUILD_OPTIONS -a $PROCESSOR -b $BUILDTARGET -t $TARGET_TOOLS -D BUILD_$ARCH_SIZE $NETWORK_SUPPORT $BUILD_NEW_SHELL $BUILD_FAT -n 3
   cp "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$HOST_TOOLS/$PROCESSOR/Host" $BUILD_ROOT_ARCH
 fi
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 03/10] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 01/10] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 02/10] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 04/10] EmulatorPkg: Add support for NOOPT target Michael D Kinney
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni, Hao A Wu

https://bugzilla.tianocore.org/show_bug.cgi?id=162

Update EmulatorPkg specific modules and libraries to use
safe string functions in BaseLib and safe PcdSetxx()
functions in PcdLib.  With these updates, the define
DISABLE_NEW_DEPRECATED_INTERFACES is enabled in the DSC
file.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
---
 EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c |   9 +-
 EmulatorPkg/EmulatorPkg.dsc                   |   6 +-
 EmulatorPkg/FlashMapPei/FlashMapPei.c         |   8 +-
 EmulatorPkg/Library/SmbiosLib/SmbiosLib.c     |   4 +-
 .../ThunkProtocolList/ThunkProtocolList.c     |  11 +-
 EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c  |  10 +-
 EmulatorPkg/Unix/Host/PosixFileSystem.c       |  80 ++++++++----
 EmulatorPkg/Unix/Host/X11GraphicsWindow.c     |   4 +-
 EmulatorPkg/Win/Host/WinFileSystem.c          | 116 ++++++++++++------
 9 files changed, 172 insertions(+), 76 deletions(-)

diff --git a/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c b/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c
index 0bf6e723a1..d8380f2be9 100644
--- a/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c
+++ b/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c
@@ -1,7 +1,7 @@
 /** @file
  Emu Bus driver
 
-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 Portions copyright (c) 2011, Apple Inc. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -256,7 +256,12 @@ EmuBusDriverBindingStart (
 
       EmuDevice->ControllerNameTable = NULL;
 
-      StrnCpy (ComponentName, EmuIoThunk->ConfigString, sizeof (ComponentName)/sizeof (CHAR16));
+      StrnCpyS (
+        ComponentName,
+        sizeof (ComponentName) / sizeof (CHAR16),
+        EmuIoThunk->ConfigString,
+        sizeof (ComponentName) / sizeof (CHAR16)
+        );
 
       EmuDevice->DevicePath = EmuBusCreateDevicePath (
                                   ParentDevicePath,
diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 153da464f1..529adfe1fa 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -408,10 +408,14 @@ [Components]
 !include NetworkPkg/Network.dsc.inc
 
 [BuildOptions]
+  #
+  # Disable deprecated APIs.
+  #
+  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
+
   MSFT:DEBUG_*_*_CC_FLAGS = /Od /Oy-
   MSFT:NOOPT_*_*_CC_FLAGS = /Od /Oy-
 
   MSFT:*_*_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
   MSFT:DEBUG_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
   MSFT:NOOPT_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
-
diff --git a/EmulatorPkg/FlashMapPei/FlashMapPei.c b/EmulatorPkg/FlashMapPei/FlashMapPei.c
index 2a468e43ac..7744065dd6 100644
--- a/EmulatorPkg/FlashMapPei/FlashMapPei.c
+++ b/EmulatorPkg/FlashMapPei/FlashMapPei.c
@@ -1,7 +1,7 @@
 /*++ @file
   PEIM to build GUIDed HOBs for platform specific flash map
 
-Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 Portions copyright (c) 2011, Apple Inc. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -69,9 +69,9 @@ Returns:
     return Status;
   }
 
-  PcdSet64 (PcdFlashNvStorageVariableBase64, PcdGet64 (PcdEmuFlashNvStorageVariableBase) + FdFixUp);
-  PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, PcdGet64 (PcdEmuFlashNvStorageFtwWorkingBase) + FdFixUp);
-  PcdSet64 (PcdFlashNvStorageFtwSpareBase64, PcdGet64 (PcdEmuFlashNvStorageFtwSpareBase) + FdFixUp);
+  PcdSet64S (PcdFlashNvStorageVariableBase64, PcdGet64 (PcdEmuFlashNvStorageVariableBase) + FdFixUp);
+  PcdSet64S (PcdFlashNvStorageFtwWorkingBase64, PcdGet64 (PcdEmuFlashNvStorageFtwWorkingBase) + FdFixUp);
+  PcdSet64S (PcdFlashNvStorageFtwSpareBase64, PcdGet64 (PcdEmuFlashNvStorageFtwSpareBase) + FdFixUp);
 
   return EFI_SUCCESS;
 }
diff --git a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c
index 331122e200..3acbb23644 100644
--- a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c
+++ b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c
@@ -4,7 +4,7 @@
 
 
 Copyright (c) 2012, Apple Inc. All rights reserved.
-Portitions Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+Portitions Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -223,7 +223,7 @@ SmbiosLibUpdateUnicodeString (
   if (Ascii == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
-  UnicodeStrToAsciiStr (String, Ascii);
+  UnicodeStrToAsciiStrS (String, Ascii, StrSize (String));
 
   StringIndex = StringNumber;
   Status = gSmbios->UpdateString (gSmbios, &SmbiosHandle, &StringIndex, Ascii);
diff --git a/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c
index b7aacc851c..3a7b6d1ceb 100644
--- a/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c
+++ b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c
@@ -2,7 +2,7 @@
   Emulator Thunk to abstract OS services from pure EFI code
 
   Copyright (c) 2008 - 2011, Apple Inc. All rights reserved.<BR>
-  Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -37,6 +37,7 @@ AddThunkProtocol (
   IN  BOOLEAN                 EmuBusDriver
   )
 {
+  UINTN                       Size;
   CHAR16                      *StartString;
   CHAR16                      *SubString;
   UINTN                       Instance;
@@ -47,8 +48,12 @@ AddThunkProtocol (
   }
 
   Instance = 0;
-  StartString = AllocatePool (StrSize (ConfigString));
-  StrCpy (StartString, ConfigString);
+  Size = StrSize (ConfigString);
+  StartString = AllocatePool (Size);
+  if (StartString == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+  StrCpyS (StartString, Size / sizeof (CHAR16), ConfigString);
   while (*StartString != '\0') {
 
     //
diff --git a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
index e318a90740..8d0eb0d197 100644
--- a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
+++ b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
@@ -4,7 +4,7 @@
 
  Tested on Mac OS X.
 
-Copyright (c) 2004 - 2009, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR>
 Portitions copyright (c) 2011, Apple Inc. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -242,7 +242,7 @@ EmuSnpStart (
     //
     // Associate our interface with this BPF file descriptor.
     //
-    AsciiStrCpy (BoundIf.ifr_name, Private->InterfaceName);
+    AsciiStrCpyS (BoundIf.ifr_name, sizeof (BoundIf.ifr_name), Private->InterfaceName);
     if (ioctl (Private->BpfFd, BIOCSETIF, &BoundIf) < 0) {
       goto DeviceErrorExit;
     }
@@ -1016,7 +1016,11 @@ GetInterfaceMacAddr (
     goto Exit;
   }
 
-  UnicodeStrToAsciiStr (Private->Thunk->ConfigString, Private->InterfaceName);
+  UnicodeStrToAsciiStrS (
+    Private->Thunk->ConfigString,
+    Private->InterfaceName,
+    StrSize (Private->Thunk->ConfigString)
+    );
 
   Status = EFI_NOT_FOUND;
   If = IfAddrs;
diff --git a/EmulatorPkg/Unix/Host/PosixFileSystem.c b/EmulatorPkg/Unix/Host/PosixFileSystem.c
index 6ba3b59d7a..0a618abcd8 100644
--- a/EmulatorPkg/Unix/Host/PosixFileSystem.c
+++ b/EmulatorPkg/Unix/Host/PosixFileSystem.c
@@ -127,7 +127,11 @@ PosixOpenVolume (
   if (PrivateFile->FileName == NULL) {
     goto Done;
   }
-  AsciiStrCpy (PrivateFile->FileName, Private->FilePath);
+  AsciiStrCpyS (
+    PrivateFile->FileName,
+    AsciiStrSize (Private->FilePath),
+    Private->FilePath
+    );
 
   PrivateFile->Signature            = EMU_EFI_FILE_PRIVATE_SIGNATURE;
   PrivateFile->Thunk                = Private->Thunk;
@@ -377,7 +381,7 @@ PosixFileOpen (
   EFI_FILE_INFO                     *Info;
   struct stat                       finfo;
   int                               res;
-
+  UINTN                             Size;
 
   PrivateFile     = EMU_EFI_FILE_PRIVATE_DATA_FROM_THIS (This);
   PrivateRoot     = EMU_SIMPLE_FILE_SYSTEM_PRIVATE_DATA_FROM_THIS (PrivateFile->SimpleFileSystem);
@@ -412,17 +416,18 @@ OpenRoot:
 
   CopyMem (NewPrivateFile, PrivateFile, sizeof (EMU_EFI_FILE_PRIVATE));
 
-  NewPrivateFile->FileName = malloc (AsciiStrSize (PrivateFile->FileName) + 1 + StrLen (FileName) + 1);
+  Size = AsciiStrSize (PrivateFile->FileName) + 1 + StrLen (FileName) + 1;
+  NewPrivateFile->FileName = malloc (Size);
   if (NewPrivateFile->FileName == NULL) {
     goto Done;
   }
 
   if (*FileName == L'\\') {
-    AsciiStrCpy (NewPrivateFile->FileName, PrivateRoot->FilePath);
+    AsciiStrCpyS (NewPrivateFile->FileName, Size, PrivateRoot->FilePath);
     // Skip first '\'.
     Src = FileName + 1;
   } else {
-    AsciiStrCpy (NewPrivateFile->FileName, PrivateFile->FileName);
+    AsciiStrCpyS (NewPrivateFile->FileName, Size, PrivateFile->FileName);
     Src = FileName;
   }
   Dst = NewPrivateFile->FileName + AsciiStrLen (NewPrivateFile->FileName);
@@ -748,7 +753,7 @@ PosixFileRead (
   UINTN                   NameSize;
   UINTN                   ResultSize;
   CHAR8                   *FullFileName;
-
+  UINTN                   FullFileNameSize;
 
   PrivateFile = EMU_EFI_FILE_PRIVATE_DATA_FROM_THIS (This);
 
@@ -798,15 +803,16 @@ PosixFileRead (
 
   *BufferSize = ResultSize;
 
-  FullFileName = malloc (AsciiStrLen(PrivateFile->FileName) + 1 + NameSize);
+  FullFileNameSize = AsciiStrLen(PrivateFile->FileName) + 1 + NameSize;
+  FullFileName = malloc (FullFileNameSize);
   if (FullFileName == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
     goto Done;
   }
 
-  AsciiStrCpy (FullFileName, PrivateFile->FileName);
-  AsciiStrCat (FullFileName, "/");
-  AsciiStrCat (FullFileName, PrivateFile->Dirent->d_name);
+  AsciiStrCpyS (FullFileName, FullFileNameSize, PrivateFile->FileName);
+  AsciiStrCatS (FullFileName, FullFileNameSize, "/");
+  AsciiStrCatS (FullFileName, FullFileNameSize, PrivateFile->Dirent->d_name);
   Status = UnixSimpleFileSystemFileInfo (
             PrivateFile,
             FullFileName,
@@ -1017,7 +1023,11 @@ PosixFileGetInfo (
     FileSystemInfoBuffer->BlockSize   = buf.f_bsize;
 
 
-    StrCpy ((CHAR16 *) FileSystemInfoBuffer->VolumeLabel, PrivateRoot->VolumeLabel);
+    StrCpyS (
+      (CHAR16 *) FileSystemInfoBuffer->VolumeLabel,
+      (*BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO) / sizeof (CHAR16),
+      PrivateRoot->VolumeLabel
+      );
     *BufferSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (PrivateRoot->VolumeLabel);
 
   } else if (CompareGuid (InformationType, &gEfiFileSystemVolumeLabelInfoIdGuid)) {
@@ -1026,7 +1036,11 @@ PosixFileGetInfo (
       return EFI_BUFFER_TOO_SMALL;
     }
 
-    StrCpy ((CHAR16 *) Buffer, PrivateRoot->VolumeLabel);
+    StrCpyS (
+      (CHAR16 *) Buffer,
+      *BufferSize / sizeof (CHAR16),
+      PrivateRoot->VolumeLabel
+      );
     *BufferSize = StrSize (PrivateRoot->VolumeLabel);
 
   }
@@ -1082,7 +1096,7 @@ PosixFileSetInfo (
   CHAR16                            *UnicodeFilePtr;
   int                               UnixStatus;
   struct utimbuf                    Utime;
-
+  UINTN                             Size;
 
   PrivateFile = EMU_EFI_FILE_PRIVATE_DATA_FROM_THIS (This);
   PrivateRoot = EMU_SIMPLE_FILE_SYSTEM_PRIVATE_DATA_FROM_THIS (PrivateFile->SimpleFileSystem);
@@ -1110,7 +1124,11 @@ PosixFileSetInfo (
       goto Done;
     }
 
-    StrCpy (PrivateRoot->VolumeLabel, NewFileSystemInfo->VolumeLabel);
+    StrCpyS (
+      PrivateRoot->VolumeLabel,
+      StrSize (NewFileSystemInfo->VolumeLabel) / sizeof (CHAR16),
+      NewFileSystemInfo->VolumeLabel
+      );
 
     Status = EFI_SUCCESS;
     goto Done;
@@ -1125,7 +1143,11 @@ PosixFileSetInfo (
       goto Done;
     }
 
-    StrCpy (PrivateRoot->VolumeLabel, (CHAR16 *) Buffer);
+    StrCpyS (
+      PrivateRoot->VolumeLabel,
+      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),
+      (CHAR16 *) Buffer
+      );
 
     Status = EFI_SUCCESS;
     goto Done;
@@ -1183,28 +1205,34 @@ PosixFileSetInfo (
     goto Done;
   }
 
-  AsciiStrCpy (OldFileName, PrivateFile->FileName);
+  AsciiStrCpyS (
+    OldFileName,
+    AsciiStrSize (PrivateFile->FileName),
+    PrivateFile->FileName
+    );
 
   //
   // Make full pathname from new filename and rootpath.
   //
   if (NewFileInfo->FileName[0] == '\\') {
-    NewFileName = malloc (AsciiStrLen (PrivateRoot->FilePath) + 1 + StrLen (NewFileInfo->FileName) + 1);
+    Size = AsciiStrLen (PrivateRoot->FilePath) + 1 + StrLen (NewFileInfo->FileName) + 1;
+    NewFileName = malloc (Size);
     if (NewFileName == NULL) {
       goto Done;
     }
 
-    AsciiStrCpy (NewFileName, PrivateRoot->FilePath);
+    AsciiStrCpyS (NewFileName, Size, PrivateRoot->FilePath);
     AsciiFilePtr = NewFileName + AsciiStrLen(NewFileName);
     UnicodeFilePtr = NewFileInfo->FileName + 1;
     *AsciiFilePtr++ ='/';
   } else {
-    NewFileName = malloc (AsciiStrLen (PrivateFile->FileName) + 2 + StrLen (NewFileInfo->FileName) + 1);
+    Size = AsciiStrLen (PrivateFile->FileName) + 2 + StrLen (NewFileInfo->FileName) + 1;
+    NewFileName = malloc (Size);
     if (NewFileName == NULL) {
       goto Done;
     }
 
-    AsciiStrCpy (NewFileName, PrivateRoot->FilePath);
+    AsciiStrCpyS (NewFileName, Size, PrivateRoot->FilePath);
     AsciiFilePtr = NewFileName + AsciiStrLen(NewFileName);
     if ((AsciiFilePtr[-1] != '/') && (NewFileInfo->FileName[0] != '/')) {
       // make sure there is a / between Root FilePath and NewFileInfo Filename
@@ -1312,7 +1340,11 @@ PosixFileSetInfo (
         goto Done;
       }
 
-      AsciiStrCpy (PrivateFile->FileName, NewFileName);
+      AsciiStrCpyS (
+        PrivateFile->FileName,
+        AsciiStrSize (NewFileName),
+        NewFileName
+        );
     } else {
       Status    = EFI_DEVICE_ERROR;
       goto Done;
@@ -1493,7 +1525,11 @@ PosixFileSystmeThunkOpen (
     free (Private);
     return EFI_OUT_OF_RESOURCES;
   }
-  StrCpy (Private->VolumeLabel, L"EFI_EMULATED");
+  StrCpyS (
+    Private->VolumeLabel,
+    StrSize (L"EFI_EMULATED") / sizeof (CHAR16),
+    L"EFI_EMULATED"
+    );
 
   Private->Signature = EMU_SIMPLE_FILE_SYSTEM_PRIVATE_SIGNATURE;
   Private->Thunk     = This;
diff --git a/EmulatorPkg/Unix/Host/X11GraphicsWindow.c b/EmulatorPkg/Unix/Host/X11GraphicsWindow.c
index 9d03c13011..5325a0e35b 100644
--- a/EmulatorPkg/Unix/Host/X11GraphicsWindow.c
+++ b/EmulatorPkg/Unix/Host/X11GraphicsWindow.c
@@ -1,6 +1,6 @@
 /*++ @file
 
-Copyright (c) 2004 - 2011, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR>
 Portions copyright (c) 2008 - 2011, Apple Inc. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -957,7 +957,7 @@ X11GraphicsWindowOpen (
   XDefineCursor (Drv->display, Drv->win, XCreateFontCursor (Drv->display, XC_pirate));
 
   Drv->Title = malloc (StrSize (This->ConfigString));
-  UnicodeStrToAsciiStr (This->ConfigString, Drv->Title);
+  UnicodeStrToAsciiStrS (This->ConfigString, Drv->Title, StrSize (This->ConfigString));
   XStoreName (Drv->display, Drv->win, Drv->Title);
 
 //  XAutoRepeatOff (Drv->display);
diff --git a/EmulatorPkg/Win/Host/WinFileSystem.c b/EmulatorPkg/Win/Host/WinFileSystem.c
index da6595228d..aab926889e 100644
--- a/EmulatorPkg/Win/Host/WinFileSystem.c
+++ b/EmulatorPkg/Win/Host/WinFileSystem.c
@@ -1,7 +1,7 @@
 /*++ @file
   Support OS native directory access.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 
@@ -205,8 +205,14 @@ WinNtOpenVolume (
     goto Done;
   }
 
-  StrCpy (PrivateFile->FilePath, Private->FilePath);
-  StrCpy (PrivateFile->FileName, PrivateFile->FilePath);
+  StrCpyS (PrivateFile->FilePath,
+    StrSize (Private->FilePath) / sizeof (CHAR16),
+    Private->FilePath
+    );
+  StrCpyS (PrivateFile->FileName,
+    StrSize (Private->FilePath) / sizeof (CHAR16),
+    PrivateFile->FilePath
+    );
   PrivateFile->Signature = WIN_NT_EFI_FILE_PRIVATE_SIGNATURE;
   PrivateFile->Thunk = Private->Thunk;
   PrivateFile->SimpleFileSystem = This;
@@ -243,8 +249,8 @@ WinNtOpenVolume (
   if (TempFileName == NULL) {
     goto Done;
   }
-  StrCpy (TempFileName, PrivateFile->FilePath);
-  StrCat (TempFileName, L"\\*");
+  StrCpyS (TempFileName, Size / sizeof (CHAR16), PrivateFile->FilePath);
+  StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");
 
   PrivateFile->LHandle = FindFirstFile (TempFileName, &PrivateFile->FindBuf);
   FreePool (TempFileName);
@@ -362,7 +368,7 @@ GetNextFileNameToken (
   } else {
     Offset = SlashPos - *FileName;
     Token = AllocateZeroPool ((Offset + 1) * sizeof (CHAR16));
-    StrnCpy (Token, *FileName, Offset);
+    StrnCpyS (Token, Offset + 1, *FileName, Offset);
     //
     // Point *FileName to the next character after L'\'.
     //
@@ -496,7 +502,7 @@ WinNtFileOpen (
   if (TempFileName == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
-  StrCpy (TempFileName, FileName);
+  StrCpyS (TempFileName, StrSize (FileName) / sizeof (CHAR16), FileName);
   FileName = TempFileName;
 
   if (FileName[StrLen (FileName) - 1] == L'\\') {
@@ -548,9 +554,17 @@ WinNtFileOpen (
   }
 
   if (PrivateFile->IsDirectoryPath) {
-    StrCpy (NewPrivateFile->FilePath, PrivateFile->FileName);
+    StrCpyS (
+      NewPrivateFile->FilePath,
+      StrSize (PrivateFile->FileName) / sizeof (CHAR16),
+      PrivateFile->FileName
+      );
   } else {
-    StrCpy (NewPrivateFile->FilePath, PrivateFile->FilePath);
+    StrCpyS (
+      NewPrivateFile->FilePath,
+      StrSize (PrivateFile->FileName) / sizeof (CHAR16),
+      PrivateFile->FilePath
+      );
   }
 
   Size = StrSize (NewPrivateFile->FilePath);
@@ -563,17 +577,17 @@ WinNtFileOpen (
   }
 
   if (*FileName == L'\\') {
-    StrCpy (NewPrivateFile->FileName, PrivateRoot->FilePath);
-    StrCat (NewPrivateFile->FileName, L"\\");
-    StrCat (NewPrivateFile->FileName, FileName + 1);
+    StrCpyS (NewPrivateFile->FileName, Size / sizeof (CHAR16), PrivateRoot->FilePath);
+    StrCatS (NewPrivateFile->FileName, Size / sizeof (CHAR16), L"\\");
+    StrCatS (NewPrivateFile->FileName, Size / sizeof (CHAR16), FileName + 1);
   } else {
-    StrCpy (NewPrivateFile->FileName, NewPrivateFile->FilePath);
+    StrCpyS (NewPrivateFile->FileName, Size / sizeof (CHAR16), NewPrivateFile->FilePath);
     if (StrCmp (FileName, L"") != 0) {
       //
       // In case the filename becomes empty, especially after trimming dots and blanks
       //
-      StrCat (NewPrivateFile->FileName, L"\\");
-      StrCat (NewPrivateFile->FileName, FileName);
+      StrCatS (NewPrivateFile->FileName, Size / sizeof (CHAR16), L"\\");
+      StrCatS (NewPrivateFile->FileName, Size / sizeof (CHAR16), FileName);
     }
   }
 
@@ -657,7 +671,11 @@ WinNtFileOpen (
     goto Done;
   }
 
-  StrCpy (NewPrivateFile->FilePath, NewPrivateFile->FileName);
+  StrCpyS (
+    NewPrivateFile->FilePath,
+    StrSize (NewPrivateFile->FileName) / sizeof (CHAR16),
+    NewPrivateFile->FileName
+    );
   if (TempChar != 0) {
     *(RealFileName - 1) = TempChar;
   }
@@ -715,7 +733,7 @@ WinNtFileOpen (
       goto Done;
     }
 
-    StrCpy (TempFileName, NewPrivateFile->FileName);
+    StrCpyS (TempFileName, Size / sizeof (CHAR16), NewPrivateFile->FileName);
 
     if ((OpenMode & EFI_FILE_MODE_CREATE)) {
       //
@@ -769,7 +787,7 @@ WinNtFileOpen (
     //
     // Find the first file under it
     //
-    StrCat (TempFileName, L"\\*");
+    StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");
     NewPrivateFile->LHandle = FindFirstFile (TempFileName, &NewPrivateFile->FindBuf);
     FreePool (TempFileName);
 
@@ -1330,8 +1348,8 @@ WinNtFileSetPossition (
       goto Done;
     }
 
-    StrCpy (FileName, PrivateFile->FileName);
-    StrCat (FileName, L"\\*");
+    StrCpyS (FileName, Size / sizeof (CHAR16), PrivateFile->FileName);
+    StrCatS (FileName, Size / sizeof (CHAR16), L"\\*");
 
     if (PrivateFile->LHandle != INVALID_HANDLE_VALUE) {
       FindClose (PrivateFile->LHandle);
@@ -1599,7 +1617,11 @@ WinNtFileGetInfo (
       goto Done;
     }
 
-    StrCpy (DriveName, PrivateFile->FilePath);
+    StrCpyS (
+      DriveName,
+      (StrSize (PrivateFile->FilePath) + 1) / sizeof (CHAR16),
+      PrivateFile->FilePath
+      );
     for (Index = 0; DriveName[Index] != 0 && DriveName[Index] != ':'; Index++) {
       ;
     }
@@ -1664,7 +1686,11 @@ WinNtFileGetInfo (
       }
     }
 
-    StrCpy ((CHAR16 *)FileSystemInfoBuffer->VolumeLabel, PrivateRoot->VolumeLabel);
+    StrCpyS (
+      (CHAR16 *)FileSystemInfoBuffer->VolumeLabel,
+      (*BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO) / sizeof (CHAR16),
+      PrivateRoot->VolumeLabel
+      );
     *BufferSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (PrivateRoot->VolumeLabel);
     Status = EFI_SUCCESS;
   }
@@ -1676,7 +1702,11 @@ WinNtFileGetInfo (
       goto Done;
     }
 
-    StrCpy ((CHAR16 *)Buffer, PrivateRoot->VolumeLabel);
+    StrCpyS (
+      (CHAR16 *)Buffer,
+      *BufferSize / sizeof (CHAR16),
+      PrivateRoot->VolumeLabel
+      );
     *BufferSize = StrSize (PrivateRoot->VolumeLabel);
     Status = EFI_SUCCESS;
   }
@@ -1768,7 +1798,11 @@ WinNtFileSetInfo (
       goto Done;
     }
 
-    StrCpy (PrivateRoot->VolumeLabel, NewFileSystemInfo->VolumeLabel);
+    StrCpyS (
+      PrivateRoot->VolumeLabel,
+      StrSize (NewFileSystemInfo->VolumeLabel) / sizeof (CHAR16),
+      NewFileSystemInfo->VolumeLabel
+      );
 
     Status = EFI_SUCCESS;
     goto Done;
@@ -1783,7 +1817,11 @@ WinNtFileSetInfo (
       goto Done;
     }
 
-    StrCpy (PrivateRoot->VolumeLabel, (CHAR16 *)Buffer);
+    StrCpyS (
+      PrivateRoot->VolumeLabel,
+      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),
+      (CHAR16 *)Buffer
+      );
 
     Status = EFI_SUCCESS;
     goto Done;
@@ -1852,7 +1890,11 @@ WinNtFileSetInfo (
     goto Done;
   }
 
-  StrCpy (OldFileName, PrivateFile->FileName);
+  StrCpyS (
+    OldFileName,
+    StrSize (PrivateFile->FileName) / sizeof (CHAR16),
+    PrivateFile->FileName
+    );
 
   //
   // Make full pathname from new filename and rootpath.
@@ -1867,9 +1909,9 @@ WinNtFileSetInfo (
       goto Done;
     }
 
-    StrCpy (NewFileName, PrivateRoot->FilePath);
-    StrCat (NewFileName, L"\\");
-    StrCat (NewFileName, NewFileInfo->FileName + 1);
+    StrCpyS (NewFileName, Size / sizeof (CHAR16), PrivateRoot->FilePath);
+    StrCatS (NewFileName, Size / sizeof (CHAR16), L"\\");
+    StrCatS (NewFileName, Size / sizeof (CHAR16), NewFileInfo->FileName + 1);
   } else {
     Size = StrSize (PrivateFile->FilePath);
     Size += StrSize (L"\\");
@@ -1880,9 +1922,9 @@ WinNtFileSetInfo (
       goto Done;
     }
 
-    StrCpy (NewFileName, PrivateFile->FilePath);
-    StrCat (NewFileName, L"\\");
-    StrCat (NewFileName, NewFileInfo->FileName);
+    StrCpyS (NewFileName, Size / sizeof (CHAR16), PrivateFile->FilePath);
+    StrCatS (NewFileName, Size / sizeof (CHAR16), L"\\");
+    StrCatS (NewFileName, Size / sizeof (CHAR16), NewFileInfo->FileName);
   }
 
   //
@@ -1990,13 +2032,13 @@ WinNtFileSetInfo (
         goto Done;
       }
 
-      StrCpy (PrivateFile->FileName, NewFileName);
+      StrCpyS (PrivateFile->FileName, StrSize (NewFileName) / sizeof (CHAR16), NewFileName);
 
       Size = StrSize (NewFileName);
       Size += StrSize (L"\\*");
       TempFileName = AllocatePool (Size);
 
-      StrCpy (TempFileName, NewFileName);
+      StrCpyS (TempFileName, Size / sizeof (CHAR16), NewFileName);
 
       if (!PrivateFile->IsDirectoryPath) {
         PrivateFile->LHandle = CreateFile (
@@ -2029,7 +2071,7 @@ WinNtFileSetInfo (
           NULL
         );
 
-        StrCat (TempFileName, L"\\*");
+        StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");
         PrivateFile->LHandle = FindFirstFile (TempFileName, &FindBuf);
 
         FreePool (TempFileName);
@@ -2048,7 +2090,7 @@ WinNtFileSetInfo (
       Size += StrSize (L"\\*");
       TempFileName = AllocatePool (Size);
 
-      StrCpy (TempFileName, OldFileName);
+      StrCpyS (TempFileName, Size / sizeof (CHAR16), OldFileName);
 
       if (!PrivateFile->IsDirectoryPath) {
         PrivateFile->LHandle = CreateFile (
@@ -2071,7 +2113,7 @@ WinNtFileSetInfo (
           NULL
         );
 
-        StrCat (TempFileName, L"\\*");
+        StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");
         PrivateFile->LHandle = FindFirstFile (TempFileName, &FindBuf);
       }
 
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 04/10] EmulatorPkg: Add support for NOOPT target
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
                   ` (2 preceding siblings ...)
  2019-08-16  2:14 ` [Patch V4 03/10] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 05/10] EmulatorPkg/Unix/Host: Disable inline/optimizations Michael D Kinney
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni, Hao A Wu

Add NOOPT to BUILD_TARGETS in DSC file.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
---
 EmulatorPkg/EmulatorPkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 529adfe1fa..0af2d1ff95 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -19,7 +19,7 @@ [Defines]
   OUTPUT_DIRECTORY               = Build/Emulator$(ARCH)
 
   SUPPORTED_ARCHITECTURES        = X64|IA32
-  BUILD_TARGETS                  = DEBUG|RELEASE
+  BUILD_TARGETS                  = DEBUG|RELEASE|NOOPT
   SKUID_IDENTIFIER               = DEFAULT
   FLASH_DEFINITION               = EmulatorPkg/EmulatorPkg.fdf
 
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 05/10] EmulatorPkg/Unix/Host: Disable inline/optimizations
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
                   ` (3 preceding siblings ...)
  2019-08-16  2:14 ` [Patch V4 04/10] EmulatorPkg: Add support for NOOPT target Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues Michael D Kinney
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni

* Disable XCODE5 compiler optimizations fort Unix/Host.
* Disable inline of SecGdbScriptBreak() to improve
  compatibility with XCODE5
* For X64 XCODE5 builds place output Host application
  in $(BIN_DIR) to match all other EmulatorPkg Host
  application builds.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 EmulatorPkg/Unix/Host/Host.c   | 3 +++
 EmulatorPkg/Unix/Host/Host.inf | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/Unix/Host/Host.c b/EmulatorPkg/Unix/Host/Host.c
index febfb1f44c..b431a4c2ed 100644
--- a/EmulatorPkg/Unix/Host/Host.c
+++ b/EmulatorPkg/Unix/Host/Host.c
@@ -1113,6 +1113,9 @@ DlLoadImage (
 }
 
 
+#ifdef __APPLE__
+__attribute__((noinline))
+#endif
 VOID
 SecGdbScriptBreak (
   char                *FileName,
diff --git a/EmulatorPkg/Unix/Host/Host.inf b/EmulatorPkg/Unix/Host/Host.inf
index ca4294249b..c479d2b7d0 100644
--- a/EmulatorPkg/Unix/Host/Host.inf
+++ b/EmulatorPkg/Unix/Host/Host.inf
@@ -137,6 +137,6 @@ [BuildOptions]
    XCODE:*_*_IA32_ASM_FLAGS == -arch i386 -g
 
    XCODE:*_*_X64_DLINK_PATH == gcc
-   XCODE:*_*_X64_DLINK_FLAGS == -L/usr/X11R6/lib -lXext -lX11 -framework Carbon -Wl,-no_pie
+   XCODE:*_*_X64_DLINK_FLAGS == -o $(BIN_DIR)/Host -L/usr/X11R6/lib -lXext -lX11 -framework Carbon -Wl,-no_pie
    XCODE:*_*_X64_ASM_FLAGS == -g
-   XCODE:*_*_X64_CC_FLAGS = -target x86_64-apple-darwin -I$(WORKSPACE)/EmulatorPkg/Unix/Host/X11IncludeHack "-DEFIAPI=__attribute__((ms_abi))"
+   XCODE:*_*_X64_CC_FLAGS = -O0 -target x86_64-apple-darwin -I$(WORKSPACE)/EmulatorPkg/Unix/Host/X11IncludeHack "-DEFIAPI=__attribute__((ms_abi))"
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
                   ` (4 preceding siblings ...)
  2019-08-16  2:14 ` [Patch V4 05/10] EmulatorPkg/Unix/Host: Disable inline/optimizations Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  7:40   ` Jordan Justen
  2019-08-16  2:14 ` [Patch V4 07/10] EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues Michael D Kinney
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Ray Ni, Andrew Fish

Fix scripts to support lldb symbolic debugging when
using XCODE5 tool chain.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Andrew Fish <afish@apple.com>
---
 EmulatorPkg/Unix/lldbefi.py |  8 +++++---
 EmulatorPkg/build.sh        | 17 ++---------------
 2 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/EmulatorPkg/Unix/lldbefi.py b/EmulatorPkg/Unix/lldbefi.py
index 218326b8cb..099192d8b5 100755
--- a/EmulatorPkg/Unix/lldbefi.py
+++ b/EmulatorPkg/Unix/lldbefi.py
@@ -346,6 +346,7 @@ def TypePrintFormating(debugger):
     debugger.HandleCommand("type summary add CHAR8 --python-function lldbefi.CHAR8_TypeSummary")
     debugger.HandleCommand('type summary add --regex "CHAR8 \[[0-9]+\]" --python-function lldbefi.CHAR8_TypeSummary')
 
+    debugger.HandleCommand('setting set frame-format "frame #${frame.index}: ${frame.pc}{ ${module.file.basename}{:${function.name}()${function.pc-offset}}}{ at ${line.file.fullpath}:${line.number}}\n"')
 
 gEmulatorBreakWorkaroundNeeded = True
 
@@ -381,15 +382,16 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
     Error = lldb.SBError()
     FileNamePtr = frame.FindVariable ("FileName").GetValueAsUnsigned()
     FileNameLen = frame.FindVariable ("FileNameLength").GetValueAsUnsigned()
+
     FileName = frame.thread.process.ReadCStringFromMemory (FileNamePtr, FileNameLen, Error)
     if not Error.Success():
         print "!ReadCStringFromMemory() did not find a %d byte C string at %x" % (FileNameLen, FileNamePtr)
         # make breakpoint command contiue
-        frame.GetThread().GetProcess().Continue()
+        return False
 
     debugger = frame.thread.process.target.debugger
     if frame.FindVariable ("AddSymbolFlag").GetValueAsUnsigned() == 1:
-        LoadAddress = frame.FindVariable ("LoadAddress").GetValueAsUnsigned()
+        LoadAddress = frame.FindVariable ("LoadAddress").GetValueAsUnsigned() - 0x240
 
         debugger.HandleCommand ("target modules add  %s" % FileName)
         print "target modules load --slid 0x%x %s" % (LoadAddress, FileName)
@@ -405,7 +407,7 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
                     print "!lldb.target.RemoveModule (%s) FAILED" % SBModule
 
     # make breakpoint command contiue
-    frame.thread.process.Continue()
+    return False
 
 def GuidToCStructStr (guid, Name=False):
   #
diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
index 60056e1b6c..35912a7775 100755
--- a/EmulatorPkg/build.sh
+++ b/EmulatorPkg/build.sh
@@ -209,21 +209,8 @@ fi
 if [[ "$RUN_EMULATOR" == "yes" ]]; then
   case `uname` in
     Darwin*)
-      #
-      # On Darwin we can't use dlopen, so we have to load the real PE/COFF images.
-      # This .gdbinit script sets a breakpoint that loads symbols for the PE/COFFEE
-      # images that get loaded in Host
-      #
-      if [[ "$CLANG_VER" == *-ccc-host-triple* ]]
-      then
-      # only older versions of Xcode support -ccc-host-tripe, for newer versions
-      # it is -target
-        cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
-        cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
-        exit $? 
-      else
-        cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
-      fi
+      cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o "command script import $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" -o 'script lldb.debugger.SetAsync(True)' -o "run" ./Host
+      exit $?
       ;;
   esac
 
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 07/10] EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
                   ` (5 preceding siblings ...)
  2019-08-16  2:14 ` [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  7:47   ` Jordan Justen
  2019-08-16  2:14 ` [Patch V4 08/10] EmulatorPkg: Disable TftpDynamicCommand and LogoDxe for XCODE5 Michael D Kinney
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Ray Ni, Andrew Fish

* Fix uninitialized Private->ReadBuffer.
* Remove old debug code that generates an exception.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Andrew Fish <afish@apple.com>
---
 EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
index 8d0eb0d197..3013bbc86b 100644
--- a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
+++ b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
@@ -216,6 +216,7 @@ EmuSnpStart (
   }
 
   Status = EFI_SUCCESS;
+  Private->ReadBuffer = NULL;
   if (Private->BpfFd == 0) {
     Status = OpenBpfFileDescriptor (Private, &Private->BpfFd);
     if (EFI_ERROR (Status)) {
@@ -766,10 +767,6 @@ EmuSnpGetStatus (
 
   Private = EMU_SNP_PRIVATE_DATA_FROM_THIS (This);
 
-  if (TxBuf != NULL) {
-    *((UINT8 **)TxBuf) =  (UINT8 *)1;
-  }
-
   if ( InterruptStatus != NULL ) {
     *InterruptStatus = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
   }
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 08/10] EmulatorPkg: Disable TftpDynamicCommand and LogoDxe for XCODE5
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
                   ` (6 preceding siblings ...)
  2019-08-16  2:14 ` [Patch V4 07/10] EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope " Michael D Kinney
  2019-08-16  2:14 ` [Patch V4 10/10] BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64 Michael D Kinney
  9 siblings, 0 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Ray Ni, Andrew Fish

Disable TftpDynamicCommand for XCODE5 because this command
places HII content in an PE/COFF resource section that is not
supported by the XCODE5 tool chain, and the missing HII
content causes the load of this command to ASSERT().

Disable the LogoDxe module that places the logo bitmap in
a PE/COFF resource section that is not supported by the
XCODE5 tool chain, and the missing HII content causes
the load of this module to ASSERT().

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Andrew Fish <afish@apple.com>
---
 EmulatorPkg/EmulatorPkg.dsc | 4 ++++
 EmulatorPkg/EmulatorPkg.fdf | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 0af2d1ff95..20f1187713 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -332,7 +332,9 @@ [Components]
 
   MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+!if "XCODE5" not in $(TOOL_CHAIN_TAG)
   MdeModulePkg/Logo/LogoDxe.inf
+!endif
   MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
   MdeModulePkg/Application/UiApp/UiApp.inf {
    <LibraryClasses>
@@ -375,10 +377,12 @@ [Components]
 
   FatPkg/EnhancedFatDxe/Fat.inf
 
+!if "XCODE5" not in $(TOOL_CHAIN_TAG)
   ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
     <PcdsFixedAtBuild>
       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
   }
+!endif
   ShellPkg/Application/Shell/Shell.inf {
     <LibraryClasses>
       ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
diff --git a/EmulatorPkg/EmulatorPkg.fdf b/EmulatorPkg/EmulatorPkg.fdf
index ec411e82b4..295f6f1db8 100644
--- a/EmulatorPkg/EmulatorPkg.fdf
+++ b/EmulatorPkg/EmulatorPkg.fdf
@@ -178,7 +178,9 @@ [FV.FvRecovery]
 INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF  MdeModulePkg/Universal/PrintDxe/PrintDxe.inf
 INF  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+!if "XCODE5" not in $(TOOL_CHAIN_TAG)
 INF  MdeModulePkg/Logo/LogoDxe.inf
+!endif
 INF  MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
 INF  RuleOverride = UI MdeModulePkg/Application/UiApp/UiApp.inf
 INF  MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
@@ -194,7 +196,9 @@ [FV.FvRecovery]
 
 INF FatPkg/EnhancedFatDxe/Fat.inf
 
+!if "XCODE5" not in $(TOOL_CHAIN_TAG)
 INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+!endif
 INF  ShellPkg/Application/Shell/Shell.inf
 
 [Rule.Common.SEC]
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope for XCODE5
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
                   ` (7 preceding siblings ...)
  2019-08-16  2:14 ` [Patch V4 08/10] EmulatorPkg: Disable TftpDynamicCommand and LogoDxe for XCODE5 Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16 18:29   ` Jordan Justen
  2019-08-16  2:14 ` [Patch V4 10/10] BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64 Michael D Kinney
  9 siblings, 1 reply; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Ray Ni, Andrew Fish

The local variable PpiArray[10] is declared in middle
of the SEC module _ModuleEntryPoint().  When building
for XCODE5 with optimizations enabled, this results in
corruption and an exception.  The fix is to move the
declaration of PpiArray[10] to the standard location
at the beginning of the function so the storage for
this local variable is allocated for the entire
lifetime of the function.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Andrew Fish <afish@apple.com>
---
 EmulatorPkg/Sec/Sec.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/EmulatorPkg/Sec/Sec.c b/EmulatorPkg/Sec/Sec.c
index 701032233b..b734d2bb87 100644
--- a/EmulatorPkg/Sec/Sec.c
+++ b/EmulatorPkg/Sec/Sec.c
@@ -75,6 +75,7 @@ _ModuleEntryPoint (
   EFI_PEI_PPI_DESCRIPTOR    *SecPpiList;
   UINTN                     SecReseveredMemorySize;
   UINTN                     Index;
+  EFI_PEI_PPI_DESCRIPTOR    PpiArray[10];
 
   EMU_MAGIC_PAGE()->PpiList = PpiList;
   ProcessLibraryConstructorList ();
@@ -104,16 +105,13 @@ _ModuleEntryPoint (
   SecCoreData->PeiTemporaryRamBase = (VOID *)((UINTN)SecCoreData->PeiTemporaryRamBase + SecReseveredMemorySize);
   SecCoreData->PeiTemporaryRamSize -= SecReseveredMemorySize;
 #else
-  {
-    //
-    // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? Either there is a bug
-    // or I don't understand temp RAM correctly?
-    //
-    EFI_PEI_PPI_DESCRIPTOR    PpiArray[10];
+  //
+  // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? Either there is a bug
+  // or I don't understand temp RAM correctly?
+  //
 
-    SecPpiList = &PpiArray[0];
-    ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
-  }
+  SecPpiList = &PpiArray[0];
+  ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
 #endif
   // Copy existing list, and append our entries.
   CopyMem (SecPpiList, PpiList, sizeof (EFI_PEI_PPI_DESCRIPTOR) * Index);
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Patch V4 10/10] BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64
  2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
                   ` (8 preceding siblings ...)
  2019-08-16  2:14 ` [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope " Michael D Kinney
@ 2019-08-16  2:14 ` Michael D Kinney
  2019-08-16  4:50   ` [edk2-devel] " Liming Gao
  9 siblings, 1 reply; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16  2:14 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Ray Ni, Andrew Fish

Add -gdwarf to XCODE5 X64 builds to generate symbols for
source level debug using lldb.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Andrew Fish <afish@apple.com>
---
 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 26a2cf604f..8f0e6cb6c2 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -2593,8 +2593,8 @@ RELEASE_XCODE5_X64_ASM_FLAGS  = -arch x86_64
 *_XCODE5_*_PP_FLAGS         = -E -x assembler-with-cpp -include $(DEST_DIR_DEBUG)/AutoGen.h
 *_XCODE5_*_VFRPP_FLAGS      = -x c -E -P -DVFRCOMPILE -include $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
 
-  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -Os       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
-  NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -O0       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
+  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -gdwarf -Os       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
+  NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -gdwarf -O0       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
 RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -Os       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -Wno-unused-const-variable -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
 
 ####################################################################################
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [Patch V4 10/10] BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64
  2019-08-16  2:14 ` [Patch V4 10/10] BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64 Michael D Kinney
@ 2019-08-16  4:50   ` Liming Gao
  0 siblings, 0 replies; 19+ messages in thread
From: Liming Gao @ 2019-08-16  4:50 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Ni, Ray, Andrew Fish

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Michael D Kinney
>Sent: Friday, August 16, 2019 10:15 AM
>To: devel@edk2.groups.io
>Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ni, Ray <ray.ni@intel.com>;
>Andrew Fish <afish@apple.com>
>Subject: [edk2-devel] [Patch V4 10/10] BaseTools/tools_def.template: Add -
>gdwarf to XCODE5 X64
>
>Add -gdwarf to XCODE5 X64 builds to generate symbols for
>source level debug using lldb.
>
>Cc: Jordan Justen <jordan.l.justen@intel.com>
>Cc: Ray Ni <ray.ni@intel.com>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>Signed-off-by: Andrew Fish <afish@apple.com>
>---
> BaseTools/Conf/tools_def.template | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/BaseTools/Conf/tools_def.template
>b/BaseTools/Conf/tools_def.template
>index 26a2cf604f..8f0e6cb6c2 100755
>--- a/BaseTools/Conf/tools_def.template
>+++ b/BaseTools/Conf/tools_def.template
>@@ -2593,8 +2593,8 @@ RELEASE_XCODE5_X64_ASM_FLAGS  = -arch x86_64
> *_XCODE5_*_PP_FLAGS         = -E -x assembler-with-cpp -include
>$(DEST_DIR_DEBUG)/AutoGen.h
> *_XCODE5_*_VFRPP_FLAGS      = -x c -E -P -DVFRCOMPILE -include
>$(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
>
>-  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -
>Os       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-
>extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float
>-mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-
>field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs
>-ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -
>D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
>-  NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -
>O0       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-
>extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float
>-mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-
>field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs
>-ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -
>D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
>+  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -
>gdwarf -Os       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -
>fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-
>implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -
>Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare
>-Wno-varargs -ftrap-
>function=undefined_behavior_has_been_optimized_away_by_clang -D
>NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
>+  NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -
>gdwarf -O0       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -
>fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-
>implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -
>Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare
>-Wno-varargs -ftrap-
>function=undefined_behavior_has_been_optimized_away_by_clang -D
>NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
> RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -Os
>-Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-
>extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float
>-mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-
>field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs
>-Wno-unused-const-variable -ftrap-
>function=undefined_behavior_has_been_optimized_away_by_clang -D
>NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
>
>
>###########################################################
>#########################
>--
>2.21.0.windows.1
>
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
  2019-08-16  2:14 ` [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues Michael D Kinney
@ 2019-08-16  7:40   ` Jordan Justen
  2019-08-16 15:09     ` Michael D Kinney
  0 siblings, 1 reply; 19+ messages in thread
From: Jordan Justen @ 2019-08-16  7:40 UTC (permalink / raw)
  To: Michael D Kinney, devel; +Cc: Ray Ni, Andrew Fish

On 2019-08-15 19:14:33, Michael D Kinney wrote:
> Fix scripts to support lldb symbolic debugging when
> using XCODE5 tool chain.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Andrew Fish <afish@apple.com>

Is this a Cc/Signed-off-by typo? (See also, patches 7-10).

This makes me wonder if you are taking advantage of the git commit -s
switch to add your Signed-off-by.

Also, I'm wondering if you are taking advantage of git-send-email
automatically Cc'ing the addresses you listed in the commit message.
(I thought it Cc'd for the author and Cc tags, but I wasn't sure about
the Signed-off-by tag, and yet I see Andrew was Cc'd.)

There's a couple long lines below. You could use \ at the end of the
line to split the .sh line. I think the cd can be a separate command
in a shell script. (Not in make)

I hope someone that uses the XCODE toolchain could review/check the
XCODE patches.

-Jordan

> ---
>  EmulatorPkg/Unix/lldbefi.py |  8 +++++---
>  EmulatorPkg/build.sh        | 17 ++---------------
>  2 files changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/EmulatorPkg/Unix/lldbefi.py b/EmulatorPkg/Unix/lldbefi.py
> index 218326b8cb..099192d8b5 100755
> --- a/EmulatorPkg/Unix/lldbefi.py
> +++ b/EmulatorPkg/Unix/lldbefi.py
> @@ -346,6 +346,7 @@ def TypePrintFormating(debugger):
>      debugger.HandleCommand("type summary add CHAR8 --python-function lldbefi.CHAR8_TypeSummary")
>      debugger.HandleCommand('type summary add --regex "CHAR8 \[[0-9]+\]" --python-function lldbefi.CHAR8_TypeSummary')
>  
> +    debugger.HandleCommand('setting set frame-format "frame #${frame.index}: ${frame.pc}{ ${module.file.basename}{:${function.name}()${function.pc-offset}}}{ at ${line.file.fullpath}:${line.number}}\n"')
>  
>  gEmulatorBreakWorkaroundNeeded = True
>  
> @@ -381,15 +382,16 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
>      Error = lldb.SBError()
>      FileNamePtr = frame.FindVariable ("FileName").GetValueAsUnsigned()
>      FileNameLen = frame.FindVariable ("FileNameLength").GetValueAsUnsigned()
> +
>      FileName = frame.thread.process.ReadCStringFromMemory (FileNamePtr, FileNameLen, Error)
>      if not Error.Success():
>          print "!ReadCStringFromMemory() did not find a %d byte C string at %x" % (FileNameLen, FileNamePtr)
>          # make breakpoint command contiue
> -        frame.GetThread().GetProcess().Continue()
> +        return False
>  
>      debugger = frame.thread.process.target.debugger
>      if frame.FindVariable ("AddSymbolFlag").GetValueAsUnsigned() == 1:
> -        LoadAddress = frame.FindVariable ("LoadAddress").GetValueAsUnsigned()
> +        LoadAddress = frame.FindVariable ("LoadAddress").GetValueAsUnsigned() - 0x240
>  
>          debugger.HandleCommand ("target modules add  %s" % FileName)
>          print "target modules load --slid 0x%x %s" % (LoadAddress, FileName)
> @@ -405,7 +407,7 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
>                      print "!lldb.target.RemoveModule (%s) FAILED" % SBModule
>  
>      # make breakpoint command contiue
> -    frame.thread.process.Continue()
> +    return False
>  
>  def GuidToCStructStr (guid, Name=False):
>    #
> diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
> index 60056e1b6c..35912a7775 100755
> --- a/EmulatorPkg/build.sh
> +++ b/EmulatorPkg/build.sh
> @@ -209,21 +209,8 @@ fi
>  if [[ "$RUN_EMULATOR" == "yes" ]]; then
>    case `uname` in
>      Darwin*)
> -      #
> -      # On Darwin we can't use dlopen, so we have to load the real PE/COFF images.
> -      # This .gdbinit script sets a breakpoint that loads symbols for the PE/COFFEE
> -      # images that get loaded in Host
> -      #
> -      if [[ "$CLANG_VER" == *-ccc-host-triple* ]]
> -      then
> -      # only older versions of Xcode support -ccc-host-tripe, for newer versions
> -      # it is -target
> -        cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
> -        cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
> -        exit $? 
> -      else
> -        cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCESSOR"
> -      fi
> +      cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o "command script import $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" -o 'script lldb.debugger.SetAsync(True)' -o "run" ./Host
> +      exit $?
>        ;;
>    esac
>  
> -- 
> 2.21.0.windows.1
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch V4 07/10] EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues
  2019-08-16  2:14 ` [Patch V4 07/10] EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues Michael D Kinney
@ 2019-08-16  7:47   ` Jordan Justen
  0 siblings, 0 replies; 19+ messages in thread
From: Jordan Justen @ 2019-08-16  7:47 UTC (permalink / raw)
  To: Michael D Kinney, devel; +Cc: Ray Ni, Andrew Fish

On 2019-08-15 19:14:34, Michael D Kinney wrote:
> * Fix uninitialized Private->ReadBuffer.
> * Remove old debug code that generates an exception.

Bulleted lists in the commit message often makes me think they should
be separate patches. Maybe that could improve the commit message
subject line for the patches too. :)

-Jordan

> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Andrew Fish <afish@apple.com>
> ---
>  EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> index 8d0eb0d197..3013bbc86b 100644
> --- a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> +++ b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> @@ -216,6 +216,7 @@ EmuSnpStart (
>    }
>  
>    Status = EFI_SUCCESS;
> +  Private->ReadBuffer = NULL;
>    if (Private->BpfFd == 0) {
>      Status = OpenBpfFileDescriptor (Private, &Private->BpfFd);
>      if (EFI_ERROR (Status)) {
> @@ -766,10 +767,6 @@ EmuSnpGetStatus (
>  
>    Private = EMU_SNP_PRIVATE_DATA_FROM_THIS (This);
>  
> -  if (TxBuf != NULL) {
> -    *((UINT8 **)TxBuf) =  (UINT8 *)1;
> -  }
> -
>    if ( InterruptStatus != NULL ) {
>      *InterruptStatus = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
>    }
> -- 
> 2.21.0.windows.1
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
  2019-08-16  7:40   ` Jordan Justen
@ 2019-08-16 15:09     ` Michael D Kinney
  2019-08-16 17:30       ` [edk2-devel] " Andrew Fish
  2019-08-16 18:21       ` Jordan Justen
  0 siblings, 2 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16 15:09 UTC (permalink / raw)
  To: Justen, Jordan L, devel@edk2.groups.io, Kinney, Michael D
  Cc: Ni, Ray, Andrew Fish

Jordan,

It is not a typo.

Andrew generated the XCODE specific changes, so they have 
been tested by him.  I have also reviewed and tested the XCODE
changes and verified that all 6 combinations build and boot
to shell (IA32/X64 for RELEASE/DEBUG/NOOPT). Since they are
all related to making EmulatorPkg work, we decided to fold
them into the same patch set that was already being reviewed.

I also verified build and boot to shell for 6 combinations
on GCC5 (IA32/X64 for RELEASE/DEBUG/NOOPT) and the 12
combinations of VS2015/VS2017, IA323/X64, RELEASE/DEBUG/NOOPT.

I have been working on some CI experiments using Azure Pipelines.
Here is a pointer to the build logs for all the combinations 
listed above.

https://dev.azure.com/mikekinney/edk2-ci/_build/results?buildId=312

Mike

> -----Original Message-----
> From: Justen, Jordan L
> Sent: Friday, August 16, 2019 12:41 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5
> lldb issues
> 
> On 2019-08-15 19:14:33, Michael D Kinney wrote:
> > Fix scripts to support lldb symbolic debugging when
> using XCODE5 tool
> > chain.
> >
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Signed-off-by: Andrew Fish <afish@apple.com>
> 
> Is this a Cc/Signed-off-by typo? (See also, patches 7-
> 10).
> 
> This makes me wonder if you are taking advantage of the
> git commit -s switch to add your Signed-off-by.
> 
> Also, I'm wondering if you are taking advantage of git-
> send-email automatically Cc'ing the addresses you
> listed in the commit message.
> (I thought it Cc'd for the author and Cc tags, but I
> wasn't sure about the Signed-off-by tag, and yet I see
> Andrew was Cc'd.)
> 
> There's a couple long lines below. You could use \ at
> the end of the line to split the .sh line. I think the
> cd can be a separate command in a shell script. (Not in
> make)
> 
> I hope someone that uses the XCODE toolchain could
> review/check the XCODE patches.
> 
> -Jordan
> 
> > ---
> >  EmulatorPkg/Unix/lldbefi.py |  8 +++++---
> >  EmulatorPkg/build.sh        | 17 ++---------------
> >  2 files changed, 7 insertions(+), 18 deletions(-)
> >
> > diff --git a/EmulatorPkg/Unix/lldbefi.py
> b/EmulatorPkg/Unix/lldbefi.py
> > index 218326b8cb..099192d8b5 100755
> > --- a/EmulatorPkg/Unix/lldbefi.py
> > +++ b/EmulatorPkg/Unix/lldbefi.py
> > @@ -346,6 +346,7 @@ def TypePrintFormating(debugger):
> >      debugger.HandleCommand("type summary add CHAR8 -
> -python-function lldbefi.CHAR8_TypeSummary")
> >      debugger.HandleCommand('type summary add --regex
> "CHAR8
> > \[[0-9]+\]" --python-function
> lldbefi.CHAR8_TypeSummary')
> >
> > +    debugger.HandleCommand('setting set frame-format
> "frame
> > + #${frame.index}: ${frame.pc}{
> > +
> ${module.file.basename}{:${function.name}()${function.p
> c-offset}}}{
> > + at ${line.file.fullpath}:${line.number}}\n"')
> >
> >  gEmulatorBreakWorkaroundNeeded = True
> >
> > @@ -381,15 +382,16 @@ def
> LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
> >      Error = lldb.SBError()
> >      FileNamePtr = frame.FindVariable
> ("FileName").GetValueAsUnsigned()
> >      FileNameLen = frame.FindVariable
> > ("FileNameLength").GetValueAsUnsigned()
> > +
> >      FileName =
> frame.thread.process.ReadCStringFromMemory
> (FileNamePtr, FileNameLen, Error)
> >      if not Error.Success():
> >          print "!ReadCStringFromMemory() did not find
> a %d byte C string at %x" % (FileNameLen, FileNamePtr)
> >          # make breakpoint command contiue
> > -        frame.GetThread().GetProcess().Continue()
> > +        return False
> >
> >      debugger = frame.thread.process.target.debugger
> >      if frame.FindVariable
> ("AddSymbolFlag").GetValueAsUnsigned() == 1:
> > -        LoadAddress = frame.FindVariable
> ("LoadAddress").GetValueAsUnsigned()
> > +        LoadAddress = frame.FindVariable
> > + ("LoadAddress").GetValueAsUnsigned() - 0x240
> >
> >          debugger.HandleCommand ("target modules add
> %s" % FileName)
> >          print "target modules load --slid 0x%x %s" %
> (LoadAddress,
> > FileName) @@ -405,7 +407,7 @@ def
> LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
> >                      print "!lldb.target.RemoveModule
> (%s) FAILED" %
> > SBModule
> >
> >      # make breakpoint command contiue
> > -    frame.thread.process.Continue()
> > +    return False
> >
> >  def GuidToCStructStr (guid, Name=False):
> >    #
> > diff --git a/EmulatorPkg/build.sh
> b/EmulatorPkg/build.sh index
> > 60056e1b6c..35912a7775 100755
> > --- a/EmulatorPkg/build.sh
> > +++ b/EmulatorPkg/build.sh
> > @@ -209,21 +209,8 @@ fi
> >  if [[ "$RUN_EMULATOR" == "yes" ]]; then
> >    case `uname` in
> >      Darwin*)
> > -      #
> > -      # On Darwin we can't use dlopen, so we have to
> load the real PE/COFF images.
> > -      # This .gdbinit script sets a breakpoint that
> loads symbols for the PE/COFFEE
> > -      # images that get loaded in Host
> > -      #
> > -      if [[ "$CLANG_VER" == *-ccc-host-triple* ]]
> > -      then
> > -      # only older versions of Xcode support -ccc-
> host-tripe, for newer versions
> > -      # it is -target
> > -        cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> SOR"
> > -        cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source
> $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
> > -        exit $?
> > -      else
> > -        cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> SOR"
> > -      fi
> > +      cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o "command
> script import $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" -
> o 'script lldb.debugger.SetAsync(True)' -o "run" ./Host
> > +      exit $?
> >        ;;
> >    esac
> >
> > --
> > 2.21.0.windows.1
> >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
  2019-08-16 15:09     ` Michael D Kinney
@ 2019-08-16 17:30       ` Andrew Fish
  2019-08-16 18:21       ` Jordan Justen
  1 sibling, 0 replies; 19+ messages in thread
From: Andrew Fish @ 2019-08-16 17:30 UTC (permalink / raw)
  To: devel, Mike Kinney; +Cc: Jordan Justen, Ni, Ray

[-- Attachment #1: Type: text/plain, Size: 6554 bytes --]

Tested-by: Andrew Fish <afish@apple.com>

I tested the XCODE builds of the emulator and made sure that source level debugging worked probably for builds that generate symbols. 

Thanks,

Andrew Fish

> On Aug 16, 2019, at 8:09 AM, Michael D Kinney <michael.d.kinney@intel.com> wrote:
> 
> Jordan,
> 
> It is not a typo.
> 
> Andrew generated the XCODE specific changes, so they have 
> been tested by him.  I have also reviewed and tested the XCODE
> changes and verified that all 6 combinations build and boot
> to shell (IA32/X64 for RELEASE/DEBUG/NOOPT). Since they are
> all related to making EmulatorPkg work, we decided to fold
> them into the same patch set that was already being reviewed.
> 
> I also verified build and boot to shell for 6 combinations
> on GCC5 (IA32/X64 for RELEASE/DEBUG/NOOPT) and the 12
> combinations of VS2015/VS2017, IA323/X64, RELEASE/DEBUG/NOOPT.
> 
> I have been working on some CI experiments using Azure Pipelines.
> Here is a pointer to the build logs for all the combinations 
> listed above.
> 
> https://dev.azure.com/mikekinney/edk2-ci/_build/results?buildId=312
> 
> Mike
> 
>> -----Original Message-----
>> From: Justen, Jordan L
>> Sent: Friday, August 16, 2019 12:41 AM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>;
>> devel@edk2.groups.io
>> Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish
>> <afish@apple.com>
>> Subject: Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5
>> lldb issues
>> 
>> On 2019-08-15 19:14:33, Michael D Kinney wrote:
>>> Fix scripts to support lldb symbolic debugging when
>> using XCODE5 tool
>>> chain.
>>> 
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Ray Ni <ray.ni@intel.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Signed-off-by: Andrew Fish <afish@apple.com>
>> 
>> Is this a Cc/Signed-off-by typo? (See also, patches 7-
>> 10).
>> 
>> This makes me wonder if you are taking advantage of the
>> git commit -s switch to add your Signed-off-by.
>> 
>> Also, I'm wondering if you are taking advantage of git-
>> send-email automatically Cc'ing the addresses you
>> listed in the commit message.
>> (I thought it Cc'd for the author and Cc tags, but I
>> wasn't sure about the Signed-off-by tag, and yet I see
>> Andrew was Cc'd.)
>> 
>> There's a couple long lines below. You could use \ at
>> the end of the line to split the .sh line. I think the
>> cd can be a separate command in a shell script. (Not in
>> make)
>> 
>> I hope someone that uses the XCODE toolchain could
>> review/check the XCODE patches.
>> 
>> -Jordan
>> 
>>> ---
>>> EmulatorPkg/Unix/lldbefi.py |  8 +++++---
>>> EmulatorPkg/build.sh        | 17 ++---------------
>>> 2 files changed, 7 insertions(+), 18 deletions(-)
>>> 
>>> diff --git a/EmulatorPkg/Unix/lldbefi.py
>> b/EmulatorPkg/Unix/lldbefi.py
>>> index 218326b8cb..099192d8b5 100755
>>> --- a/EmulatorPkg/Unix/lldbefi.py
>>> +++ b/EmulatorPkg/Unix/lldbefi.py
>>> @@ -346,6 +346,7 @@ def TypePrintFormating(debugger):
>>>     debugger.HandleCommand("type summary add CHAR8 -
>> -python-function lldbefi.CHAR8_TypeSummary")
>>>     debugger.HandleCommand('type summary add --regex
>> "CHAR8
>>> \[[0-9]+\]" --python-function
>> lldbefi.CHAR8_TypeSummary')
>>> 
>>> +    debugger.HandleCommand('setting set frame-format
>> "frame
>>> + #${frame.index}: ${frame.pc}{
>>> +
>> ${module.file.basename}{:${function.name}()${function.p
>> c-offset}}}{
>>> + at ${line.file.fullpath}:${line.number}}\n"')
>>> 
>>> gEmulatorBreakWorkaroundNeeded = True
>>> 
>>> @@ -381,15 +382,16 @@ def
>> LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
>>>     Error = lldb.SBError()
>>>     FileNamePtr = frame.FindVariable
>> ("FileName").GetValueAsUnsigned()
>>>     FileNameLen = frame.FindVariable
>>> ("FileNameLength").GetValueAsUnsigned()
>>> +
>>>     FileName =
>> frame.thread.process.ReadCStringFromMemory
>> (FileNamePtr, FileNameLen, Error)
>>>     if not Error.Success():
>>>         print "!ReadCStringFromMemory() did not find
>> a %d byte C string at %x" % (FileNameLen, FileNamePtr)
>>>         # make breakpoint command contiue
>>> -        frame.GetThread().GetProcess().Continue()
>>> +        return False
>>> 
>>>     debugger = frame.thread.process.target.debugger
>>>     if frame.FindVariable
>> ("AddSymbolFlag").GetValueAsUnsigned() == 1:
>>> -        LoadAddress = frame.FindVariable
>> ("LoadAddress").GetValueAsUnsigned()
>>> +        LoadAddress = frame.FindVariable
>>> + ("LoadAddress").GetValueAsUnsigned() - 0x240
>>> 
>>>         debugger.HandleCommand ("target modules add
>> %s" % FileName)
>>>         print "target modules load --slid 0x%x %s" %
>> (LoadAddress,
>>> FileName) @@ -405,7 +407,7 @@ def
>> LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
>>>                     print "!lldb.target.RemoveModule
>> (%s) FAILED" %
>>> SBModule
>>> 
>>>     # make breakpoint command contiue
>>> -    frame.thread.process.Continue()
>>> +    return False
>>> 
>>> def GuidToCStructStr (guid, Name=False):
>>>   #
>>> diff --git a/EmulatorPkg/build.sh
>> b/EmulatorPkg/build.sh index
>>> 60056e1b6c..35912a7775 100755
>>> --- a/EmulatorPkg/build.sh
>>> +++ b/EmulatorPkg/build.sh
>>> @@ -209,21 +209,8 @@ fi
>>> if [[ "$RUN_EMULATOR" == "yes" ]]; then
>>>   case `uname` in
>>>     Darwin*)
>>> -      #
>>> -      # On Darwin we can't use dlopen, so we have to
>> load the real PE/COFF images.
>>> -      # This .gdbinit script sets a breakpoint that
>> loads symbols for the PE/COFFEE
>>> -      # images that get loaded in Host
>>> -      #
>>> -      if [[ "$CLANG_VER" == *-ccc-host-triple* ]]
>>> -      then
>>> -      # only older versions of Xcode support -ccc-
>> host-tripe, for newer versions
>>> -      # it is -target
>>> -        cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
>> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
>> SOR"
>>> -        cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source
>> $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
>>> -        exit $?
>>> -      else
>>> -        cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
>> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
>> SOR"
>>> -      fi
>>> +      cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o "command
>> script import $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" -
>> o 'script lldb.debugger.SetAsync(True)' -o "run" ./Host
>>> +      exit $?
>>>       ;;
>>>   esac
>>> 
>>> --
>>> 2.21.0.windows.1
>>> 
> 
> 
> 


[-- Attachment #2: Type: text/html, Size: 11923 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
  2019-08-16 15:09     ` Michael D Kinney
  2019-08-16 17:30       ` [edk2-devel] " Andrew Fish
@ 2019-08-16 18:21       ` Jordan Justen
  2019-08-16 21:14         ` Michael D Kinney
  1 sibling, 1 reply; 19+ messages in thread
From: Jordan Justen @ 2019-08-16 18:21 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Ni, Ray, Andrew Fish

On 2019-08-16 08:09:55, Kinney, Michael D wrote:
> Jordan,
> 
> It is not a typo.
> 
> Andrew generated the XCODE specific changes, so they have 
> been tested by him.

In that case, is the git author for the patches set to Andrew? If it
was, I would expect to see a line inside the email body with:

From: Andrew Fish <afish@apple.com>

Git does that for patches where the sender doesn't match the patch
author.

You might want to rebase and run:

git commit --amend --author="Andrew Fish <afish@apple.com>" --reset-author

to change the author.

I expect you might want to add Reviewed-by for yourself on these
patches to help speed things along. If Andrew authored the patch, you
reviewed it, and with a quick review it looked good to me, I would
probably add an Acked-by for some of the patches.

-Jordan

> I have also reviewed and tested the XCODE
> changes and verified that all 6 combinations build and boot
> to shell (IA32/X64 for RELEASE/DEBUG/NOOPT). Since they are
> all related to making EmulatorPkg work, we decided to fold
> them into the same patch set that was already being reviewed.
> 
> I also verified build and boot to shell for 6 combinations
> on GCC5 (IA32/X64 for RELEASE/DEBUG/NOOPT) and the 12
> combinations of VS2015/VS2017, IA323/X64, RELEASE/DEBUG/NOOPT.
> 
> I have been working on some CI experiments using Azure Pipelines.
> Here is a pointer to the build logs for all the combinations 
> listed above.
> 
> https://dev.azure.com/mikekinney/edk2-ci/_build/results?buildId=312
> 
> Mike
> 
> > -----Original Message-----
> > From: Justen, Jordan L
> > Sent: Friday, August 16, 2019 12:41 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish
> > <afish@apple.com>
> > Subject: Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5
> > lldb issues
> > 
> > On 2019-08-15 19:14:33, Michael D Kinney wrote:
> > > Fix scripts to support lldb symbolic debugging when
> > using XCODE5 tool
> > > chain.
> > >
> > > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Signed-off-by: Andrew Fish <afish@apple.com>
> > 
> > Is this a Cc/Signed-off-by typo? (See also, patches 7-
> > 10).
> > 
> > This makes me wonder if you are taking advantage of the
> > git commit -s switch to add your Signed-off-by.
> > 
> > Also, I'm wondering if you are taking advantage of git-
> > send-email automatically Cc'ing the addresses you
> > listed in the commit message.
> > (I thought it Cc'd for the author and Cc tags, but I
> > wasn't sure about the Signed-off-by tag, and yet I see
> > Andrew was Cc'd.)
> > 
> > There's a couple long lines below. You could use \ at
> > the end of the line to split the .sh line. I think the
> > cd can be a separate command in a shell script. (Not in
> > make)
> > 
> > I hope someone that uses the XCODE toolchain could
> > review/check the XCODE patches.
> > 
> > -Jordan
> > 
> > > ---
> > >  EmulatorPkg/Unix/lldbefi.py |  8 +++++---
> > >  EmulatorPkg/build.sh        | 17 ++---------------
> > >  2 files changed, 7 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/EmulatorPkg/Unix/lldbefi.py
> > b/EmulatorPkg/Unix/lldbefi.py
> > > index 218326b8cb..099192d8b5 100755
> > > --- a/EmulatorPkg/Unix/lldbefi.py
> > > +++ b/EmulatorPkg/Unix/lldbefi.py
> > > @@ -346,6 +346,7 @@ def TypePrintFormating(debugger):
> > >      debugger.HandleCommand("type summary add CHAR8 -
> > -python-function lldbefi.CHAR8_TypeSummary")
> > >      debugger.HandleCommand('type summary add --regex
> > "CHAR8
> > > \[[0-9]+\]" --python-function
> > lldbefi.CHAR8_TypeSummary')
> > >
> > > +    debugger.HandleCommand('setting set frame-format
> > "frame
> > > + #${frame.index}: ${frame.pc}{
> > > +
> > ${module.file.basename}{:${function.name}()${function.p
> > c-offset}}}{
> > > + at ${line.file.fullpath}:${line.number}}\n"')
> > >
> > >  gEmulatorBreakWorkaroundNeeded = True
> > >
> > > @@ -381,15 +382,16 @@ def
> > LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
> > >      Error = lldb.SBError()
> > >      FileNamePtr = frame.FindVariable
> > ("FileName").GetValueAsUnsigned()
> > >      FileNameLen = frame.FindVariable
> > > ("FileNameLength").GetValueAsUnsigned()
> > > +
> > >      FileName =
> > frame.thread.process.ReadCStringFromMemory
> > (FileNamePtr, FileNameLen, Error)
> > >      if not Error.Success():
> > >          print "!ReadCStringFromMemory() did not find
> > a %d byte C string at %x" % (FileNameLen, FileNamePtr)
> > >          # make breakpoint command contiue
> > > -        frame.GetThread().GetProcess().Continue()
> > > +        return False
> > >
> > >      debugger = frame.thread.process.target.debugger
> > >      if frame.FindVariable
> > ("AddSymbolFlag").GetValueAsUnsigned() == 1:
> > > -        LoadAddress = frame.FindVariable
> > ("LoadAddress").GetValueAsUnsigned()
> > > +        LoadAddress = frame.FindVariable
> > > + ("LoadAddress").GetValueAsUnsigned() - 0x240
> > >
> > >          debugger.HandleCommand ("target modules add
> > %s" % FileName)
> > >          print "target modules load --slid 0x%x %s" %
> > (LoadAddress,
> > > FileName) @@ -405,7 +407,7 @@ def
> > LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
> > >                      print "!lldb.target.RemoveModule
> > (%s) FAILED" %
> > > SBModule
> > >
> > >      # make breakpoint command contiue
> > > -    frame.thread.process.Continue()
> > > +    return False
> > >
> > >  def GuidToCStructStr (guid, Name=False):
> > >    #
> > > diff --git a/EmulatorPkg/build.sh
> > b/EmulatorPkg/build.sh index
> > > 60056e1b6c..35912a7775 100755
> > > --- a/EmulatorPkg/build.sh
> > > +++ b/EmulatorPkg/build.sh
> > > @@ -209,21 +209,8 @@ fi
> > >  if [[ "$RUN_EMULATOR" == "yes" ]]; then
> > >    case `uname` in
> > >      Darwin*)
> > > -      #
> > > -      # On Darwin we can't use dlopen, so we have to
> > load the real PE/COFF images.
> > > -      # This .gdbinit script sets a breakpoint that
> > loads symbols for the PE/COFFEE
> > > -      # images that get loaded in Host
> > > -      #
> > > -      if [[ "$CLANG_VER" == *-ccc-host-triple* ]]
> > > -      then
> > > -      # only older versions of Xcode support -ccc-
> > host-tripe, for newer versions
> > > -      # it is -target
> > > -        cp $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
> > "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> > SOR"
> > > -        cd $BUILD_ROOT_ARCH; /usr/bin/lldb --source
> > $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
> > > -        exit $?
> > > -      else
> > > -        cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
> > "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> > SOR"
> > > -      fi
> > > +      cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o "command
> > script import $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" -
> > o 'script lldb.debugger.SetAsync(True)' -o "run" ./Host
> > > +      exit $?
> > >        ;;
> > >    esac
> > >
> > > --
> > > 2.21.0.windows.1
> > >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope for XCODE5
  2019-08-16  2:14 ` [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope " Michael D Kinney
@ 2019-08-16 18:29   ` Jordan Justen
  0 siblings, 0 replies; 19+ messages in thread
From: Jordan Justen @ 2019-08-16 18:29 UTC (permalink / raw)
  To: Michael D Kinney, devel; +Cc: Ray Ni, Andrew Fish

On 2019-08-15 19:14:36, Michael D Kinney wrote:
> The local variable PpiArray[10] is declared in middle
> of the SEC module _ModuleEntryPoint().  When building
> for XCODE5 with optimizations enabled, this results in
> corruption and an exception.

It looks like with old code, the scope containing PpiArray was closed,
but a dangling reference to had been made to it's location on the
stack. So, I think the change is good but we should add this extra
detail to the commit message.

-Jordan

> The fix is to move the
> declaration of PpiArray[10] to the standard location
> at the beginning of the function so the storage for
> this local variable is allocated for the entire
> lifetime of the function.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Andrew Fish <afish@apple.com>
> ---
>  EmulatorPkg/Sec/Sec.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/EmulatorPkg/Sec/Sec.c b/EmulatorPkg/Sec/Sec.c
> index 701032233b..b734d2bb87 100644
> --- a/EmulatorPkg/Sec/Sec.c
> +++ b/EmulatorPkg/Sec/Sec.c
> @@ -75,6 +75,7 @@ _ModuleEntryPoint (
>    EFI_PEI_PPI_DESCRIPTOR    *SecPpiList;
>    UINTN                     SecReseveredMemorySize;
>    UINTN                     Index;
> +  EFI_PEI_PPI_DESCRIPTOR    PpiArray[10];
>  
>    EMU_MAGIC_PAGE()->PpiList = PpiList;
>    ProcessLibraryConstructorList ();
> @@ -104,16 +105,13 @@ _ModuleEntryPoint (
>    SecCoreData->PeiTemporaryRamBase = (VOID *)((UINTN)SecCoreData->PeiTemporaryRamBase + SecReseveredMemorySize);
>    SecCoreData->PeiTemporaryRamSize -= SecReseveredMemorySize;
>  #else
> -  {
> -    //
> -    // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? Either there is a bug
> -    // or I don't understand temp RAM correctly?
> -    //
> -    EFI_PEI_PPI_DESCRIPTOR    PpiArray[10];
> +  //
> +  // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? Either there is a bug
> +  // or I don't understand temp RAM correctly?
> +  //
>  
> -    SecPpiList = &PpiArray[0];
> -    ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
> -  }
> +  SecPpiList = &PpiArray[0];
> +  ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
>  #endif
>    // Copy existing list, and append our entries.
>    CopyMem (SecPpiList, PpiList, sizeof (EFI_PEI_PPI_DESCRIPTOR) * Index);
> -- 
> 2.21.0.windows.1
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues
  2019-08-16 18:21       ` Jordan Justen
@ 2019-08-16 21:14         ` Michael D Kinney
  0 siblings, 0 replies; 19+ messages in thread
From: Michael D Kinney @ 2019-08-16 21:14 UTC (permalink / raw)
  To: Justen, Jordan L, devel@edk2.groups.io, Kinney, Michael D
  Cc: Ni, Ray, Andrew Fish

Jordan,

Thanks.  Yes.  Updating the Author sounds like the
right fix.  I will send an updated series soon.

Thanks,

Mike

> -----Original Message-----
> From: Justen, Jordan L
> Sent: Friday, August 16, 2019 11:21 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: RE: [Patch V4 06/10] EmulatorPkg: Fix XCODE5
> lldb issues
> 
> On 2019-08-16 08:09:55, Kinney, Michael D wrote:
> > Jordan,
> >
> > It is not a typo.
> >
> > Andrew generated the XCODE specific changes, so they
> have been tested
> > by him.
> 
> In that case, is the git author for the patches set to
> Andrew? If it was, I would expect to see a line inside
> the email body with:
> 
> From: Andrew Fish <afish@apple.com>
> 
> Git does that for patches where the sender doesn't
> match the patch author.
> 
> You might want to rebase and run:
> 
> git commit --amend --author="Andrew Fish
> <afish@apple.com>" --reset-author
> 
> to change the author.
> 
> I expect you might want to add Reviewed-by for yourself
> on these patches to help speed things along. If Andrew
> authored the patch, you reviewed it, and with a quick
> review it looked good to me, I would probably add an
> Acked-by for some of the patches.
> 
> -Jordan
> 
> > I have also reviewed and tested the XCODE changes and
> verified that
> > all 6 combinations build and boot to shell (IA32/X64
> for
> > RELEASE/DEBUG/NOOPT). Since they are all related to
> making EmulatorPkg
> > work, we decided to fold them into the same patch set
> that was already
> > being reviewed.
> >
> > I also verified build and boot to shell for 6
> combinations on GCC5
> > (IA32/X64 for RELEASE/DEBUG/NOOPT) and the 12
> combinations of
> > VS2015/VS2017, IA323/X64, RELEASE/DEBUG/NOOPT.
> >
> > I have been working on some CI experiments using
> Azure Pipelines.
> > Here is a pointer to the build logs for all the
> combinations listed
> > above.
> >
> > https://dev.azure.com/mikekinney/edk2-
> ci/_build/results?buildId=312
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Justen, Jordan L
> > > Sent: Friday, August 16, 2019 12:41 AM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish
> <afish@apple.com>
> > > Subject: Re: [Patch V4 06/10] EmulatorPkg: Fix
> XCODE5 lldb issues
> > >
> > > On 2019-08-15 19:14:33, Michael D Kinney wrote:
> > > > Fix scripts to support lldb symbolic debugging
> when
> > > using XCODE5 tool
> > > > chain.
> > > >
> > > > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > Signed-off-by: Andrew Fish <afish@apple.com>
> > >
> > > Is this a Cc/Signed-off-by typo? (See also, patches
> 7- 10).
> > >
> > > This makes me wonder if you are taking advantage of
> the git commit
> > > -s switch to add your Signed-off-by.
> > >
> > > Also, I'm wondering if you are taking advantage of
> git- send-email
> > > automatically Cc'ing the addresses you listed in
> the commit message.
> > > (I thought it Cc'd for the author and Cc tags, but
> I wasn't sure
> > > about the Signed-off-by tag, and yet I see Andrew
> was Cc'd.)
> > >
> > > There's a couple long lines below. You could use \
> at the end of the
> > > line to split the .sh line. I think the cd can be a
> separate command
> > > in a shell script. (Not in
> > > make)
> > >
> > > I hope someone that uses the XCODE toolchain could
> review/check the
> > > XCODE patches.
> > >
> > > -Jordan
> > >
> > > > ---
> > > >  EmulatorPkg/Unix/lldbefi.py |  8 +++++---
> > > >  EmulatorPkg/build.sh        | 17 ++-------------
> --
> > > >  2 files changed, 7 insertions(+), 18 deletions(-
> )
> > > >
> > > > diff --git a/EmulatorPkg/Unix/lldbefi.py
> > > b/EmulatorPkg/Unix/lldbefi.py
> > > > index 218326b8cb..099192d8b5 100755
> > > > --- a/EmulatorPkg/Unix/lldbefi.py
> > > > +++ b/EmulatorPkg/Unix/lldbefi.py
> > > > @@ -346,6 +346,7 @@ def
> TypePrintFormating(debugger):
> > > >      debugger.HandleCommand("type summary add
> CHAR8 -
> > > -python-function lldbefi.CHAR8_TypeSummary")
> > > >      debugger.HandleCommand('type summary add --
> regex
> > > "CHAR8
> > > > \[[0-9]+\]" --python-function
> > > lldbefi.CHAR8_TypeSummary')
> > > >
> > > > +    debugger.HandleCommand('setting set frame-
> format
> > > "frame
> > > > + #${frame.index}: ${frame.pc}{
> > > > +
> > >
> ${module.file.basename}{:${function.name}()${function.p
> > > c-offset}}}{
> > > > + at ${line.file.fullpath}:${line.number}}\n"')
> > > >
> > > >  gEmulatorBreakWorkaroundNeeded = True
> > > >
> > > > @@ -381,15 +382,16 @@ def
> > > LoadEmulatorEfiSymbols(frame, bp_loc ,
> internal_dict):
> > > >      Error = lldb.SBError()
> > > >      FileNamePtr = frame.FindVariable
> > > ("FileName").GetValueAsUnsigned()
> > > >      FileNameLen = frame.FindVariable
> > > > ("FileNameLength").GetValueAsUnsigned()
> > > > +
> > > >      FileName =
> > > frame.thread.process.ReadCStringFromMemory
> > > (FileNamePtr, FileNameLen, Error)
> > > >      if not Error.Success():
> > > >          print "!ReadCStringFromMemory() did not
> find
> > > a %d byte C string at %x" % (FileNameLen,
> FileNamePtr)
> > > >          # make breakpoint command contiue
> > > > -
> frame.GetThread().GetProcess().Continue()
> > > > +        return False
> > > >
> > > >      debugger =
> frame.thread.process.target.debugger
> > > >      if frame.FindVariable
> > > ("AddSymbolFlag").GetValueAsUnsigned() == 1:
> > > > -        LoadAddress = frame.FindVariable
> > > ("LoadAddress").GetValueAsUnsigned()
> > > > +        LoadAddress = frame.FindVariable
> > > > + ("LoadAddress").GetValueAsUnsigned() - 0x240
> > > >
> > > >          debugger.HandleCommand ("target modules
> add
> > > %s" % FileName)
> > > >          print "target modules load --slid 0x%x
> %s" %
> > > (LoadAddress,
> > > > FileName) @@ -405,7 +407,7 @@ def
> > > LoadEmulatorEfiSymbols(frame, bp_loc ,
> internal_dict):
> > > >                      print
> "!lldb.target.RemoveModule
> > > (%s) FAILED" %
> > > > SBModule
> > > >
> > > >      # make breakpoint command contiue
> > > > -    frame.thread.process.Continue()
> > > > +    return False
> > > >
> > > >  def GuidToCStructStr (guid, Name=False):
> > > >    #
> > > > diff --git a/EmulatorPkg/build.sh
> > > b/EmulatorPkg/build.sh index
> > > > 60056e1b6c..35912a7775 100755
> > > > --- a/EmulatorPkg/build.sh
> > > > +++ b/EmulatorPkg/build.sh
> > > > @@ -209,21 +209,8 @@ fi
> > > >  if [[ "$RUN_EMULATOR" == "yes" ]]; then
> > > >    case `uname` in
> > > >      Darwin*)
> > > > -      #
> > > > -      # On Darwin we can't use dlopen, so we
> have to
> > > load the real PE/COFF images.
> > > > -      # This .gdbinit script sets a breakpoint
> that
> > > loads symbols for the PE/COFFEE
> > > > -      # images that get loaded in Host
> > > > -      #
> > > > -      if [[ "$CLANG_VER" == *-ccc-host-triple*
> ]]
> > > > -      then
> > > > -      # only older versions of Xcode support -
> ccc-
> > > host-tripe, for newer versions
> > > > -      # it is -target
> > > > -        cp
> $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
> > >
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> > > SOR"
> > > > -        cd $BUILD_ROOT_ARCH; /usr/bin/lldb --
> source
> > > $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
> > > > -        exit $?
> > > > -      else
> > > > -        cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
> > >
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> > > SOR"
> > > > -      fi
> > > > +      cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o
> "command
> > > script import
> $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" - o 'script
> > > lldb.debugger.SetAsync(True)' -o "run" ./Host
> > > > +      exit $?
> > > >        ;;
> > > >    esac
> > > >
> > > > --
> > > > 2.21.0.windows.1
> > > >

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-08-16 21:14 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-16  2:14 [Patch V4 00/10] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
2019-08-16  2:14 ` [Patch V4 01/10] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
2019-08-16  2:14 ` [Patch V4 02/10] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Michael D Kinney
2019-08-16  2:14 ` [Patch V4 03/10] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Michael D Kinney
2019-08-16  2:14 ` [Patch V4 04/10] EmulatorPkg: Add support for NOOPT target Michael D Kinney
2019-08-16  2:14 ` [Patch V4 05/10] EmulatorPkg/Unix/Host: Disable inline/optimizations Michael D Kinney
2019-08-16  2:14 ` [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues Michael D Kinney
2019-08-16  7:40   ` Jordan Justen
2019-08-16 15:09     ` Michael D Kinney
2019-08-16 17:30       ` [edk2-devel] " Andrew Fish
2019-08-16 18:21       ` Jordan Justen
2019-08-16 21:14         ` Michael D Kinney
2019-08-16  2:14 ` [Patch V4 07/10] EmulatorPkg/Unix/Host: Fix BerkeleyPacketFilter.c issues Michael D Kinney
2019-08-16  7:47   ` Jordan Justen
2019-08-16  2:14 ` [Patch V4 08/10] EmulatorPkg: Disable TftpDynamicCommand and LogoDxe for XCODE5 Michael D Kinney
2019-08-16  2:14 ` [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope " Michael D Kinney
2019-08-16 18:29   ` Jordan Justen
2019-08-16  2:14 ` [Patch V4 10/10] BaseTools/tools_def.template: Add -gdwarf to XCODE5 X64 Michael D Kinney
2019-08-16  4:50   ` [edk2-devel] " Liming Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox