Re: [edk2-devel] [Patch v2 10/11] EmulatorPkg: Enable CLANG9 tool chain

["Liming Gao" <liming.gao@intel.com>]

Ray:
 
>-----Original Message-----
>From: Ni, Ray
>Sent: Wednesday, October 16, 2019 5:32 PM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>Cc: 'Andrew Fish (afish@apple.com)' <afish@apple.com>
>Subject: RE: [edk2-devel] [Patch v2 10/11] EmulatorPkg: Enable CLANG9 tool
>chain
>
>Liming,
>You missed the Cc tag so Andrew and I were not CCed.
I CC you in the cover letter, not single patch. I will add you in next version. 

>
>3 comments in below.
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming
>> Gao
>> Sent: Tuesday, October 15, 2019 8:27 AM
>> To: devel@edk2.groups.io
>> Subject: [edk2-devel] [Patch v2 10/11] EmulatorPkg: Enable CLANG9 tool
>> chain
>>
>> 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);
>
>1. Why move the assignment to a later place? Can you explain a bit more?
The hang issue here. 

I investigate this issue more. The root cause is that GetProcessAffinityMask() API
requires the 64bit pointer as the input for 64bit ARCH. So, the correct fix should declare 
ProcessAffinityMask and SystemAffinityMask as UINTN. I will update the patch. 

(GetProcessAffinityMask (GetCurrentProcess (), &ProcessAffinityMask, &SystemAffinityMask))

>
>>
>>    //
>>    // 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)
>
>2. If CLANG9 is used in Linux, WinHost is still used. That seems not good.
>
Now, CLANG9 is for Windows Emulator only. The key problem is that 
there is no way to know which OS is used in DSC. So for now, I will add the 
limitation here. For long term, we need to think the way to support Emulator usage.

>>      ##
>>      #  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)
>
>3. Why TftpDynamicCommand cannot be built with CLANG9?
>
CLANG9 supports RC section. I will update patch to remove this change. 

Thanks
Liming
>>
>> 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
>>
>>
>>