[Patch 0/3] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [Patch 0/3] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config
@ 2019-08-07  4:20 Michael D Kinney
  2019-08-07  4:20 ` [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Michael D Kinney @ 2019-08-07  4:20 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni

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 (3):
  EmulatorPkg: Fix VS20xx IA32 boot failure
  EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES

 EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c |   9 +-
 EmulatorPkg/EmulatorPkg.dsc                   |  36 +++---
 EmulatorPkg/FlashMapPei/FlashMapPei.c         |   8 +-
 EmulatorPkg/Library/SmbiosLib/SmbiosLib.c     |   4 +-
 .../ThunkProtocolList/ThunkProtocolList.c     |  11 +-
 EmulatorPkg/Readme.md                         |   8 +-
 EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c  |   8 +-
 EmulatorPkg/Unix/Host/PosixFileSystem.c       |  30 ++++-
 EmulatorPkg/Unix/Host/X11GraphicsWindow.c     |   4 +-
 EmulatorPkg/Win/Host/WinFileSystem.c          | 116 ++++++++++++------
 EmulatorPkg/Win/VS2017/BuildVS.bat            |   2 +-
 EmulatorPkg/build.sh                          |   8 +-
 12 files changed, 164 insertions(+), 80 deletions(-)

-- 
2.21.0.windows.1


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

* [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure
  2019-08-07  4:20 [Patch 0/3] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
@ 2019-08-07  4:20 ` Michael D Kinney
  2019-08-07  6:18   ` [edk2-devel] " Wu, Hao A
  2019-08-07  4:20 ` [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Michael D Kinney
  2019-08-07  4:20 ` [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Michael D Kinney
  2 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2019-08-07  4:20 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni

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
is set for X64 Windows Host environments.  If the
EmulatorPkg is build for an IA32 Windows Host environment,
then set this PCD to 0x13000000 that can be mapped into a
32-bit 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>
---
 EmulatorPkg/EmulatorPkg.dsc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index ea8b6ce76e..c4ec10d1d8 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
@@ -225,6 +225,10 @@ [PcdsFixedAtBuild]
   #  0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
 
+!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
+  gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13000000
+!endif
+
 [PcdsDynamicDefault.common.DEFAULT]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0
-- 
2.21.0.windows.1


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

* [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  2019-08-07  4:20 [Patch 0/3] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
  2019-08-07  4:20 ` [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
@ 2019-08-07  4:20 ` Michael D Kinney
  2019-08-07  6:41   ` [edk2-devel] " Wu, Hao A
  2019-08-07  4:20 ` [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Michael D Kinney
  2 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2019-08-07  4:20 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni

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>
---
 EmulatorPkg/EmulatorPkg.dsc        | 24 ++++++++++++------------
 EmulatorPkg/Readme.md              |  8 ++++----
 EmulatorPkg/Win/VS2017/BuildVS.bat |  2 +-
 EmulatorPkg/build.sh               |  8 ++++----
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index c4ec10d1d8..c9e4a5b34d 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -240,18 +240,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] 15+ messages in thread

* [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES
  2019-08-07  4:20 [Patch 0/3] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
  2019-08-07  4:20 ` [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
  2019-08-07  4:20 ` [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Michael D Kinney
@ 2019-08-07  4:20 ` Michael D Kinney
  2019-08-07  7:58   ` [edk2-devel] " Wu, Hao A
  2 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2019-08-07  4:20 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Andrew Fish, Ray Ni

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>
---
 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  |   8 +-
 EmulatorPkg/Unix/Host/PosixFileSystem.c       |  30 ++++-
 EmulatorPkg/Unix/Host/X11GraphicsWindow.c     |   4 +-
 EmulatorPkg/Win/Host/WinFileSystem.c          | 116 ++++++++++++------
 9 files changed, 138 insertions(+), 58 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 c9e4a5b34d..39a6658427 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -412,10 +412,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..18cb3831a4 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
@@ -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..b2b2d011c9 100644
--- a/EmulatorPkg/Unix/Host/PosixFileSystem.c
+++ b/EmulatorPkg/Unix/Host/PosixFileSystem.c
@@ -1017,7 +1017,11 @@ PosixFileGetInfo (
     FileSystemInfoBuffer->BlockSize   = buf.f_bsize;
 
 
-    StrCpy ((CHAR16 *) FileSystemInfoBuffer->VolumeLabel, PrivateRoot->VolumeLabel);
+    StrCpyS (
+      (CHAR16 *) FileSystemInfoBuffer->VolumeLabel,
+      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),
+      PrivateRoot->VolumeLabel
+      );
     *BufferSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (PrivateRoot->VolumeLabel);
 
   } else if (CompareGuid (InformationType, &gEfiFileSystemVolumeLabelInfoIdGuid)) {
@@ -1026,7 +1030,11 @@ PosixFileGetInfo (
       return EFI_BUFFER_TOO_SMALL;
     }
 
-    StrCpy ((CHAR16 *) Buffer, PrivateRoot->VolumeLabel);
+    StrCpyS (
+      (CHAR16 *) Buffer,
+      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),
+      PrivateRoot->VolumeLabel
+      );
     *BufferSize = StrSize (PrivateRoot->VolumeLabel);
 
   }
@@ -1110,7 +1118,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 +1137,11 @@ PosixFileSetInfo (
       goto Done;
     }
 
-    StrCpy (PrivateRoot->VolumeLabel, (CHAR16 *) Buffer);
+    StrCpyS (
+      PrivateRoot->VolumeLabel,
+      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),
+      (CHAR16 *) Buffer
+      );
 
     Status = EFI_SUCCESS;
     goto Done;
@@ -1493,7 +1509,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..bb64439007 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, L"\\");
+      StrCatS (NewPrivateFile->FileName, Size, 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, 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,
+      (StrSize (PrivateRoot->VolumeLabel) + 1) / 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,
+      StrSize (PrivateRoot->VolumeLabel) / 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, PrivateFile->FilePath);
+    StrCatS (NewFileName, Size, L"\\");
+    StrCatS (NewFileName, Size, 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, 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, L"\\*");
         PrivateFile->LHandle = FindFirstFile (TempFileName, &FindBuf);
       }
 
-- 
2.21.0.windows.1


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

* Re: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure
  2019-08-07  4:20 ` [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
@ 2019-08-07  6:18   ` Wu, Hao A
  2019-08-07  7:42     ` Michael D Kinney
  0 siblings, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2019-08-07  6:18 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael D Kinney
> Sent: Wednesday, August 07, 2019 12:20 PM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> Subject: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure
> 
> 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
> is set for X64 Windows Host environments.  If the
> EmulatorPkg is build for an IA32 Windows Host environment,
> then set this PCD to 0x13000000 that can be mapped into a
> 32-bit user process.


Hello Mike,

I have a question for this patch.

For the main() function within file EmulatorPkg/Win/Host/WinHost.c, I saw
the below codes:

  EmuMagicPage = (VOID *)(UINTN)(FixedPcdGet64 (PcdPeiServicesTablePage) & MAX_UINTN);
  if (EmuMagicPage != NULL) {
    UINT64  Size;
    Status = WinNtOpenFile (
              NULL,
              SIZE_4KB,
              0,
              &EmuMagicPage,
              &Size
              );
    if (EFI_ERROR (Status)) {
      SecPrint ("ERROR : Could not allocate PeiServicesTablePage @ %p\n", EmuMagicPage);
      return EFI_DEVICE_ERROR;
    }
  }

My understanding is that the WinNtOpenFile() function call is attempting
to map the address specified by 'EmuMagicPage'. For IA32 case, the value
in 'EmuMagicPage' here has already been truncated (0x03000000). If the
WinNtOpenFile() call returned successfully, the address should be mapped.

Is it the case that even if WinNtOpenFile() returns with no error, the
specified address (0x03000000) is not actually being mapped?

Best Regards,
Hao Wu


> 
> 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/EmulatorPkg.dsc | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
> index ea8b6ce76e..c4ec10d1d8 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
> @@ -225,6 +225,10 @@ [PcdsFixedAtBuild]
>    #  0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
>    gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
> 
> +!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
> +  gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13000000
> +!endif
> +
>  [PcdsDynamicDefault.common.DEFAULT]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|
> 0
> --
> 2.21.0.windows.1
> 
> 
> 


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

* Re: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  2019-08-07  4:20 ` [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Michael D Kinney
@ 2019-08-07  6:41   ` Wu, Hao A
  2019-08-07  7:45     ` Michael D Kinney
  0 siblings, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2019-08-07  6:41 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael D Kinney
> Sent: Wednesday, August 07, 2019 12:20 PM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> Subject: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove
> UNIX_SEC_BUILD/WIN_SEC_BUILD
> 
> 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>
> ---
>  EmulatorPkg/EmulatorPkg.dsc        | 24 ++++++++++++------------
>  EmulatorPkg/Readme.md              |  8 ++++----
>  EmulatorPkg/Win/VS2017/BuildVS.bat |  2 +-
>  EmulatorPkg/build.sh               |  8 ++++----
>  4 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
> index c4ec10d1d8..c9e4a5b34d 100644
> --- a/EmulatorPkg/EmulatorPkg.dsc
> +++ b/EmulatorPkg/EmulatorPkg.dsc
> @@ -240,18 +240,18 @@ [PcdsDynamicHii.common.DEFAULT]
> 
> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlo
> balVariableGuid|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)


Just a thought, the only supported architectures within EmulatorPkg.dsc
are IA32 and X64 at this moment. Do you think we can omit the arch check
here?

Anyway, the patch looks good to me:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> +  !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	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure
  2019-08-07  6:18   ` [edk2-devel] " Wu, Hao A
@ 2019-08-07  7:42     ` Michael D Kinney
  2019-08-07  8:15       ` Wu, Hao A
  0 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2019-08-07  7:42 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

I observed 2 failure modes

1) The range could not be mapped at the request address
   And an error message was displayed and the app exited.
2) The mapping passed, but when it was accessed by the
   Sec module, the app generated an exception.

Mike

> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, August 6, 2019 11:19 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
> VS20xx IA32 boot failure
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of
> > Michael D Kinney
> > Sent: Wednesday, August 07, 2019 12:20 PM
> > To: devel@edk2.groups.io
> > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > Subject: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
> VS20xx IA32 boot
> > failure
> >
> > 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 is set
> for X64 Windows
> > Host environments.  If the EmulatorPkg is build for
> an IA32 Windows
> > Host environment, then set this PCD to 0x13000000
> that can be mapped
> > into a 32-bit user process.
> 
> 
> Hello Mike,
> 
> I have a question for this patch.
> 
> For the main() function within file
> EmulatorPkg/Win/Host/WinHost.c, I saw the below codes:
> 
>   EmuMagicPage = (VOID *)(UINTN)(FixedPcdGet64
> (PcdPeiServicesTablePage) & MAX_UINTN);
>   if (EmuMagicPage != NULL) {
>     UINT64  Size;
>     Status = WinNtOpenFile (
>               NULL,
>               SIZE_4KB,
>               0,
>               &EmuMagicPage,
>               &Size
>               );
>     if (EFI_ERROR (Status)) {
>       SecPrint ("ERROR : Could not allocate
> PeiServicesTablePage @ %p\n", EmuMagicPage);
>       return EFI_DEVICE_ERROR;
>     }
>   }
> 
> My understanding is that the WinNtOpenFile() function
> call is attempting to map the address specified by
> 'EmuMagicPage'. For IA32 case, the value in
> 'EmuMagicPage' here has already been truncated
> (0x03000000). If the
> WinNtOpenFile() call returned successfully, the address
> should be mapped.
> 
> Is it the case that even if WinNtOpenFile() returns
> with no error, the specified address (0x03000000) is
> not actually being mapped?
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > 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/EmulatorPkg.dsc | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> b/EmulatorPkg/EmulatorPkg.dsc
> > index ea8b6ce76e..c4ec10d1d8 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 @@
> > -225,6 +225,10 @@ [PcdsFixedAtBuild]
> >    #  0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
> >    gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
> >
> > +!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
> > +
> gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13
> 000000
> > +!endif
> > +
> >  [PcdsDynamicDefault.common.DEFAULT]
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpar
> eBase64|0
> >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWork
> ingBase64|
> > 0
> > --
> > 2.21.0.windows.1
> >
> >
> > 


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

* Re: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  2019-08-07  6:41   ` [edk2-devel] " Wu, Hao A
@ 2019-08-07  7:45     ` Michael D Kinney
  2019-08-07  7:59       ` Wu, Hao A
  0 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2019-08-07  7:45 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

Hao Wu,

I put the arch check in on purpose because more
CPU archs might be added in the future.

Mike

> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, August 6, 2019 11:42 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 2/3] EmulatorPkg:
> Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of
> > Michael D Kinney
> > Sent: Wednesday, August 07, 2019 12:20 PM
> > To: devel@edk2.groups.io
> > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > Subject: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove
> > UNIX_SEC_BUILD/WIN_SEC_BUILD
> >
> > 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>
> > ---
> >  EmulatorPkg/EmulatorPkg.dsc        | 24
> ++++++++++++------------
> >  EmulatorPkg/Readme.md              |  8 ++++----
> >  EmulatorPkg/Win/VS2017/BuildVS.bat |  2 +-
> >  EmulatorPkg/build.sh               |  8 ++++----
> >  4 files changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> b/EmulatorPkg/EmulatorPkg.dsc
> > index c4ec10d1d8..c9e4a5b34d 100644
> > --- a/EmulatorPkg/EmulatorPkg.dsc
> > +++ b/EmulatorPkg/EmulatorPkg.dsc
> > @@ -240,18 +240,18 @@ [PcdsDynamicHii.common.DEFAULT]
> >
> >
> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeo
> ut"|gEfiGlo
> > balVariableGuid|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)
> 
> 
> Just a thought, the only supported architectures within
> EmulatorPkg.dsc are IA32 and X64 at this moment. Do you
> think we can omit the arch check here?
> 
> Anyway, the patch looks good to me:
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> Best Regards,
> Hao Wu
> 
> 
> > +  !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/E
> mulatorPkg
> >  **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/$PROCESSO
> R/Host"
> > $BUILD_ROOT_ARCH
> >  fi
> > --
> > 2.21.0.windows.1
> >
> >
> > 


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

* Re: [edk2-devel] [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES
  2019-08-07  4:20 ` [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Michael D Kinney
@ 2019-08-07  7:58   ` Wu, Hao A
  2019-08-08  2:31     ` Michael D Kinney
  0 siblings, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2019-08-07  7:58 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

Hello Mike

Some inline comments below:


> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael D Kinney
> Sent: Wednesday, August 07, 2019 12:20 PM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> Subject: [edk2-devel] [Patch 3/3] EmulatorPkg: Add -D
> DISABLE_NEW_DEPRECATED_INTERFACES
> 
> 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>
> ---
>  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  |   8 +-
>  EmulatorPkg/Unix/Host/PosixFileSystem.c       |  30 ++++-
>  EmulatorPkg/Unix/Host/X11GraphicsWindow.c     |   4 +-
>  EmulatorPkg/Win/Host/WinFileSystem.c          | 116 ++++++++++++------
>  9 files changed, 138 insertions(+), 58 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 c9e4a5b34d..39a6658427 100644
> --- a/EmulatorPkg/EmulatorPkg.dsc
> +++ b/EmulatorPkg/EmulatorPkg.dsc
> @@ -412,10 +412,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..18cb3831a4 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
> @@ -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..b2b2d011c9 100644
> --- a/EmulatorPkg/Unix/Host/PosixFileSystem.c
> +++ b/EmulatorPkg/Unix/Host/PosixFileSystem.c
> @@ -1017,7 +1017,11 @@ PosixFileGetInfo (
>      FileSystemInfoBuffer->BlockSize   = buf.f_bsize;
> 
> 
> -    StrCpy ((CHAR16 *) FileSystemInfoBuffer->VolumeLabel, PrivateRoot-
> >VolumeLabel);
> +    StrCpyS (
> +      (CHAR16 *) FileSystemInfoBuffer->VolumeLabel,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


I think it will be better to use:

(*BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO) / sizeof (CHAR16)

here. Even though the usage of function PosixFileGetInfo() would ensures
that:

StrSize (PrivateRoot->VolumeLabel) == *BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO


> +      PrivateRoot->VolumeLabel
> +      );
>      *BufferSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (PrivateRoot-
> >VolumeLabel);
> 
>    } else if (CompareGuid (InformationType,
> &gEfiFileSystemVolumeLabelInfoIdGuid)) {
> @@ -1026,7 +1030,11 @@ PosixFileGetInfo (
>        return EFI_BUFFER_TOO_SMALL;
>      }
> 
> -    StrCpy ((CHAR16 *) Buffer, PrivateRoot->VolumeLabel);
> +    StrCpyS (
> +      (CHAR16 *) Buffer,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


Similarly, I would suggest using:
*BufferSize / sizeof (CHAR16)

instead of:
StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16)


> +      PrivateRoot->VolumeLabel
> +      );
>      *BufferSize = StrSize (PrivateRoot->VolumeLabel);
> 
>    }
> @@ -1110,7 +1118,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 +1137,11 @@ PosixFileSetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (PrivateRoot->VolumeLabel, (CHAR16 *) Buffer);
> +    StrCpyS (
> +      PrivateRoot->VolumeLabel,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


I think the size for 'PrivateRoot->VolumeLabel' is good here.

Since within this driver, 'PrivateRoot->VolumeLabel' is always allocated
with the size to just hold the current string it stores.


> +      (CHAR16 *) Buffer
> +      );
> 
>      Status = EFI_SUCCESS;
>      goto Done;
> @@ -1493,7 +1509,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..bb64439007 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, L"\\");
> +      StrCatS (NewPrivateFile->FileName, Size, FileName);


For the above 2 lines, the 2nd parameter for StrCatS() should be:
Size / sizeof (CHAR16)


>      }
>    }
> 
> @@ -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, L"\\*");


Should be:
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,
> +      (StrSize (PrivateRoot->VolumeLabel) + 1) / sizeof (CHAR16),


I would suggest here using:
(*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,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


I would suggest here using:
*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),


Similar to the PosixFileSetInfo() case above, I think the size for
'PrivateRoot->VolumeLabel' is good here.


> +      (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, PrivateFile->FilePath);
> +    StrCatS (NewFileName, Size, L"\\");
> +    StrCatS (NewFileName, Size, NewFileInfo->FileName);


The 2nd parameter for the above StrCatS() calls should be:
Size / sizeof (CHAR16)


>    }
> 
>    //
> @@ -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, L"\\*");


Should be:
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, L"\\*");


Should be:
StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");

Best Regards,
Hao Wu


>          PrivateFile->LHandle = FindFirstFile (TempFileName, &FindBuf);
>        }
> 
> --
> 2.21.0.windows.1
> 
> 
> 


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

* Re: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  2019-08-07  7:45     ` Michael D Kinney
@ 2019-08-07  7:59       ` Wu, Hao A
  2019-08-08  2:28         ` Michael D Kinney
  0 siblings, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2019-08-07  7:59 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, August 07, 2019 3:45 PM
> To: Wu, Hao A; devel@edk2.groups.io; Kinney, Michael D
> Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> Subject: RE: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove
> UNIX_SEC_BUILD/WIN_SEC_BUILD
> 
> Hao Wu,
> 
> I put the arch check in on purpose because more
> CPU archs might be added in the future.


Got it. I agree.
Please keep my RB tag in the previous reply.

Best Regards,
Hao Wu


> 
> Mike
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Tuesday, August 6, 2019 11:42 PM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> > Andrew Fish <afish@apple.com>; Ni, Ray
> > <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [Patch 2/3] EmulatorPkg:
> > Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io
> > [mailto:devel@edk2.groups.io] On Behalf Of
> > > Michael D Kinney
> > > Sent: Wednesday, August 07, 2019 12:20 PM
> > > To: devel@edk2.groups.io
> > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > Subject: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove
> > > UNIX_SEC_BUILD/WIN_SEC_BUILD
> > >
> > > 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>
> > > ---
> > >  EmulatorPkg/EmulatorPkg.dsc        | 24
> > ++++++++++++------------
> > >  EmulatorPkg/Readme.md              |  8 ++++----
> > >  EmulatorPkg/Win/VS2017/BuildVS.bat |  2 +-
> > >  EmulatorPkg/build.sh               |  8 ++++----
> > >  4 files changed, 21 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> > b/EmulatorPkg/EmulatorPkg.dsc
> > > index c4ec10d1d8..c9e4a5b34d 100644
> > > --- a/EmulatorPkg/EmulatorPkg.dsc
> > > +++ b/EmulatorPkg/EmulatorPkg.dsc
> > > @@ -240,18 +240,18 @@ [PcdsDynamicHii.common.DEFAULT]
> > >
> > >
> > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeo
> > ut"|gEfiGlo
> > > balVariableGuid|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)
> >
> >
> > Just a thought, the only supported architectures within
> > EmulatorPkg.dsc are IA32 and X64 at this moment. Do you
> > think we can omit the arch check here?
> >
> > Anyway, the patch looks good to me:
> > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > > +  !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/E
> > mulatorPkg
> > >  **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/$PROCESSO
> > R/Host"
> > > $BUILD_ROOT_ARCH
> > >  fi
> > > --
> > > 2.21.0.windows.1
> > >
> > >
> > > 


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

* Re: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure
  2019-08-07  7:42     ` Michael D Kinney
@ 2019-08-07  8:15       ` Wu, Hao A
  2019-08-07 15:52         ` Michael D Kinney
  0 siblings, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2019-08-07  8:15 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, August 07, 2019 3:43 PM
> To: Wu, Hao A; devel@edk2.groups.io; Kinney, Michael D
> Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot
> failure
> 
> I observed 2 failure modes
> 
> 1) The range could not be mapped at the request address
>    And an error message was displayed and the app exited.
> 2) The mapping passed, but when it was accessed by the
>    Sec module, the app generated an exception.


Thanks for the additional information.

For case 2), even though it might not be directly related with this issue.
I think it would be the best to ensure WinNtOpenFile() returns with
success only when the map operation takes effect.

For this patch:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> 
> Mike
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Tuesday, August 6, 2019 11:19 PM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> > Andrew Fish <afish@apple.com>; Ni, Ray
> > <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
> > VS20xx IA32 boot failure
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io
> > [mailto:devel@edk2.groups.io] On Behalf Of
> > > Michael D Kinney
> > > Sent: Wednesday, August 07, 2019 12:20 PM
> > > To: devel@edk2.groups.io
> > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > Subject: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
> > VS20xx IA32 boot
> > > failure
> > >
> > > 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 is set
> > for X64 Windows
> > > Host environments.  If the EmulatorPkg is build for
> > an IA32 Windows
> > > Host environment, then set this PCD to 0x13000000
> > that can be mapped
> > > into a 32-bit user process.
> >
> >
> > Hello Mike,
> >
> > I have a question for this patch.
> >
> > For the main() function within file
> > EmulatorPkg/Win/Host/WinHost.c, I saw the below codes:
> >
> >   EmuMagicPage = (VOID *)(UINTN)(FixedPcdGet64
> > (PcdPeiServicesTablePage) & MAX_UINTN);
> >   if (EmuMagicPage != NULL) {
> >     UINT64  Size;
> >     Status = WinNtOpenFile (
> >               NULL,
> >               SIZE_4KB,
> >               0,
> >               &EmuMagicPage,
> >               &Size
> >               );
> >     if (EFI_ERROR (Status)) {
> >       SecPrint ("ERROR : Could not allocate
> > PeiServicesTablePage @ %p\n", EmuMagicPage);
> >       return EFI_DEVICE_ERROR;
> >     }
> >   }
> >
> > My understanding is that the WinNtOpenFile() function
> > call is attempting to map the address specified by
> > 'EmuMagicPage'. For IA32 case, the value in
> > 'EmuMagicPage' here has already been truncated
> > (0x03000000). If the
> > WinNtOpenFile() call returned successfully, the address
> > should be mapped.
> >
> > Is it the case that even if WinNtOpenFile() returns
> > with no error, the specified address (0x03000000) is
> > not actually being mapped?
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > 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/EmulatorPkg.dsc | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> > b/EmulatorPkg/EmulatorPkg.dsc
> > > index ea8b6ce76e..c4ec10d1d8 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 @@
> > > -225,6 +225,10 @@ [PcdsFixedAtBuild]
> > >    #  0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
> > >    gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
> > >
> > > +!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
> > > +
> > gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13
> > 000000
> > > +!endif
> > > +
> > >  [PcdsDynamicDefault.common.DEFAULT]
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpar
> > eBase64|0
> > >
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWork
> > ingBase64|
> > > 0
> > > --
> > > 2.21.0.windows.1
> > >
> > >
> > > 


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

* Re: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure
  2019-08-07  8:15       ` Wu, Hao A
@ 2019-08-07 15:52         ` Michael D Kinney
  2019-08-08  2:27           ` Michael D Kinney
  0 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2019-08-07 15:52 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

Hao Wu,

I agree that the exception is not an expected result
at all.  I will debug a bit more to see why the mapping
was allowed to pass.  The expected result should be
an error message with normal app exit if the address
can not be mapped.

Mike

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, August 7, 2019 1:15 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
> VS20xx IA32 boot failure
> 
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Wednesday, August 07, 2019 3:43 PM
> > To: Wu, Hao A; devel@edk2.groups.io; Kinney, Michael
> D
> > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
> Fix VS20xx IA32
> > boot failure
> >
> > I observed 2 failure modes
> >
> > 1) The range could not be mapped at the request
> address
> >    And an error message was displayed and the app
> exited.
> > 2) The mapping passed, but when it was accessed by
> the
> >    Sec module, the app generated an exception.
> 
> 
> Thanks for the additional information.
> 
> For case 2), even though it might not be directly
> related with this issue.
> I think it would be the best to ensure WinNtOpenFile()
> returns with success only when the map operation takes
> effect.
> 
> For this patch:
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Wu, Hao A
> > > Sent: Tuesday, August 6, 2019 11:19 PM
> > > To: devel@edk2.groups.io; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish
> > > <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
> Fix VS20xx IA32
> > > boot failure
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io
> > > [mailto:devel@edk2.groups.io] On Behalf Of
> > > > Michael D Kinney
> > > > Sent: Wednesday, August 07, 2019 12:20 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > > Subject: [edk2-devel] [Patch 1/3] EmulatorPkg:
> Fix
> > > VS20xx IA32 boot
> > > > failure
> > > >
> > > >
> 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 is
> set
> > > for X64 Windows
> > > > Host environments.  If the EmulatorPkg is build
> for
> > > an IA32 Windows
> > > > Host environment, then set this PCD to 0x13000000
> > > that can be mapped
> > > > into a 32-bit user process.
> > >
> > >
> > > Hello Mike,
> > >
> > > I have a question for this patch.
> > >
> > > For the main() function within file
> > > EmulatorPkg/Win/Host/WinHost.c, I saw the below
> codes:
> > >
> > >   EmuMagicPage = (VOID *)(UINTN)(FixedPcdGet64
> > > (PcdPeiServicesTablePage) & MAX_UINTN);
> > >   if (EmuMagicPage != NULL) {
> > >     UINT64  Size;
> > >     Status = WinNtOpenFile (
> > >               NULL,
> > >               SIZE_4KB,
> > >               0,
> > >               &EmuMagicPage,
> > >               &Size
> > >               );
> > >     if (EFI_ERROR (Status)) {
> > >       SecPrint ("ERROR : Could not allocate
> PeiServicesTablePage @
> > > %p\n", EmuMagicPage);
> > >       return EFI_DEVICE_ERROR;
> > >     }
> > >   }
> > >
> > > My understanding is that the WinNtOpenFile()
> function call is
> > > attempting to map the address specified by
> 'EmuMagicPage'. For IA32
> > > case, the value in 'EmuMagicPage' here has already
> been truncated
> > > (0x03000000). If the
> > > WinNtOpenFile() call returned successfully, the
> address should be
> > > mapped.
> > >
> > > Is it the case that even if WinNtOpenFile() returns
> with no error,
> > > the specified address (0x03000000) is not actually
> being mapped?
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > >
> > > > 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/EmulatorPkg.dsc | 6 +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> > > b/EmulatorPkg/EmulatorPkg.dsc
> > > > index ea8b6ce76e..c4ec10d1d8 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 @@
> > > > -225,6 +225,10 @@ [PcdsFixedAtBuild]
> > > >    #  0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-
> TTYTERM
> > > >
> gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
> > > >
> > > > +!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
> > > > +
> > >
> gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13
> > > 000000
> > > > +!endif
> > > > +
> > > >  [PcdsDynamicDefault.common.DEFAULT]
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpar
> > > eBase64|0
> > > >
> > > >
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWork
> > > ingBase64|
> > > > 0
> > > > --
> > > > 2.21.0.windows.1
> > > >
> > > >
> > > > 


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

* Re: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure
  2019-08-07 15:52         ` Michael D Kinney
@ 2019-08-08  2:27           ` Michael D Kinney
  0 siblings, 0 replies; 15+ messages in thread
From: Michael D Kinney @ 2019-08-08  2:27 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

Hao Wu,

I could not reproduce the exception case today.

There must have been something incorrect about my
build environment.  I have verified an error message
with normal exit always occurs.

Mike

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, August 7, 2019 8:52 AM
> To: Wu, Hao A <hao.a.wu@intel.com>;
> devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
> VS20xx IA32 boot failure
> 
> Hao Wu,
> 
> I agree that the exception is not an expected result at
> all.  I will debug a bit more to see why the mapping
> was allowed to pass.  The expected result should be an
> error message with normal app exit if the address can
> not be mapped.
> 
> Mike
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Wednesday, August 7, 2019 1:15 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish
> > <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
> Fix VS20xx IA32
> > boot failure
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D
> > > Sent: Wednesday, August 07, 2019 3:43 PM
> > > To: Wu, Hao A; devel@edk2.groups.io; Kinney,
> Michael
> > D
> > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
> > Fix VS20xx IA32
> > > boot failure
> > >
> > > I observed 2 failure modes
> > >
> > > 1) The range could not be mapped at the request
> > address
> > >    And an error message was displayed and the app
> > exited.
> > > 2) The mapping passed, but when it was accessed by
> > the
> > >    Sec module, the app generated an exception.
> >
> >
> > Thanks for the additional information.
> >
> > For case 2), even though it might not be directly
> related with this
> > issue.
> > I think it would be the best to ensure
> WinNtOpenFile() returns with
> > success only when the map operation takes effect.
> >
> > For this patch:
> > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Wu, Hao A
> > > > Sent: Tuesday, August 6, 2019 11:19 PM
> > > > To: devel@edk2.groups.io; Kinney, Michael D
> > > > <michael.d.kinney@intel.com>
> > > > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> > Andrew Fish
> > > > <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
> > > > Subject: RE: [edk2-devel] [Patch 1/3]
> EmulatorPkg:
> > Fix VS20xx IA32
> > > > boot failure
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io
> > > > [mailto:devel@edk2.groups.io] On Behalf Of
> > > > > Michael D Kinney
> > > > > Sent: Wednesday, August 07, 2019 12:20 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > > > Subject: [edk2-devel] [Patch 1/3] EmulatorPkg:
> > Fix
> > > > VS20xx IA32 boot
> > > > > failure
> > > > >
> > > > >
> > 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 is
> > set
> > > > for X64 Windows
> > > > > Host environments.  If the EmulatorPkg is build
> > for
> > > > an IA32 Windows
> > > > > Host environment, then set this PCD to
> 0x13000000
> > > > that can be mapped
> > > > > into a 32-bit user process.
> > > >
> > > >
> > > > Hello Mike,
> > > >
> > > > I have a question for this patch.
> > > >
> > > > For the main() function within file
> > > > EmulatorPkg/Win/Host/WinHost.c, I saw the below
> > codes:
> > > >
> > > >   EmuMagicPage = (VOID *)(UINTN)(FixedPcdGet64
> > > > (PcdPeiServicesTablePage) & MAX_UINTN);
> > > >   if (EmuMagicPage != NULL) {
> > > >     UINT64  Size;
> > > >     Status = WinNtOpenFile (
> > > >               NULL,
> > > >               SIZE_4KB,
> > > >               0,
> > > >               &EmuMagicPage,
> > > >               &Size
> > > >               );
> > > >     if (EFI_ERROR (Status)) {
> > > >       SecPrint ("ERROR : Could not allocate
> > PeiServicesTablePage @
> > > > %p\n", EmuMagicPage);
> > > >       return EFI_DEVICE_ERROR;
> > > >     }
> > > >   }
> > > >
> > > > My understanding is that the WinNtOpenFile()
> > function call is
> > > > attempting to map the address specified by
> > 'EmuMagicPage'. For IA32
> > > > case, the value in 'EmuMagicPage' here has
> already
> > been truncated
> > > > (0x03000000). If the
> > > > WinNtOpenFile() call returned successfully, the
> > address should be
> > > > mapped.
> > > >
> > > > Is it the case that even if WinNtOpenFile()
> returns
> > with no error,
> > > > the specified address (0x03000000) is not
> actually
> > being mapped?
> > > >
> > > > Best Regards,
> > > > Hao Wu
> > > >
> > > >
> > > > >
> > > > > 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/EmulatorPkg.dsc | 6 +++++-
> > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> > > > b/EmulatorPkg/EmulatorPkg.dsc
> > > > > index ea8b6ce76e..c4ec10d1d8 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 @@
> > > > > -225,6 +225,10 @@ [PcdsFixedAtBuild]
> > > > >    #  0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-
> > TTYTERM
> > > > >
> > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
> > > > >
> > > > > +!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
> > > > > +
> > > >
> >
> gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13
> > > > 000000
> > > > > +!endif
> > > > > +
> > > > >  [PcdsDynamicDefault.common.DEFAULT]
> > > > >
> > > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpar
> > > > eBase64|0
> > > > >
> > > > >
> > > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWork
> > > > ingBase64|
> > > > > 0
> > > > > --
> > > > > 2.21.0.windows.1
> > > > >
> > > > >
> > > > > 


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

* Re: [edk2-devel] [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
  2019-08-07  7:59       ` Wu, Hao A
@ 2019-08-08  2:28         ` Michael D Kinney
  0 siblings, 0 replies; 15+ messages in thread
From: Michael D Kinney @ 2019-08-08  2:28 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

Hao Wu,

In the V2 patch series there are now fewer checks
for IA32/X64.  I was able to update the DEC file 
with a new default value that works for all
combinations.

Mike

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, August 7, 2019 1:00 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 2/3] EmulatorPkg:
> Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
> 
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Wednesday, August 07, 2019 3:45 PM
> > To: Wu, Hao A; devel@edk2.groups.io; Kinney, Michael
> D
> > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > Subject: RE: [edk2-devel] [Patch 2/3] EmulatorPkg:
> Remove
> > UNIX_SEC_BUILD/WIN_SEC_BUILD
> >
> > Hao Wu,
> >
> > I put the arch check in on purpose because more CPU
> archs might be
> > added in the future.
> 
> 
> Got it. I agree.
> Please keep my RB tag in the previous reply.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Wu, Hao A
> > > Sent: Tuesday, August 6, 2019 11:42 PM
> > > To: devel@edk2.groups.io; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish
> > > <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: RE: [edk2-devel] [Patch 2/3] EmulatorPkg:
> > > Remove UNIX_SEC_BUILD/WIN_SEC_BUILD
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io
> > > [mailto:devel@edk2.groups.io] On Behalf Of
> > > > Michael D Kinney
> > > > Sent: Wednesday, August 07, 2019 12:20 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > > Subject: [edk2-devel] [Patch 2/3] EmulatorPkg:
> Remove
> > > > UNIX_SEC_BUILD/WIN_SEC_BUILD
> > > >
> > > >
> 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>
> > > > ---
> > > >  EmulatorPkg/EmulatorPkg.dsc        | 24
> > > ++++++++++++------------
> > > >  EmulatorPkg/Readme.md              |  8 ++++----
> > > >  EmulatorPkg/Win/VS2017/BuildVS.bat |  2 +-
> > > >  EmulatorPkg/build.sh               |  8 ++++----
> > > >  4 files changed, 21 insertions(+), 21
> deletions(-)
> > > >
> > > > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> > > b/EmulatorPkg/EmulatorPkg.dsc
> > > > index c4ec10d1d8..c9e4a5b34d 100644
> > > > --- a/EmulatorPkg/EmulatorPkg.dsc
> > > > +++ b/EmulatorPkg/EmulatorPkg.dsc
> > > > @@ -240,18 +240,18 @@
> [PcdsDynamicHii.common.DEFAULT]
> > > >
> > > >
> > >
> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeo
> > > ut"|gEfiGlo
> > > > balVariableGuid|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)
> > >
> > >
> > > Just a thought, the only supported architectures
> within
> > > EmulatorPkg.dsc are IA32 and X64 at this moment. Do
> you think we can
> > > omit the arch check here?
> > >
> > > Anyway, the patch looks good to me:
> > > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > > +  !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/E
> > > mulatorPkg
> > > >  **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/$PROCESSO
> > > R/Host"
> > > > $BUILD_ROOT_ARCH
> > > >  fi
> > > > --
> > > > 2.21.0.windows.1
> > > >
> > > >
> > > > 


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

* Re: [edk2-devel] [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES
  2019-08-07  7:58   ` [edk2-devel] " Wu, Hao A
@ 2019-08-08  2:31     ` Michael D Kinney
  0 siblings, 0 replies; 15+ messages in thread
From: Michael D Kinney @ 2019-08-08  2:31 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io, Kinney, Michael D
  Cc: Justen, Jordan L, Andrew Fish, Ni, Ray

Hao Wu,

Thanks for the detailed feedback on the size
Parameters.  I have fixed all these issues in
V2.  I also found some additional functions
that needed to be updated to safe string 
functions for the POSIX host module.

I have verified the EmulatorPkg builds and 
boots on Windows and Linux for IA32 and X64
with these patches applied.

Mike

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, August 7, 2019 12:59 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 3/3] EmulatorPkg: Add
> -D DISABLE_NEW_DEPRECATED_INTERFACES
> 
> Hello Mike
> 
> Some inline comments below:
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of
> > Michael D Kinney
> > Sent: Wednesday, August 07, 2019 12:20 PM
> > To: devel@edk2.groups.io
> > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > Subject: [edk2-devel] [Patch 3/3] EmulatorPkg: Add -D
> > DISABLE_NEW_DEPRECATED_INTERFACES
> >
> > 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>
> > ---
> >  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  |   8
> +-
> >  EmulatorPkg/Unix/Host/PosixFileSystem.c       |  30
> ++++-
> >  EmulatorPkg/Unix/Host/X11GraphicsWindow.c     |   4
> +-
> >  EmulatorPkg/Win/Host/WinFileSystem.c          | 116
> ++++++++++++------
> >  9 files changed, 138 insertions(+), 58 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
> > c9e4a5b34d..39a6658427 100644
> > --- a/EmulatorPkg/EmulatorPkg.dsc
> > +++ b/EmulatorPkg/EmulatorPkg.dsc
> > @@ -412,10 +412,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/ThunkProtocolLi
> st.c
> >
> b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolLi
> st.c
> > index b7aacc851c..3a7b6d1ceb 100644
> > ---
> a/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolLi
> st.c
> > +++
> b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolLi
> st.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..18cb3831a4 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 @@ -
> 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..b2b2d011c9 100644
> > --- a/EmulatorPkg/Unix/Host/PosixFileSystem.c
> > +++ b/EmulatorPkg/Unix/Host/PosixFileSystem.c
> > @@ -1017,7 +1017,11 @@ PosixFileGetInfo (
> >      FileSystemInfoBuffer->BlockSize   = buf.f_bsize;
> >
> >
> > -    StrCpy ((CHAR16 *) FileSystemInfoBuffer-
> >VolumeLabel, PrivateRoot-
> > >VolumeLabel);
> > +    StrCpyS (
> > +      (CHAR16 *) FileSystemInfoBuffer->VolumeLabel,
> > +      StrSize (PrivateRoot->VolumeLabel) / sizeof
> (CHAR16),
> 
> 
> I think it will be better to use:
> 
> (*BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO) / sizeof
> (CHAR16)
> 
> here. Even though the usage of function
> PosixFileGetInfo() would ensures
> that:
> 
> StrSize (PrivateRoot->VolumeLabel) == *BufferSize -
> SIZE_OF_EFI_FILE_SYSTEM_INFO
> 
> 
> > +      PrivateRoot->VolumeLabel
> > +      );
> >      *BufferSize = SIZE_OF_EFI_FILE_SYSTEM_INFO +
> StrSize
> > (PrivateRoot-
> > >VolumeLabel);
> >
> >    } else if (CompareGuid (InformationType,
> > &gEfiFileSystemVolumeLabelInfoIdGuid)) { @@ -1026,7
> +1030,11 @@
> > PosixFileGetInfo (
> >        return EFI_BUFFER_TOO_SMALL;
> >      }
> >
> > -    StrCpy ((CHAR16 *) Buffer, PrivateRoot-
> >VolumeLabel);
> > +    StrCpyS (
> > +      (CHAR16 *) Buffer,
> > +      StrSize (PrivateRoot->VolumeLabel) / sizeof
> (CHAR16),
> 
> 
> Similarly, I would suggest using:
> *BufferSize / sizeof (CHAR16)
> 
> instead of:
> StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16)
> 
> 
> > +      PrivateRoot->VolumeLabel
> > +      );
> >      *BufferSize = StrSize (PrivateRoot-
> >VolumeLabel);
> >
> >    }
> > @@ -1110,7 +1118,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 +1137,11 @@ PosixFileSetInfo (
> >        goto Done;
> >      }
> >
> > -    StrCpy (PrivateRoot->VolumeLabel, (CHAR16 *)
> Buffer);
> > +    StrCpyS (
> > +      PrivateRoot->VolumeLabel,
> > +      StrSize (PrivateRoot->VolumeLabel) / sizeof
> (CHAR16),
> 
> 
> I think the size for 'PrivateRoot->VolumeLabel' is good
> here.
> 
> Since within this driver, 'PrivateRoot->VolumeLabel' is
> always allocated with the size to just hold the current
> string it stores.
> 
> 
> > +      (CHAR16 *) Buffer
> > +      );
> >
> >      Status = EFI_SUCCESS;
> >      goto Done;
> > @@ -1493,7 +1509,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..bb64439007 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,
> L"\\");
> > +      StrCatS (NewPrivateFile->FileName, Size,
> FileName);
> 
> 
> For the above 2 lines, the 2nd parameter for StrCatS()
> should be:
> Size / sizeof (CHAR16)
> 
> 
> >      }
> >    }
> >
> > @@ -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, L"\\*");
> 
> 
> Should be:
> 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,
> > +      (StrSize (PrivateRoot->VolumeLabel) + 1) /
> sizeof (CHAR16),
> 
> 
> I would suggest here using:
> (*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,
> > +      StrSize (PrivateRoot->VolumeLabel) / sizeof
> (CHAR16),
> 
> 
> I would suggest here using:
> *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),
> 
> 
> Similar to the PosixFileSetInfo() case above, I think
> the size for 'PrivateRoot->VolumeLabel' is good here.
> 
> 
> > +      (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, PrivateFile-
> >FilePath);
> > +    StrCatS (NewFileName, Size, L"\\");
> > +    StrCatS (NewFileName, Size, NewFileInfo-
> >FileName);
> 
> 
> The 2nd parameter for the above StrCatS() calls should
> be:
> Size / sizeof (CHAR16)
> 
> 
> >    }
> >
> >    //
> > @@ -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, L"\\*");
> 
> 
> Should be:
> 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, L"\\*");
> 
> 
> Should be:
> StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");
> 
> Best Regards,
> Hao Wu
> 
> 
> >          PrivateFile->LHandle = FindFirstFile
> (TempFileName, &FindBuf);
> >        }
> >
> > --
> > 2.21.0.windows.1
> >
> >
> > 


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

end of thread, other threads:[~2019-08-08  2:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-07  4:20 [Patch 0/3] EmulatorPkg: Fix VS20xx IA32 boot and simplify build config Michael D Kinney
2019-08-07  4:20 ` [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure Michael D Kinney
2019-08-07  6:18   ` [edk2-devel] " Wu, Hao A
2019-08-07  7:42     ` Michael D Kinney
2019-08-07  8:15       ` Wu, Hao A
2019-08-07 15:52         ` Michael D Kinney
2019-08-08  2:27           ` Michael D Kinney
2019-08-07  4:20 ` [Patch 2/3] EmulatorPkg: Remove UNIX_SEC_BUILD/WIN_SEC_BUILD Michael D Kinney
2019-08-07  6:41   ` [edk2-devel] " Wu, Hao A
2019-08-07  7:45     ` Michael D Kinney
2019-08-07  7:59       ` Wu, Hao A
2019-08-08  2:28         ` Michael D Kinney
2019-08-07  4:20 ` [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES Michael D Kinney
2019-08-07  7:58   ` [edk2-devel] " Wu, Hao A
2019-08-08  2:31     ` Michael D Kinney

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