[Patch 00/12] New Cross OS tool chain CLANG9

[Patch 00/12] New Cross OS tool chain CLANG9

[Liming Gao]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1603
Code: https://github.com/lgao4/edk2/tree/CLANG9
Wiki: https://github.com/lgao4/edk2/wiki/CLANG9-Tools-Chain

CLANG9 tool chain is added to directly generate PE/COFF image (EFI image).
This tool chain uses LLVM clang C compiler and lld linker, generates PE/COFF
image and PDB compatible debug symbol format. Now, it supports IA32/X64 Archs.
It must use LLVM 9 or above release. LLVM 9 is ready on 
http://releases.llvm.org/download.html#9.0.0.

CLANG9 is the cross OS tool chain. It can work on Windows/Linux/Mac host OS. 
For the same source code, with the same version LLVM tool chain, 
CLANG9 can generate the same binary image. So, the developer can 
choose the different development environment and work on the same 
code base. Besides, EDKII project build also requires third party 
tools: nasm and iasl. They both keep the same version. If so, the same 
binary image can be generated on the different host OS.

LLVM tool chain provides the compiler and linker. To build EDK2 project, 
some other tools are still required. On Windows OS, nmake and Visual Studio 
are required to call Makefile and compile BaseTools C tools. 
On Linux/Mac, binutils and gcc are required to make and compile BaseTools 
C tools. Because VS or GCC are mainly used to compile BaseTools and provide 
nmake/make tool, they can keep on the stable version without update.

To build source code, CLANG9 tool chain (-t CLANG9) can be specified
on Windows OS, set CLANG_HOST_BIN=n, set CLANG9_BIN=LLVM installed directory
CLANG_HOST_BIN is used CLANG_HOST_PREFIX. Prefix n is for nmake.
For example:
*  set CLANG_HOST_BIN=n
*  set CLANG9_BIN=C:\Program Files\LLVM\bin\
*  set IASL_PREFIX=C:\Asl\

On Linux/Mac, export CLANG9_BIN=LLVM installed directory, CLANG_HOST_BIN is 
not required, because there is no prefix for make.
For example:
*  export CLANG9_BIN=/home/clang9/bin/

Now, CLANG9 tool chain has been verified in Edk2 packages and Ovmf/Emulator
with LLVM 9.0.0 on Windows and Linux OS. 
OVMF IA32/X64/IA32X64 all boots to Shell on Windows and Linux OS. 
Emulator can boot to Shell on Windows only with CLANG9.
OVMF Ia32X64 RELEASE build generates the same BIOS images on Windows and Linux OS.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Steven Shi <steven.shi@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Liming Gao (12):
  BaseTools tools_def.template: Remove unnecessary $(DEST_DIR_DEBUG)
    path
  BaseTools tools_def: Add CLANG9 tool chain to directly generate PE
    image
  BaseTools GenFw: Fix the issue to update the wrong size as SectionSize
  MdePkg Base.h: Add definition for CLANG9 tool chain
  MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO
    functions
  MdeModulePkg LzmaCustomDecompressLib: Update macro to be same in CLANG
    tool
  MdeModulePkg RegularExpressionDxe: Disable warning for CLANG9 tool
    chain
  CryptoPkg: Append options to make CLANG9 tool chain pass build
  CryptoPkg IntrinsicLib: Make _fltused always be used
  EmulatorPkg: Enable CLANG9 tool chain
  OvmfPkg: Enable CLANG9 tool chain
  OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9
    X64

 BaseTools/Source/C/GenFw/GenFw.c                                |   8 +-
 CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c               |  10 ++-
 EmulatorPkg/Win/Host/WinHost.c                                  |  11 ++-
 MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c                    |   6 --
 BaseTools/Conf/build_rule.template                              |  26 +++---
 BaseTools/Conf/tools_def.template                               | 124 +++++++++++++++++++++++++---
 CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf                 |   1 +
 CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf                  |   1 +
 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf              |   1 +
 CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf                  |   1 +
 CryptoPkg/Library/OpensslLib/OpensslLib.inf                     |   1 +
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf               |   1 +
 EmulatorPkg/EmulatorPkg.dsc                                     |   8 +-
 EmulatorPkg/EmulatorPkg.fdf                                     |   2 +-
 EmulatorPkg/Win/Host/WinHost.inf                                |   6 ++
 MdeModulePkg/Library/LzmaCustomDecompressLib/Sdk/C/7zTypes.h    |   2 +-
 .../Universal/RegularExpressionDxe/RegularExpressionDxe.inf     |   3 +
 MdePkg/Include/Base.h                                           |   6 +-
 MdePkg/Include/Ia32/ProcessorBind.h                             |   4 +-
 MdePkg/Include/X64/ProcessorBind.h                              |   2 +-
 OvmfPkg/OvmfPkgIa32.dsc                                         |   4 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                                      |   4 +-
 OvmfPkg/OvmfPkgX64.dsc                                          |   4 +-
 OvmfPkg/Sec/SecMain.inf                                         |   4 +
 24 files changed, 192 insertions(+), 48 deletions(-)

-- 
2.13.0.windows.1

[Patch 01/12] BaseTools tools_def.template: Remove unnecessary $(DEST_DIR_DEBUG) path

[Liming Gao]

$(DEST_DIR_DEBUG) path is in Include directory.
It is not required to be specified again.

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 BaseTools/Conf/tools_def.template | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 88a6764f43..fd6fca542d 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -350,8 +350,8 @@ DEFINE DTC_BIN                 = ENV(DTC_PREFIX)dtc
 # *_*_EBC_SLINK_PATH                 = C:\Program Files\Intel\EBC\Bin\link.exe
 #
 # *_*_EBC_SLINK_FLAGS                = /lib /NOLOGO /MACHINE:EBC
-# *_*_EBC_PP_FLAGS                   = /nologo /E /TC /FI$(DEST_DIR_DEBUG)/AutoGen.h
-# *_*_EBC_CC_FLAGS                   = /nologo /FAcs /c /W3 /WX /FI$(DEST_DIR_DEBUG)/AutoGen.h
+# *_*_EBC_PP_FLAGS                   = /nologo /E /TC /FIAutoGen.h
+# *_*_EBC_CC_FLAGS                   = /nologo /FAcs /c /W3 /WX /FIAutoGen.h
 # *_*_EBC_DLINK_FLAGS                = "C:\Program Files\Intel\EBC\Lib\EbcLib.lib" /NOLOGO /MACHINE:EBC /OPT:REF /NODEFAULTLIB /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /ALIGN:32 /DRIVER
 #
 ####################################################################################
@@ -1906,13 +1906,13 @@ DEFINE GCC_IA32_X64_ASLDLINK_FLAGS = DEF(GCC_IA32_X64_DLINK_COMMON) --entry _Ref
 DEFINE GCC_ARM_ASLDLINK_FLAGS      = DEF(GCC_ARM_DLINK_FLAGS) -Wl,--entry,ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT) DEF(GCC_ARM_AARCH64_ASLDLINK_FLAGS)
 DEFINE GCC_AARCH64_ASLDLINK_FLAGS  = DEF(GCC_AARCH64_DLINK_FLAGS) -Wl,--entry,ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT) DEF(GCC_ARM_AARCH64_ASLDLINK_FLAGS)
 DEFINE GCC_IA32_X64_DLINK_FLAGS    = DEF(GCC_IA32_X64_DLINK_COMMON) --entry _$(IMAGE_ENTRY_POINT) --file-alignment 0x20 --section-alignment 0x20 -Map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
-DEFINE GCC_ASM_FLAGS               = -c -x assembler -imacros $(DEST_DIR_DEBUG)/AutoGen.h
-DEFINE GCC_PP_FLAGS                = -E -x assembler-with-cpp -include $(DEST_DIR_DEBUG)/AutoGen.h
-DEFINE GCC_VFRPP_FLAGS             = -x c -E -P -DVFRCOMPILE --include $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
+DEFINE GCC_ASM_FLAGS               = -c -x assembler -imacros AutoGen.h
+DEFINE GCC_PP_FLAGS                = -E -x assembler-with-cpp -include AutoGen.h
+DEFINE GCC_VFRPP_FLAGS             = -x c -E -P -DVFRCOMPILE --include $(MODULE_NAME)StrDefs.h
 DEFINE GCC_ASLPP_FLAGS             = -x c -E -include AutoGen.h
 DEFINE GCC_ASLCC_FLAGS             = -x c
 DEFINE GCC_WINDRES_FLAGS           = -J rc -O coff
-DEFINE GCC_DTCPP_FLAGS             = -E -x assembler-with-cpp -imacros $(DEST_DIR_DEBUG)/AutoGen.h -nostdinc -undef
+DEFINE GCC_DTCPP_FLAGS             = -E -x assembler-with-cpp -imacros AutoGen.h -nostdinc -undef
 DEFINE GCC_IA32_RC_FLAGS           = -I binary -O elf32-i386          -B i386    --rename-section .data=.hii
 DEFINE GCC_X64_RC_FLAGS            = -I binary -O elf64-x86-64        -B i386    --rename-section .data=.hii
 DEFINE GCC_ARM_RC_FLAGS            = -I binary -O elf32-littlearm     -B arm     --rename-section .data=.hii
@@ -2772,8 +2772,8 @@ RELEASE_XCODE5_X64_DLINK_FLAGS      = -arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _
   NOOPT_XCODE5_X64_ASM_FLAGS  = -arch x86_64 -g
 RELEASE_XCODE5_X64_ASM_FLAGS  = -arch x86_64
       *_XCODE5_X64_NASM_FLAGS = -f macho64
-*_XCODE5_*_PP_FLAGS         = -E -x assembler-with-cpp -include $(DEST_DIR_DEBUG)/AutoGen.h
-*_XCODE5_*_VFRPP_FLAGS      = -x c -E -P -DVFRCOMPILE -include $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
+*_XCODE5_*_PP_FLAGS         = -E -x assembler-with-cpp -include AutoGen.h
+*_XCODE5_*_VFRPP_FLAGS      = -x c -E -P -DVFRCOMPILE -include $(MODULE_NAME)StrDefs.h
 
   DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -gdwarf -Os       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
   NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -gdwarf -O0       -Wall -Werror -Wextra -include AutoGen.h -funsigned-char -fno-ms-extensions -fno-stack-protector -fno-builtin -fshort-wchar -mno-implicit-float -mms-bitfields -Wno-unused-parameter -Wno-missing-braces -Wno-missing-field-initializers -Wno-tautological-compare -Wno-sign-compare -Wno-varargs -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
@@ -2812,7 +2812,7 @@ RELEASE_RVCT_ARM_DLINK_FLAGS     = $(ARCHDLINK_FLAGS) DEF(RVCT_ALL_DLINK_FLAGS)
 
 *_RVCT_ARM_ASM_FLAGS       = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(RVCT_ALL_ASM_FLAGS)
 *_RVCT_ARM_PP_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E --preinclude AutoGen.h
-*_RVCT_ARM_VFRPP_FLAGS     = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E  -DVFRCOMPILE --preinclude $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
+*_RVCT_ARM_VFRPP_FLAGS     = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E  -DVFRCOMPILE --preinclude $(MODULE_NAME)StrDefs.h
 *_RVCT_ARM_MAKE_PATH       = nmake /NOLOGO
 *_RVCT_ARM_SLINK_FLAGS     = --partial -o
   DEBUG_RVCT_ARM_CC_FLAGS  = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(RVCT_ALL_CC_FLAGS) -O1 -g
@@ -2853,7 +2853,7 @@ RELEASE_RVCTLINUX_ARM_DLINK_FLAGS   = $(ARCHDLINK_FLAGS) DEF(RVCT_ALL_DLINK_FLAG
 
 *_RVCTLINUX_ARM_ASM_FLAGS       = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(RVCT_ALL_ASM_FLAGS)
 *_RVCTLINUX_ARM_PP_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E
-*_RVCTLINUX_ARM_VFRPP_FLAGS     = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E  -DVFRCOMPILE --preinclude $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
+*_RVCTLINUX_ARM_VFRPP_FLAGS     = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E  -DVFRCOMPILE --preinclude $(MODULE_NAME)StrDefs.h
 *_RVCTLINUX_ARM_SLINK_FLAGS     = --partial -o
   DEBUG_RVCTLINUX_ARM_CC_FLAGS  = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(RVCT_ALL_CC_FLAGS) -O1 -g
 RELEASE_RVCTLINUX_ARM_CC_FLAGS  = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) --diag_suppress=550 DEF(RVCT_ALL_CC_FLAGS) -O2
@@ -2900,7 +2900,7 @@ RELEASE_RVCTCYGWIN_ARM_DLINK_FLAGS     = "$(DLINKPATH_FLAG)" $(ARCHDLINK_FLAGS)
 
 *_RVCTCYGWIN_ARM_ASM_FLAGS       = "$(ASMPATH_FLAG)" $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(RVCT_ALL_ASM_FLAGS)
 *_RVCTCYGWIN_ARM_PP_FLAGS        = "$(CCPATH_FLAG)" $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E
-*_RVCTCYGWIN_ARM_VFRPP_FLAGS     = "$(CCPATH_FLAG)" $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E  -DVFRCOMPILE --preinclude `cygpath -m $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h`
+*_RVCTCYGWIN_ARM_VFRPP_FLAGS     = "$(CCPATH_FLAG)" $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -E  -DVFRCOMPILE --preinclude `cygpath -m $(MODULE_NAME)StrDefs.h`
 *_RVCTCYGWIN_ARM_MAKE_PATH       = make
 *_RVCTCYGWIN_ARM_SLINK_FLAGS     = "$(SLINKPATH_FLAG)" --partial -o
   DEBUG_RVCTCYGWIN_ARM_CC_FLAGS  = "$(CCPATH_FLAG)" $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(RVCT_ALL_CC_FLAGS) -O1 -g
-- 
2.13.0.windows.1

[Patch 02/12] BaseTools tools_def: Add CLANG9 tool chain to directly generate PE image

[Liming Gao]

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 BaseTools/Conf/build_rule.template |  26 +++++++++------
 BaseTools/Conf/tools_def.template  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+), 10 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template
index db06d3a6b4..3a58ac8015 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -260,7 +260,7 @@
     <OutputFile>
         $(OUTPUT_DIR)(+)$(MODULE_NAME).lib
 
-    <Command.MSFT, Command.INTEL>
+    <Command.MSFT, Command.INTEL, Command.CLANGPE>
         "$(SLINK)" $(SLINK_FLAGS) /OUT:${dst} @$(OBJECT_FILES_LIST)
 
     <Command.GCC>
@@ -291,6 +291,9 @@
         "$(DLINK)" /OUT:${dst} $(DLINK_FLAGS) $(DLINK2_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)
         "$(DLINK)" /OUT:${dst} $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)
 
+    <Command.CLANGPE>
+        "$(DLINK)" /OUT:${dst} $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST) $(DLINK2_FLAGS)
+
     <Command.GCC>
         "$(DLINK)" -o ${dst} $(DLINK_FLAGS) -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS) $(DLINK2_FLAGS)
         "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
@@ -331,7 +334,7 @@
     <OutputFile>
         $(DEBUG_DIR)(+)$(MODULE_NAME)
 
-    <Command.MSFT, Command.INTEL>
+    <Command.MSFT, Command.INTEL, Command.CLANGPE>
         "$(DLINK)" $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)
 
     <Command.GCC>
@@ -355,7 +358,7 @@
     <OutputFile>
         $(OUTPUT_DIR)(+)$(MODULE_NAME).efi
 
-    <Command.MSFT, Command.INTEL, Command.RVCT>
+    <Command.MSFT, Command.INTEL, Command.RVCT, Command.CLANGPE>
         "$(GENFW)" -e $(MODULE_TYPE) -o ${dst} ${src} $(GENFW_FLAGS)
         $(CP) ${dst} $(DEBUG_DIR)
         $(CP) ${dst} $(BIN_DIR)(+)$(MODULE_NAME_GUID).efi
@@ -460,9 +463,14 @@
 
     <Command.GCC>
         "$(ASLCC)" -c -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS) $(INC) ${src}
-        "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
+        "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS)
         "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(GENFW_FLAGS)
-        
+
+    <Command.CLANGPE>
+        "$(ASLCC)" -c -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS) $(INC) ${src}
+        "$(ASLDLINK)" /OUT:$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
+        "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(GENFW_FLAGS)
+
     <Command.XCODE>        
         "$(ASLCC)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj  $(ASLCC_FLAGS) $(INC) ${src}
         "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
@@ -622,21 +630,19 @@
     <InputFile>
         *.hpk
 
-    <OutputFile.MSFT, OutputFile.INTEL, OutputFile.GCC>
+    <OutputFile.MSFT, OutputFile.INTEL, OutputFile.GCC, OutputFile.CLANGPE>
         $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.lib
 
     <OutputFile.XCODE, OutputFile.RVCT>
         $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc
 
-    <Command.MSFT, Command.INTEL>
+    <Command.MSFT, Command.INTEL, Command.CLANGPE>
         "$(GENFW)" -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiipackage $(HII_BINARY_PACKAGES) $(GENFW_FLAGS)
         "$(RC)" /Fo${dst} $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc
 
     <Command.GCC>
         "$(GENFW)" -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiibinpackage $(HII_BINARY_PACKAGES) $(GENFW_FLAGS)
         "$(RC)" $(RC_FLAGS) $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc ${dst}
-        
+
     <Command.XCODE, Command.RVCT>
         GenFw -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiibinpackage $(HII_BINARY_PACKAGES)
-        
-        
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index fd6fca542d..e009f195b9 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -268,6 +268,15 @@ DEFINE DTC_BIN                 = ENV(DTC_PREFIX)dtc
 #                             Required to build platforms or ACPI tables:
 #                               Intel(r) ACPI Compiler from
 #                               https://acpica.org/downloads
+#   CLANG9   -Linux, Windows, Mac-  Requires:
+#                             Clang 9 or above from http://releases.llvm.org/
+#                        Optional:
+#                             Required to compile nasm source:
+#                               nasm compiler from
+#                               NASM -- http://www.nasm.us/
+#                             Required to build platforms or ACPI tables:
+#                               Intel(r) ACPI Compiler from
+#                               https://acpica.org/downloads
 #   VS2008x86   -win64-  Requires:
 #                             Microsoft Visual Studio 2008 (x86)
 #                             Microsoft Windows Server 2003 Driver Development Kit (Microsoft WINDDK) version 3790.1830
@@ -2698,6 +2707,99 @@ DEFINE CLANG38_AARCH64_DLINK_FLAGS  = DEF(CLANG38_AARCH64_TARGET) DEF(GCC_AARCH6
 RELEASE_CLANG38_AARCH64_CC_FLAGS    = DEF(CLANG38_AARCH64_CC_FLAGS) $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -flto -O3
 RELEASE_CLANG38_AARCH64_DLINK_FLAGS = DEF(CLANG38_AARCH64_DLINK_FLAGS) -flto -Wl,-O3 -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64
 
+####################################################################################
+#
+# CLANG9 - This configuration is used to compile under Windows/Linux/Mac to produce
+#  PE/COFF binaries using LLVM/Clang/LLD with Link Time Optimization enabled
+#
+####################################################################################
+*_CLANG9_*_*_FAMILY                = GCC
+*_CLANG9_*_*_BUILDRULEFAMILY       = CLANGPE
+*_CLANG9_*_MAKE_PATH               = ENV(CLANG_HOST_BIN)make
+*_CLANG9_*_*_DLL                   = ENV(CLANG9_DLL)
+*_CLANG9_*_ASL_PATH                = DEF(UNIX_IASL_BIN)
+
+*_CLANG9_*_APP_FLAGS               =
+*_CLANG9_*_ASL_FLAGS               = DEF(DEFAULT_WIN_ASL_FLAGS)
+*_CLANG9_*_ASL_OUTFLAGS            = DEF(DEFAULT_WIN_ASL_OUTFLAGS)
+*_CLANG9_*_ASLDLINK_FLAGS          = DEF(MSFT_ASLDLINK_FLAGS)
+
+DEFINE CLANG9_IA32_PREFIX          = ENV(CLANG9_BIN)
+DEFINE CLANG9_X64_PREFIX           = ENV(CLANG9_BIN)
+
+DEFINE CLANG9_IA32_TARGET          = -target i686-unknown-windows
+DEFINE CLANG9_X64_TARGET           = -target x86_64-unknown-windows
+
+DEFINE CLANG9_WARNING_OVERRIDES    = -Wno-parentheses-equality -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -Wno-empty-body -Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option -Wno-microsoft-enum-forward-reference
+DEFINE CLANG9_ALL_CC_FLAGS         = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANG9_WARNING_OVERRIDES) -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-implicit-float  -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference -fms-compatibility -mno-stack-arg-probe
+
+###########################
+# CLANG9 IA32 definitions
+###########################
+*_CLANG9_IA32_CC_PATH              = DEF(CLANG9_IA32_PREFIX)clang
+*_CLANG9_IA32_SLINK_PATH           = DEF(CLANG9_IA32_PREFIX)llvm-lib
+*_CLANG9_IA32_DLINK_PATH           = DEF(CLANG9_IA32_PREFIX)lld-link
+*_CLANG9_IA32_ASLDLINK_PATH        = DEF(CLANG9_IA32_PREFIX)lld-link
+*_CLANG9_IA32_ASM_PATH             = DEF(CLANG9_IA32_PREFIX)clang
+*_CLANG9_IA32_PP_PATH              = DEF(CLANG9_IA32_PREFIX)clang
+*_CLANG9_IA32_VFRPP_PATH           = DEF(CLANG9_IA32_PREFIX)clang
+*_CLANG9_IA32_ASLCC_PATH           = DEF(CLANG9_IA32_PREFIX)clang
+*_CLANG9_IA32_ASLPP_PATH           = DEF(CLANG9_IA32_PREFIX)clang
+*_CLANG9_IA32_RC_PATH              = DEF(CLANG9_IA32_PREFIX)llvm-rc
+
+*_CLANG9_IA32_ASLCC_FLAGS          = DEF(GCC_ASLCC_FLAGS) -m32 -fno-lto DEF(CLANG9_IA32_TARGET)
+*_CLANG9_IA32_ASM_FLAGS            = DEF(GCC5_ASM_FLAGS) -m32 -march=i386 DEF(CLANG9_IA32_TARGET)
+*_CLANG9_IA32_OBJCOPY_FLAGS        =
+*_CLANG9_IA32_NASM_FLAGS           = -f win32
+*_CLANG9_IA32_PP_FLAGS             = DEF(GCC_PP_FLAGS) DEF(CLANG9_IA32_TARGET)
+*_CLANG9_IA32_ASLPP_FLAGS          = DEF(GCC_ASLPP_FLAGS) DEF(CLANG9_IA32_TARGET)
+*_CLANG9_IA32_VFRPP_FLAGS          = DEF(GCC_VFRPP_FLAGS) DEF(CLANG9_IA32_TARGET)
+
+DEBUG_CLANG9_IA32_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m32 -Oz -flto -march=i586 DEF(CLANG9_IA32_TARGET) -gcodeview
+DEBUG_CLANG9_IA32_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
+DEBUG_CLANG9_IA32_DLINK2_FLAGS     =
+
+RELEASE_CLANG9_IA32_CC_FLAGS       = DEF(CLANG9_ALL_CC_FLAGS) -m32 -Oz -flto -march=i586 DEF(CLANG9_IA32_TARGET)
+RELEASE_CLANG9_IA32_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /IGNORE:4254 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /MERGE:.rdata=.data  /lldmap
+RELEASE_CLANG9_IA32_DLINK2_FLAGS   = 
+
+NOOPT_CLANG9_IA32_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m32 -O0 -march=i586 DEF(CLANG9_IA32_TARGET) -gcodeview
+NOOPT_CLANG9_IA32_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
+NOOPT_CLANG9_IA32_DLINK2_FLAGS     = 
+
+##########################
+# CLANGWIN X64 definitions
+##########################
+*_CLANG9_X64_CC_PATH              = DEF(CLANG9_X64_PREFIX)clang
+*_CLANG9_X64_SLINK_PATH           = DEF(CLANG9_X64_PREFIX)llvm-lib
+*_CLANG9_X64_DLINK_PATH           = DEF(CLANG9_X64_PREFIX)lld-link
+*_CLANG9_X64_ASLDLINK_PATH        = DEF(CLANG9_X64_PREFIX)lld-link
+*_CLANG9_X64_ASM_PATH             = DEF(CLANG9_X64_PREFIX)clang
+*_CLANG9_X64_PP_PATH              = DEF(CLANG9_X64_PREFIX)clang
+*_CLANG9_X64_VFRPP_PATH           = DEF(CLANG9_X64_PREFIX)clang
+*_CLANG9_X64_ASLCC_PATH           = DEF(CLANG9_X64_PREFIX)clang
+*_CLANG9_X64_ASLPP_PATH           = DEF(CLANG9_X64_PREFIX)clang
+*_CLANG9_X64_RC_PATH              = DEF(CLANG9_IA32_PREFIX)llvm-rc
+
+*_CLANG9_X64_ASLCC_FLAGS          = DEF(GCC_ASLCC_FLAGS) -m64 -fno-lto DEF(CLANG9_X64_TARGET)
+*_CLANG9_X64_ASM_FLAGS            = DEF(GCC5_ASM_FLAGS) -m64 DEF(CLANG9_X64_TARGET)
+*_CLANG9_X64_OBJCOPY_FLAGS        =
+*_CLANG9_X64_NASM_FLAGS           = -f win64
+*_CLANG9_X64_PP_FLAGS             = DEF(GCC_PP_FLAGS) DEF(CLANG9_X64_TARGET)
+*_CLANG9_X64_ASLPP_FLAGS          = DEF(GCC_ASLPP_FLAGS) DEF(CLANG9_X64_TARGET)
+*_CLANG9_X64_VFRPP_FLAGS          = DEF(GCC_VFRPP_FLAGS) DEF(CLANG9_X64_TARGET)
+
+DEBUG_CLANG9_X64_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -Oz -flto DEF(CLANG9_X64_TARGET) -gcodeview
+DEBUG_CLANG9_X64_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
+DEBUG_CLANG9_X64_DLINK2_FLAGS     = 
+
+RELEASE_CLANG9_X64_CC_FLAGS       = DEF(CLANG9_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -Oz -flto DEF(CLANG9_X64_TARGET)
+RELEASE_CLANG9_X64_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /IGNORE:4254 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /MERGE:.rdata=.data  /lldmap
+RELEASE_CLANG9_X64_DLINK2_FLAGS   = 
+
+NOOPT_CLANG9_X64_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -O0 DEF(CLANG9_X64_TARGET) -gcodeview
+NOOPT_CLANG9_X64_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
+NOOPT_CLANG9_X64_DLINK2_FLAGS     = 
 
 
 #
-- 
2.13.0.windows.1

[Patch 03/12] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize

[Liming Gao]

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 BaseTools/Source/C/GenFw/GenFw.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index c99782b78e..d8d3360c24 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -653,7 +653,11 @@ PeCoffConvertImageToXip (
     //
     // Make the size of raw data in section header alignment.
     //
-    SectionHeader->SizeOfRawData = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
+    SectionSize = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
+    if (SectionSize < SectionHeader->SizeOfRawData) {
+      SectionHeader->SizeOfRawData = SectionSize;
+    }
+    
     SectionHeader->PointerToRawData = SectionHeader->VirtualAddress;
   }
 
@@ -999,7 +1003,7 @@ Returns:
     CopyMem (
       FileBuffer + SectionHeader->PointerToRawData,
       (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress),
-      SectionHeader->SizeOfRawData
+      SectionHeader->SizeOfRawData < SectionHeader->Misc.VirtualSize ? SectionHeader->SizeOfRawData : SectionHeader->Misc.VirtualSize
       );
   }
 
-- 
2.13.0.windows.1

[Patch 04/12] MdePkg Base.h: Add definition for CLANG9 tool chain

[Liming Gao]

CLANG9 tool chain defines __clang__ macro only,
doesn't define __GNUC__ macro. But, it uses some same definitions with GCC.
So, update base definition for CLANG9 tool chain.

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdePkg/Include/Base.h               | 6 +++---
 MdePkg/Include/Ia32/ProcessorBind.h | 4 ++--
 MdePkg/Include/X64/ProcessorBind.h  | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index d94b8a5f93..4680e64136 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -621,9 +621,9 @@ typedef char* VA_LIST;
 #define VA_END(Marker)                  (Marker = (VA_LIST) 0)
 #define VA_COPY(Dest, Start)            ((void)((Dest) = (Start)))
 
-#elif defined(__GNUC__)
+#elif defined(__GNUC__) || defined(__clang__)
 
-#if defined(MDE_CPU_X64) && !defined(NO_MSABI_VA_FUNCS)
+#if defined(MDE_CPU_X64) && !defined(NO_MSABI_VA_FUNCS) && !defined(__clang__)
 //
 // X64 only. Use MS ABI version of GCC built-in macros for variable argument lists.
 //
@@ -1274,7 +1274,7 @@ typedef UINTN RETURN_STATUS;
 
   **/
   #define RETURN_ADDRESS(L)     ((L == 0) ? _ReturnAddress() : (VOID *) 0)
-#elif defined(__GNUC__)
+#elif defined (__GNUC__) || defined (__clang__)
   void * __builtin_return_address (unsigned int level);
   /**
     Get the return address of the calling function.
diff --git a/MdePkg/Include/Ia32/ProcessorBind.h b/MdePkg/Include/Ia32/ProcessorBind.h
index 497c58b33b..fa4b7e8e98 100644
--- a/MdePkg/Include/Ia32/ProcessorBind.h
+++ b/MdePkg/Include/Ia32/ProcessorBind.h
@@ -281,7 +281,7 @@ typedef INT32   INTN;
   /// Microsoft* compiler specific method for EFIAPI calling convention.
   ///
   #define EFIAPI __cdecl
-#elif defined(__GNUC__)
+#elif defined(__GNUC__) || defined(__clang__)
   ///
   /// GCC specific method for EFIAPI calling convention.
   ///
@@ -294,7 +294,7 @@ typedef INT32   INTN;
   #define EFIAPI
 #endif
 
-#if defined(__GNUC__)
+#if defined(__GNUC__) || defined(__clang__)
   ///
   /// For GNU assembly code, .global or .globl can declare global symbols.
   /// Define this macro to unify the usage.
diff --git a/MdePkg/Include/X64/ProcessorBind.h b/MdePkg/Include/X64/ProcessorBind.h
index 6f65acd609..387e9c5c9c 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -313,7 +313,7 @@ typedef INT64   INTN;
   #define EFIAPI
 #endif
 
-#if defined(__GNUC__)
+#if defined(__GNUC__) || defined(__clang__)
   ///
   /// For GNU assembly code, .global or .globl can declare global symbols.
   /// Define this macro to unify the usage.
-- 
2.13.0.windows.1

[Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Liming Gao]

__inline__ attribute will make the functions not be exposed as the
library interface. It will cause CLANG9 compiler fail.

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
index 055f0a947e..b3a1a20256 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
+++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
@@ -32,7 +32,6 @@
   @return The value read.
 
 **/
-__inline__
 UINT8
 EFIAPI
 IoRead8 (
@@ -60,7 +59,6 @@ IoRead8 (
   @return The value written the I/O port.
 
 **/
-__inline__
 UINT8
 EFIAPI
 IoWrite8 (
@@ -87,7 +85,6 @@ IoWrite8 (
   @return The value read.
 
 **/
-__inline__
 UINT16
 EFIAPI
 IoRead16 (
@@ -117,7 +114,6 @@ IoRead16 (
   @return The value written the I/O port.
 
 **/
-__inline__
 UINT16
 EFIAPI
 IoWrite16 (
@@ -145,7 +141,6 @@ IoWrite16 (
   @return The value read.
 
 **/
-__inline__
 UINT32
 EFIAPI
 IoRead32 (
@@ -175,7 +170,6 @@ IoRead32 (
   @return The value written the I/O port.
 
 **/
-__inline__
 UINT32
 EFIAPI
 IoWrite32 (
-- 
2.13.0.windows.1

[Patch 06/12] MdeModulePkg LzmaCustomDecompressLib: Update macro to be same in CLANG tool

[Liming Gao]

Define the same macro in the different OS. It can make CLANG generate the same
image in the different host OS.

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/Library/LzmaCustomDecompressLib/Sdk/C/7zTypes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/LzmaCustomDecompressLib/Sdk/C/7zTypes.h b/MdeModulePkg/Library/LzmaCustomDecompressLib/Sdk/C/7zTypes.h
index a5fcb50bae..3ce087ecf7 100644
--- a/MdeModulePkg/Library/LzmaCustomDecompressLib/Sdk/C/7zTypes.h
+++ b/MdeModulePkg/Library/LzmaCustomDecompressLib/Sdk/C/7zTypes.h
@@ -118,7 +118,7 @@ typedef int Bool;
 #define MY_STD_CALL
 #endif
 
-#ifdef _MSC_VER
+#if defined(_MSC_VER) && !defined(__clang__)
 
 #if _MSC_VER >= 1300
 #define MY_NO_INLINE __declspec(noinline)
-- 
2.13.0.windows.1

[Patch 07/12] MdeModulePkg RegularExpressionDxe: Disable warning for CLANG9 tool chain

[Liming Gao]

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
index 883d5f1127..e9c885465d 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
@@ -107,6 +107,9 @@
   # Oniguruma: tag_end in parse_callout_of_name
   GCC:*_*_*_CC_FLAGS = -Wno-error=maybe-uninitialized
 
+  # Oniguruma: implicit conversion from 'UINTN' (aka 'unsigned long long') to 'long'
+  GCC:*_CLANG9_*_CC_FLAGS = -Wno-error=constant-conversion
+
   # Not add -Wno-error=maybe-uninitialized option for XCODE
   # XCODE doesn't know this option
   XCODE:*_*_*_CC_FLAGS =
-- 
2.13.0.windows.1

[Patch 08/12] CryptoPkg: Append options to make CLANG9 tool chain pass build

[Liming Gao]

Disable warning reported from CLANG9.

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf    | 1 +
 CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf     | 1 +
 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf | 1 +
 CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf     | 1 +
 CryptoPkg/Library/OpensslLib/OpensslLib.inf        | 1 +
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  | 1 +
 6 files changed, 6 insertions(+)

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 8d4988e8c6..a98be2cd95 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -101,5 +101,6 @@
 
   GCC:*_CLANG35_*_CC_FLAGS = -std=c99
   GCC:*_CLANG38_*_CC_FLAGS = -std=c99
+  GCC:*_CLANG9_*_CC_FLAGS = -std=c99 -Wno-error=incompatible-pointer-types
 
   XCODE:*_*_*_CC_FLAGS = -std=c99
diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
index 3da8bd8480..7b07dd13d2 100644
--- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
@@ -96,5 +96,6 @@
 
   GCC:*_CLANG35_*_CC_FLAGS = -std=c99
   GCC:*_CLANG38_*_CC_FLAGS = -std=c99
+  GCC:*_CLANG9_*_CC_FLAGS = -std=c99 -Wno-error=incompatible-pointer-types
 
   XCODE:*_*_*_CC_FLAGS = -std=c99
diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
index 21a481eb77..d9e29ef660 100644
--- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
@@ -107,5 +107,6 @@
 
   GCC:*_CLANG35_*_CC_FLAGS = -std=c99
   GCC:*_CLANG38_*_CC_FLAGS = -std=c99
+  GCC:*_CLANG9_*_CC_FLAGS = -std=c99 -Wno-error=incompatible-pointer-types
 
   XCODE:*_*_*_CC_FLAGS = -std=c99
diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
index 7c187e21b3..b4faaf3f80 100644
--- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
@@ -104,3 +104,4 @@
 
   GCC:*_CLANG35_*_CC_FLAGS = -std=c99
   GCC:*_CLANG38_*_CC_FLAGS = -std=c99
+  GCC:*_CLANG9_*_CC_FLAGS = -std=c99 -Wno-error=incompatible-pointer-types
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index 7432321fd4..b40d82783b 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -662,6 +662,7 @@
   GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-format -Wno-error=unused-but-set-variable
   GCC:*_CLANG35_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
   GCC:*_CLANG38_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
+  GCC:*_CLANG9_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized -Wno-error=incompatible-pointer-types -Wno-error=pointer-sign -Wno-error=implicit-function-declaration -Wno-error=ignored-pragma-optimize
 
   # suppress the following warnings in openssl so we don't break the build with warnings-as-errors:
   # 1295: Deprecated declaration <entity> - give arg types
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index 8134b45eda..0a60196c8a 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
@@ -610,6 +610,7 @@
   GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-format -Wno-error=unused-but-set-variable
   GCC:*_CLANG35_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
   GCC:*_CLANG38_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
+  GCC:*_CLANG9_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized -Wno-error=incompatible-pointer-types -Wno-error=pointer-sign -Wno-error=implicit-function-declaration -Wno-error=ignored-pragma-optimize
 
   # suppress the following warnings in openssl so we don't break the build with warnings-as-errors:
   # 1295: Deprecated declaration <entity> - give arg types
-- 
2.13.0.windows.1

[Patch 09/12] CryptoPkg IntrinsicLib: Make _fltused always be used

[Liming Gao]

With this change, global variable _fltused will not be removed by LTO

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
index 6e4d4a68cc..94fe341bec 100644
--- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
+++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
@@ -2,7 +2,7 @@
   Intrinsic Memory Routines Wrapper Implementation for OpenSSL-based
   Cryptographic Library.
 
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -13,9 +13,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 typedef UINTN  size_t;
 
+#if defined(__GNUC__) || defined(__clang__)
+  #define GLOBAL_USED __attribute__((used))
+#else
+  #define GLOBAL_USED
+#endif
+
 /* OpenSSL will use floating point support, and C compiler produces the _fltused
    symbol by default. Simply define this symbol here to satisfy the linker. */
-int _fltused = 1;
+int  GLOBAL_USED _fltused = 1;
 
 /* Sets buffers to a specified character */
 void * memset (void *dest, int ch, size_t count)
-- 
2.13.0.windows.1

[Patch 10/12] EmulatorPkg: Enable CLANG9 tool chain

[Liming Gao]

1. Append CLANG CC and LINK flags to generate windows HOST.
2. Modify WinHost to move PCD getting in the late position

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 EmulatorPkg/Win/Host/WinHost.c   | 11 +++++++----
 EmulatorPkg/EmulatorPkg.dsc      |  8 ++++++--
 EmulatorPkg/EmulatorPkg.fdf      |  2 +-
 EmulatorPkg/Win/Host/WinHost.inf |  6 ++++++
 4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c
index 9aba3c8959..d51a96de7b 100644
--- a/EmulatorPkg/Win/Host/WinHost.c
+++ b/EmulatorPkg/Win/Host/WinHost.c
@@ -356,7 +356,7 @@ Returns:
 INTN
 EFIAPI
 main (
-  IN  INTN  Argc,
+  IN  INT  Argc,
   IN  CHAR8 **Argv,
   IN  CHAR8 **Envp
   )
@@ -405,9 +405,6 @@ Returns:
     AdjustTokenPrivileges(Token, FALSE, &TokenPrivileges, 0, (PTOKEN_PRIVILEGES) NULL, 0);
   }
 
-  MemorySizeStr      = (CHAR16 *) PcdGetPtr (PcdEmuMemorySize);
-  FirmwareVolumesStr = (CHAR16 *) PcdGetPtr (PcdEmuFirmwareVolume);
-
   SecPrint ("\n\rEDK II WIN Host Emulation Environment from http://www.tianocore.org/edk2/\n\r");
 
   //
@@ -423,6 +420,12 @@ Returns:
       SetProcessAffinityMask (GetCurrentProcess (), (INTN)(BIT0 << LowBit));
     }
   }
+  
+  //
+  // Move PCD getting late
+  //
+  MemorySizeStr      = (CHAR16 *) PcdGetPtr (PcdEmuMemorySize);
+  FirmwareVolumesStr = (CHAR16 *) PcdGetPtr (PcdEmuFirmwareVolume);
 
   //
   // Make some Windows calls to Set the process to the highest priority in the
diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 20f1187713..21620d4e94 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -237,7 +237,7 @@
 
 [Components]
 !if "IA32" in $(ARCH) || "X64" in $(ARCH)
-  !if "MSFT" in $(FAMILY)
+  !if "MSFT" in $(FAMILY) || "CLANG9" in $(TOOL_CHAIN_TAG)
     ##
     #  Emulator, OS WIN application
     ##
@@ -377,7 +377,7 @@
 
   FatPkg/EnhancedFatDxe/Fat.inf
 
-!if "XCODE5" not in $(TOOL_CHAIN_TAG)
+!if "XCODE5" not in $(TOOL_CHAIN_TAG) and "CLANG9" not in $(TOOL_CHAIN_TAG)
   ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
     <PcdsFixedAtBuild>
       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
@@ -419,7 +419,11 @@
 
   MSFT:DEBUG_*_*_CC_FLAGS = /Od /Oy-
   MSFT:NOOPT_*_*_CC_FLAGS = /Od /Oy-
+  GCC:DEBUG_CLANG9_*_CC_FLAGS =-O0 -D UNICODE -Wno-unused-command-line-argument -Wno-incompatible-pointer-types -Wno-enum-conversion -Wno-incompatible-pointer-types -Wno-sometimes-uninitialized -Wno-constant-conversion -Wno-main-return-type
 
   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
+  GCC:*_CLANG9_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
+  GCC:DEBUG_CLANG9_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
+  GCC:NOOPT_CLANG9_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
diff --git a/EmulatorPkg/EmulatorPkg.fdf b/EmulatorPkg/EmulatorPkg.fdf
index 295f6f1db8..59d9927d29 100644
--- a/EmulatorPkg/EmulatorPkg.fdf
+++ b/EmulatorPkg/EmulatorPkg.fdf
@@ -196,7 +196,7 @@ INF  EmulatorPkg/EmuSnpDxe/EmuSnpDxe.inf
 
 INF FatPkg/EnhancedFatDxe/Fat.inf
 
-!if "XCODE5" not in $(TOOL_CHAIN_TAG)
+!if "XCODE5" not in $(TOOL_CHAIN_TAG) and "CLANG9" not in $(TOOL_CHAIN_TAG)
 INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
 !endif
 INF  ShellPkg/Application/Shell/Shell.inf
diff --git a/EmulatorPkg/Win/Host/WinHost.inf b/EmulatorPkg/Win/Host/WinHost.inf
index 631d5a6470..1adca10d79 100644
--- a/EmulatorPkg/Win/Host/WinHost.inf
+++ b/EmulatorPkg/Win/Host/WinHost.inf
@@ -95,3 +95,9 @@
   MSFT:*_VS2017_X64_DLINK_FLAGS      = /LIBPATH:"%VCToolsInstallDir%lib\x64" /LIBPATH:"%UniversalCRTSdkDir%lib\%UCRTVersion%\ucrt\x64" /LIBPATH:"%WindowsSdkDir%lib\%WindowsSDKLibVersion%\um\x64" /NOLOGO /SUBSYSTEM:CONSOLE /NODEFAULTLIB /IGNORE:4086 /MAP /OPT:REF /DEBUG /MACHINE:AMD64 /LTCG Kernel32.lib MSVCRTD.lib vcruntimed.lib ucrtd.lib Gdi32.lib User32.lib Winmm.lib Advapi32.lib
   MSFT:*_*_X64_ASM_FLAGS            == /nologo /W3 /WX /c /Cx /Zd /W0 /Zi
   MSFT:*_*_X64_ASMLINK_FLAGS        == /link /nologo
+
+  GCC:*_CLANG9_X64_DLINK_FLAGS == /out:"$(BIN_DIR)\$(BASE_NAME).exe" /base:0x10000000 /pdb:"$(BIN_DIR)\$(BASE_NAME).pdb"  /LIBPATH:"%UniversalCRTSdkDir%lib\%UCRTVersion%\ucrt\x64" /LIBPATH:"%WindowsSdkDir%lib\%WindowsSDKLibVersion%\um\x64" /LIBPATH:"%VCToolsInstallDir%lib\x64"   /NOLOGO /SUBSYSTEM:CONSOLE /NODEFAULTLIB /IGNORE:4086  /OPT:REF /DEBUG /MACHINE:AMD64 Kernel32.lib MSVCRTD.lib vcruntimed.lib ucrtd.lib Gdi32.lib User32.lib Winmm.lib Advapi32.lib /lldmap  /EXPORT:InitializeDriver=_ModuleEntryPoint
+  GCC:*_CLANG9_X64_CC_FLAGS == -m64 -g -fshort-wchar -fno-strict-aliasing -Wall -c -include AutoGen.h -D _CRT_SECURE_NO_WARNINGS -Wnonportable-include-path  -D UNICODE -D _CRT_SECURE_NO_DEPRECATE
+
+  GCC:*_CLANG9_IA32_DLINK_FLAGS == /out:"$(BIN_DIR)\$(BASE_NAME).exe" /base:0x10000000 /pdb:"$(BIN_DIR)\$(BASE_NAME).pdb" /LIBPATH:"%UniversalCRTSdkDir%lib\%UCRTVersion%\ucrt\x86" /LIBPATH:"%WindowsSdkDir%lib\%WindowsSDKLibVersion%\um\x86" /LIBPATH:"%VCToolsInstallDir%ib\x86"   /NOLOGO /SUBSYSTEM:CONSOLE /NODEFAULTLIB /IGNORE:4086  /OPT:REF /DEBUG /MACHINE:I386 Kernel32.lib MSVCRTD.lib vcruntimed.lib ucrtd.lib Gdi32.lib User32.lib Winmm.lib Advapi32.lib /lldmap  /EXPORT:InitializeDriver=_ModuleEntryPoint
+  GCC:*_CLANG9_IA32_CC_FLAGS == -m32 -g -fshort-wchar -fno-strict-aliasing -Wall -c -include AutoGen.h -D _CRT_SECURE_NO_WARNINGS -Wnonportable-include-path  -D UNICODE -D _CRT_SECURE_NO_DEPRECATE
-- 
2.13.0.windows.1

[Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Liming Gao]

1. Apply CLANG9 Linker option.
2. Add -mno-mmx -mno-sse compiler option

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 4 +++-
 OvmfPkg/OvmfPkgIa32X64.dsc | 4 +++-
 OvmfPkg/OvmfPkgX64.dsc     | 4 +++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 66e944436a..0fde8e6e84 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -66,7 +66,7 @@
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
   MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
-!if $(TOOL_CHAIN_TAG) != "XCODE5"
+!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
   GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
 !endif
 
@@ -80,12 +80,14 @@
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
   XCODE:*_*_*_DLINK_FLAGS =
+  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
 
 # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
 # protection of DXE_SMM_DRIVER/SMM_CORE modules
 [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
   XCODE:*_*_*_DLINK_FLAGS =
+  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
 
 ################################################################################
 #
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 51c2bfb44f..c17329878e 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -66,7 +66,7 @@
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
   MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
-!if $(TOOL_CHAIN_TAG) != "XCODE5"
+!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
   GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
 !endif
 !ifdef $(SOURCE_DEBUG_ENABLE)
@@ -85,12 +85,14 @@
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
   XCODE:*_*_*_DLINK_FLAGS =
+  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
 
 # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
 # protection of DXE_SMM_DRIVER/SMM_CORE modules
 [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
   XCODE:*_*_*_DLINK_FLAGS =
+  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
 
 ################################################################################
 #
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index ba7a758844..af91265d05 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -66,7 +66,7 @@
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
   MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
-!if $(TOOL_CHAIN_TAG) != "XCODE5"
+!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
   GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
 !endif
 !ifdef $(SOURCE_DEBUG_ENABLE)
@@ -85,12 +85,14 @@
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
   XCODE:*_*_*_DLINK_FLAGS =
+  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
 
 # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
 # protection of DXE_SMM_DRIVER/SMM_CORE modules
 [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
   XCODE:*_*_*_DLINK_FLAGS =
+  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
 
 ################################################################################
 #
-- 
2.13.0.windows.1

[Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

[Liming Gao]

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

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 OvmfPkg/Sec/SecMain.inf | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 63ba4cb555..cd765cac25 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -69,3 +69,7 @@
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
+
+[BuildOptions]
+  # CLANG9 X64 requires it.
+  GCC:*_CLANG9_X64_CC_FLAGS = -fno-omit-frame-pointer
-- 
2.13.0.windows.1

Re: [edk2-devel] [Patch 00/12] New Cross OS tool chain CLANG9

[Yao, Jiewen]

Thank you Liming. That is cool feature.


Thank you
Yao Jiewen



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao
> Sent: Friday, September 27, 2019 3:46 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Shi, Steven <steven.shi@intel.com>;
> Justen, Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [Patch 00/12] New Cross OS tool chain CLANG9
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1603
> Code: https://github.com/lgao4/edk2/tree/CLANG9
> Wiki: https://github.com/lgao4/edk2/wiki/CLANG9-Tools-Chain
> 
> CLANG9 tool chain is added to directly generate PE/COFF image (EFI image).
> This tool chain uses LLVM clang C compiler and lld linker, generates PE/COFF
> image and PDB compatible debug symbol format. Now, it supports IA32/X64
> Archs.
> It must use LLVM 9 or above release. LLVM 9 is ready on
> http://releases.llvm.org/download.html#9.0.0.
> 
> CLANG9 is the cross OS tool chain. It can work on Windows/Linux/Mac host OS.
> For the same source code, with the same version LLVM tool chain,
> CLANG9 can generate the same binary image. So, the developer can
> choose the different development environment and work on the same
> code base. Besides, EDKII project build also requires third party
> tools: nasm and iasl. They both keep the same version. If so, the same
> binary image can be generated on the different host OS.
> 
> LLVM tool chain provides the compiler and linker. To build EDK2 project,
> some other tools are still required. On Windows OS, nmake and Visual Studio
> are required to call Makefile and compile BaseTools C tools.
> On Linux/Mac, binutils and gcc are required to make and compile BaseTools
> C tools. Because VS or GCC are mainly used to compile BaseTools and provide
> nmake/make tool, they can keep on the stable version without update.
> 
> To build source code, CLANG9 tool chain (-t CLANG9) can be specified
> on Windows OS, set CLANG_HOST_BIN=n, set CLANG9_BIN=LLVM installed
> directory
> CLANG_HOST_BIN is used CLANG_HOST_PREFIX. Prefix n is for nmake.
> For example:
> *  set CLANG_HOST_BIN=n
> *  set CLANG9_BIN=C:\Program Files\LLVM\bin\
> *  set IASL_PREFIX=C:\Asl\
> 
> On Linux/Mac, export CLANG9_BIN=LLVM installed directory,
> CLANG_HOST_BIN is
> not required, because there is no prefix for make.
> For example:
> *  export CLANG9_BIN=/home/clang9/bin/
> 
> Now, CLANG9 tool chain has been verified in Edk2 packages and Ovmf/Emulator
> with LLVM 9.0.0 on Windows and Linux OS.
> OVMF IA32/X64/IA32X64 all boots to Shell on Windows and Linux OS.
> Emulator can boot to Shell on Windows only with CLANG9.
> OVMF Ia32X64 RELEASE build generates the same BIOS images on Windows and
> Linux OS.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Steven Shi <steven.shi@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Liming Gao (12):
>   BaseTools tools_def.template: Remove unnecessary $(DEST_DIR_DEBUG)
>     path
>   BaseTools tools_def: Add CLANG9 tool chain to directly generate PE
>     image
>   BaseTools GenFw: Fix the issue to update the wrong size as SectionSize
>   MdePkg Base.h: Add definition for CLANG9 tool chain
>   MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO
>     functions
>   MdeModulePkg LzmaCustomDecompressLib: Update macro to be same in
> CLANG
>     tool
>   MdeModulePkg RegularExpressionDxe: Disable warning for CLANG9 tool
>     chain
>   CryptoPkg: Append options to make CLANG9 tool chain pass build
>   CryptoPkg IntrinsicLib: Make _fltused always be used
>   EmulatorPkg: Enable CLANG9 tool chain
>   OvmfPkg: Enable CLANG9 tool chain
>   OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9
>     X64
> 
>  BaseTools/Source/C/GenFw/GenFw.c                                |   8 +-
>  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c               |  10 ++-
>  EmulatorPkg/Win/Host/WinHost.c                                  |  11 ++-
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c                    |   6 --
>  BaseTools/Conf/build_rule.template                              |  26 +++---
>  BaseTools/Conf/tools_def.template                               | 124
> +++++++++++++++++++++++++---
>  CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf                 |   1 +
>  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf                  |   1 +
>  CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf              |   1 +
>  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf                  |   1 +
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf                     |   1 +
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf               |   1 +
>  EmulatorPkg/EmulatorPkg.dsc                                     |   8 +-
>  EmulatorPkg/EmulatorPkg.fdf                                     |   2 +-
>  EmulatorPkg/Win/Host/WinHost.inf                                |   6 ++
>  MdeModulePkg/Library/LzmaCustomDecompressLib/Sdk/C/7zTypes.h    |   2 +-
>  .../Universal/RegularExpressionDxe/RegularExpressionDxe.inf     |   3 +
>  MdePkg/Include/Base.h                                           |   6 +-
>  MdePkg/Include/Ia32/ProcessorBind.h                             |   4 +-
>  MdePkg/Include/X64/ProcessorBind.h                              |   2 +-
>  OvmfPkg/OvmfPkgIa32.dsc                                         |   4 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                                      |   4 +-
>  OvmfPkg/OvmfPkgX64.dsc                                          |   4 +-
>  OvmfPkg/Sec/SecMain.inf                                         |   4 +
>  24 files changed, 192 insertions(+), 48 deletions(-)
> 
> --
> 2.13.0.windows.1
> 
> 
> 

Re: [edk2-devel] [Patch 09/12] CryptoPkg IntrinsicLib: Make _fltused always be used

[Yao, Jiewen]

Hi

+int  GLOBAL_USED _fltused = 1;

May I know what is the use of GLOBAL_USED? Only for compiler stub symbol?

If so, why we add __GNUC__ here? Any other usage?

+#if defined(__GNUC__) || defined(__clang__)
+  #define GLOBAL_USED __attribute__((used))
+#else
+  #define GLOBAL_USED
+#endif

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao
> Sent: Friday, September 27, 2019 3:47 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch 09/12] CryptoPkg IntrinsicLib: Make _fltused always
> be used
> 
> With this change, global variable _fltused will not be removed by LTO
> 
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> ---
>  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> index 6e4d4a68cc..94fe341bec 100644
> --- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> +++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> @@ -2,7 +2,7 @@
>    Intrinsic Memory Routines Wrapper Implementation for OpenSSL-based
>    Cryptographic Library.
> 
> -Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -13,9 +13,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  typedef UINTN  size_t;
> 
> +#if defined(__GNUC__) || defined(__clang__)
> +  #define GLOBAL_USED __attribute__((used))
> +#else
> +  #define GLOBAL_USED
> +#endif
> +
>  /* OpenSSL will use floating point support, and C compiler produces the _fltused
>     symbol by default. Simply define this symbol here to satisfy the linker. */
> -int _fltused = 1;
> +int  GLOBAL_USED _fltused = 1;
> 
>  /* Sets buffers to a specified character */
>  void * memset (void *dest, int ch, size_t count)
> --
> 2.13.0.windows.1
> 
> 
> 

Re: [edk2-devel] [Patch 02/12] BaseTools tools_def: Add CLANG9 tool chain to directly generate PE image

[Philippe Mathieu-Daudé]

Hi Liming,

On 9/27/19 9:46 AM, Liming Gao wrote:
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> ---
>  BaseTools/Conf/build_rule.template |  26 +++++++++------
>  BaseTools/Conf/tools_def.template  | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 118 insertions(+), 10 deletions(-)
> 
> diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template
> index db06d3a6b4..3a58ac8015 100755
> --- a/BaseTools/Conf/build_rule.template
> +++ b/BaseTools/Conf/build_rule.template
> @@ -260,7 +260,7 @@
>      <OutputFile>
>          $(OUTPUT_DIR)(+)$(MODULE_NAME).lib
>  
> -    <Command.MSFT, Command.INTEL>
> +    <Command.MSFT, Command.INTEL, Command.CLANGPE>
>          "$(SLINK)" $(SLINK_FLAGS) /OUT:${dst} @$(OBJECT_FILES_LIST)
>  
>      <Command.GCC>
> @@ -291,6 +291,9 @@
>          "$(DLINK)" /OUT:${dst} $(DLINK_FLAGS) $(DLINK2_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)
>          "$(DLINK)" /OUT:${dst} $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)
>  
> +    <Command.CLANGPE>
> +        "$(DLINK)" /OUT:${dst} $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST) $(DLINK2_FLAGS)
> +
>      <Command.GCC>
>          "$(DLINK)" -o ${dst} $(DLINK_FLAGS) -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS) $(DLINK2_FLAGS)
>          "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
> @@ -331,7 +334,7 @@
>      <OutputFile>
>          $(DEBUG_DIR)(+)$(MODULE_NAME)
>  
> -    <Command.MSFT, Command.INTEL>
> +    <Command.MSFT, Command.INTEL, Command.CLANGPE>
>          "$(DLINK)" $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)
>  
>      <Command.GCC>
> @@ -355,7 +358,7 @@
>      <OutputFile>
>          $(OUTPUT_DIR)(+)$(MODULE_NAME).efi
>  
> -    <Command.MSFT, Command.INTEL, Command.RVCT>
> +    <Command.MSFT, Command.INTEL, Command.RVCT, Command.CLANGPE>
>          "$(GENFW)" -e $(MODULE_TYPE) -o ${dst} ${src} $(GENFW_FLAGS)
>          $(CP) ${dst} $(DEBUG_DIR)
>          $(CP) ${dst} $(BIN_DIR)(+)$(MODULE_NAME_GUID).efi
> @@ -460,9 +463,14 @@
>  
>      <Command.GCC>
>          "$(ASLCC)" -c -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS) $(INC) ${src}
> -        "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
> +        "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS)
>          "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(GENFW_FLAGS)
> -        
> +
> +    <Command.CLANGPE>
> +        "$(ASLCC)" -c -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS) $(INC) ${src}
> +        "$(ASLDLINK)" /OUT:$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
> +        "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(GENFW_FLAGS)
> +
>      <Command.XCODE>        
>          "$(ASLCC)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj  $(ASLCC_FLAGS) $(INC) ${src}
>          "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
> @@ -622,21 +630,19 @@
>      <InputFile>
>          *.hpk
>  
> -    <OutputFile.MSFT, OutputFile.INTEL, OutputFile.GCC>
> +    <OutputFile.MSFT, OutputFile.INTEL, OutputFile.GCC, OutputFile.CLANGPE>
>          $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.lib
>  
>      <OutputFile.XCODE, OutputFile.RVCT>
>          $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc
>  
> -    <Command.MSFT, Command.INTEL>
> +    <Command.MSFT, Command.INTEL, Command.CLANGPE>
>          "$(GENFW)" -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiipackage $(HII_BINARY_PACKAGES) $(GENFW_FLAGS)
>          "$(RC)" /Fo${dst} $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc
>  
>      <Command.GCC>
>          "$(GENFW)" -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiibinpackage $(HII_BINARY_PACKAGES) $(GENFW_FLAGS)
>          "$(RC)" $(RC_FLAGS) $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc ${dst}
> -        
> +
>      <Command.XCODE, Command.RVCT>
>          GenFw -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID) --hiibinpackage $(HII_BINARY_PACKAGES)
> -        
> -        
> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
> index fd6fca542d..e009f195b9 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -268,6 +268,15 @@ DEFINE DTC_BIN                 = ENV(DTC_PREFIX)dtc
>  #                             Required to build platforms or ACPI tables:
>  #                               Intel(r) ACPI Compiler from
>  #                               https://acpica.org/downloads
> +#   CLANG9   -Linux, Windows, Mac-  Requires:
> +#                             Clang 9 or above from http://releases.llvm.org/
> +#                        Optional:
> +#                             Required to compile nasm source:
> +#                               nasm compiler from
> +#                               NASM -- http://www.nasm.us/
> +#                             Required to build platforms or ACPI tables:
> +#                               Intel(r) ACPI Compiler from
> +#                               https://acpica.org/downloads
>  #   VS2008x86   -win64-  Requires:
>  #                             Microsoft Visual Studio 2008 (x86)
>  #                             Microsoft Windows Server 2003 Driver Development Kit (Microsoft WINDDK) version 3790.1830
> @@ -2698,6 +2707,99 @@ DEFINE CLANG38_AARCH64_DLINK_FLAGS  = DEF(CLANG38_AARCH64_TARGET) DEF(GCC_AARCH6
>  RELEASE_CLANG38_AARCH64_CC_FLAGS    = DEF(CLANG38_AARCH64_CC_FLAGS) $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -flto -O3
>  RELEASE_CLANG38_AARCH64_DLINK_FLAGS = DEF(CLANG38_AARCH64_DLINK_FLAGS) -flto -Wl,-O3 -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64
>  
> +####################################################################################
> +#
> +# CLANG9 - This configuration is used to compile under Windows/Linux/Mac to produce
> +#  PE/COFF binaries using LLVM/Clang/LLD with Link Time Optimization enabled
> +#
> +####################################################################################
> +*_CLANG9_*_*_FAMILY                = GCC
> +*_CLANG9_*_*_BUILDRULEFAMILY       = CLANGPE
> +*_CLANG9_*_MAKE_PATH               = ENV(CLANG_HOST_BIN)make
> +*_CLANG9_*_*_DLL                   = ENV(CLANG9_DLL)
> +*_CLANG9_*_ASL_PATH                = DEF(UNIX_IASL_BIN)
> +
> +*_CLANG9_*_APP_FLAGS               =
> +*_CLANG9_*_ASL_FLAGS               = DEF(DEFAULT_WIN_ASL_FLAGS)
> +*_CLANG9_*_ASL_OUTFLAGS            = DEF(DEFAULT_WIN_ASL_OUTFLAGS)
> +*_CLANG9_*_ASLDLINK_FLAGS          = DEF(MSFT_ASLDLINK_FLAGS)
> +
> +DEFINE CLANG9_IA32_PREFIX          = ENV(CLANG9_BIN)
> +DEFINE CLANG9_X64_PREFIX           = ENV(CLANG9_BIN)
> +
> +DEFINE CLANG9_IA32_TARGET          = -target i686-unknown-windows
> +DEFINE CLANG9_X64_TARGET           = -target x86_64-unknown-windows
> +
> +DEFINE CLANG9_WARNING_OVERRIDES    = -Wno-parentheses-equality -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -Wno-empty-body -Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option -Wno-microsoft-enum-forward-reference
> +DEFINE CLANG9_ALL_CC_FLAGS         = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANG9_WARNING_OVERRIDES) -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-implicit-float  -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference -fms-compatibility -mno-stack-arg-probe
> +
> +###########################
> +# CLANG9 IA32 definitions
> +###########################
> +*_CLANG9_IA32_CC_PATH              = DEF(CLANG9_IA32_PREFIX)clang
> +*_CLANG9_IA32_SLINK_PATH           = DEF(CLANG9_IA32_PREFIX)llvm-lib
> +*_CLANG9_IA32_DLINK_PATH           = DEF(CLANG9_IA32_PREFIX)lld-link
> +*_CLANG9_IA32_ASLDLINK_PATH        = DEF(CLANG9_IA32_PREFIX)lld-link
> +*_CLANG9_IA32_ASM_PATH             = DEF(CLANG9_IA32_PREFIX)clang
> +*_CLANG9_IA32_PP_PATH              = DEF(CLANG9_IA32_PREFIX)clang
> +*_CLANG9_IA32_VFRPP_PATH           = DEF(CLANG9_IA32_PREFIX)clang
> +*_CLANG9_IA32_ASLCC_PATH           = DEF(CLANG9_IA32_PREFIX)clang
> +*_CLANG9_IA32_ASLPP_PATH           = DEF(CLANG9_IA32_PREFIX)clang
> +*_CLANG9_IA32_RC_PATH              = DEF(CLANG9_IA32_PREFIX)llvm-rc
> +
> +*_CLANG9_IA32_ASLCC_FLAGS          = DEF(GCC_ASLCC_FLAGS) -m32 -fno-lto DEF(CLANG9_IA32_TARGET)
> +*_CLANG9_IA32_ASM_FLAGS            = DEF(GCC5_ASM_FLAGS) -m32 -march=i386 DEF(CLANG9_IA32_TARGET)
> +*_CLANG9_IA32_OBJCOPY_FLAGS        =
> +*_CLANG9_IA32_NASM_FLAGS           = -f win32
> +*_CLANG9_IA32_PP_FLAGS             = DEF(GCC_PP_FLAGS) DEF(CLANG9_IA32_TARGET)
> +*_CLANG9_IA32_ASLPP_FLAGS          = DEF(GCC_ASLPP_FLAGS) DEF(CLANG9_IA32_TARGET)
> +*_CLANG9_IA32_VFRPP_FLAGS          = DEF(GCC_VFRPP_FLAGS) DEF(CLANG9_IA32_TARGET)
> +
> +DEBUG_CLANG9_IA32_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m32 -Oz -flto -march=i586 DEF(CLANG9_IA32_TARGET) -gcodeview
> +DEBUG_CLANG9_IA32_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
> +DEBUG_CLANG9_IA32_DLINK2_FLAGS     =
> +
> +RELEASE_CLANG9_IA32_CC_FLAGS       = DEF(CLANG9_ALL_CC_FLAGS) -m32 -Oz -flto -march=i586 DEF(CLANG9_IA32_TARGET)
> +RELEASE_CLANG9_IA32_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /IGNORE:4254 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /MERGE:.rdata=.data  /lldmap
> +RELEASE_CLANG9_IA32_DLINK2_FLAGS   = 

git complains for trailing whitespace  ^

> +
> +NOOPT_CLANG9_IA32_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m32 -O0 -march=i586 DEF(CLANG9_IA32_TARGET) -gcodeview
> +NOOPT_CLANG9_IA32_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
> +NOOPT_CLANG9_IA32_DLINK2_FLAGS     = 

Here too,

> +
> +##########################
> +# CLANGWIN X64 definitions
> +##########################
> +*_CLANG9_X64_CC_PATH              = DEF(CLANG9_X64_PREFIX)clang
> +*_CLANG9_X64_SLINK_PATH           = DEF(CLANG9_X64_PREFIX)llvm-lib
> +*_CLANG9_X64_DLINK_PATH           = DEF(CLANG9_X64_PREFIX)lld-link
> +*_CLANG9_X64_ASLDLINK_PATH        = DEF(CLANG9_X64_PREFIX)lld-link
> +*_CLANG9_X64_ASM_PATH             = DEF(CLANG9_X64_PREFIX)clang
> +*_CLANG9_X64_PP_PATH              = DEF(CLANG9_X64_PREFIX)clang
> +*_CLANG9_X64_VFRPP_PATH           = DEF(CLANG9_X64_PREFIX)clang
> +*_CLANG9_X64_ASLCC_PATH           = DEF(CLANG9_X64_PREFIX)clang
> +*_CLANG9_X64_ASLPP_PATH           = DEF(CLANG9_X64_PREFIX)clang
> +*_CLANG9_X64_RC_PATH              = DEF(CLANG9_IA32_PREFIX)llvm-rc
> +
> +*_CLANG9_X64_ASLCC_FLAGS          = DEF(GCC_ASLCC_FLAGS) -m64 -fno-lto DEF(CLANG9_X64_TARGET)
> +*_CLANG9_X64_ASM_FLAGS            = DEF(GCC5_ASM_FLAGS) -m64 DEF(CLANG9_X64_TARGET)
> +*_CLANG9_X64_OBJCOPY_FLAGS        =
> +*_CLANG9_X64_NASM_FLAGS           = -f win64
> +*_CLANG9_X64_PP_FLAGS             = DEF(GCC_PP_FLAGS) DEF(CLANG9_X64_TARGET)
> +*_CLANG9_X64_ASLPP_FLAGS          = DEF(GCC_ASLPP_FLAGS) DEF(CLANG9_X64_TARGET)
> +*_CLANG9_X64_VFRPP_FLAGS          = DEF(GCC_VFRPP_FLAGS) DEF(CLANG9_X64_TARGET)
> +
> +DEBUG_CLANG9_X64_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -Oz -flto DEF(CLANG9_X64_TARGET) -gcodeview
> +DEBUG_CLANG9_X64_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
> +DEBUG_CLANG9_X64_DLINK2_FLAGS     = 

Here

> +
> +RELEASE_CLANG9_X64_CC_FLAGS       = DEF(CLANG9_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -Oz -flto DEF(CLANG9_X64_TARGET)
> +RELEASE_CLANG9_X64_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /IGNORE:4254 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /MERGE:.rdata=.data  /lldmap
> +RELEASE_CLANG9_X64_DLINK2_FLAGS   = 

Here

> +
> +NOOPT_CLANG9_X64_CC_FLAGS         = DEF(CLANG9_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -O0 DEF(CLANG9_X64_TARGET) -gcodeview
> +NOOPT_CLANG9_X64_DLINK_FLAGS      = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH  /lldmap
> +NOOPT_CLANG9_X64_DLINK2_FLAGS     = 

And here.

Just few nits ;)

Regards,

Phil.

Re: [edk2-devel] [Patch 03/12] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize

[Philippe Mathieu-Daudé]

On 9/27/19 9:46 AM, Liming Gao wrote:
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> ---
>  BaseTools/Source/C/GenFw/GenFw.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index c99782b78e..d8d3360c24 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -653,7 +653,11 @@ PeCoffConvertImageToXip (
>      //
>      // Make the size of raw data in section header alignment.
>      //
> -    SectionHeader->SizeOfRawData = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
> +    SectionSize = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
> +    if (SectionSize < SectionHeader->SizeOfRawData) {
> +      SectionHeader->SizeOfRawData = SectionSize;
> +    }
> +    
>      SectionHeader->PointerToRawData = SectionHeader->VirtualAddress;
>    }
>  
> @@ -999,7 +1003,7 @@ Returns:
>      CopyMem (
>        FileBuffer + SectionHeader->PointerToRawData,
>        (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress),
> -      SectionHeader->SizeOfRawData
> +      SectionHeader->SizeOfRawData < SectionHeader->Misc.VirtualSize ? SectionHeader->SizeOfRawData : SectionHeader->Misc.VirtualSize
>        );
>    }
>  
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

Re: [edk2-devel] [Patch 09/12] CryptoPkg IntrinsicLib: Make _fltused always be used

[Liming Gao]

Jiewen:
  When CLANG9 is used, CLANG compiler always removes _fltused symbol, and cause the linker issue. 
 __attribute__((used)) variable attribute informs the compiler that a static variable is to be retained in the object file, even if it is unreferenced.

 So, I add this attribute for _fltused symbol and let compiler keep this symbol for linker. I find GCC and CLANG both supports this attribute. Then, I add it for GCC and CLANG both. 

Thanks
Liming
>-----Original Message-----
>From: Yao, Jiewen
>Sent: Friday, September 27, 2019 4:34 PM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>Subject: RE: [edk2-devel] [Patch 09/12] CryptoPkg IntrinsicLib: Make _fltused
>always be used
>
>Hi
>
>+int  GLOBAL_USED _fltused = 1;
>
>May I know what is the use of GLOBAL_USED? Only for compiler stub symbol?
>
>If so, why we add __GNUC__ here? Any other usage?
>
>+#if defined(__GNUC__) || defined(__clang__)
>+  #define GLOBAL_USED __attribute__((used))
>+#else
>+  #define GLOBAL_USED
>+#endif
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming
>Gao
>> Sent: Friday, September 27, 2019 3:47 PM
>> To: devel@edk2.groups.io
>> Subject: [edk2-devel] [Patch 09/12] CryptoPkg IntrinsicLib: Make _fltused
>always
>> be used
>>
>> With this change, global variable _fltused will not be removed by LTO
>>
>> Signed-off-by: Liming Gao <liming.gao@intel.com>
>> ---
>>  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
>> b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
>> index 6e4d4a68cc..94fe341bec 100644
>> --- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
>> +++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
>> @@ -2,7 +2,7 @@
>>    Intrinsic Memory Routines Wrapper Implementation for OpenSSL-based
>>    Cryptographic Library.
>>
>> -Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>>  SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>>  **/
>> @@ -13,9 +13,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>>  typedef UINTN  size_t;
>>
>> +#if defined(__GNUC__) || defined(__clang__)
>> +  #define GLOBAL_USED __attribute__((used))
>> +#else
>> +  #define GLOBAL_USED
>> +#endif
>> +
>>  /* OpenSSL will use floating point support, and C compiler produces the
>_fltused
>>     symbol by default. Simply define this symbol here to satisfy the linker. */
>> -int _fltused = 1;
>> +int  GLOBAL_USED _fltused = 1;
>>
>>  /* Sets buffers to a specified character */
>>  void * memset (void *dest, int ch, size_t count)
>> --
>> 2.13.0.windows.1
>>
>>
>> 

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Laszlo Ersek]

Hi Liming,

On 09/27/19 09:46, Liming Gao wrote:
> __inline__ attribute will make the functions not be exposed as the
> library interface. It will cause CLANG9 compiler fail.
> 
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> ---
>  MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c | 6 ------
>  1 file changed, 6 deletions(-)

Did you regression-test this change against GCC48 (for example)?

I can't tell why we have the __inline__'s in the first place. They date
back to historical commit e1f414b6a7d8 ("Import some basic libraries
instances for Mde Packages.", 2007-06-22). And that commit does not
explain __inline__.

If we remove __inline__ for the whole GCC toolchain *family*, then I
think we need a better justification than just "makes CLANG9 fail".

Thanks
Laszlo

> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
> index 055f0a947e..b3a1a20256 100644
> --- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
> @@ -32,7 +32,6 @@
>    @return The value read.
>  
>  **/
> -__inline__
>  UINT8
>  EFIAPI
>  IoRead8 (
> @@ -60,7 +59,6 @@ IoRead8 (
>    @return The value written the I/O port.
>  
>  **/
> -__inline__
>  UINT8
>  EFIAPI
>  IoWrite8 (
> @@ -87,7 +85,6 @@ IoWrite8 (
>    @return The value read.
>  
>  **/
> -__inline__
>  UINT16
>  EFIAPI
>  IoRead16 (
> @@ -117,7 +114,6 @@ IoRead16 (
>    @return The value written the I/O port.
>  
>  **/
> -__inline__
>  UINT16
>  EFIAPI
>  IoWrite16 (
> @@ -145,7 +141,6 @@ IoWrite16 (
>    @return The value read.
>  
>  **/
> -__inline__
>  UINT32
>  EFIAPI
>  IoRead32 (
> @@ -175,7 +170,6 @@ IoRead32 (
>    @return The value written the I/O port.
>  
>  **/
> -__inline__
>  UINT32
>  EFIAPI
>  IoWrite32 (
> 

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Laszlo Ersek]

Hi Liming,

On 09/27/19 09:46, Liming Gao wrote:
> 1. Apply CLANG9 Linker option.

I'm confused by this, in two regards.

- Why do we refer to CLANG9 first as being in the GCC toolchain family
(see near mmx/sse), and then under a totally different family name
(CLANGPE?)

- Regarding the CLANGPE option, does the "/ALIGN:4096" option format
work on Linux too? (It seems quite unusual).

Another question:

- There is another XCODE5-specific exception in OvmfPkg, namely
TftpDynamicCommand. Does that build OK with CLANG9?

> 2. Add -mno-mmx -mno-sse compiler option

I think this remark should be clarified -- the options are *excluded*
from the CLANG9 build.

Thanks
Laszlo

> 
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 4 +++-
>  OvmfPkg/OvmfPkgIa32X64.dsc | 4 +++-
>  OvmfPkg/OvmfPkgX64.dsc     | 4 +++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 66e944436a..0fde8e6e84 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -66,7 +66,7 @@
>    GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
>    INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
>    MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
> -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> +!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
>    GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>  !endif
>  
> @@ -80,12 +80,14 @@
>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>    XCODE:*_*_*_DLINK_FLAGS =
> +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>  
>  # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
>  # protection of DXE_SMM_DRIVER/SMM_CORE modules
>  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
>    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>    XCODE:*_*_*_DLINK_FLAGS =
> +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>  
>  ################################################################################
>  #
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 51c2bfb44f..c17329878e 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -66,7 +66,7 @@
>    GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
>    INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
>    MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
> -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> +!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
>    GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>  !endif
>  !ifdef $(SOURCE_DEBUG_ENABLE)
> @@ -85,12 +85,14 @@
>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>    XCODE:*_*_*_DLINK_FLAGS =
> +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>  
>  # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
>  # protection of DXE_SMM_DRIVER/SMM_CORE modules
>  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
>    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>    XCODE:*_*_*_DLINK_FLAGS =
> +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>  
>  ################################################################################
>  #
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index ba7a758844..af91265d05 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -66,7 +66,7 @@
>    GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
>    INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
>    MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
> -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> +!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
>    GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>  !endif
>  !ifdef $(SOURCE_DEBUG_ENABLE)
> @@ -85,12 +85,14 @@
>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>    XCODE:*_*_*_DLINK_FLAGS =
> +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>  
>  # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
>  # protection of DXE_SMM_DRIVER/SMM_CORE modules
>  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
>    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>    XCODE:*_*_*_DLINK_FLAGS =
> +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>  
>  ################################################################################
>  #
> 

Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

[Laszlo Ersek]

+ Jordan

On 09/27/19 09:46, Liming Gao wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2024
>
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> ---
>  OvmfPkg/Sec/SecMain.inf | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
> index 63ba4cb555..cd765cac25 100644
> --- a/OvmfPkg/Sec/SecMain.inf
> +++ b/OvmfPkg/Sec/SecMain.inf
> @@ -69,3 +69,7 @@
>
>  [FeaturePcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> +
> +[BuildOptions]
> +  # CLANG9 X64 requires it.
> +  GCC:*_CLANG9_X64_CC_FLAGS = -fno-omit-frame-pointer
>

I disagree with this patch for two reasons.

First, the comment "CLANG9 X64 requires it" is quite lacking. It does
nothing to explain the issue; it doesn't even provide a pointer in the
code comment (only in the code message).

Second, Jordan suggested an alternative approach at the end of
<https://bugzilla.tianocore.org/show_bug.cgi?id=2024#c3>, which I prefer
to the one seen above. Can we please try that with CLANG9 too?


... Oh wait I see there are new comments in BZ#2024.

Apparently,

  #pragma GCC optimize ("no-omit-frame-pointer")

does not work with CLANG9.

That's not a problem: we still have two options that are superior to the
present patch, and should be tested.

(a) Does Jordan's series linked below fix the problem with CLANG9?

  [edk2-devel] [PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration
  http://mid.mail-archive.com/20190410084000.19660-1-jordan.l.justen@intel.com
  https://edk2.groups.io/g/devel/message/38785

(b) Jordan's full #pragma suggestion was:

#ifdef __GNUC__
#pragma GCC push_options
#pragma GCC optimize ("no-omit-frame-pointer")
#else
#pragma optimize ("y", off)
#endif

If the '#pragme GCC' branch doesn't help, can we customize the *other*
branch for CLANG9?

For example, in patch#4, we rely on defined(__clang__). Therefore, can
we try the following:

#ifdef __GNUC__
#pragma GCC push_options
#pragma GCC optimize ("no-omit-frame-pointer")
#elif defined(__clang__)
#pragma clang optimize off  <----- NOTE THIS <http://clang.llvm.org/docs/LanguageExtensions.html>
#else
#pragma optimize ("y", off)
#endif

In summary, the fact that CLANG9 breaks is just a symptom; it shows that
the PEI Core issue fixed by Jordan is real. We should go for the real
fix (a).

Alternatively, use a clang-specific optimization override, as tightly as
possible; (b) might work.

-*-

In fact I think this is the perfect time to fix the PEI Core issue: we
now have a feature request that depends on fixing the bug in the PEI
Core. We should fix technical debt in edk2 at least *sometimes*. If we
are not willing to fix technical debt in edk2 for the sake of new
features, we will *never* fix technical debt.

(Well, to be technically correct, we also fix technical debt when it
turns into security issues. Yay!)

Please test this series with the present patch removed, and Jordan's v2
series (linked above) applied.

Thanks
Laszlo

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Andrew Fish]

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



> On Sep 30, 2019, at 3:35 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> Hi Liming,
> 
> On 09/27/19 09:46, Liming Gao wrote:
>> __inline__ attribute will make the functions not be exposed as the
>> library interface. It will cause CLANG9 compiler fail.
>> 
>> Signed-off-by: Liming Gao <liming.gao@intel.com <mailto:liming.gao@intel.com>>
>> ---
>> MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c | 6 ------
>> 1 file changed, 6 deletions(-)
> 
> Did you regression-test this change against GCC48 (for example)?
> 
> I can't tell why we have the __inline__'s in the first place. They date
> back to historical commit e1f414b6a7d8 ("Import some basic libraries
> instances for Mde Packages.", 2007-06-22). And that commit does not
> explain __inline__.
> 
> If we remove __inline__ for the whole GCC toolchain *family*, then I
> think we need a better justification than just "makes CLANG9 fail".
> 

Yikes,

Looks like __inline__ is the C89 version of inline. 

I'm kind of surprised clang with LTO would just not ignore the inline, but then I came across....

https://en.wikipedia.org/wiki/Inline_function
"gnu89 semantics of inline and extern inline are essentially the exact opposite of those in C99[4] <https://en.wikipedia.org/wiki/Inline_function#cite_note-4>, with the exception that gnu89 permits redefinition of an extern inline function as an unqualified function, while C99 inline does not[5] <https://en.wikipedia.org/wiki/Inline_function#cite_note-gcc-5-porting-5>. Thus, gnu89 extern inline without redefinition is like C99 inline, and gnu89 inline is like C99 extern inline; in other words, in gnu89, a function defined inline will always and a function defined extern inline will never emit an externally visible function. The rationale for this is that it matches variables, for which storage will never be reserved if defined as extern and always if defined without. The rationale for C99, in contrast, is that it would be astonishing <https://en.wikipedia.org/wiki/Principle_of_least_astonishment> if using inline would have a side-effect—to always emit a non-inlined version of the function—that is contrary to what its name suggests.
The remarks for C99 about the need to provide exactly one externally visible function instance for inlined functions and about the resulting problem with unreachable code apply mutatis mutandis to gnu89 as well.
gcc up to and including version 4.2 used gnu89 inline semantics even when -std=c99 was explicitly specified.[6] <https://en.wikipedia.org/wiki/Inline_function#cite_note-6> With version 5[5] <https://en.wikipedia.org/wiki/Inline_function#cite_note-gcc-5-porting-5>, gcc switched from gnu89 to the gnu11 dialect, effectively enabling C99 inline semantics by default. To use gnu89 semantics instead, they have to be enabled explicitly, either with -std=gnu89 or, to only affect inlining, -fgnu89-inline, or by adding the gnu_inline attribute to all inline declarations. To ensure C99 semantics, either -std=c99, -std=c11, -std=gnu99 or -std=gnu11 (without -fgnu89-inline) can be used.[3] <https://en.wikipedia.org/wiki/Inline_function#cite_note-gcc-inline-3>"

And the above makes you look at the C99 definition "In C99, a function defined inline will never, and a function defined extern inline will always, emit an externally visible function. ". So this make me wonder if clang is getting more pedantic about the C99 definition of inline (__inline__). So I wonder if we could use an` if ( __STDC_VERSION__ < 199901L)` to turn off the __inline__ to fix the clang issue?

It also seems strange to me the __inline__ only exists next to the library function. Given it is not in the header (and the code is not in the header) I'm not really sure what the compiler can do? When the BaseIoLibIntrinsic library gets compiled it is going to create the intrinsic functions. It seems like code only comes together a link time? So unless the GCC linker was doing inline code generation at link time I'm not sure  how the __inline__ helps. Does the compiler tag the object with some kind of hint? If you are doing Link Time Optimization (LTO) the __inline__ is kind of a moot point as the code gen will always inline simple stuff like this. 

I'd point out when we ported to GCC we came from VC++ and always had LTO, so it is likely we did not have a good grasp of GCC inlining. Thus there is a non-zero chance this code is a no-op even on old GCC versions. But it is worth checking out. 

[1]  $ git grep __inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:35:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:63:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:90:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:120:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:148:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:178:__inline__


Thanks,

Andrew Fish

> Thanks
> Laszlo
> 
>> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
>> index 055f0a947e..b3a1a20256 100644
>> --- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
>> +++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
>> @@ -32,7 +32,6 @@
>>   @return The value read.
>> 
>> **/
>> -__inline__
>> UINT8
>> EFIAPI
>> IoRead8 (
>> @@ -60,7 +59,6 @@ IoRead8 (
>>   @return The value written the I/O port.
>> 
>> **/
>> -__inline__
>> UINT8
>> EFIAPI
>> IoWrite8 (
>> @@ -87,7 +85,6 @@ IoWrite8 (
>>   @return The value read.
>> 
>> **/
>> -__inline__
>> UINT16
>> EFIAPI
>> IoRead16 (
>> @@ -117,7 +114,6 @@ IoRead16 (
>>   @return The value written the I/O port.
>> 
>> **/
>> -__inline__
>> UINT16
>> EFIAPI
>> IoWrite16 (
>> @@ -145,7 +141,6 @@ IoWrite16 (
>>   @return The value read.
>> 
>> **/
>> -__inline__
>> UINT32
>> EFIAPI
>> IoRead32 (
>> @@ -175,7 +170,6 @@ IoRead32 (
>>   @return The value written the I/O port.
>> 
>> **/
>> -__inline__
>> UINT32
>> EFIAPI
>> IoWrite32 (
>> 
> 
> 
> 


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

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Liming Gao]

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



From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrew Fish via Groups.Io
Sent: Tuesday, October 1, 2019 5:12 AM
To: devel@edk2.groups.io; lersek@redhat.com
Cc: Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions




On Sep 30, 2019, at 3:35 PM, Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:

Hi Liming,

On 09/27/19 09:46, Liming Gao wrote:

__inline__ attribute will make the functions not be exposed as the
library interface. It will cause CLANG9 compiler fail.

Signed-off-by: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
---
MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c | 6 ------
1 file changed, 6 deletions(-)

Did you regression-test this change against GCC48 (for example)?

I can't tell why we have the __inline__'s in the first place. They date
back to historical commit e1f414b6a7d8 ("Import some basic libraries
instances for Mde Packages.", 2007-06-22). And that commit does not
explain __inline__.

If we remove __inline__ for the whole GCC toolchain *family*, then I
think we need a better justification than just "makes CLANG9 fail".

[Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and XCODE5.
I don’t know the specific reason about __inline__. If there is no impact on
other GCC tool chain, I prefer to remove them.

Yikes,

Looks like __inline__ is the C89 version of inline.

I'm kind of surprised clang with LTO would just not ignore the inline, but then I came across....

https://en.wikipedia.org/wiki/Inline_function
"gnu89 semantics of inline and extern inline are essentially the exact opposite of those in C99[4]<https://en.wikipedia.org/wiki/Inline_function#cite_note-4>, with the exception that gnu89 permits redefinition of an extern inline function as an unqualified function, while C99 inline does not[5]<https://en.wikipedia.org/wiki/Inline_function#cite_note-gcc-5-porting-5>. Thus, gnu89 extern inline without redefinition is like C99 inline, and gnu89 inline is like C99 extern inline; in other words, in gnu89, a function defined inline will always and a function defined extern inline will never emit an externally visible function. The rationale for this is that it matches variables, for which storage will never be reserved if defined as extern and always if defined without. The rationale for C99, in contrast, is that it would be astonishing<https://en.wikipedia.org/wiki/Principle_of_least_astonishment> if using inline would have a side-effect—to always emit a non-inlined version of the function—that is contrary to what its name suggests.
The remarks for C99 about the need to provide exactly one externally visible function instance for inlined functions and about the resulting problem with unreachable code apply mutatis mutandis to gnu89 as well.
gcc up to and including version 4.2 used gnu89 inline semantics even when -std=c99 was explicitly specified.[6]<https://en.wikipedia.org/wiki/Inline_function#cite_note-6> With version 5[5]<https://en.wikipedia.org/wiki/Inline_function#cite_note-gcc-5-porting-5>, gcc switched from gnu89 to the gnu11 dialect, effectively enabling C99 inline semantics by default. To use gnu89 semantics instead, they have to be enabled explicitly, either with -std=gnu89 or, to only affect inlining, -fgnu89-inline, or by adding the gnu_inline attribute to all inline declarations. To ensure C99 semantics, either -std=c99, -std=c11, -std=gnu99 or -std=gnu11 (without -fgnu89-inline) can be used.[3]<https://en.wikipedia.org/wiki/Inline_function#cite_note-gcc-inline-3>"

And the above makes you look at the C99 definition "In C99, a function defined inline will never, and a function defined extern inline will always, emit an externally visible function. ". So this make me wonder if clang is getting more pedantic about the C99 definition of inline (__inline__). So I wonder if we could use an` if ( __STDC_VERSION__ < 199901L)` to turn off the __inline__ to fix the clang issue?

It also seems strange to me the __inline__ only exists next to the library function. Given it is not in the header (and the code is not in the header) I'm not really sure what the compiler can do? When the BaseIoLibIntrinsic library gets compiled it is going to create the intrinsic functions. It seems like code only comes together a link time? So unless the GCC linker was doing inline code generation at link time I'm not sure  how the __inline__ helps. Does the compiler tag the object with some kind of hint? If you are doing Link Time Optimization (LTO) the __inline__ is kind of a moot point as the code gen will always inline simple stuff like this.

I'd point out when we ported to GCC we came from VC++ and always had LTO, so it is likely we did not have a good grasp of GCC inlining. Thus there is a non-zero chance this code is a no-op even on old GCC versions. But it is worth checking out.

[Liming] This seems the remaining clean up task. So, I prefer to remove __inline__ if no impact on GCC tool chain.

Thanks
Liming
[1]  $ git grep __inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:35:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:63:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:90:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:120:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:148:__inline__
Library/BaseIoLibIntrinsic/IoLibGcc.c:178:__inline__



Thanks,

Andrew Fish


Thanks
Laszlo


diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
index 055f0a947e..b3a1a20256 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
+++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c
@@ -32,7 +32,6 @@
  @return The value read.

**/
-__inline__
UINT8
EFIAPI
IoRead8 (
@@ -60,7 +59,6 @@ IoRead8 (
  @return The value written the I/O port.

**/
-__inline__
UINT8
EFIAPI
IoWrite8 (
@@ -87,7 +85,6 @@ IoWrite8 (
  @return The value read.

**/
-__inline__
UINT16
EFIAPI
IoRead16 (
@@ -117,7 +114,6 @@ IoRead16 (
  @return The value written the I/O port.

**/
-__inline__
UINT16
EFIAPI
IoWrite16 (
@@ -145,7 +141,6 @@ IoWrite16 (
  @return The value read.

**/
-__inline__
UINT32
EFIAPI
IoRead32 (
@@ -175,7 +170,6 @@ IoRead32 (
  @return The value written the I/O port.

**/
-__inline__
UINT32
EFIAPI
IoWrite32 (






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

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Liming Gao]

Laszlo:

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, October 1, 2019 4:42 AM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain
> 
> Hi Liming,
> 
> On 09/27/19 09:46, Liming Gao wrote:
> > 1. Apply CLANG9 Linker option.
> 
> I'm confused by this, in two regards.
> 
> - Why do we refer to CLANG9 first as being in the GCC toolchain family
> (see near mmx/sse), and then under a totally different family name
> (CLANGPE?)

CLANGPE is used to override GCC option. GCC is used to append GCC options. 
You can see XCODE5 take the same way. 
CLANG9 tool chain doesn't recognize -z common-page-size=0x1000 option. 
So, here CLANGPE is used to specify /ALIGN:4096 option. 

> 
> - Regarding the CLANGPE option, does the "/ALIGN:4096" option format
> work on Linux too? (It seems quite unusual).
> 
Yes. CLANG9 tool chain is CLANG compiler + LLVM LLD linker. 
This linker is to generate PE/COFF image. Its linker option is same to VS linker. 
This linker works on Linux and Mac. 

> Another question:
> 
> - There is another XCODE5-specific exception in OvmfPkg, namely
> TftpDynamicCommand. Does that build OK with CLANG9?
> 
Yes. LLVM provides llvm-rc to generate resource section. 

> > 2. Add -mno-mmx -mno-sse compiler option
> 
> I think this remark should be clarified -- the options are *excluded*
> from the CLANG9 build.

Right. I will update comments.
> 
> Thanks
> Laszlo
> 
> >
> > Signed-off-by: Liming Gao <liming.gao@intel.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc    | 4 +++-
> >  OvmfPkg/OvmfPkgIa32X64.dsc | 4 +++-
> >  OvmfPkg/OvmfPkgX64.dsc     | 4 +++-
> >  3 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index 66e944436a..0fde8e6e84 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -66,7 +66,7 @@
> >    GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
> >    INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
> >    MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
> > -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> > +!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
> >    GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
> >  !endif
> >
> > @@ -80,12 +80,14 @@
> >  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> >    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >    XCODE:*_*_*_DLINK_FLAGS =
> > +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
> >
> >  # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
> >  # protection of DXE_SMM_DRIVER/SMM_CORE modules
> >  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
> >    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >    XCODE:*_*_*_DLINK_FLAGS =
> > +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
> >
> >  ################################################################################
> >  #
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index 51c2bfb44f..c17329878e 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -66,7 +66,7 @@
> >    GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
> >    INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
> >    MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
> > -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> > +!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
> >    GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
> >  !endif
> >  !ifdef $(SOURCE_DEBUG_ENABLE)
> > @@ -85,12 +85,14 @@
> >  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> >    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >    XCODE:*_*_*_DLINK_FLAGS =
> > +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
> >
> >  # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
> >  # protection of DXE_SMM_DRIVER/SMM_CORE modules
> >  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
> >    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >    XCODE:*_*_*_DLINK_FLAGS =
> > +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
> >
> >  ################################################################################
> >  #
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index ba7a758844..af91265d05 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -66,7 +66,7 @@
> >    GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
> >    INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
> >    MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
> > -!if $(TOOL_CHAIN_TAG) != "XCODE5"
> > +!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
> >    GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
> >  !endif
> >  !ifdef $(SOURCE_DEBUG_ENABLE)
> > @@ -85,12 +85,14 @@
> >  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> >    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >    XCODE:*_*_*_DLINK_FLAGS =
> > +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
> >
> >  # Force PE/COFF sections to be aligned at 4KB boundaries to support page level
> >  # protection of DXE_SMM_DRIVER/SMM_CORE modules
> >  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
> >    GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >    XCODE:*_*_*_DLINK_FLAGS =
> > +  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
> >
> >  ################################################################################
> >  #
> >

Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

[Liming Gao]

Laszlo:
  I will verify your option (a) and (b). The problem is described in https://bugzilla.tianocore.org/show_bug.cgi?id=2024. 
  I know there are some discussion in Jordan patch for PeiCore change. Before the conclusion for Jordan change is made, 
  I expect to find the alternate option to resolve CLANG9 tool chain issue first. 

Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, October 1, 2019 5:10 AM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64
> 
> + Jordan
> 
> On 09/27/19 09:46, Liming Gao wrote:
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2024
> >
> > Signed-off-by: Liming Gao <liming.gao@intel.com>
> > ---
> >  OvmfPkg/Sec/SecMain.inf | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
> > index 63ba4cb555..cd765cac25 100644
> > --- a/OvmfPkg/Sec/SecMain.inf
> > +++ b/OvmfPkg/Sec/SecMain.inf
> > @@ -69,3 +69,7 @@
> >
> >  [FeaturePcd]
> >    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> > +
> > +[BuildOptions]
> > +  # CLANG9 X64 requires it.
> > +  GCC:*_CLANG9_X64_CC_FLAGS = -fno-omit-frame-pointer
> >
> 
> I disagree with this patch for two reasons.
> 
> First, the comment "CLANG9 X64 requires it" is quite lacking. It does
> nothing to explain the issue; it doesn't even provide a pointer in the
> code comment (only in the code message).
> 
> Second, Jordan suggested an alternative approach at the end of
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2024#c3>, which I prefer
> to the one seen above. Can we please try that with CLANG9 too?
> 
> 
> ... Oh wait I see there are new comments in BZ#2024.
> 
> Apparently,
> 
>   #pragma GCC optimize ("no-omit-frame-pointer")
> 
> does not work with CLANG9.
> 
> That's not a problem: we still have two options that are superior to the
> present patch, and should be tested.
> 
> (a) Does Jordan's series linked below fix the problem with CLANG9?
> 
>   [edk2-devel] [PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration
>   http://mid.mail-archive.com/20190410084000.19660-1-jordan.l.justen@intel.com
>   https://edk2.groups.io/g/devel/message/38785
> 
> (b) Jordan's full #pragma suggestion was:
> 
> #ifdef __GNUC__
> #pragma GCC push_options
> #pragma GCC optimize ("no-omit-frame-pointer")
> #else
> #pragma optimize ("y", off)
> #endif
> 
> If the '#pragme GCC' branch doesn't help, can we customize the *other*
> branch for CLANG9?
> 
> For example, in patch#4, we rely on defined(__clang__). Therefore, can
> we try the following:
> 
> #ifdef __GNUC__
> #pragma GCC push_options
> #pragma GCC optimize ("no-omit-frame-pointer")
> #elif defined(__clang__)
> #pragma clang optimize off  <----- NOTE THIS <http://clang.llvm.org/docs/LanguageExtensions.html>
> #else
> #pragma optimize ("y", off)
> #endif
> 
> In summary, the fact that CLANG9 breaks is just a symptom; it shows that
> the PEI Core issue fixed by Jordan is real. We should go for the real
> fix (a).
> 
> Alternatively, use a clang-specific optimization override, as tightly as
> possible; (b) might work.
> 
> -*-
> 
> In fact I think this is the perfect time to fix the PEI Core issue: we
> now have a feature request that depends on fixing the bug in the PEI
> Core. We should fix technical debt in edk2 at least *sometimes*. If we
> are not willing to fix technical debt in edk2 for the sake of new
> features, we will *never* fix technical debt.
> 
> (Well, to be technically correct, we also fix technical debt when it
> turns into security issues. Yay!)
> 
> Please test this series with the present patch removed, and Jordan's v2
> series (linked above) applied.
> 
> Thanks
> Laszlo

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Laszlo Ersek]

On 10/08/19 16:47, Gao, Liming wrote:

>     [Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and
>     XCODE5.
> 
>     I don’t know the specific reason about __inline__. If there is no
>     impact on
> 
>     other GCC tool chain, I prefer to remove them.

> [Liming] This seems the remaining clean up task. So, I prefer to remove
> __inline__ if no impact on GCC tool chain.

OK. Given your testing with GCC48, I'm fine.

Thanks
Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Laszlo Ersek]

On 10/08/19 17:02, Gao, Liming wrote:
> Laszlo:
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, October 1, 2019 4:42 AM
>> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain
>>
>> Hi Liming,
>>
>> On 09/27/19 09:46, Liming Gao wrote:
>>> 1. Apply CLANG9 Linker option.
>>
>> I'm confused by this, in two regards.
>>
>> - Why do we refer to CLANG9 first as being in the GCC toolchain family
>> (see near mmx/sse), and then under a totally different family name
>> (CLANGPE?)
> 
> CLANGPE is used to override GCC option. GCC is used to append GCC options. 
> You can see XCODE5 take the same way. 
> CLANG9 tool chain doesn't recognize -z common-page-size=0x1000 option. 
> So, here CLANGPE is used to specify /ALIGN:4096 option. 

Wait, so the "GCC" toolchain *family* applies to:
- actual GCC toolchains (such as GCC48, GCC49, GCC5)
- XCODE toolchains (such as XCODE5)
- CLANG toolchains (such as CLANG9)

but the "XCODE" toolchain *family* only applies to XCODE toolchains
(such as XCODE5), and similarly, the CLANGPE toolchain *family* only
applies to CLANG toolchains (such as CLANG9)?

Put differently, is XCODE in two toolchain families at the same time
(GCC and XCODE)?

Similarly, is CLANG9 in two toolchain families at the same time (GCC and
CLANGPE)?

Wait... from "BaseTools/Conf/tools_def.template":

> #
> #
> # XCODE5 support
> #
>
> *_XCODE5_*_*_FAMILY            = GCC
> *_XCODE5_*_*_BUILDRULEFAMILY   = XCODE

This makes me very unhappy. I don't know how anyone can follow this.

What is the difference between "FAMILY" and "BUILDRULEFAMILY"?

When I see

   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
   XCODE:*_*_*_DLINK_FLAGS =

how can I know that the first line applies due to "FAMILY", but the
second line also applies due to "BUILDRULEFAMILY" (and overrides the
first line)?

Hmmm....

https://edk2-docs.gitbooks.io/edk-ii-build-specification/5_meta-data_file_specifications/52_tools_def_txt.html

"""
FAMILY -- A flag to the build command that will be used to ensure the
          correct commands and flags are used in the generated Makefile
          or GNUMakefile, as well as to use the correct options for
          independent tools, such as the ACPI compiler. This is
          typically used to identify the type of Makefile that needs to
          be generated.

BUILDRULEFAMILY -- This flag is used by some tool chain tags to set a
                   special FAMILY value when processing the
                   build_rule.txt file. Normally, the FAMILY attribute
                   is used to identify the type of makefile the tools
                   need to generate. Tools such as XCODE will use GCC as
                   the FAMILY, but uses different (from GCC) processing
                   rules. If present and if a build rule (in
                   build_rules.txt) contains an attribute with the value
                   specified in this entry, that rule will be processed
                   and the rule with the FAMILY attribute will be
                   ignored.
"""

Well, I can't say that it's entirely clear to me what applies when. :/

In particular because in this patch, we don't use BUILDRULEFAMILY for
overrides in "build_rules.txt", we use BUILDRULEFAMILY for overrides in
DSC files.


Anyway, let's say that we use the following syntax for linker flag
overrides:

[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
  XCODE:*_*_*_DLINK_FLAGS =
  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096

Fine.

But then, why don't we similarly use:

[BuildOptions]
  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
  XCODE:*_*_*_CC_FLAGS                 =
  CLANGPE:*_*_*_CC_FLAGS               =

for CC flag overrides?

Because, the proposal is:

!if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
!endif

I mean, in one case (for the linker flags), we rely on BUILDRULEFAMILY
for specifying the override.

But in the other case (for the C compilation flags), we do not rely on
BUILDRULEFAMILY; instead we check $(TOOL_CHAIN_TAG).

Is this difference justified? Why?


The rest of your answers sounds good to me. Thanks!
Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Andrew Fish]

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



> On Oct 8, 2019, at 3:29 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 10/08/19 17:02, Gao, Liming wrote:
>> Laszlo:
>> 
>>> -----Original Message-----
>>> From: Laszlo Ersek <lersek@redhat.com>
>>> Sent: Tuesday, October 1, 2019 4:42 AM
>>> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain
>>> 
>>> Hi Liming,
>>> 
>>> On 09/27/19 09:46, Liming Gao wrote:
>>>> 1. Apply CLANG9 Linker option.
>>> 
>>> I'm confused by this, in two regards.
>>> 
>>> - Why do we refer to CLANG9 first as being in the GCC toolchain family
>>> (see near mmx/sse), and then under a totally different family name
>>> (CLANGPE?)
>> 
>> CLANGPE is used to override GCC option. GCC is used to append GCC options. 
>> You can see XCODE5 take the same way. 
>> CLANG9 tool chain doesn't recognize -z common-page-size=0x1000 option. 
>> So, here CLANGPE is used to specify /ALIGN:4096 option. 
> 
> Wait, so the "GCC" toolchain *family* applies to:
> - actual GCC toolchains (such as GCC48, GCC49, GCC5)
> - XCODE toolchains (such as XCODE5)
> - CLANG toolchains (such as CLANG9)
> 
> but the "XCODE" toolchain *family* only applies to XCODE toolchains
> (such as XCODE5), and similarly, the CLANGPE toolchain *family* only
> applies to CLANG toolchains (such as CLANG9)?
> 
> Put differently, is XCODE in two toolchain families at the same time
> (GCC and XCODE)?
> 
> Similarly, is CLANG9 in two toolchain families at the same time (GCC and
> CLANGPE)?
> 
> Wait... from "BaseTools/Conf/tools_def.template":

> 
>> #
>> #
>> # XCODE5 support
>> #
>> 
>> *_XCODE5_*_*_FAMILY            = GCC
>> *_XCODE5_*_*_BUILDRULEFAMILY   = XCODE
> 
> This makes me very unhappy. I don't know how anyone can follow this.
> 
> What is the difference between "FAMILY" and "BUILDRULEFAMILY"?
> 

> When I see
> 
>   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>   XCODE:*_*_*_DLINK_FLAGS =
> 
> how can I know that the first line applies due to "FAMILY", but the
> second line also applies due to "BUILDRULEFAMILY" (and overrides the
> first line)?
> 
> Hmmm....
> 
> https://edk2-docs.gitbooks.io/edk-ii-build-specification/5_meta-data_file_specifications/52_tools_def_txt.html <https://edk2-docs.gitbooks.io/edk-ii-build-specification/5_meta-data_file_specifications/52_tools_def_txt.html>
> 
> """
> FAMILY -- A flag to the build command that will be used to ensure the
>          correct commands and flags are used in the generated Makefile
>          or GNUMakefile, as well as to use the correct options for
>          independent tools, such as the ACPI compiler. This is
>          typically used to identify the type of Makefile that needs to
>          be generated.
> 
> BUILDRULEFAMILY -- This flag is used by some tool chain tags to set a
>                   special FAMILY value when processing the
>                   build_rule.txt file. Normally, the FAMILY attribute
>                   is used to identify the type of makefile the tools
>                   need to generate. Tools such as XCODE will use GCC as
>                   the FAMILY, but uses different (from GCC) processing
>                   rules. If present and if a build rule (in
>                   build_rules.txt) contains an attribute with the value
>                   specified in this entry, that rule will be processed
>                   and the rule with the FAMILY attribute will be
>                   ignored.
> """
> 
> Well, I can't say that it's entirely clear to me what applies when. :/
> 


Laszlo,

If you just have  *_*_*_*_FAMILY then that matches the statements in the INF and DSC files (It also matters for build_rule.txt) like this example for GCC [1]. 

When you have *_*_*_*_FAMILY and *_*_*_*_BUILDRULEFAMILY you get 2 levels. Thus XCODE is generally compatible with GCC so it is in that *_*_*_*_FAMILY, but if you need to do something XCODE specific you can use the XCODE *_*_*_*_BUILDRULEFAMILY. If we did not have 2 levels every place we had GCC would need XCODE and see the list [1]. Thus XCODE can use GCC when it is compatible with GCC (which is a majority of the time) and then use XCODE to have XCODE rules that are different than GCC. 

So I guess the way to describe it is XCODE inherits GCC and only needs to override when it is different.

As per usual we seem to have picked confusing names. 


> In particular because in this patch, we don't use BUILDRULEFAMILY for
> overrides in "build_rules.txt", we use BUILDRULEFAMILY for overrides in
> DSC files.
> 
> 
> Anyway, let's say that we use the following syntax for linker flag
> overrides:
> 
> [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>  XCODE:*_*_*_DLINK_FLAGS =
>  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
> 
> Fine.
> 
> But then, why don't we similarly use:
> 
> [BuildOptions]
>  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>  XCODE:*_*_*_CC_FLAGS                 =
>  CLANGPE:*_*_*_CC_FLAGS               =
> 
> for CC flag overrides?
> 
> Because, the proposal is:
> 
> !if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANG9"
>  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
> !endif
> 

Is this really the same as:
  XCODE: *_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
  GCC:*_CLANG8_*_CC_FLAGS            = -mno-mmx -mno-sse

Also for CLANG38 the above flags are part of CLANG38_ALL_CC_FLAGS. I they are needed then why are they not part of the generic tool chain definition? 

[1] git grep GCC -- *.inf *.dsc
ArmPkg/ArmPkg.dsc:93:  # Add support for GCC stack protector
ArmPkg/Drivers/ArmGic/ArmGicLib.inf:24:  GicV3/Arm/ArmGicV3.S     | GCC
ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf:37:  Arm/ExceptionSupport.S   | GCC
ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf:32:  Arm/ExceptionSupport.S   | GCC
ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf:19:  Arm/ArmHvc.S      | GCC
ArmPkg/Library/ArmLib/ArmBaseLib.inf:28:  Arm/ArmLibSupport.S           | GCC
ArmPkg/Library/ArmLib/ArmBaseLib.inf:29:  Arm/ArmLibSupportV7.S         | GCC
ArmPkg/Library/ArmLib/ArmBaseLib.inf:30:  Arm/ArmV7Support.S            | GCC
ArmPkg/Library/ArmLib/ArmBaseLib.inf:31:  Arm/ArmV7ArchTimerSupport.S   | GCC
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf:25:  Arm/ArmMmuLibV7Support.S   |GCC
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf:18:  Arm/ArmSmc.S      | GCC
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf:19:  AArch64/Reset.S   | GCC
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf:23:  Arm/Reset.S       | GCC
ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf:57:  berkeley-softfloat-3/source/include/opts-GCC.h
ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf:92:  GCC:*_*_*_CC_FLAGS = -fno-lto -ffreestanding -Wno-unused-label
ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf:18:  Arm/ArmSvc.S      | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:24:  memcpy.c             | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:25:  memset.c             | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:45:  Arm/ashrdi3.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:46:  Arm/ashldi3.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:47:  Arm/div.S            | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:48:  Arm/divdi3.S         | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:49:  Arm/divsi3.S         | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:50:  Arm/lshrdi3.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:51:  Arm/memmove.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:52:  Arm/modsi3.S         | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:53:  Arm/moddi3.S         | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:54:  Arm/muldi3.S         | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:55:  Arm/mullu.S          | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:56:  Arm/udivsi3.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:57:  Arm/umodsi3.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:58:  Arm/udivdi3.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:59:  Arm/umoddi3.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:60:  Arm/udivmoddi4.S     | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:61:  Arm/clzsi2.S         | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:62:  Arm/ctzsi2.S         | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:63:  Arm/ucmpdi2.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:64:  Arm/switch8.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:65:  Arm/switchu8.S       | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:66:  Arm/switch16.S       | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:67:  Arm/switch32.S       | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:68:  Arm/sourcery.S       | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:69:  Arm/uldiv.S          | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:70:  Arm/ldivmod.S        | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:71:  Arm/lasr.S           | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:72:  Arm/llsr.S           | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:73:  Arm/llsl.S           | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:74:  Arm/uread.S          | GCC
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf:75:  Arm/uwrite.S         | GCC
ArmPkg/Library/SemihostLib/SemihostLib.inf:30:  Arm/GccSemihost.S | GCC
ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf:31:  Arm/ArmPlatformHelper.S    | GCC
ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf:25:  Arm/ArmPlatformStackLib.S       | GCC
ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf:25:  Arm/PrePeiCoreEntryPoint.S   | GCC
ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf:27:  Arm/SwitchStack.S        | GCC
ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf:29:  Arm/Exception.S          | GCC
ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf:25:  Arm/PrePeiCoreEntryPoint.S   | GCC
ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf:27:  Arm/SwitchStack.S        | GCC
ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf:29:  Arm/Exception.S          | GCC
ArmPlatformPkg/PrePi/PeiMPCore.inf:24:  Arm/ModuleEntryPoint.S   | GCC
ArmPlatformPkg/PrePi/PeiUniCore.inf:24:  Arm/ModuleEntryPoint.S   | GCC
ArmVirtPkg/ArmVirtQemuKernel.dsc:90:  GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h
ArmVirtPkg/ArmVirtXen.dsc:61:  GCC:*_CLANG38_*_CC_FLAGS = -include $(WORKSPACE)/ArmVirtPkg/Include/Platform/Hidden.h
ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf:100:  GCC:*_*_*_DLINK_FLAGS = -Wl,-Bsymbolic,-pie,-T,$(MODULE_DIR)/Scripts/PrePi-PIE.lds
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf:102:  GCC:*_CLANG35_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf:103:  GCC:*_CLANG38_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf:97:  GCC:*_CLANG35_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf:98:  GCC:*_CLANG38_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf:108:  GCC:*_CLANG35_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf:109:  GCC:*_CLANG38_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf:105:  GCC:*_CLANG35_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf:106:  GCC:*_CLANG38_*_CC_FLAGS = -std=c99
CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf:38:  Ia32/MathLShiftS64.nasm   | GCC
CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf:39:  Ia32/MathRShiftU64.nasm   | GCC
CryptoPkg/Library/OpensslLib/OpensslLib.inf:659:  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
CryptoPkg/Library/OpensslLib/OpensslLib.inf:660:  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=format -Wno-format -Wno-error=unused-but-set-variable -DNO_MSABI_VA_FUNCS
CryptoPkg/Library/OpensslLib/OpensslLib.inf:661:  GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
CryptoPkg/Library/OpensslLib/OpensslLib.inf:662:  GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-format -Wno-error=unused-but-set-variable
CryptoPkg/Library/OpensslLib/OpensslLib.inf:663:  GCC:*_CLANG35_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
CryptoPkg/Library/OpensslLib/OpensslLib.inf:664:  GCC:*_CLANG38_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
CryptoPkg/Library/OpensslLib/OpensslLib.inf:698:  GCC:*_*_AARCH64_CC_XIPFLAGS ==
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:607:  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:608:  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=format -Wno-format -Wno-error=unused-but-set-variable -DNO_MSABI_VA_FUNCS
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:609:  GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:610:  GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-format -Wno-error=unused-but-set-variable
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:611:  GCC:*_CLANG35_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:612:  GCC:*_CLANG38_*_CC_FLAGS = -std=c99 -Wno-error=uninitialized
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:646:  GCC:*_*_AARCH64_CC_XIPFLAGS ==
EmbeddedPkg/EmbeddedPkg.dsc:128:  # Add support for GCC stack protector
EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf:35:# Current usage of this library expects GCC in a UNIX-like shell environment with the date command
EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf:37:  GCC:*_*_*_CC_FLAGS = -DBUILD_EPOCH=`date +%s`
EmulatorPkg/Unix/Host/Host.inf:115:   GCC:*_*_IA32_DLINK_FLAGS == -o $(BIN_DIR)/Host -m32 -L/usr/X11R6/lib
EmulatorPkg/Unix/Host/Host.inf:116:   GCC:*_*_IA32_CC_FLAGS == -m32 -g -fshort-wchar -fno-strict-aliasing -Wall -malign-double -idirafter/usr/include -c -include $(DEST_DIR_DEBUG)/AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
EmulatorPkg/Unix/Host/Host.inf:117:   GCC:*_*_IA32_PP_FLAGS == -m32 -E -x assembler-with-cpp -include $(DEST_DIR_DEBUG)/AutoGen.h
EmulatorPkg/Unix/Host/Host.inf:118:   GCC:*_*_IA32_ASM_FLAGS == -m32 -c -x assembler -imacros $(DEST_DIR_DEBUG)/AutoGen.h
EmulatorPkg/Unix/Host/Host.inf:120:   GCC:*_*_X64_DLINK_FLAGS == -o $(BIN_DIR)/Host -m64 -L/usr/X11R6/lib
EmulatorPkg/Unix/Host/Host.inf:121:   GCC:*_GCC5_X64_DLINK_FLAGS == -flto -o $(BIN_DIR)/Host -m64 -L/usr/X11R6/lib
EmulatorPkg/Unix/Host/Host.inf:122:   GCC:*_*_X64_CC_FLAGS == -m64 -g -fshort-wchar -fno-strict-aliasing -Wall -malign-double -idirafter/usr/include -c -include $(DEST_DIR_DEBUG)/AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
EmulatorPkg/Unix/Host/Host.inf:123:   GCC:*_GCC48_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))"
EmulatorPkg/Unix/Host/Host.inf:124:   GCC:*_GCC49_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))"
EmulatorPkg/Unix/Host/Host.inf:125:   GCC:*_GCC5_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))" -flto -DUSING_LTO -Os
EmulatorPkg/Unix/Host/Host.inf:126:   GCC:*_*_X64_PP_FLAGS == -m64 -E -x assembler-with-cpp -include $(DEST_DIR_DEBUG)/AutoGen.h
EmulatorPkg/Unix/Host/Host.inf:127:   GCC:*_*_X64_ASM_FLAGS == -m64 -c -x assembler -imacros $(DEST_DIR_DEBUG)/AutoGen.h
EmulatorPkg/Unix/Host/Host.inf:129:   GCC:*_*_*_DLINK2_FLAGS == -lpthread -ldl -lXext -lX11
FatPkg/FatPkg.dsc:23:  GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf:39:  RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf:34:  RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf:33:  RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf:108:  GCC:*_*_*_CC_FLAGS = -Wno-error=maybe-uninitialized
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf:37:  Ia32/CpuSleepGcc.c | GCC
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf:38:  Ia32/CpuFlushTlbGcc.c | GCC
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf:53:  Arm/CpuFlushTlb.S   | GCC
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf:54:  Arm/CpuSleep.S      | GCC
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf:57:  AArch64/CpuFlushTlb.S   | GCC
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf:58:  AArch64/CpuSleep.S      | GCC
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf:37:  IoLibGcc.c    | GCC
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf:43:  IoLibGcc.c    | GCC
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf:33:  Arm/ArmVirtMmio.S       | GCC
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf:38:  AArch64/ArmVirtMmio.S   | GCC
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf:35:  IoLibGcc.c    | GCC
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf:41:  IoLibGcc.c    | GCC
MdePkg/Library/BaseLib/BaseLib.inf:157:  Ia32/GccInline.c | GCC
MdePkg/Library/BaseLib/BaseLib.inf:159:  Ia32/EnableDisableInterrupts.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:161:  Ia32/DisablePaging32.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:162:  Ia32/EnablePaging32.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:163:  Ia32/Mwait.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:164:  Ia32/Monitor.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:165:  Ia32/CpuIdEx.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:166:  Ia32/CpuId.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:169:  Ia32/SwapBytes64.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:171:  Ia32/DivU64x32Remainder.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:172:  Ia32/ModU64x32.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:173:  Ia32/DivU64x32.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:174:  Ia32/MultU64x64.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:175:  Ia32/MultU64x32.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:176:  Ia32/RRotU64.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:177:  Ia32/LRotU64.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:178:  Ia32/ARShiftU64.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:179:  Ia32/RShiftU64.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:180:  Ia32/LShiftU64.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:181:  Ia32/EnableCache.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:182:  Ia32/DisableCache.nasm| GCC
MdePkg/Library/BaseLib/BaseLib.inf:187:  Ia32/InternalSwitchStack.nasm | GCC
MdePkg/Library/BaseLib/BaseLib.inf:311:  X64/GccInline.c | GCC
MdePkg/Library/BaseLib/BaseLib.inf:315:  ChkStkGcc.c  | GCC
MdePkg/Library/BaseLib/BaseLib.inf:351:  Arm/Math64.S                  | GCC
MdePkg/Library/BaseLib/BaseLib.inf:352:  Arm/SwitchStack.S             | GCC
MdePkg/Library/BaseLib/BaseLib.inf:353:  Arm/EnableInterrupts.S        | GCC
MdePkg/Library/BaseLib/BaseLib.inf:354:  Arm/DisableInterrupts.S       | GCC
MdePkg/Library/BaseLib/BaseLib.inf:355:  Arm/GetInterruptsState.S      | GCC
MdePkg/Library/BaseLib/BaseLib.inf:356:  Arm/SetJumpLongJump.S         | GCC
MdePkg/Library/BaseLib/BaseLib.inf:357:  Arm/CpuBreakpoint.S           | GCC
MdePkg/Library/BaseLib/BaseLib.inf:358:  Arm/MemoryFence.S             | GCC
MdePkg/Library/BaseLib/BaseLib.inf:359:  Arm/SpeculationBarrier.S      | GCC
MdePkg/Library/BaseLib/BaseLib.inf:366:  AArch64/MemoryFence.S             | GCC
MdePkg/Library/BaseLib/BaseLib.inf:367:  AArch64/SwitchStack.S             | GCC
MdePkg/Library/BaseLib/BaseLib.inf:368:  AArch64/EnableInterrupts.S        | GCC
MdePkg/Library/BaseLib/BaseLib.inf:369:  AArch64/DisableInterrupts.S       | GCC
MdePkg/Library/BaseLib/BaseLib.inf:370:  AArch64/GetInterruptsState.S      | GCC
MdePkg/Library/BaseLib/BaseLib.inf:371:  AArch64/SetJumpLongJump.S         | GCC
MdePkg/Library/BaseLib/BaseLib.inf:372:  AArch64/CpuBreakpoint.S           | GCC
MdePkg/Library/BaseLib/BaseLib.inf:373:  AArch64/SpeculationBarrier.S      | GCC
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:82:  Arm/ScanMem.S       |GCC
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:83:  Arm/SetMem.S        |GCC
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:84:  Arm/CopyMem.S       |GCC
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:85:  Arm/CompareMem.S    |GCC
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:86:  Arm/CompareGuid.S   |GCC
MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf:28:  BaseStackCheckGcc.c  | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:43:  Ia32/InternalGetSpinLockProperties.c | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:44:  Ia32/GccInline.c | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:45:  SynchronizationGcc.c  | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:63:  Ia32/InternalGetSpinLockProperties.c | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:64:  X64/GccInline.c | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:65:  SynchronizationGcc.c  | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:74:  Arm/Synchronization.S         | GCC
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf:78:  AArch64/Synchronization.S     | GCC
OvmfPkg/OvmfPkgIa32.dsc:66:  GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
OvmfPkg/OvmfPkgIa32.dsc:70:  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
OvmfPkg/OvmfPkgIa32.dsc:78:  GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
OvmfPkg/OvmfPkgIa32.dsc:81:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
OvmfPkg/OvmfPkgIa32.dsc:87:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
OvmfPkg/OvmfPkgIa32X64.dsc:66:  GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
OvmfPkg/OvmfPkgIa32X64.dsc:70:  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
OvmfPkg/OvmfPkgIa32X64.dsc:74:  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
OvmfPkg/OvmfPkgIa32X64.dsc:83:  GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
OvmfPkg/OvmfPkgIa32X64.dsc:86:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
OvmfPkg/OvmfPkgIa32X64.dsc:92:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
OvmfPkg/OvmfPkgX64.dsc:66:  GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
OvmfPkg/OvmfPkgX64.dsc:70:  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
OvmfPkg/OvmfPkgX64.dsc:74:  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:83:  GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
OvmfPkg/OvmfPkgX64.dsc:86:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
OvmfPkg/OvmfPkgX64.dsc:92:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
OvmfPkg/OvmfXen.dsc:63:  GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
OvmfPkg/OvmfXen.dsc:67:  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
OvmfPkg/OvmfXen.dsc:71:  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
OvmfPkg/OvmfXen.dsc:80:  GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
OvmfPkg/OvmfXen.dsc:83:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
OvmfPkg/OvmfXen.dsc:89:  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
SecurityPkg/SecurityPkg.dsc:78:  # Add support for GCC stack protector
ShellPkg/ShellPkg.dsc:69:  # Add support for GCC stack protector
SignedCapsulePkg/SignedCapsulePkg.dsc:107:  # Add support for GCC stack protector
StandaloneMmPkg/StandaloneMmPkg.dsc:116:GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -march=armv8-a+nofp -mstrict-align
StandaloneMmPkg/StandaloneMmPkg.dsc:117:GCC:*_*_*_CC_FLAGS = -mstrict-align
UefiPayloadPkg/UefiPayloadPkgIa32.dsc:86:  GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
UefiPayloadPkg/UefiPayloadPkgIa32.dsc:87:  GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc:86:  GCC:*_UNIXGCC_*_CC_FLAGS       = -DMDEPKG_NDEBUG
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc:87:  GCC:RELEASE_*_*_CC_FLAGS       = -DMDEPKG_NDEBUG

[2] git grep XCODE  -- *.inf *.dsc
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf:105:  XCODE:*_*_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf:100:  XCODE:*_*_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf:111:  XCODE:*_*_*_CC_FLAGS = -std=c99
CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf:103:  XCODE:*_*_*_CC_FLAGS = -mmmx -msse -std=c99
CryptoPkg/Library/OpensslLib/OpensslLib.inf:686:  XCODE:*_*_IA32_CC_FLAGS   = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -w -std=c99 -Wno-error=uninitialized
CryptoPkg/Library/OpensslLib/OpensslLib.inf:687:  XCODE:*_*_X64_CC_FLAGS    = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -w -std=c99 -Wno-error=uninitialized
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:634:  XCODE:*_*_IA32_CC_FLAGS   = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -w -std=c99 -Wno-error=uninitialized
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf:635:  XCODE:*_*_X64_CC_FLAGS    = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -w -std=c99 -Wno-error=uninitialized
EmulatorPkg/EmulatorPkg.dsc:335:!if "XCODE5" not in $(TOOL_CHAIN_TAG)
EmulatorPkg/EmulatorPkg.dsc:380:!if "XCODE5" not in $(TOOL_CHAIN_TAG)
EmulatorPkg/Library/ThunkPpiList/ThunkPpiList.inf:32:   XCODE:*_*_*_DLINK_PATH == gcc
EmulatorPkg/Unix/Host/Host.inf:134:   XCODE:*_*_IA32_DLINK_PATH == gcc
EmulatorPkg/Unix/Host/Host.inf:135:   XCODE:*_*_IA32_CC_FLAGS = -I$(WORKSPACE)/EmulatorPkg/Unix/Host/X11IncludeHack
EmulatorPkg/Unix/Host/Host.inf:136:   XCODE:*_*_IA32_DLINK_FLAGS == -arch i386 -o $(BIN_DIR)/Host -L/usr/X11R6/lib -lXext -lX11 -framework Carbon
EmulatorPkg/Unix/Host/Host.inf:137:   XCODE:*_*_IA32_ASM_FLAGS == -arch i386 -g
EmulatorPkg/Unix/Host/Host.inf:139:   XCODE:*_*_X64_DLINK_PATH == gcc
EmulatorPkg/Unix/Host/Host.inf:140:   XCODE:*_*_X64_DLINK_FLAGS == -o $(BIN_DIR)/Host -L/usr/X11R6/lib -lXext -lX11 -framework Carbon -Wl,-no_pie
EmulatorPkg/Unix/Host/Host.inf:141:   XCODE:*_*_X64_ASM_FLAGS == -g
EmulatorPkg/Unix/Host/Host.inf:142:   XCODE:*_*_X64_CC_FLAGS = -O0 -target x86_64-apple-darwin -I$(WORKSPACE)/EmulatorPkg/Unix/Host/X11IncludeHack "-DEFIAPI=__attribute__((ms_abi))"
MdeModulePkg/MdeModulePkg.dsc:442:!if $(TOOL_CHAIN_TAG) != "XCODE5"
MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf:110:  # Not add -Wno-error=maybe-uninitialized option for XCODE
MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf:111:  # XCODE doesn't know this option
MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf:112:  XCODE:*_*_*_CC_FLAGS =
OvmfPkg/OvmfPkgIa32.dsc:69:!if $(TOOL_CHAIN_TAG) != "XCODE5"
OvmfPkg/OvmfPkgIa32.dsc:82:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfPkgIa32.dsc:88:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfPkgIa32.dsc:807:!if $(TOOL_CHAIN_TAG) != "XCODE5"
OvmfPkg/OvmfPkgIa32X64.dsc:69:!if $(TOOL_CHAIN_TAG) != "XCODE5"
OvmfPkg/OvmfPkgIa32X64.dsc:87:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfPkgIa32X64.dsc:93:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfPkgIa32X64.dsc:820:!if $(TOOL_CHAIN_TAG) != "XCODE5"
OvmfPkg/OvmfPkgX64.dsc:69:!if $(TOOL_CHAIN_TAG) != "XCODE5"
OvmfPkg/OvmfPkgX64.dsc:87:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfPkgX64.dsc:93:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfPkgX64.dsc:818:!if $(TOOL_CHAIN_TAG) != "XCODE5"
OvmfPkg/OvmfXen.dsc:66:!if $(TOOL_CHAIN_TAG) != "XCODE5"
OvmfPkg/OvmfXen.dsc:84:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfXen.dsc:90:  XCODE:*_*_*_DLINK_FLAGS =
OvmfPkg/OvmfXen.dsc:689:!if $(TOOL_CHAIN_TAG) != "XCODE5"
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf:63:  XCODE:*_*_*_CC_FLAGS = -mmmx -msse

Thanks,

Andrew Fish

> I mean, in one case (for the linker flags), we rely on BUILDRULEFAMILY
> for specifying the override.
> 
> But in the other case (for the C compilation flags), we do not rely on
> BUILDRULEFAMILY; instead we check $(TOOL_CHAIN_TAG).
> 
> Is this difference justified? Why?
> 
> 
> The rest of your answers sounds good to me. Thanks!
> Laszlo
> 
> 


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

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Laszlo Ersek]

On 10/09/19 01:08, Andrew Fish wrote:

> So I guess the way to describe it is XCODE inherits GCC and only needs to override when it is different.

Thank you for the explanation!

I've been trying to figure out why this inheritance bothers me so much.
I guess the reason is the following:

I'm a user of the actual GCCxx toolchains (such as GCC48, GCC49, GCC5).
Assume that I realize that a gcc-specific change is needed to some part
of the code. (It could be build options, or gcc specific source code,
etc.) Then, I go to the gcc documentation, and verify whether the change
is indeed supported by all the relevant gcc versions.

If the change is possible with only some gcc versions, then I likely
write the change for a specific toolchain only, such as GCC5. Fine.

Now assume that my documentation review proves that *all* GCCxx
toolchains, supported by edk2, should receive the update. This could be
a #pragma, a command line flag, some __attribute__, etc. I'd very likely
express the change for the whole GCC toolchain *family*, and not
enumerate it for GCC48, GCC49, GCC5 individually.

And now I learn that said approach would be wrong, because such a change
would be inherited by XCODExx and CLANGxx too (via FAMILY), fully
against my intent, unless XCODExx and CLANGxx overrode the particular
aspect (via BUILDRULEFAMILY).

This means that, after checking the gcc documentation meticulously
(multiple releases), and possibly even testing multiple gcc
installations, I could still regress edk2 (CLANGxx and/or XCODExx
specific parts), because they could inherit my GCC-oriented changes that
I never meant for CLANGxx and/or XCODExx.

I don't use XCODE or CLANG, I don't test on them, and now it seems I can
restrict changes to the *actual* gcc compilers only if I keep spelling
out the individual toolchain names, such as GCC48, GCC49, GCC5.

This makes the toolchain family concept totally useless to me (and to
other users that only care about actual gcc compilers). There is no
*family* facility available that allows people to restrict settings to
actual gcc compilers. In other words, we can't express "all GCCxx
toolchains, regardless of toolchain version, but not XCODE, and also not
CLANG".

Thanks
Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Liming Gao]

Laszlo:

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, October 9, 2019 9:44 PM
> To: Andrew Fish <afish@apple.com>; devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain
> 
> On 10/09/19 01:08, Andrew Fish wrote:
> 
> > So I guess the way to describe it is XCODE inherits GCC and only needs to override when it is different.
> 
> Thank you for the explanation!
> 
> I've been trying to figure out why this inheritance bothers me so much.
> I guess the reason is the following:
> 
> I'm a user of the actual GCCxx toolchains (such as GCC48, GCC49, GCC5).
> Assume that I realize that a gcc-specific change is needed to some part
> of the code. (It could be build options, or gcc specific source code,
> etc.) Then, I go to the gcc documentation, and verify whether the change
> is indeed supported by all the relevant gcc versions.
> 
> If the change is possible with only some gcc versions, then I likely
> write the change for a specific toolchain only, such as GCC5. Fine.
> 
> Now assume that my documentation review proves that *all* GCCxx
> toolchains, supported by edk2, should receive the update. This could be
> a #pragma, a command line flag, some __attribute__, etc. I'd very likely
> express the change for the whole GCC toolchain *family*, and not
> enumerate it for GCC48, GCC49, GCC5 individually.
> 
> And now I learn that said approach would be wrong, because such a change
> would be inherited by XCODExx and CLANGxx too (via FAMILY), fully
> against my intent, unless XCODExx and CLANGxx overrode the particular
> aspect (via BUILDRULEFAMILY).

The difference between XCODE/CLANG and GCCXX is the linker. Current patches are
introduced for the different linker. Clang supports most usage of GCC compiler.
So, CLANG and XCODE uses GCC family. When I enable XCODE or CLANG tool chain
in the platform, I don't find other incompatible behavior with GCC compiler.
So, I think it is safe to let CLANG inherit GCC option except for these two cases.
When you make new changes, and verify them with GCC compiler, that's enough.
You don't need to specially verify them with XCODE or CLANG. In most case, GCC 
compiler pass, XCODE or CLANG can also pass.

Thanks
Liming
> 
> This means that, after checking the gcc documentation meticulously
> (multiple releases), and possibly even testing multiple gcc
> installations, I could still regress edk2 (CLANGxx and/or XCODExx
> specific parts), because they could inherit my GCC-oriented changes that
> I never meant for CLANGxx and/or XCODExx.
> 
> I don't use XCODE or CLANG, I don't test on them, and now it seems I can
> restrict changes to the *actual* gcc compilers only if I keep spelling
> out the individual toolchain names, such as GCC48, GCC49, GCC5.
> 
> This makes the toolchain family concept totally useless to me (and to
> other users that only care about actual gcc compilers). There is no
> *family* facility available that allows people to restrict settings to
> actual gcc compilers. In other words, we can't express "all GCCxx
> toolchains, regardless of toolchain version, but not XCODE, and also not
> CLANG".
> 
> Thanks
> Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -

[Andrew Fish]

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



> On Oct 9, 2019, at 7:44 AM, Gao, Liming <liming.gao@intel.com> wrote:
> 
> Laszlo:
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Wednesday, October 9, 2019 9:44 PM
>> To: Andrew Fish <afish@apple.com>; devel@edk2.groups.io
>> Cc: Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain
>> 
>> On 10/09/19 01:08, Andrew Fish wrote:
>> 
>>> So I guess the way to describe it is XCODE inherits GCC and only needs to override when it is different.
>> 
>> Thank you for the explanation!
>> 
>> I've been trying to figure out why this inheritance bothers me so much.
>> I guess the reason is the following:
>> 
>> I'm a user of the actual GCCxx toolchains (such as GCC48, GCC49, GCC5).
>> Assume that I realize that a gcc-specific change is needed to some part
>> of the code. (It could be build options, or gcc specific source code,
>> etc.) Then, I go to the gcc documentation, and verify whether the change
>> is indeed supported by all the relevant gcc versions.
>> 
>> If the change is possible with only some gcc versions, then I likely
>> write the change for a specific toolchain only, such as GCC5. Fine.
>> 
>> Now assume that my documentation review proves that *all* GCCxx
>> toolchains, supported by edk2, should receive the update. This could be
>> a #pragma, a command line flag, some __attribute__, etc. I'd very likely
>> express the change for the whole GCC toolchain *family*, and not
>> enumerate it for GCC48, GCC49, GCC5 individually.
>> 
>> And now I learn that said approach would be wrong, because such a change
>> would be inherited by XCODExx and CLANGxx too (via FAMILY), fully
>> against my intent, unless XCODExx and CLANGxx overrode the particular
>> aspect (via BUILDRULEFAMILY).
> 
> The difference between XCODE/CLANG and GCCXX is the linker. Current patches are
> introduced for the different linker. Clang supports most usage of GCC compiler.
> So, CLANG and XCODE uses GCC family. When I enable XCODE or CLANG tool chain
> in the platform, I don't find other incompatible behavior with GCC compiler.
> So, I think it is safe to let CLANG inherit GCC option except for these two cases.
> When you make new changes, and verify them with GCC compiler, that's enough.
> You don't need to specially verify them with XCODE or CLANG. In most case, GCC 
> compiler pass, XCODE or CLANG can also pass.
> 

Liming,

I agree that clang attempts to be drop in compatible with GCC and the big difference is the linker. For the most part XCODE means macOS linker + clang. 

I thought the thing we were discussing was compiler flags. Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition for those compilers?  As far as I can tell -mno-implicit-float should prevent the optimizer from using floating point. The -mno-mmx -mno-sse flags most change how floating point code gets compiled [1]. it looks like -mno-mmx -mno-sse just down grade floating point instructions that get used. Thus it seems like either we have some code doing float and that code should set -mno-mmx -mno-sse, or the -mno-mmx -mno-sse should be set generically. 

[1]
~/work/Compiler/float>cat c.c
int main()
{
    float a = 10;
    float b = 20;

    float c = a * b;

    return 0;
}
~/work/Compiler/float>clang -S c.c -mno-implicit-float
~/work/Compiler/float>cat c.S
	.section	__TEXT,__text,regular,pure_instructions
	.build_version macos, 10, 15	sdk_version 10, 15
	.section	__TEXT,__literal4,4byte_literals
	.p2align	2               ## -- Begin function main
LCPI0_0:
	.long	1101004800              ## float 20
LCPI0_1:
	.long	1092616192              ## float 10
	.section	__TEXT,__text,regular,pure_instructions
	.globl	_main
	.p2align	4, 0x90
_main:                                  ## @main
	.cfi_startproc
## %bb.0:
	pushq	%rbp
	.cfi_def_cfa_offset 16
	.cfi_offset %rbp, -16
	movq	%rsp, %rbp
	.cfi_def_cfa_register %rbp
	xorl	%eax, %eax
	movss	LCPI0_0(%rip), %xmm0    ## xmm0 = mem[0],zero,zero,zero
	movss	LCPI0_1(%rip), %xmm1    ## xmm1 = mem[0],zero,zero,zero
	movl	$0, -4(%rbp)
	movss	%xmm1, -8(%rbp)
	movss	%xmm0, -12(%rbp)
	movss	-8(%rbp), %xmm0         ## xmm0 = mem[0],zero,zero,zero
	mulss	-12(%rbp), %xmm0
	movss	%xmm0, -16(%rbp)
	popq	%rbp
	retq
	.cfi_endproc
                                        ## -- End function

.subsections_via_symbols
~/work/Compiler/float>clang -S c.c -mno-implicit-float -mno-mmx -mno-sse
~/work/Compiler/float>cat c.S
	.section	__TEXT,__text,regular,pure_instructions
	.build_version macos, 10, 15	sdk_version 10, 15
	.globl	_main                   ## -- Begin function main
	.p2align	4, 0x90
_main:                                  ## @main
	.cfi_startproc
## %bb.0:
	pushq	%rbp
	.cfi_def_cfa_offset 16
	.cfi_offset %rbp, -16
	movq	%rsp, %rbp
	.cfi_def_cfa_register %rbp
	xorl	%eax, %eax
	movl	$0, -4(%rbp)
	movl	$1092616192, -8(%rbp)   ## imm = 0x41200000
	movl	$1101004800, -12(%rbp)  ## imm = 0x41A00000
	flds	-8(%rbp)
	flds	-12(%rbp)
	fmulp	%st(1)
	fstps	-16(%rbp)
	popq	%rbp
	retq
	.cfi_endproc
                                        ## -- End function

.subsections_via_symbols
~/work/Compiler/float>

Thanks,

Andrew Fish

> Thanks
> Liming
>> 
>> This means that, after checking the gcc documentation meticulously
>> (multiple releases), and possibly even testing multiple gcc
>> installations, I could still regress edk2 (CLANGxx and/or XCODExx
>> specific parts), because they could inherit my GCC-oriented changes that
>> I never meant for CLANGxx and/or XCODExx.
>> 
>> I don't use XCODE or CLANG, I don't test on them, and now it seems I can
>> restrict changes to the *actual* gcc compilers only if I keep spelling
>> out the individual toolchain names, such as GCC48, GCC49, GCC5.
>> 
>> This makes the toolchain family concept totally useless to me (and to
>> other users that only care about actual gcc compilers). There is no
>> *family* facility available that allows people to restrict settings to
>> actual gcc compilers. In other words, we can't express "all GCCxx
>> toolchains, regardless of toolchain version, but not XCODE, and also not
>> CLANG".
>> 
>> Thanks
>> Laszlo


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

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -

[Laszlo Ersek]

Hi Andrew,

On 10/09/19 18:22, Andrew Fish wrote:

> I thought the thing we were discussing was compiler flags.
> Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires
> -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition
> for those compilers?  As far as I can tell -mno-implicit-float should
> prevent the optimizer from using floating point. The -mno-mmx
> -mno-sse flags most change how floating point code gets compiled [1].
> it looks like -mno-mmx -mno-sse just down grade floating point
> instructions that get used. Thus it seems like either we have some
> code doing float and that code should set -mno-mmx -mno-sse, or the
> -mno-mmx -mno-sse should be set generically.

The origin of those build flags in OVMF is commit 4a8266f570ef
("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31):

    OvmfPkg: Work around issue seen with kvm + grub2 (efi)

    When OVMF is run with kvm and grub2 (efi), an exception
    occurs when mmx/sse registers are accessed.

    As a work around, this change eliminates firmware usage
    of these register types.

    First, only the BaseMemoryLib implementation
    MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
    is used.

    Second, the GCC compiler is passes the additional
    '-mno-mmx -mno-sse' parameters.

The eternal problem with this kind of workaround is that we never know
when it becomes safe to remove.

It would be nice to understand the exact details around the problem.
Given that the commit is from 2010, I assume the issue is difficult to
reproduce with recent components (KVM, grub2). On the other hand, if we
just remove the flags (which we should, in a perfect world), someone
could report a bug in three months: "my host distro upgraded the OVMF
package to the next edk2-stableYYYYMM tag, and now my virtual machine,
which runs a distro from 2009, no longer boots". (We've seen similar
reports before.)

Thanks
Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -

[Liming Gao]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Thursday, October 10, 2019 3:35 PM
> To: Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io
> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -
> 
> Hi Andrew,
> 
> On 10/09/19 18:22, Andrew Fish wrote:
> 
> > I thought the thing we were discussing was compiler flags.
> > Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires
> > -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition
> > for those compilers?  As far as I can tell -mno-implicit-float should
> > prevent the optimizer from using floating point. The -mno-mmx
> > -mno-sse flags most change how floating point code gets compiled [1].
> > it looks like -mno-mmx -mno-sse just down grade floating point
> > instructions that get used. Thus it seems like either we have some
> > code doing float and that code should set -mno-mmx -mno-sse, or the
> > -mno-mmx -mno-sse should be set generically.
> 
> The origin of those build flags in OVMF is commit 4a8266f570ef
> ("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31):
> 
>     OvmfPkg: Work around issue seen with kvm + grub2 (efi)
> 
>     When OVMF is run with kvm and grub2 (efi), an exception
>     occurs when mmx/sse registers are accessed.
> 
>     As a work around, this change eliminates firmware usage
>     of these register types.
> 
>     First, only the BaseMemoryLib implementation
>     MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
>     is used.
> 
>     Second, the GCC compiler is passes the additional
>     '-mno-mmx -mno-sse' parameters.
> 
> The eternal problem with this kind of workaround is that we never know
> when it becomes safe to remove.
> 
> It would be nice to understand the exact details around the problem.
> Given that the commit is from 2010, I assume the issue is difficult to
> reproduce with recent components (KVM, grub2). On the other hand, if we
> just remove the flags (which we should, in a perfect world), someone
> could report a bug in three months: "my host distro upgraded the OVMF
> package to the next edk2-stableYYYYMM tag, and now my virtual machine,
> which runs a distro from 2009, no longer boots". (We've seen similar
> reports before.)

Yes. I agree it is hard to decide to remove this option, because we don't know its impact. 
With the option -mno-mmx -mno-sse, it will cause CLANG9 linker failure like below. So, I say 
this patch is to support the different linker.

0.      Program arguments: C:\Program Files\LLVM\bin\lld-link.EXE /OUT:d:\allpkg\edk2\Build\Ovmf3264\DEBUG_CLANG9\X64\Se
curityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDxe\DEBUG\SecureBootConfigDxe.dll /NOLOGO /NODEFAULT
LIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT
RY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH /lldmap @d:\allpkg\edk2\Build\O
vmf3264\DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDxe\OUTPUT\static_library
_files.lst
1.      Running pass 'Function Pass Manager' on module 'ld-temp.o'.
2.      Running pass 'X86 FP Stackifier' on function '@drbg_add'
 #0 0x00007ff696bad7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x148d7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x142ba7f

Thanks
Liming
> 
> Thanks
> Laszlo
> 
> 

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Liming Gao]

Laszlo:

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, October 9, 2019 4:22 AM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; afish@apple.com
> Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions
> 
> On 10/08/19 16:47, Gao, Liming wrote:
> 
> >     [Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and
> >     XCODE5.
> >
> >     I don’t know the specific reason about __inline__. If there is no
> >     impact on
> >
> >     other GCC tool chain, I prefer to remove them.
> 
> > [Liming] This seems the remaining clean up task. So, I prefer to remove
> > __inline__ if no impact on GCC tool chain.
> 
> OK. Given your testing with GCC48, I'm fine.
> 
With this patch set, I verify GCC48/GCC49/GCC5 on Ovmf3264. They can all boot to Shell. 
Are they enough?

Thanks
Liming
> Thanks
> Laszlo

Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

[Liming Gao]

Laszlo:
  Option (a) works. Jordan patch can fix this issue. 
  Option (b) doesn't work. Even if disable optimization, CLANG doesn't generate the code with push rbp & pop rbp.
  
  So, Jordan patch becomes only option. We can discuss this topic again. But, I don't think this is the block issue
  to add new CLANG9 tool chain. I will try to add CLANG9 tool chain patch without this change, and use BZ 2024 to track this issue. 

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao
> Sent: Tuesday, October 8, 2019 11:10 PM
> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64
> 
> Laszlo:
>   I will verify your option (a) and (b). The problem is described in https://bugzilla.tianocore.org/show_bug.cgi?id=2024.
>   I know there are some discussion in Jordan patch for PeiCore change. Before the conclusion for Jordan change is made,
>   I expect to find the alternate option to resolve CLANG9 tool chain issue first.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Tuesday, October 1, 2019 5:10 AM
> > To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> > Subject: Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64
> >
> > + Jordan
> >
> > On 09/27/19 09:46, Liming Gao wrote:
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2024
> > >
> > > Signed-off-by: Liming Gao <liming.gao@intel.com>
> > > ---
> > >  OvmfPkg/Sec/SecMain.inf | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
> > > index 63ba4cb555..cd765cac25 100644
> > > --- a/OvmfPkg/Sec/SecMain.inf
> > > +++ b/OvmfPkg/Sec/SecMain.inf
> > > @@ -69,3 +69,7 @@
> > >
> > >  [FeaturePcd]
> > >    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> > > +
> > > +[BuildOptions]
> > > +  # CLANG9 X64 requires it.
> > > +  GCC:*_CLANG9_X64_CC_FLAGS = -fno-omit-frame-pointer
> > >
> >
> > I disagree with this patch for two reasons.
> >
> > First, the comment "CLANG9 X64 requires it" is quite lacking. It does
> > nothing to explain the issue; it doesn't even provide a pointer in the
> > code comment (only in the code message).
> >
> > Second, Jordan suggested an alternative approach at the end of
> > <https://bugzilla.tianocore.org/show_bug.cgi?id=2024#c3>, which I prefer
> > to the one seen above. Can we please try that with CLANG9 too?
> >
> >
> > ... Oh wait I see there are new comments in BZ#2024.
> >
> > Apparently,
> >
> >   #pragma GCC optimize ("no-omit-frame-pointer")
> >
> > does not work with CLANG9.
> >
> > That's not a problem: we still have two options that are superior to the
> > present patch, and should be tested.
> >
> > (a) Does Jordan's series linked below fix the problem with CLANG9?
> >
> >   [edk2-devel] [PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration
> >   http://mid.mail-archive.com/20190410084000.19660-1-jordan.l.justen@intel.com
> >   https://edk2.groups.io/g/devel/message/38785
> >
> > (b) Jordan's full #pragma suggestion was:
> >
> > #ifdef __GNUC__
> > #pragma GCC push_options
> > #pragma GCC optimize ("no-omit-frame-pointer")
> > #else
> > #pragma optimize ("y", off)
> > #endif
> >
> > If the '#pragme GCC' branch doesn't help, can we customize the *other*
> > branch for CLANG9?
> >
> > For example, in patch#4, we rely on defined(__clang__). Therefore, can
> > we try the following:
> >
> > #ifdef __GNUC__
> > #pragma GCC push_options
> > #pragma GCC optimize ("no-omit-frame-pointer")
> > #elif defined(__clang__)
> > #pragma clang optimize off  <----- NOTE THIS <http://clang.llvm.org/docs/LanguageExtensions.html>
> > #else
> > #pragma optimize ("y", off)
> > #endif
> >
> > In summary, the fact that CLANG9 breaks is just a symptom; it shows that
> > the PEI Core issue fixed by Jordan is real. We should go for the real
> > fix (a).
> >
> > Alternatively, use a clang-specific optimization override, as tightly as
> > possible; (b) might work.
> >
> > -*-
> >
> > In fact I think this is the perfect time to fix the PEI Core issue: we
> > now have a feature request that depends on fixing the bug in the PEI
> > Core. We should fix technical debt in edk2 at least *sometimes*. If we
> > are not willing to fix technical debt in edk2 for the sake of new
> > features, we will *never* fix technical debt.
> >
> > (Well, to be technically correct, we also fix technical debt when it
> > turns into security issues. Yay!)
> >
> > Please test this series with the present patch removed, and Jordan's v2
> > series (linked above) applied.
> >
> > Thanks
> > Laszlo
> 
> 

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Andrew Fish]

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

Liming,

I'm OK with not regressing OVMF, I understand the concept of technical debt. 

I'd point out that this is likely a generic issue with the SecurityPkg that was worked around in OVMF. 

Also it looks to me like passing those flags violate the calling convention [1]. Maybe that is why the linker had an issue? 
int drbg_add(const void *buf, int num, double randomness);

>From what I can tell without the flag randomness gets passed in XMM2, with the flags it gets passed on the stack. Seems clang is not happy with that. But the optimizer violates the calling conventions too, so as long as nothing is public with a floating point API it should not break anything. 

Yikes I crashed clang passing in -mno-mmx -mno-sse. It might be a good idea to not pass -mno-mmx -mno-sse to clang in general for any target that is x86_64-pc-win32.

[1] UEFI Spec...
Return values that fix into 64-bits are returned in the Rax register. If the return value does not fit within 64-bits, then the caller must allocate and pass a pointer for the return value as the first argument, Rcx. Subsequent arguments are then shifted one argument to the right, so for example argument one would be passed in Rdx. User-defined types to be returned must be 1,2,4,8,16,32, or 64 bits in length.
The registers Rax, Rcx Rdx R8, R9, R10, R11, and XMM0-XMM5 are volatile and are, therefore, destroyed on function calls.
The registers RBX, RBP, RDI, RSI, R12, R13, R14, R15, and XMM6-XMM15 are considered nonvolatile and must be saved and restored by a function that uses them.
Function pointers are pointers to the label of the respective function and don’t require special treatment. A caller must always call with the stack 16-byte aligned.
For MMX, XMM and floating-point values, return values that can fit into 64-bits are returned through RAX (including MMX types). However, XMM 128-bit types, floats, and doubles are returned in XMM0. The floating point status register is not saved by the target function. Floating-point and double-precision arguments are passed in XMM0 - XMM3 (up to 4) with the integer slot (RCX, RDX, R8, and R9) that would normally be used for that cardinal slot being ignored (see example) and vice versa. XMM types are never passed by immediate value but rather a pointer will be passed to memory allocated by the caller. MMX types will be passed as if they were integers of the same size. Callees must not unmask exceptions without providing correct exception handlers.
In addition, unless otherwise specified by the function definition, all other CPU registers (including MMX and XMM) are preserved.

Thanks,

Andrew Fish

> On Oct 10, 2019, at 5:18 AM, Gao, Liming <liming.gao@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
>> Sent: Thursday, October 10, 2019 3:35 PM
>> To: Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>
>> Cc: devel@edk2.groups.io
>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -
>> 
>> Hi Andrew,
>> 
>> On 10/09/19 18:22, Andrew Fish wrote:
>> 
>>> I thought the thing we were discussing was compiler flags.
>>> Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires
>>> -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition
>>> for those compilers?  As far as I can tell -mno-implicit-float should
>>> prevent the optimizer from using floating point. The -mno-mmx
>>> -mno-sse flags most change how floating point code gets compiled [1].
>>> it looks like -mno-mmx -mno-sse just down grade floating point
>>> instructions that get used. Thus it seems like either we have some
>>> code doing float and that code should set -mno-mmx -mno-sse, or the
>>> -mno-mmx -mno-sse should be set generically.
>> 
>> The origin of those build flags in OVMF is commit 4a8266f570ef
>> ("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31):
>> 
>>    OvmfPkg: Work around issue seen with kvm + grub2 (efi)
>> 
>>    When OVMF is run with kvm and grub2 (efi), an exception
>>    occurs when mmx/sse registers are accessed.
>> 
>>    As a work around, this change eliminates firmware usage
>>    of these register types.
>> 
>>    First, only the BaseMemoryLib implementation
>>    MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
>>    is used.
>> 
>>    Second, the GCC compiler is passes the additional
>>    '-mno-mmx -mno-sse' parameters.
>> 
>> The eternal problem with this kind of workaround is that we never know
>> when it becomes safe to remove.
>> 
>> It would be nice to understand the exact details around the problem.
>> Given that the commit is from 2010, I assume the issue is difficult to
>> reproduce with recent components (KVM, grub2). On the other hand, if we
>> just remove the flags (which we should, in a perfect world), someone
>> could report a bug in three months: "my host distro upgraded the OVMF
>> package to the next edk2-stableYYYYMM tag, and now my virtual machine,
>> which runs a distro from 2009, no longer boots". (We've seen similar
>> reports before.)
> 
> Yes. I agree it is hard to decide to remove this option, because we don't know its impact. 
> With the option -mno-mmx -mno-sse, it will cause CLANG9 linker failure like below. So, I say 
> this patch is to support the different linker.
> 
> 0.      Program arguments: C:\Program Files\LLVM\bin\lld-link.EXE /OUT:d:\allpkg\edk2\Build\Ovmf3264\DEBUG_CLANG9\X64\Se
> curityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDxe\DEBUG\SecureBootConfigDxe.dll /NOLOGO /NODEFAULT
> LIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT
> RY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH /lldmap @d:\allpkg\edk2\Build\O
> vmf3264\DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDxe\OUTPUT\static_library
> _files.lst
> 1.      Running pass 'Function Pass Manager' on module 'ld-temp.o'.
> 2.      Running pass 'X86 FP Stackifier' on function '@drbg_add'
> #0 0x00007ff696bad7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x148d7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x142ba7f
> 
> Thanks
> Liming
>> 
>> Thanks
>> Laszlo
>> 
>> 


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

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Laszlo Ersek]

Hi Liming, Andrew,

On 10/10/19 14:32, Liming Gao wrote:
> Laszlo:
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Wednesday, October 9, 2019 4:22 AM
>> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; afish@apple.com
>> Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions
>>
>> On 10/08/19 16:47, Gao, Liming wrote:
>>
>>>     [Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and
>>>     XCODE5.
>>>
>>>     I don’t know the specific reason about __inline__. If there is no
>>>     impact on
>>>
>>>     other GCC tool chain, I prefer to remove them.
>>
>>> [Liming] This seems the remaining clean up task. So, I prefer to remove
>>> __inline__ if no impact on GCC tool chain.
>>
>> OK. Given your testing with GCC48, I'm fine.
>>
> With this patch set, I verify GCC48/GCC49/GCC5 on Ovmf3264. They can all boot to Shell. 
> Are they enough?

Would you guys agree with the following commit message, on this patch?

"""
__inline__ has no discernible effect with the GCC48 / GCC49 / GCC5
toolchains, but it breaks the build with CLANG9.

Remove __inline__.
"""

If you can update the commit message like that, then you can add:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -

[Laszlo Ersek]

Hi Liming,

On 10/10/19 14:18, Liming Gao wrote:
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
>> Sent: Thursday, October 10, 2019 3:35 PM
>> To: Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>
>> Cc: devel@edk2.groups.io
>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -
>>
>> Hi Andrew,
>>
>> On 10/09/19 18:22, Andrew Fish wrote:
>>
>>> I thought the thing we were discussing was compiler flags.
>>> Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires
>>> -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition
>>> for those compilers?  As far as I can tell -mno-implicit-float should
>>> prevent the optimizer from using floating point. The -mno-mmx
>>> -mno-sse flags most change how floating point code gets compiled [1].
>>> it looks like -mno-mmx -mno-sse just down grade floating point
>>> instructions that get used. Thus it seems like either we have some
>>> code doing float and that code should set -mno-mmx -mno-sse, or the
>>> -mno-mmx -mno-sse should be set generically.
>>
>> The origin of those build flags in OVMF is commit 4a8266f570ef
>> ("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31):
>>
>>     OvmfPkg: Work around issue seen with kvm + grub2 (efi)
>>
>>     When OVMF is run with kvm and grub2 (efi), an exception
>>     occurs when mmx/sse registers are accessed.
>>
>>     As a work around, this change eliminates firmware usage
>>     of these register types.
>>
>>     First, only the BaseMemoryLib implementation
>>     MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
>>     is used.
>>
>>     Second, the GCC compiler is passes the additional
>>     '-mno-mmx -mno-sse' parameters.
>>
>> The eternal problem with this kind of workaround is that we never know
>> when it becomes safe to remove.
>>
>> It would be nice to understand the exact details around the problem.
>> Given that the commit is from 2010, I assume the issue is difficult to
>> reproduce with recent components (KVM, grub2). On the other hand, if we
>> just remove the flags (which we should, in a perfect world), someone
>> could report a bug in three months: "my host distro upgraded the OVMF
>> package to the next edk2-stableYYYYMM tag, and now my virtual machine,
>> which runs a distro from 2009, no longer boots". (We've seen similar
>> reports before.)
> 
> Yes. I agree it is hard to decide to remove this option, because we don't know its impact. 
> With the option -mno-mmx -mno-sse, it will cause CLANG9 linker failure like below. So, I say 
> this patch is to support the different linker.
> 
> 0.      Program arguments: C:\Program Files\LLVM\bin\lld-link.EXE /OUT:d:\allpkg\edk2\Build\Ovmf3264\DEBUG_CLANG9\X64\Se
> curityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDxe\DEBUG\SecureBootConfigDxe.dll /NOLOGO /NODEFAULT
> LIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT
> RY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH /lldmap @d:\allpkg\edk2\Build\O
> vmf3264\DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDxe\OUTPUT\static_library
> _files.lst
> 1.      Running pass 'Function Pass Manager' on module 'ld-temp.o'.
> 2.      Running pass 'X86 FP Stackifier' on function '@drbg_add'
>  #0 0x00007ff696bad7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x148d7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x142ba7f

can you please try the following:

(a) replace this patch with two patches, as follows

(b) in the first patch, only rework

!if $(TOOL_CHAIN_TAG) != "XCODE5"
  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
!endif

into:

  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
  XCODE:*_*_*_CC_FLAGS                 =

(c) in the second patch, append:

  CLANGPE:*_*_*_CC_FLAGS               =

and also introduce

  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096


If (b) preserves the intended effect for the XCODE5 toolchain, and (c)
does the right thing for the CLANG9 toolchain as well, then that
approach would be preferable to the currently proposed patch#11.

If the above does *not* work, then I'm fine with the currently proposed
patch, but then please update the commit message; it should say that
"-mno-mmx -mno-sse" is *excluded* for CLANG9.

Thanks
Laszlo

Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

[Laszlo Ersek]

Hi Liming,

On 10/10/19 16:08, Liming Gao wrote:
> Laszlo:
>   Option (a) works. Jordan patch can fix this issue. 
>   Option (b) doesn't work. Even if disable optimization, CLANG doesn't generate the code with push rbp & pop rbp.
>   
>   So, Jordan patch becomes only option. We can discuss this topic again. But, I don't think this is the block issue
>   to add new CLANG9 tool chain. I will try to add CLANG9 tool chain patch without this change, and use BZ 2024 to track this issue. 

not sure I understand you correctly.

- If you intend to drop this patch from the series, and enable CLANG9
for edk2 without enabling it for OvmfPkg specifically (for the time
being), that's fine with me.

- If you'd like to push this patch first, and work on the general PEI
Core issue second, then I defer to Jordan for reviewing this patch. I'd
like to neither approve nor reject this patch myself, in that case.

Thanks
Laszlo

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Liming Gao]

Laszlo:

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Laszlo Ersek
>Sent: Friday, October 11, 2019 12:32 AM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>;
>afish@apple.com
>Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove
>__inline__ attribute for IO functions
>
>Hi Liming, Andrew,
>
>On 10/10/19 14:32, Liming Gao wrote:
>> Laszlo:
>>
>>> -----Original Message-----
>>> From: Laszlo Ersek <lersek@redhat.com>
>>> Sent: Wednesday, October 9, 2019 4:22 AM
>>> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io;
>afish@apple.com
>>> Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic:
>Remove __inline__ attribute for IO functions
>>>
>>> On 10/08/19 16:47, Gao, Liming wrote:
>>>
>>>>     [Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and
>>>>     XCODE5.
>>>>
>>>>     I don’t know the specific reason about __inline__. If there is no
>>>>     impact on
>>>>
>>>>     other GCC tool chain, I prefer to remove them.
>>>
>>>> [Liming] This seems the remaining clean up task. So, I prefer to remove
>>>> __inline__ if no impact on GCC tool chain.
>>>
>>> OK. Given your testing with GCC48, I'm fine.
>>>
>> With this patch set, I verify GCC48/GCC49/GCC5 on Ovmf3264. They can all
>boot to Shell.
>> Are they enough?
>
>Would you guys agree with the following commit message, on this patch?
>
>"""
>__inline__ has no discernible effect with the GCC48 / GCC49 / GCC5
>toolchains, but it breaks the build with CLANG9.
>
>Remove __inline__.
>"""
>
Agree. I will update the commit message. Thanks for your suggestion. 

Liming

>If you can update the commit message like that, then you can add:
>
>Acked-by: Laszlo Ersek <lersek@redhat.com>
>
>Thanks!
>Laszlo
>
>

Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

[Liming Gao]

Laszlo:

>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Friday, October 11, 2019 1:35 AM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Justen,
>Jordan L <jordan.l.justen@intel.com>
>Subject: Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-
>fno-omit-frame-pointer" for CLANG9 X64
>
>Hi Liming,
>
>On 10/10/19 16:08, Liming Gao wrote:
>> Laszlo:
>>   Option (a) works. Jordan patch can fix this issue.
>>   Option (b) doesn't work. Even if disable optimization, CLANG doesn't
>generate the code with push rbp & pop rbp.
>>
>>   So, Jordan patch becomes only option. We can discuss this topic again. But,
>I don't think this is the block issue
>>   to add new CLANG9 tool chain. I will try to add CLANG9 tool chain patch
>without this change, and use BZ 2024 to track this issue.
>
>not sure I understand you correctly.
>
>- If you intend to drop this patch from the series, and enable CLANG9
>for edk2 without enabling it for OvmfPkg specifically (for the time
>being), that's fine with me.
>
I mean to drop this patch from this patch set. I will use BZ 2024 to track this issue.
Without this fix, CLANG9 OVMF X64 boot will hang. CLANG9 OVMF Ia32 and Ia32X64 boots fine.

Thanks
Liming
>- If you'd like to push this patch first, and work on the general PEI
>Core issue second, then I defer to Jordan for reviewing this patch. I'd
>like to neither approve nor reject this patch myself, in that case.
>
>Thanks
>Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -

[Liming Gao]

Laszlo:

>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Friday, October 11, 2019 12:43 AM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Andrew Fish
><afish@apple.com>
>Cc: Justen, Jordan L <jordan.l.justen@intel.com>
>Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -
>
>Hi Liming,
>
>On 10/10/19 14:18, Liming Gao wrote:
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
>Ersek
>>> Sent: Thursday, October 10, 2019 3:35 PM
>>> To: Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>
>>> Cc: devel@edk2.groups.io
>>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool
>chain -
>>>
>>> Hi Andrew,
>>>
>>> On 10/09/19 18:22, Andrew Fish wrote:
>>>
>>>> I thought the thing we were discussing was compiler flags.
>>>> Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires
>>>> -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition
>>>> for those compilers?  As far as I can tell -mno-implicit-float should
>>>> prevent the optimizer from using floating point. The -mno-mmx
>>>> -mno-sse flags most change how floating point code gets compiled [1].
>>>> it looks like -mno-mmx -mno-sse just down grade floating point
>>>> instructions that get used. Thus it seems like either we have some
>>>> code doing float and that code should set -mno-mmx -mno-sse, or the
>>>> -mno-mmx -mno-sse should be set generically.
>>>
>>> The origin of those build flags in OVMF is commit 4a8266f570ef
>>> ("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31):
>>>
>>>     OvmfPkg: Work around issue seen with kvm + grub2 (efi)
>>>
>>>     When OVMF is run with kvm and grub2 (efi), an exception
>>>     occurs when mmx/sse registers are accessed.
>>>
>>>     As a work around, this change eliminates firmware usage
>>>     of these register types.
>>>
>>>     First, only the BaseMemoryLib implementation
>>>     MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
>>>     is used.
>>>
>>>     Second, the GCC compiler is passes the additional
>>>     '-mno-mmx -mno-sse' parameters.
>>>
>>> The eternal problem with this kind of workaround is that we never know
>>> when it becomes safe to remove.
>>>
>>> It would be nice to understand the exact details around the problem.
>>> Given that the commit is from 2010, I assume the issue is difficult to
>>> reproduce with recent components (KVM, grub2). On the other hand, if
>we
>>> just remove the flags (which we should, in a perfect world), someone
>>> could report a bug in three months: "my host distro upgraded the OVMF
>>> package to the next edk2-stableYYYYMM tag, and now my virtual machine,
>>> which runs a distro from 2009, no longer boots". (We've seen similar
>>> reports before.)
>>
>> Yes. I agree it is hard to decide to remove this option, because we don't
>know its impact.
>> With the option -mno-mmx -mno-sse, it will cause CLANG9 linker failure like
>below. So, I say
>> this patch is to support the different linker.
>>
>> 0.      Program arguments: C:\Program Files\LLVM\bin\lld-link.EXE
>/OUT:d:\allpkg\edk2\Build\Ovmf3264\DEBUG_CLANG9\X64\Se
>>
>curityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDx
>e\DEBUG\SecureBootConfigDxe.dll /NOLOGO /NODEFAULT
>> LIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32
>/SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT
>> RY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER
>/SAFESEH:NO /BASE:0 /DEBUG:GHASH /lldmap @d:\allpkg\edk2\Build\O
>>
>vmf3264\DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBo
>otConfigDxe\SecureBootConfigDxe\OUTPUT\static_library
>> _files.lst
>> 1.      Running pass 'Function Pass Manager' on module 'ld-temp.o'.
>> 2.      Running pass 'X86 FP Stackifier' on function '@drbg_add'
>>  #0 0x00007ff696bad7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x148d7f8
>C:\Program Files\LLVM\bin\lld-link.EXE 0x142ba7f
>
>can you please try the following:
>
>(a) replace this patch with two patches, as follows
>
>(b) in the first patch, only rework
>
>!if $(TOOL_CHAIN_TAG) != "XCODE5"
>  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>!endif
>
>into:
>
>  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>  XCODE:*_*_*_CC_FLAGS                 =
>
>(c) in the second patch, append:
>
>  CLANGPE:*_*_*_CC_FLAGS               =
>
This way clear CLANG9 CC flags. It forbids CLANG9 to inherit other GCC flags. 
Below flags are not applied into CLANG9. The purpose is not to include 
specific -mno-mmx -mno-sse option, and still inherit other GCC flags. 
So, I have to use $(TOOL_CHAIN_TAG) check. 

  MSFT:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES
  INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES
  GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES

>and also introduce
>
>  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>
>
>If (b) preserves the intended effect for the XCODE5 toolchain, and (c)
>does the right thing for the CLANG9 toolchain as well, then that
>approach would be preferable to the currently proposed patch#11.
>
>If the above does *not* work, then I'm fine with the currently proposed
>patch, but then please update the commit message; it should say that
>"-mno-mmx -mno-sse" is *excluded* for CLANG9.
>
Yes. I will update the commit message. 

Thanks
Liming

>Thanks
>Laszlo

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Laszlo Ersek]

Hi Liming,

On 10/09/19 16:44, Gao, Liming wrote:

> The difference between XCODE/CLANG and GCCXX is the linker. Current
> patches are introduced for the different linker. Clang supports most
> usage of GCC compiler. So, CLANG and XCODE uses GCC family. When I
> enable XCODE or CLANG tool chain in the platform, I don't find other
> incompatible behavior with GCC compiler. So, I think it is safe to let
> CLANG inherit GCC option except for these two cases. When you make new
> changes, and verify them with GCC compiler, that's enough. You don't
> need to specially verify them with XCODE or CLANG. In most case, GCC
> compiler pass, XCODE or CLANG can also pass.

I'd like to provide a counter-example for this.

Consider the issue that Tom reported, at

  http://mid.mail-archive.com/8eb55d97-0ba3-c217-a160-c24730b9f036@amd.com
  https://edk2.groups.io/g/devel/message/48762

in point (2).

(Both links point to the same message, just in different archives.)

Let me summarize that problem.

- In BZ#849, an XCODE toolchain bug was reported, and a workaround was
  requested.

- The workaround was commit 2db0ccc2d7fe ("UefiCpuPkg: Update
  CpuExceptionHandlerLib pass XCODE5 tool chain", 2018-01-16).

- The workaround is binary patching (self-modification) in the exception
  handler's assembly code.

- Unfortunately, this code is also linked into SEC modules, which run
  from flash. Therefore, self-modification is not permitted, and the
  workaround is not good enough for SEC. (Nor for PEI modules that run
  from flash.)

Now, let's consider how we could mitigate this issue for *temporarily*,
for all toolchains *except* XCODE5, until the issue is fixed. Clearly,
toolchains other than XCODE5 have no problems with the original assembly
code (i.e., with the 64-bit absolute address relocations). Therefore, we
could consider reverting the commit, and capturing the results of the
revert in a *separate* NASM source file. This source file (that is, the
original, pre-2db0ccc2d7fe assembly code) would apply to all toolchain
families, except the XCODE family.

Let's say the new NASM source files would be called
- X64/ExceptionHandlerAsmGeneric.nasm -- all except XCODE,
- X64/ExceptionHandlerAsmXcode.nasm -- XCODE,

replacing the current file

- X64/ExceptionHandlerAsm.nasm

So, how can we use the new files, in the INF file

  UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf

?

We could attempt,

> [Sources.X64]
>   X64/ExceptionHandlerAsmGeneric.nasm | GCC
>   X64/ExceptionHandlerAsmXcode.nasm   | XCODE
>   X64/ExceptionHandlerAsmGeneric.nasm | INTEL
>   X64/ExceptionHandlerAsmGeneric.nasm | MSFT

Unfortunately, this does not work.

While it solves the issue on GCC, INTEL and MSFT, it breaks on XCODE:
XCODE picks up *both* the GCC line and the XCODE line. And that's
because XCODE inherits GCC, and there is presently no way to express
"real GCC only".

But, at least, this suggests a solution too. In
"BaseTools/Conf/tools_def.template", for *every* toolchain that
specifies FAMILY=GCC, we should spell out an explicit BUILDRULEFAMILY.
Like this:

> @@ -1995,6 +1995,7 @@
>  #
>  ####################################################################################
>  *_GCC48_*_*_FAMILY               = GCC
> +*_GCC48_*_*_BUILDRULEFAMILY      = GENUINE_GCC
>
>  *_GCC48_*_MAKE_PATH                    = DEF(GCC_HOST_PREFIX)make
>  *_GCC48_*_*_DLL                        = ENV(GCC48_DLL)
> @@ -2134,6 +2135,7 @@
>  #
>  ####################################################################################
>  *_GCC49_*_*_FAMILY               = GCC
> +*_GCC49_*_*_BUILDRULEFAMILY      = GENUINE_GCC
>
>  *_GCC49_*_MAKE_PATH                    = DEF(GCC_HOST_PREFIX)make
>  *_GCC49_*_*_DLL                        = ENV(GCC49_DLL)
> @@ -2280,6 +2282,7 @@
>  #
>  ####################################################################################
>  *_GCC5_*_*_FAMILY                = GCC
> +*_GCC5_*_*_BUILDRULEFAMILY       = GENUINE_GCC
>
>  *_GCC5_*_MAKE_PATH               = DEF(GCC_HOST_PREFIX)make
>  *_GCC5_*_*_DLL                   = ENV(GCC5_DLL)
> @@ -2437,6 +2440,7 @@
>  #
>  ####################################################################################
>  *_CLANG35_*_*_FAMILY             = GCC
> +*_CLANG35_*_*_BUILDRULEFAMILY    = CLANG
>
>  *_CLANG35_*_MAKE_PATH            = make
>  *_CLANG35_*_*_DLL                = ENV(CLANG35_DLL)
> @@ -2517,6 +2521,8 @@
>  #
>  ####################################################################################
>  *_CLANG38_*_*_FAMILY                = GCC
> +*_CLANG38_*_*_BUILDRULEFAMILY       = CLANG
> +
>  *_CLANG38_*_MAKE_PATH               = make
>  *_CLANG38_*_*_DLL                   = ENV(CLANG38_DLL)
>  *_CLANG38_*_ASL_PATH                = DEF(UNIX_IASL_BIN)

And then we could write:

> [Sources.X64]
>   X64/ExceptionHandlerAsmGeneric.nasm | GENUINE_GCC
>   X64/ExceptionHandlerAsmGeneric.nasm | CLANG
>   X64/ExceptionHandlerAsmXcode.nasm   | XCODE
>   X64/ExceptionHandlerAsmGeneric.nasm | INTEL
>   X64/ExceptionHandlerAsmGeneric.nasm | MSFT

replacing plain "GCC", which XCODE inherits, with "GENUINE_GCC" and
"CLANG", neither of which XCODE inherits.

Would you accept the above patch, for
"BaseTools/Conf/tools_def.template"?

Thanks,
Laszlo

Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

[Laszlo Ersek]

On 10/11/19 03:30, Liming Gao wrote:
> Laszlo:
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, October 11, 2019 1:35 AM
>> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Justen,
>> Jordan L <jordan.l.justen@intel.com>
>> Subject: Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-
>> fno-omit-frame-pointer" for CLANG9 X64
>>
>> Hi Liming,
>>
>> On 10/10/19 16:08, Liming Gao wrote:
>>> Laszlo:
>>>   Option (a) works. Jordan patch can fix this issue.
>>>   Option (b) doesn't work. Even if disable optimization, CLANG doesn't
>> generate the code with push rbp & pop rbp.
>>>
>>>   So, Jordan patch becomes only option. We can discuss this topic again. But,
>> I don't think this is the block issue
>>>   to add new CLANG9 tool chain. I will try to add CLANG9 tool chain patch
>> without this change, and use BZ 2024 to track this issue.
>>
>> not sure I understand you correctly.
>>
>> - If you intend to drop this patch from the series, and enable CLANG9
>> for edk2 without enabling it for OvmfPkg specifically (for the time
>> being), that's fine with me.
>>
> I mean to drop this patch from this patch set. I will use BZ 2024 to track this issue.
> Without this fix, CLANG9 OVMF X64 boot will hang. CLANG9 OVMF Ia32 and Ia32X64 boots fine.

OK, thank you.
Laszlo

>> - If you'd like to push this patch first, and work on the general PEI
>> Core issue second, then I defer to Jordan for reviewing this patch. I'd
>> like to neither approve nor reject this patch myself, in that case.
>>
>> Thanks
>> Laszlo
> 
> 
> 

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -

[Laszlo Ersek]

On 10/11/19 03:47, Liming Gao wrote:
> Laszlo:
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, October 11, 2019 12:43 AM
>> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Andrew Fish
>> <afish@apple.com>
>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>
>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain -
>>
>> Hi Liming,
>>
>> On 10/10/19 14:18, Liming Gao wrote:
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
>> Ersek
>>>> Sent: Thursday, October 10, 2019 3:35 PM
>>>> To: Andrew Fish <afish@apple.com>; Gao, Liming <liming.gao@intel.com>
>>>> Cc: devel@edk2.groups.io
>>>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool
>> chain -
>>>>
>>>> Hi Andrew,
>>>>
>>>> On 10/09/19 18:22, Andrew Fish wrote:
>>>>
>>>>> I thought the thing we were discussing was compiler flags.
>>>>> Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires
>>>>> -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition
>>>>> for those compilers?  As far as I can tell -mno-implicit-float should
>>>>> prevent the optimizer from using floating point. The -mno-mmx
>>>>> -mno-sse flags most change how floating point code gets compiled [1].
>>>>> it looks like -mno-mmx -mno-sse just down grade floating point
>>>>> instructions that get used. Thus it seems like either we have some
>>>>> code doing float and that code should set -mno-mmx -mno-sse, or the
>>>>> -mno-mmx -mno-sse should be set generically.
>>>>
>>>> The origin of those build flags in OVMF is commit 4a8266f570ef
>>>> ("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31):
>>>>
>>>>     OvmfPkg: Work around issue seen with kvm + grub2 (efi)
>>>>
>>>>     When OVMF is run with kvm and grub2 (efi), an exception
>>>>     occurs when mmx/sse registers are accessed.
>>>>
>>>>     As a work around, this change eliminates firmware usage
>>>>     of these register types.
>>>>
>>>>     First, only the BaseMemoryLib implementation
>>>>     MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
>>>>     is used.
>>>>
>>>>     Second, the GCC compiler is passes the additional
>>>>     '-mno-mmx -mno-sse' parameters.
>>>>
>>>> The eternal problem with this kind of workaround is that we never know
>>>> when it becomes safe to remove.
>>>>
>>>> It would be nice to understand the exact details around the problem.
>>>> Given that the commit is from 2010, I assume the issue is difficult to
>>>> reproduce with recent components (KVM, grub2). On the other hand, if
>> we
>>>> just remove the flags (which we should, in a perfect world), someone
>>>> could report a bug in three months: "my host distro upgraded the OVMF
>>>> package to the next edk2-stableYYYYMM tag, and now my virtual machine,
>>>> which runs a distro from 2009, no longer boots". (We've seen similar
>>>> reports before.)
>>>
>>> Yes. I agree it is hard to decide to remove this option, because we don't
>> know its impact.
>>> With the option -mno-mmx -mno-sse, it will cause CLANG9 linker failure like
>> below. So, I say
>>> this patch is to support the different linker.
>>>
>>> 0.      Program arguments: C:\Program Files\LLVM\bin\lld-link.EXE
>> /OUT:d:\allpkg\edk2\Build\Ovmf3264\DEBUG_CLANG9\X64\Se
>>>
>> curityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDx
>> e\DEBUG\SecureBootConfigDxe.dll /NOLOGO /NODEFAULT
>>> LIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32
>> /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT
>>> RY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER
>> /SAFESEH:NO /BASE:0 /DEBUG:GHASH /lldmap @d:\allpkg\edk2\Build\O
>>>
>> vmf3264\DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBo
>> otConfigDxe\SecureBootConfigDxe\OUTPUT\static_library
>>> _files.lst
>>> 1.      Running pass 'Function Pass Manager' on module 'ld-temp.o'.
>>> 2.      Running pass 'X86 FP Stackifier' on function '@drbg_add'
>>>  #0 0x00007ff696bad7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x148d7f8
>> C:\Program Files\LLVM\bin\lld-link.EXE 0x142ba7f
>>
>> can you please try the following:
>>
>> (a) replace this patch with two patches, as follows
>>
>> (b) in the first patch, only rework
>>
>> !if $(TOOL_CHAIN_TAG) != "XCODE5"
>>  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>> !endif
>>
>> into:
>>
>>  GCC:*_*_*_CC_FLAGS                   = -mno-mmx -mno-sse
>>  XCODE:*_*_*_CC_FLAGS                 =
>>
>> (c) in the second patch, append:
>>
>>  CLANGPE:*_*_*_CC_FLAGS               =
>>
> This way clear CLANG9 CC flags. It forbids CLANG9 to inherit other GCC flags. 
> Below flags are not applied into CLANG9. The purpose is not to include 
> specific -mno-mmx -mno-sse option, and still inherit other GCC flags. 
> So, I have to use $(TOOL_CHAIN_TAG) check. 
> 
>   MSFT:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES
>   INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES
>   GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> 
>> and also introduce
>>
>>  CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096
>>
>>
>> If (b) preserves the intended effect for the XCODE5 toolchain, and (c)
>> does the right thing for the CLANG9 toolchain as well, then that
>> approach would be preferable to the currently proposed patch#11.
>>
>> If the above does *not* work, then I'm fine with the currently proposed
>> patch, but then please update the commit message; it should say that
>> "-mno-mmx -mno-sse" is *excluded* for CLANG9.
>>
> Yes. I will update the commit message. 

OK. With the commit message updated, for this patch:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

For later, please consider the GENUINE_GCC suggestion (for
BUILDRULEFAMILY) in my other message in the present thread:

http://mid.mail-archive.com/43b303eb-fd74-7480-aa6d-6907300af3e0@redhat.com
https://edk2.groups.io/g/devel/message/48808

If you think that the "BaseTools/Conf/tools_def.template" patch that I
suggested there is acceptable, then I would like to update the OVMF DSC
files (later) as follows:

  GENUINE_GCC:*_*_*_CC_FLAGS              = -mno-mmx -mno-sse
  CLANG:*_*_*_CC_FLAGS                    = -mno-mmx -mno-sse
  /* say nothing about XCODE */
  /* say nothing CLANGPE */

Thanks
Laszlo

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Jordan Justen]

On 2019-10-10 09:32:19, Laszlo Ersek wrote:
> Hi Liming, Andrew,
> 
> On 10/10/19 14:32, Liming Gao wrote:
> > Laszlo:
> > 
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Wednesday, October 9, 2019 4:22 AM
> >> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; afish@apple.com
> >> Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions
> >>
> >> On 10/08/19 16:47, Gao, Liming wrote:
> >>
> >>>     [Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and
> >>>     XCODE5.
> >>>
> >>>     I don’t know the specific reason about __inline__. If there is no
> >>>     impact on
> >>>
> >>>     other GCC tool chain, I prefer to remove them.
> >>
> >>> [Liming] This seems the remaining clean up task. So, I prefer to remove
> >>> __inline__ if no impact on GCC tool chain.
> >>
> >> OK. Given your testing with GCC48, I'm fine.
> >>
> > With this patch set, I verify GCC48/GCC49/GCC5 on Ovmf3264. They can all boot to Shell. 
> > Are they enough?
> 
> Would you guys agree with the following commit message, on this patch?
> 
> """
> __inline__ has no discernible effect with the GCC48 / GCC49 / GCC5
> toolchains, but it breaks the build with CLANG9.
> 
> Remove __inline__.
> """

Would it be more accurate to say it didn't have a functional
difference? Did we rule out that it might have made a difference in
code gen?

I guess I wouldn't be surprised if the older non-LTO toolchains didn't
make use of it anyway. So, maybe there is no difference in code gen.

With LTO the linker is hopefully smarter about auto-inlining anyhow.

-Jordan

Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove __inline__ attribute for IO functions

[Liming Gao]

Jordan:

>-----Original Message-----
>From: Justen, Jordan L
>Sent: Saturday, October 12, 2019 3:22 AM
>To: afish@apple.com; Laszlo Ersek <lersek@redhat.com>;
>devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic: Remove
>__inline__ attribute for IO functions
>
>On 2019-10-10 09:32:19, Laszlo Ersek wrote:
>> Hi Liming, Andrew,
>>
>> On 10/10/19 14:32, Liming Gao wrote:
>> > Laszlo:
>> >
>> >> -----Original Message-----
>> >> From: Laszlo Ersek <lersek@redhat.com>
>> >> Sent: Wednesday, October 9, 2019 4:22 AM
>> >> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io;
>afish@apple.com
>> >> Subject: Re: [edk2-devel] [Patch 05/12] MdePkg BaseIoLibIntrinsic:
>Remove __inline__ attribute for IO functions
>> >>
>> >> On 10/08/19 16:47, Gao, Liming wrote:
>> >>
>> >>>     [Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and
>> >>>     XCODE5.
>> >>>
>> >>>     I don’t know the specific reason about __inline__. If there is no
>> >>>     impact on
>> >>>
>> >>>     other GCC tool chain, I prefer to remove them.
>> >>
>> >>> [Liming] This seems the remaining clean up task. So, I prefer to remove
>> >>> __inline__ if no impact on GCC tool chain.
>> >>
>> >> OK. Given your testing with GCC48, I'm fine.
>> >>
>> > With this patch set, I verify GCC48/GCC49/GCC5 on Ovmf3264. They can all
>boot to Shell.
>> > Are they enough?
>>
>> Would you guys agree with the following commit message, on this patch?
>>
>> """
>> __inline__ has no discernible effect with the GCC48 / GCC49 / GCC5
>> toolchains, but it breaks the build with CLANG9.
>>
>> Remove __inline__.
>> """
>
>Would it be more accurate to say it didn't have a functional
>difference? Did we rule out that it might have made a difference in
>code gen?

Yes. The function are same. I verify GCC5 before and after this change. 
The generated image is also same. So, there is no different code gen. 

Thanks
Liming
>
>I guess I wouldn't be surprised if the older non-LTO toolchains didn't
>make use of it anyway. So, maybe there is no difference in code gen.
>
>With LTO the linker is hopefully smarter about auto-inlining anyhow.
>
>-Jordan

Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

[Liming Gao]

Laszlo:

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Laszlo Ersek
>Sent: Friday, October 11, 2019 5:37 PM
>To: Gao, Liming <liming.gao@intel.com>
>Cc: Andrew Fish <afish@apple.com>; devel@edk2.groups.io; Tom Lendacky
><thomas.lendacky@amd.com>
>Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain
>
>Hi Liming,
>
>On 10/09/19 16:44, Gao, Liming wrote:
>
>> The difference between XCODE/CLANG and GCCXX is the linker. Current
>> patches are introduced for the different linker. Clang supports most
>> usage of GCC compiler. So, CLANG and XCODE uses GCC family. When I
>> enable XCODE or CLANG tool chain in the platform, I don't find other
>> incompatible behavior with GCC compiler. So, I think it is safe to let
>> CLANG inherit GCC option except for these two cases. When you make new
>> changes, and verify them with GCC compiler, that's enough. You don't
>> need to specially verify them with XCODE or CLANG. In most case, GCC
>> compiler pass, XCODE or CLANG can also pass.
>
>I'd like to provide a counter-example for this.
>
>Consider the issue that Tom reported, at
>
>  http://mid.mail-archive.com/8eb55d97-0ba3-c217-a160-
>c24730b9f036@amd.com
>  https://edk2.groups.io/g/devel/message/48762
>
>in point (2).
>
>(Both links point to the same message, just in different archives.)
>
>Let me summarize that problem.
>
>- In BZ#849, an XCODE toolchain bug was reported, and a workaround was
>  requested.
>
>- The workaround was commit 2db0ccc2d7fe ("UefiCpuPkg: Update
>  CpuExceptionHandlerLib pass XCODE5 tool chain", 2018-01-16).
>
>- The workaround is binary patching (self-modification) in the exception
>  handler's assembly code.
>
>- Unfortunately, this code is also linked into SEC modules, which run
>  from flash. Therefore, self-modification is not permitted, and the
>  workaround is not good enough for SEC. (Nor for PEI modules that run
>  from flash.)
>
>Now, let's consider how we could mitigate this issue for *temporarily*,
>for all toolchains *except* XCODE5, until the issue is fixed. Clearly,
>toolchains other than XCODE5 have no problems with the original assembly
>code (i.e., with the 64-bit absolute address relocations). Therefore, we
>could consider reverting the commit, and capturing the results of the
>revert in a *separate* NASM source file. This source file (that is, the
>original, pre-2db0ccc2d7fe assembly code) would apply to all toolchain
>families, except the XCODE family.
>
>Let's say the new NASM source files would be called
>- X64/ExceptionHandlerAsmGeneric.nasm -- all except XCODE,
>- X64/ExceptionHandlerAsmXcode.nasm -- XCODE,
>
>replacing the current file
>
>- X64/ExceptionHandlerAsm.nasm
>
>So, how can we use the new files, in the INF file
>
>
>UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.i
>nf
>
>?
>
>We could attempt,
>
>> [Sources.X64]
>>   X64/ExceptionHandlerAsmGeneric.nasm | GCC
>>   X64/ExceptionHandlerAsmXcode.nasm   | XCODE
>>   X64/ExceptionHandlerAsmGeneric.nasm | INTEL
>>   X64/ExceptionHandlerAsmGeneric.nasm | MSFT
>
>Unfortunately, this does not work.
>
>While it solves the issue on GCC, INTEL and MSFT, it breaks on XCODE:
>XCODE picks up *both* the GCC line and the XCODE line. And that's
>because XCODE inherits GCC, and there is presently no way to express
>"real GCC only".
>
>But, at least, this suggests a solution too. In
>"BaseTools/Conf/tools_def.template", for *every* toolchain that
>specifies FAMILY=GCC, we should spell out an explicit BUILDRULEFAMILY.
>Like this:
>
>> @@ -1995,6 +1995,7 @@
>>  #
>>
>###########################################################
>#########################
>>  *_GCC48_*_*_FAMILY               = GCC
>> +*_GCC48_*_*_BUILDRULEFAMILY      = GENUINE_GCC
>>
>>  *_GCC48_*_MAKE_PATH                    = DEF(GCC_HOST_PREFIX)make
>>  *_GCC48_*_*_DLL                        = ENV(GCC48_DLL)
>> @@ -2134,6 +2135,7 @@
>>  #
>>
>###########################################################
>#########################
>>  *_GCC49_*_*_FAMILY               = GCC
>> +*_GCC49_*_*_BUILDRULEFAMILY      = GENUINE_GCC
>>
>>  *_GCC49_*_MAKE_PATH                    = DEF(GCC_HOST_PREFIX)make
>>  *_GCC49_*_*_DLL                        = ENV(GCC49_DLL)
>> @@ -2280,6 +2282,7 @@
>>  #
>>
>###########################################################
>#########################
>>  *_GCC5_*_*_FAMILY                = GCC
>> +*_GCC5_*_*_BUILDRULEFAMILY       = GENUINE_GCC
>>
>>  *_GCC5_*_MAKE_PATH               = DEF(GCC_HOST_PREFIX)make
>>  *_GCC5_*_*_DLL                   = ENV(GCC5_DLL)
>> @@ -2437,6 +2440,7 @@
>>  #
>>
>###########################################################
>#########################
>>  *_CLANG35_*_*_FAMILY             = GCC
>> +*_CLANG35_*_*_BUILDRULEFAMILY    = CLANG
>>
>>  *_CLANG35_*_MAKE_PATH            = make
>>  *_CLANG35_*_*_DLL                = ENV(CLANG35_DLL)
>> @@ -2517,6 +2521,8 @@
>>  #
>>
>###########################################################
>#########################
>>  *_CLANG38_*_*_FAMILY                = GCC
>> +*_CLANG38_*_*_BUILDRULEFAMILY       = CLANG
>> +
>>  *_CLANG38_*_MAKE_PATH               = make
>>  *_CLANG38_*_*_DLL                   = ENV(CLANG38_DLL)
>>  *_CLANG38_*_ASL_PATH                = DEF(UNIX_IASL_BIN)
>
>And then we could write:
>
>> [Sources.X64]
>>   X64/ExceptionHandlerAsmGeneric.nasm | GENUINE_GCC
>>   X64/ExceptionHandlerAsmGeneric.nasm | CLANG
>>   X64/ExceptionHandlerAsmXcode.nasm   | XCODE
>>   X64/ExceptionHandlerAsmGeneric.nasm | INTEL
>>   X64/ExceptionHandlerAsmGeneric.nasm | MSFT
>
>replacing plain "GCC", which XCODE inherits, with "GENUINE_GCC" and
>"CLANG", neither of which XCODE inherits.
>
>Would you accept the above patch, for
>"BaseTools/Conf/tools_def.template"?
>

This way makes other tool chain work, but doesn't resolve XCODE issue in X64 SEC/PEI. 
I reply the mail thread and collect Andrew feedback on this issue. 

Liming

>Thanks,
>Laszlo
>
>