[PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[Vitaly Cheptsov]

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

Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397.
We request this to be merged in edk2-stable202002.

Vitaly Cheptsov (1):
  MdePkg: Use _MSC_VER to determine MSVC compiler

 MdePkg/Include/AArch64/ProcessorBind.h |  2 +-
 MdePkg/Include/Arm/ProcessorBind.h     |  8 ++++----
 MdePkg/Include/Base.h                  | 10 +++++-----
 MdePkg/Include/Ia32/ProcessorBind.h    |  6 +++---
 MdePkg/Include/X64/ProcessorBind.h     |  6 +++---
 5 files changed, 16 insertions(+), 16 deletions(-)

-- 
2.21.1 (Apple Git-122.3)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

[PATCH 1/1] MdePkg: Use _MSC_VER to determine MSVC compiler

[Vitaly Cheptsov]

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

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

CLANGPDB toolchain, implemented by clang compiler, will also
define currently used _MSC_EXTENSIONS macro, which does not
behave correctly in many situations as explained in the report.

Signed-off-by: Vitaly Cheptsov <vit9696@protonmail.com>
---
 MdePkg/Include/AArch64/ProcessorBind.h |  2 +-
 MdePkg/Include/Arm/ProcessorBind.h     |  8 ++++----
 MdePkg/Include/Base.h                  | 10 +++++-----
 MdePkg/Include/Ia32/ProcessorBind.h    |  6 +++---
 MdePkg/Include/X64/ProcessorBind.h     |  6 +++---
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
index 896bf273ac..fe6b28ec9c 100644
--- a/MdePkg/Include/AArch64/ProcessorBind.h
+++ b/MdePkg/Include/AArch64/ProcessorBind.h
@@ -24,7 +24,7 @@
 #pragma pack()
 #endif
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
 
 //
 // Disable some level 4 compilation warnings (same as IA32 and X64)
diff --git a/MdePkg/Include/Arm/ProcessorBind.h b/MdePkg/Include/Arm/ProcessorBind.h
index 1264b44b46..f5a5969d4a 100644
--- a/MdePkg/Include/Arm/ProcessorBind.h
+++ b/MdePkg/Include/Arm/ProcessorBind.h
@@ -22,7 +22,7 @@
 #pragma pack()
 #endif
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
 
 //
 // Disable some level 4 compilation warnings (same as IA32 and X64)
@@ -74,11 +74,11 @@
 //
 // RVCT and MSFT don't support the __builtin_unreachable() macro
 //
-#if defined(__ARMCC_VERSION) || defined(_MSC_EXTENSIONS)
+#if defined(__ARMCC_VERSION) || defined(_MSC_VER)
 #define UNREACHABLE()
 #endif
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
   //
   // use Microsoft* C compiler dependent integer width types
   //
@@ -212,7 +212,7 @@ typedef INT32   INTN;
     #define GCC_ASM_IMPORT(name)
 
   #endif
-#elif defined(_MSC_EXTENSIONS)
+#elif defined(_MSC_VER)
   //
   // PRESERVE8 is not supported by the MSFT assembler.
   //
diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 321d729c04..bda630a2dc 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -21,7 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 #include <ProcessorBind.h>
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
 //
 // Disable warning when last field of data structure is a zero sized array.
 //
@@ -33,7 +33,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //  if the /OPT:REF linker option is used. We defined a macro as this is a
 //  a non standard extension
 //
-#if defined(_MSC_EXTENSIONS) && _MSC_VER < 1800 && !defined (MDE_CPU_EBC)
+#if defined(_MSC_VER) && _MSC_VER < 1800 && !defined (MDE_CPU_EBC)
   ///
   /// Remove global variable from the linked image if there are no references to
   /// it after all compiler and linker optimizations have been performed.
@@ -92,7 +92,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
     /// flagged with this attribute.
     ///
     #define NORETURN  __attribute__((noreturn))
-  #elif defined(_MSC_EXTENSIONS) && !defined(MDE_CPU_EBC)
+  #elif defined(_MSC_VER) && !defined(MDE_CPU_EBC)
     ///
     /// Signal compilers and analyzers that the function cannot return.
     /// It is up to the compiler to remove any code past a call to functions
@@ -799,7 +799,7 @@ typedef UINTN  *BASE_LIST;
 **/
 #ifdef MDE_CPU_EBC
   #define STATIC_ASSERT(Expression, Message)
-#elif defined(_MSC_EXTENSIONS)
+#elif defined(_MSC_VER)
   #define STATIC_ASSERT static_assert
 #else
   #define STATIC_ASSERT _Static_assert
@@ -1256,7 +1256,7 @@ typedef UINTN RETURN_STATUS;
 #define SIGNATURE_64(A, B, C, D, E, F, G, H) \
     (SIGNATURE_32 (A, B, C, D) | ((UINT64) (SIGNATURE_32 (E, F, G, H)) << 32))
 
-#if defined(_MSC_EXTENSIONS) && !defined (__INTEL_COMPILER) && !defined (MDE_CPU_EBC)
+#if defined(_MSC_VER) && !defined (__INTEL_COMPILER) && !defined (MDE_CPU_EBC)
   void * _ReturnAddress(void);
   #pragma intrinsic(_ReturnAddress)
   /**
diff --git a/MdePkg/Include/Ia32/ProcessorBind.h b/MdePkg/Include/Ia32/ProcessorBind.h
index fa4b7e8e98..bf6a8b09f5 100644
--- a/MdePkg/Include/Ia32/ProcessorBind.h
+++ b/MdePkg/Include/Ia32/ProcessorBind.h
@@ -49,7 +49,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #endif
 
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
 
 //
 // Disable warning that make it impossible to compile at /W4
@@ -110,7 +110,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #endif
 
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
 
   //
   // use Microsoft C compiler dependent integer width types
@@ -276,7 +276,7 @@ typedef INT32   INTN;
   ///
   /// If EFIAPI is already defined, then we use that definition.
   ///
-#elif defined(_MSC_EXTENSIONS)
+#elif defined(_MSC_VER)
   ///
   /// Microsoft* compiler specific method for EFIAPI calling convention.
   ///
diff --git a/MdePkg/Include/X64/ProcessorBind.h b/MdePkg/Include/X64/ProcessorBind.h
index 387e9c5c9c..2e8ac6c8d5 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -63,7 +63,7 @@
 #endif
 
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
 
 //
 // Disable warning that make it impossible to compile at /W4
@@ -124,7 +124,7 @@
 #endif
 
 
-#if defined(_MSC_EXTENSIONS)
+#if defined(_MSC_VER)
   //
   // use Microsoft C compiler dependent integer width types
   //
@@ -290,7 +290,7 @@ typedef INT64   INTN;
   ///
   /// If EFIAPI is already defined, then we use that definition.
   ///
-#elif defined(_MSC_EXTENSIONS)
+#elif defined(_MSC_VER)
   ///
   /// Microsoft* compiler specific method for EFIAPI calling convention.
   ///
-- 
2.21.1 (Apple Git-122.3)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[Liming Gao]

Vitaly:
  This change will cause CryptoPkg openssl build failure, because OpensslLib.inf undefines _MSC_VER macro to make sure openssl source code be built in edk2 project without using MS VS functions.
  
  Now, I have no good solution to fix this issue. One way is to use defined(_MSC_EXTENSIONS) && !defined(__clang__), another way is to investigate whether we can remove -fms-compatibility option in CLANGPDB.

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly Cheptsov via Groups.Io
> Sent: Saturday, February 1, 2020 5:17 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
> 
> Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397.
> We request this to be merged in edk2-stable202002.
> 
> Vitaly Cheptsov (1):
>   MdePkg: Use _MSC_VER to determine MSVC compiler
> 
>  MdePkg/Include/AArch64/ProcessorBind.h |  2 +-
>  MdePkg/Include/Arm/ProcessorBind.h     |  8 ++++----
>  MdePkg/Include/Base.h                  | 10 +++++-----
>  MdePkg/Include/Ia32/ProcessorBind.h    |  6 +++---
>  MdePkg/Include/X64/ProcessorBind.h     |  6 +++---
>  5 files changed, 16 insertions(+), 16 deletions(-)
> 
> --
> 2.21.1 (Apple Git-122.3)
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623
> Mute This Topic: https://groups.io/mt/70882954/1759384
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com]
> -=-=-=-=-=-=

Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[Vitaly Cheptsov]

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

Liming,

I believe it is reasonable to fix OpenSSL, but most likely it is faster to address EDK II code at first. In future we should still stick to _MSC_VER, but for now just ensuring that any toolchain by MSVC does not define _MSC_EXTENSIONS should work too.

I believe that -fms-compatibility option is not needed for CLANGPDB, as CLANGPDB is literally using clang, and that worked fine before in CLANG38 and XCODE5. We may still have to update some preprocessor conditions to include __clang__ near __GNUC__ if __GNUC__ is left undefined even when we remove -fms-compatibility, but that sounds fine for me.

We will investigate that on our own and submit a new patch, but help from other parties is strongly appreciated. We mostly use XCODE5 and are not well aware of other platforms.

Best wishes,
Vitaly

On Mon, Feb 3, 2020 at 11:00, Gao, Liming <liming.gao@intel.com> wrote:

> Vitaly:
> This change will cause CryptoPkg openssl build failure, because OpensslLib.inf undefines _MSC_VER macro to make sure openssl source code be built in edk2 project without using MS VS functions.
>
> Now, I have no good solution to fix this issue. One way is to use defined(_MSC_EXTENSIONS) && !defined(__clang__), another way is to investigate whether we can remove -fms-compatibility option in CLANGPDB.
>
> Thanks
> Liming
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly Cheptsov via Groups.Io
>> Sent: Saturday, February 1, 2020 5:17 AM
>> To: devel@edk2.groups.io
>> Subject: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
>>
>> Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397.
>> We request this to be merged in edk2-stable202002.
>>
>> Vitaly Cheptsov (1):
>> MdePkg: Use _MSC_VER to determine MSVC compiler
>>
>> MdePkg/Include/AArch64/ProcessorBind.h | 2 +-
>> MdePkg/Include/Arm/ProcessorBind.h | 8 ++++----
>> MdePkg/Include/Base.h | 10 +++++-----
>> MdePkg/Include/Ia32/ProcessorBind.h | 6 +++---
>> MdePkg/Include/X64/ProcessorBind.h | 6 +++---
>> 5 files changed, 16 insertions(+), 16 deletions(-)
>>
>> --
>> 2.21.1 (Apple Git-122.3)
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>>
>> View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623
>> Mute This Topic: https://groups.io/mt/70882954/1759384
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming.gao@intel.com]
>> -=-=-=-=-=-=

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

Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[Liming Gao]

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

Vitaly:
  Yes. I think we should have better solution in OpenSSL to support EDK2 usage. But, this is a long term solution. For now, I will try the solution to remove -fms-compatibility option in CLANGPDB tool chain.

Thanks
Liming
From: vit9696 <vit9696@protonmail.com>
Sent: Monday, February 3, 2020 7:29 PM
To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; Marvin Häuser <marvin.haeuser@outlook.com>
Subject: RE: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

Liming,

I believe it is reasonable to fix OpenSSL, but most likely it is faster to address EDK II code at first. In future we should still stick to _MSC_VER, but for now just ensuring that any toolchain by MSVC does not define _MSC_EXTENSIONS should work too.

I believe that -fms-compatibility option is not needed for CLANGPDB, as CLANGPDB is literally using clang, and that worked fine before in CLANG38 and XCODE5. We may still have to update some preprocessor conditions to include __clang__ near __GNUC__ if __GNUC__ is left undefined even when we remove -fms-compatibility, but that sounds fine for me.

We will investigate that on our own and submit a new patch, but help from other parties is strongly appreciated. We mostly use XCODE5 and are not well aware of other platforms.

Best wishes,
Vitaly

On Mon, Feb 3, 2020 at 11:00, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
Vitaly:
This change will cause CryptoPkg openssl build failure, because OpensslLib.inf undefines _MSC_VER macro to make sure openssl source code be built in edk2 project without using MS VS functions.

Now, I have no good solution to fix this issue. One way is to use defined(_MSC_EXTENSIONS) && !defined(__clang__), another way is to investigate whether we can remove -fms-compatibility option in CLANGPDB.

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Vitaly Cheptsov via Groups.Io
> Sent: Saturday, February 1, 2020 5:17 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Subject: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
>
> Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397.
> We request this to be merged in edk2-stable202002.
>
> Vitaly Cheptsov (1):
> MdePkg: Use _MSC_VER to determine MSVC compiler
>
> MdePkg/Include/AArch64/ProcessorBind.h | 2 +-
> MdePkg/Include/Arm/ProcessorBind.h | 8 ++++----
> MdePkg/Include/Base.h | 10 +++++-----
> MdePkg/Include/Ia32/ProcessorBind.h | 6 +++---
> MdePkg/Include/X64/ProcessorBind.h | 6 +++---
> 5 files changed, 16 insertions(+), 16 deletions(-)
>
> --
> 2.21.1 (Apple Git-122.3)
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
>
> View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623
> Mute This Topic: https://groups.io/mt/70882954/1759384
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming.gao@intel.com]
> -=-=-=-=-=-=



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

Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[Vitaly Cheptsov]

[-- Attachment #1.1: Type: text/plain, Size: 13602 bytes --]

Liming,

We performed the initial exploration of CLANGPDB toolchain issue on our end and believe we can suggest a solid solution.

In addition to all the issues I mentioned in the BZ[1] there are several more.

1. CLANGPDB uses -target x86_64-unknown-windows, and this basically means different behaviour for Windows and other operating systems:
- On Windows it will "attach" to installed Visual Studio and will gather the parameters from this installation, i.e. it will define _MSC_VER to installed Visual Studio version. For example, for me it is implicitly passing -fms-compatibility-version=19.16.27026 and setting full triple to x86_64-unknown-windows-msvc19.16.27026.
- On Mac and Linux it will obviously not find Visual Studio, and as a result the full triple will be x86_64-unknown-windows-msvc with _MSC_VER macro not being defined.

There basically is no control to it except -U_MSC_VER, which is ugly, as different include directories, other defines will still happen between installations.

2. EDK II relies on UINT32_MAX being a valid value for enum. This is not the case in the specification, as it requires enum to be either INT32 or UINT32:

Element of a standard ANSI C enum type declaration. Type INT32.or UINT32. ANSI C does not define the size of sign of an enum so they should never be used in structures. ANSI C integer promotion rules make INT32 or UINT32 interchangeable when passed as an argument to a function.

However, since I am not positive that no existing code relies on this, it is best to preserve the current behaviour. Supporting this is valid for GNU flavour or as a Microsoft Extension. Disabling -fms-compatibility will result in a compile error for enums having 0xFFFFFFFF values, like in Base.h.

All in all, we believe that CLANGPDB simply has an overlook in the -target argument due to a misconsideration of the clang triple implementation. Normally for target only 3 words are provided, but for Windows it is crucial to have 4, as there are different drivers with different automatics. To resolve the problem, we should use GNU targets i686-unknown-windows-gnu and x86_64-unknown-windows-gnu. This is basically the only and the least hurtful solution, as using MSVC mode will define _MSC_EXTENSIONS, which already breaks many places and will require a heavy codebase refactoring, and randomly define _MSC_VER and use Visual Studio headers and configuration, which makes reproducible builds on different operating systems questionable if not impossible.

I will submit another patch that will replace this one later this week. In addition to GNU targets I additionally pass -nostdlib and -nostdlibinc so that a freestanding target is used and only builtin headers are accessible (like stdint.h, stddef.h, and stdbool.h). This is not required but an extra safety measure. Our initial validation of the changes found no issues with our projects. We also performed building of most common EDK II packages like CryptoPkg, MdePkg, and MdeModulePkg. While the change is fairly minor, I will still request others to perform a brief check of their packages in case they want to use CLANGPDB. For a quick test I include the diff of the patch beforehand.

Best wishes,
Vitaly

[1] https://bugzilla.tianocore.org/show_bug.cgi?id=2397




> 4 февр. 2020 г., в 09:56, Gao, Liming <liming.gao@intel.com> написал(а):
> 
> Vitaly:
>   Yes. I think we should have better solution in OpenSSL to support EDK2 usage. But, this is a long term solution. For now, I will try the solution to remove -fms-compatibility option in CLANGPDB tool chain.
>
> Thanks
> Liming
> From: vit9696 <vit9696@protonmail.com> 
> Sent: Monday, February 3, 2020 7:29 PM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; Marvin Häuser <marvin.haeuser@outlook.com>
> Subject: RE: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
>
> Liming,
>
> I believe it is reasonable to fix OpenSSL, but most likely it is faster to address EDK II code at first. In future we should still stick to _MSC_VER, but for now just ensuring that any toolchain by MSVC does not define _MSC_EXTENSIONS should work too.
>
> I believe that -fms-compatibility option is not needed for CLANGPDB, as CLANGPDB is literally using clang, and that worked fine before in CLANG38 and XCODE5. We may still have to update some preprocessor conditions to include __clang__ near __GNUC__ if __GNUC__ is left undefined even when we remove -fms-compatibility, but that sounds fine for me.
>
> We will investigate that on our own and submit a new patch, but help from other parties is strongly appreciated. We mostly use XCODE5 and are not well aware of other platforms.
>
> Best wishes,
> Vitaly
>
> On Mon, Feb 3, 2020 at 11:00, Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>> wrote:
> Vitaly:
> This change will cause CryptoPkg openssl build failure, because OpensslLib.inf undefines _MSC_VER macro to make sure openssl source code be built in edk2 project without using MS VS functions.
> 
> Now, I have no good solution to fix this issue. One way is to use defined(_MSC_EXTENSIONS) && !defined(__clang__), another way is to investigate whether we can remove -fms-compatibility option in CLANGPDB.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Vitaly Cheptsov via Groups.Io
> > Sent: Saturday, February 1, 2020 5:17 AM
> > To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > Subject: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
> >
> > Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397 <https://bugzilla.tianocore.org/show_bug.cgi?id=2397>.
> > We request this to be merged in edk2-stable202002.
> >
> > Vitaly Cheptsov (1):
> > MdePkg: Use _MSC_VER to determine MSVC compiler
> >
> > MdePkg/Include/AArch64/ProcessorBind.h | 2 +-
> > MdePkg/Include/Arm/ProcessorBind.h | 8 ++++----
> > MdePkg/Include/Base.h | 10 +++++-----
> > MdePkg/Include/Ia32/ProcessorBind.h | 6 +++---
> > MdePkg/Include/X64/ProcessorBind.h | 6 +++---
> > 5 files changed, 16 insertions(+), 16 deletions(-)
> >
> > --
> > 2.21.1 (Apple Git-122.3)
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> >
> > View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623 <https://edk2.groups.io/g/devel/message/53623>
> > Mute This Topic: https://groups.io/mt/70882954/1759384 <https://groups.io/mt/70882954/1759384>
> > Group Owner: devel+owner@edk2.groups.io <mailto:devel+owner@edk2.groups.io>
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub <https://edk2.groups.io/g/devel/unsub> [liming.gao@intel.com]
> > -=-=-=-=-=-=
> 

Liming,

We performed the initial exploration of CLANGPDB toolchain issue on our end and believe we can suggest a solid solution.

In addition to all the issues I mentioned in the BZ[1] there are several more.

1. CLANGPDB uses -target x86_64-unknown-windows, and this basically means different behaviour for Windows and other operating systems:
- On Windows it will "attach" to installed Visual Studio and will gather the parameters from this installation, i.e. it will define _MSC_VER to installed Visual Studio version. For example, for me it is implicitly passing -fms-compatibility-version=19.16.27026 and setting full triple to x86_64-unknown-windows-msvc19.16.27026.
- On Mac and Linux it will obviously not find Visual Studio, and as a result the full triple will be x86_64-unknown-windows-msvc with _MSC_VER macro not being defined.

There basically is no control to it except -U_MSC_VER, which is ugly, as different include directories, other defines will still happen between installations.

2. EDK II relies on UINT32_MAX being a valid value for enum. This is not the case in the specification, as it requires enum to be either INT32 or UINT32:

Element of a standard ANSI C enum type declaration. Type INT32.or UINT32. ANSI C does not define the size of sign of an enum so they should never be used in structures. ANSI C integer promotion rules make INT32 or UINT32 interchangeable when passed as an argument to a function.

However, since I am not positive that no existing code relies on this, it is best to preserve the current behaviour. Supporting this is valid for GNU flavour or as a Microsoft Extension. Disabling -fms-compatibility will result in a compile error for enums having 0xFFFFFFFF values, like in Base.h.

All in all, we believe that CLANGPDB simply has an overlook in the -target argument due to a misconsideration of the clang triple implementation. Normally for target only 3 words are provided, but for Windows it is crucial to have 4, as there are different drivers with different automatics. To resolve the problem, we should use GNU targets i686-unknown-windows-gnu and x86_64-unknown-windows-gnu. This is basically the only and the least hurtful solution, as using MSVC mode will define _MSC_EXTENSIONS, which already breaks many places and will require a heavy codebase refactoring, and randomly define _MSC_VER and use Visual Studio headers and configuration, which makes reproducible builds on different operating systems questionable if not impossible.

I will submit another patch that will replace this one later this week. In addition to GNU targets I additionally pass -nostdlib and -nostdlibinc so that a freestanding target is used and only builtin headers are accessible (like stdint.h, stddef.h, and stdbool.h). This is not required but an extra safety measure. Our initial validation of the changes found no issues with our projects. We also performed building of most common EDK II packages like CryptoPkg, MdePkg, and MdeModulePkg. While the change is fairly minor, I will still request others to perform a brief check of their packages in case they want to use CLANGPDB. For a quick test I include the diff of the patch beforehand.

Best wishes,
Vitaly

[1] https://bugzilla.tianocore.org/show_bug.cgi?id=2397> 
> 4 февр. 2020 г., в 09:56, Gao, Liming < liming.gao@intel.com > написал(а):
> 
> 
> Vitaly:
> Yes. I think we should have better solution in OpenSSL to support EDK2
> usage. But, this is a long term solution. For now, I will try the solution
> to remove -fms-compatibility option in CLANGPDB tool chain.
> 
> Thanks
> Liming
> *From:* vit9696 < vit9696@protonmail.com >
> *Sent:* Monday, February 3, 2020 7:29 PM
> *To:* Gao, Liming < liming.gao@intel.com >; devel@edk2.groups.io ; Marvin
> Häuser < marvin.haeuser@outlook.com >
> *Subject:* RE: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC
> compiler
> 
> Liming,
> 
> I believe it is reasonable to fix OpenSSL, but most likely it is faster to
> address EDK II code at first. In future we should still stick to _MSC_VER,
> but for now just ensuring that any toolchain by MSVC does not define
> _MSC_EXTENSIONS should work too.
> 
> I believe that -fms-compatibility option is not needed for CLANGPDB, as
> CLANGPDB is literally using clang, and that worked fine before in CLANG38
> and XCODE5. We may still have to update some preprocessor conditions to
> include __clang__ near __GNUC__ if __GNUC__ is left undefined even when we
> remove -fms-compatibility, but that sounds fine for me.
> 
> We will investigate that on our own and submit a new patch, but help from
> other parties is strongly appreciated. We mostly use XCODE5 and are not
> well aware of other platforms.
> 
> Best wishes,
> Vitaly
> 
> On Mon, Feb 3, 2020 at 11:00, Gao, Liming < liming.gao@intel.com > wrote:
> 
>> 
>> 
>> Vitaly:
>> This change will cause CryptoPkg openssl build failure, because
>> OpensslLib.inf undefines _MSC_VER macro to make sure openssl source code
>> be built in edk2 project without using MS VS functions.
>> 
>> Now, I have no good solution to fix this issue. One way is to use
>> defined(_MSC_EXTENSIONS) && !defined(__clang__), another way is to
>> investigate whether we can remove -fms-compatibility option in CLANGPDB.
>> 
>> Thanks
>> Liming
>> > -----Original Message-----
>> > From: devel@edk2.groups.io < devel@edk2.groups.io > On Behalf Of Vitaly
>> Cheptsov via Groups.Io
>> > Sent: Saturday, February 1, 2020 5:17 AM
>> > To: devel@edk2.groups.io
>> > Subject: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC
>> compiler
>> >
>> > Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397
>> .
>> > We request this to be merged in edk2-stable202002.
>> >
>> > Vitaly Cheptsov (1):
>> > MdePkg: Use _MSC_VER to determine MSVC compiler
>> >
>> > MdePkg/Include/AArch64/ProcessorBind.h | 2 +-
>> > MdePkg/Include/Arm/ProcessorBind.h | 8 ++++----
>> > MdePkg/Include/Base.h | 10 +++++-----
>> > MdePkg/Include/Ia32/ProcessorBind.h | 6 +++---
>> > MdePkg/Include/X64/ProcessorBind.h | 6 +++---
>> > 5 files changed, 16 insertions(+), 16 deletions(-)
>> >
>> > --
>> > 2.21.1 (Apple Git-122.3)
>> >
>> >
>> > -=-=-=-=-=-=
>> > Groups.io ( http://Groups.io ) Links: You receive all messages sent to
>> this group.
>> >
>> > View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623
>> 
>> > Mute This Topic: https://groups.io/mt/70882954/1759384
>> > Group Owner: devel+owner@edk2.groups.io
>> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ liming.gao@intel.com
>> ]
>> > -=-=-=-=-=-=
>> 
>> 
> 
> 
>

[-- Attachment #1.2.1.1: Type: text/html, Size: 14342 bytes --]

[-- Attachment #1.2.2: clangpdb.diff --]
[-- Type: application/octet-stream, Size: 1931 bytes --]

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index feee2bbf16..6bf6c5768e 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -2755,11 +2755,11 @@ RELEASE_CLANG38_AARCH64_DLINK_FLAGS = DEF(CLANG38_AARCH64_DLINK_FLAGS) -flto -Wl
 DEFINE CLANGPDB_IA32_PREFIX          = ENV(CLANG_BIN)
 DEFINE CLANGPDB_X64_PREFIX           = ENV(CLANG_BIN)
 
-DEFINE CLANGPDB_IA32_TARGET          = -target i686-unknown-windows
-DEFINE CLANGPDB_X64_TARGET           = -target x86_64-unknown-windows
+DEFINE CLANGPDB_IA32_TARGET          = -target i686-unknown-windows-gnu
+DEFINE CLANGPDB_X64_TARGET           = -target x86_64-unknown-windows-gnu
 
 DEFINE CLANGPDB_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 CLANGPDB_ALL_CC_FLAGS         = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANGPDB_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
+DEFINE CLANGPDB_ALL_CC_FLAGS         = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANGPDB_WARNING_OVERRIDES) -fno-stack-protector -fno-asynchronous-unwind-tables -funsigned-char -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -Wno-null-dereference -mno-implicit-float -mms-bitfields -mno-stack-arg-probe -nostdlib -nostdlibinc
 
 ###########################
 # CLANGPDB IA32 definitions

[-- Attachment #1.2.3: Type: multipart/alternative, Size: 1 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[Liming Gao]

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

Vitaly:
  We also find _MSC_VER is defined in Windows, but not in Linux. Your analysis explains it. When use i686-unknown-windows-gnu option, __GNUC__ macro will be defined. If so, we don’t need to append the check for defined (__clang__) in Base.h. And, this change can remove -fno-ms-extensions and -fms-compatibility option. Then, CLANGPDB can keep the same behavior in Windows/Linux/MacOs host OS.

#if defined (__GNUC__) || defined (__clang__)
==>
#if defined (__GNUC__)

Thanks
Liming
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly Cheptsov via Groups.Io
Sent: Thursday, February 6, 2020 8:17 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: devel@edk2.groups.io; Marvin Häuser <marvin.haeuser@outlook.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

Liming,

We performed the initial exploration of CLANGPDB toolchain issue on our end and believe we can suggest a solid solution.

In addition to all the issues I mentioned in the BZ[1] there are several more.

1. CLANGPDB uses -target x86_64-unknown-windows, and this basically means different behaviour for Windows and other operating systems:
- On Windows it will "attach" to installed Visual Studio and will gather the parameters from this installation, i.e. it will define _MSC_VER to installed Visual Studio version. For example, for me it is implicitly passing -fms-compatibility-version=19.16.27026 and setting full triple to x86_64-unknown-windows-msvc19.16.27026.
- On Mac and Linux it will obviously not find Visual Studio, and as a result the full triple will be x86_64-unknown-windows-msvc with _MSC_VER macro not being defined.

There basically is no control to it except -U_MSC_VER, which is ugly, as different include directories, other defines will still happen between installations.

2. EDK II relies on UINT32_MAX being a valid value for enum. This is not the case in the specification, as it requires enum to be either INT32 or UINT32:

Element of a standard ANSI C enum type declaration. Type INT32.or UINT32. ANSI C does not define the size of sign of an enum so they should never be used in structures. ANSI C integer promotion rules make INT32 or UINT32 interchangeable when passed as an argument to a function.

However, since I am not positive that no existing code relies on this, it is best to preserve the current behaviour. Supporting this is valid for GNU flavour or as a Microsoft Extension. Disabling -fms-compatibility will result in a compile error for enums having 0xFFFFFFFF values, like in Base.h.

All in all, we believe that CLANGPDB simply has an overlook in the -target argument due to a misconsideration of the clang triple implementation. Normally for target only 3 words are provided, but for Windows it is crucial to have 4, as there are different drivers with different automatics. To resolve the problem, we should use GNU targets i686-unknown-windows-gnu and x86_64-unknown-windows-gnu. This is basically the only and the least hurtful solution, as using MSVC mode will define _MSC_EXTENSIONS, which already breaks many places and will require a heavy codebase refactoring, and randomly define _MSC_VER and use Visual Studio headers and configuration, which makes reproducible builds on different operating systems questionable if not impossible.


I will submit another patch that will replace this one later this week. In addition to GNU targets I additionally pass -nostdlib and -nostdlibinc so that a freestanding target is used and only builtin headers are accessible (like stdint.h, stddef.h, and stdbool.h). This is not required but an extra safety measure. Our initial validation of the changes found no issues with our projects. We also performed building of most common EDK II packages like CryptoPkg, MdePkg, and MdeModulePkg. While the change is fairly minor, I will still request others to perform a brief check of their packages in case they want to use CLANGPDB. For a quick test I include the diff of the patch beforehand.

Best wishes,
Vitaly

[1] https://bugzilla.tianocore.org/show_bug.cgi?id=2397




4 февр. 2020 г., в 09:56, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> написал(а):

Vitaly:
  Yes. I think we should have better solution in OpenSSL to support EDK2 usage. But, this is a long term solution. For now, I will try the solution to remove -fms-compatibility option in CLANGPDB tool chain.

Thanks
Liming
From: vit9696 <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Sent: Monday, February 3, 2020 7:29 PM
To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Marvin Häuser <marvin.haeuser@outlook.com<mailto:marvin.haeuser@outlook.com>>
Subject: RE: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

Liming,

I believe it is reasonable to fix OpenSSL, but most likely it is faster to address EDK II code at first. In future we should still stick to _MSC_VER, but for now just ensuring that any toolchain by MSVC does not define _MSC_EXTENSIONS should work too.

I believe that -fms-compatibility option is not needed for CLANGPDB, as CLANGPDB is literally using clang, and that worked fine before in CLANG38 and XCODE5. We may still have to update some preprocessor conditions to include __clang__ near __GNUC__ if __GNUC__ is left undefined even when we remove -fms-compatibility, but that sounds fine for me.

We will investigate that on our own and submit a new patch, but help from other parties is strongly appreciated. We mostly use XCODE5 and are not well aware of other platforms.

Best wishes,
Vitaly

On Mon, Feb 3, 2020 at 11:00, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
Vitaly:
This change will cause CryptoPkg openssl build failure, because OpensslLib.inf undefines _MSC_VER macro to make sure openssl source code be built in edk2 project without using MS VS functions.

Now, I have no good solution to fix this issue. One way is to use defined(_MSC_EXTENSIONS) && !defined(__clang__), another way is to investigate whether we can remove -fms-compatibility option in CLANGPDB.

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Vitaly Cheptsov via Groups.Io
> Sent: Saturday, February 1, 2020 5:17 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Subject: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
>
> Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397.
> We request this to be merged in edk2-stable202002.
>
> Vitaly Cheptsov (1):
> MdePkg: Use _MSC_VER to determine MSVC compiler
>
> MdePkg/Include/AArch64/ProcessorBind.h | 2 +-
> MdePkg/Include/Arm/ProcessorBind.h | 8 ++++----
> MdePkg/Include/Base.h | 10 +++++-----
> MdePkg/Include/Ia32/ProcessorBind.h | 6 +++---
> MdePkg/Include/X64/ProcessorBind.h | 6 +++---
> 5 files changed, 16 insertions(+), 16 deletions(-)
>
> --
> 2.21.1 (Apple Git-122.3)
>
>
> -=-=-=-=-=-=
> Groups.io<http://Groups.io> Links: You receive all messages sent to this group.
>
> View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623
> Mute This Topic: https://groups.io/mt/70882954/1759384
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming.gao@intel.com<mailto:liming.gao@intel.com>]
> -=-=-=-=-=-=



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

Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler

[Vitaly Cheptsov]

[-- Attachment #1.1: Type: text/plain, Size: 8894 bytes --]

Liming,

Correct, I believe it is not just Base.h but several other files. I plan to include the removal of __clang__ references in my patchset as well, since after the target change all the use of clang will be in GNU mode.

In addition to that, I believe that in GNU mode it should be also possible to support ARM and AARCH64 in CLANGPDB, but I would rather not work on this as I do not have the hardware for validation.

Best wishes,
Vitaly

> 6 февр. 2020 г., в 11:22, Gao, Liming <liming.gao@intel.com> написал(а):
> 
> Vitaly:
>   We also find _MSC_VER is defined in Windows, but not in Linux. Your analysis explains it. When use i686-unknown-windows-gnu option, __GNUC__ macro will be defined. If so, we don’t need to append the check for defined (__clang__) in Base.h. And, this change can remove -fno-ms-extensions and -fms-compatibility option. Then, CLANGPDB can keep the same behavior in Windows/Linux/MacOs host OS.
>
> #if defined (__GNUC__) || defined (__clang__)
> è
> #if defined (__GNUC__)
>
> Thanks
> Liming
> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Vitaly Cheptsov via Groups.Io
> Sent: Thursday, February 6, 2020 8:17 AM
> To: Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>
> Cc: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Marvin Häuser <marvin.haeuser@outlook.com <mailto:marvin.haeuser@outlook.com>>; Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> Subject: Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
>
> Liming,
>
> We performed the initial exploration of CLANGPDB toolchain issue on our end and believe we can suggest a solid solution.
>
> In addition to all the issues I mentioned in the BZ[1] there are several more.
>
> 1. CLANGPDB uses -target x86_64-unknown-windows, and this basically means different behaviour for Windows and other operating systems:
> - On Windows it will "attach" to installed Visual Studio and will gather the parameters from this installation, i.e. it will define _MSC_VER to installed Visual Studio version. For example, for me it is implicitly passing -fms-compatibility-version=19.16.27026 and setting full triple to x86_64-unknown-windows-msvc19.16.27026.
> - On Mac and Linux it will obviously not find Visual Studio, and as a result the full triple will be x86_64-unknown-windows-msvc with _MSC_VER macro not being defined.
>
> There basically is no control to it except -U_MSC_VER, which is ugly, as different include directories, other defines will still happen between installations.
>
> 2. EDK II relies on UINT32_MAX being a valid value for enum. This is not the case in the specification, as it requires enum to be either INT32 or UINT32:
>
> Element of a standard ANSI C enum type declaration. Type INT32.or UINT32. ANSI C does not define the size of sign of an enum so they should never be used in structures. ANSI C integer promotion rules make INT32 or UINT32 interchangeable when passed as an argument to a function.
>
> However, since I am not positive that no existing code relies on this, it is best to preserve the current behaviour. Supporting this is valid for GNU flavour or as a Microsoft Extension. Disabling -fms-compatibility will result in a compile error for enums having 0xFFFFFFFF values, like in Base.h.
>
> All in all, we believe that CLANGPDB simply has an overlook in the -target argument due to a misconsideration of the clang triple implementation. Normally for target only 3 words are provided, but for Windows it is crucial to have 4, as there are different drivers with different automatics. To resolve the problem, we should use GNU targets i686-unknown-windows-gnu and x86_64-unknown-windows-gnu. This is basically the only and the least hurtful solution, as using MSVC mode will define _MSC_EXTENSIONS, which already breaks many places and will require a heavy codebase refactoring, and randomly define _MSC_VER and use Visual Studio headers and configuration, which makes reproducible builds on different operating systems questionable if not impossible.
> 
> 
> I will submit another patch that will replace this one later this week. In addition to GNU targets I additionally pass -nostdlib and -nostdlibinc so that a freestanding target is used and only builtin headers are accessible (like stdint.h, stddef.h, and stdbool.h). This is not required but an extra safety measure. Our initial validation of the changes found no issues with our projects. We also performed building of most common EDK II packages like CryptoPkg, MdePkg, and MdeModulePkg. While the change is fairly minor, I will still request others to perform a brief check of their packages in case they want to use CLANGPDB. For a quick test I include the diff of the patch beforehand.
>
> Best wishes,
> Vitaly
>
> [1] https://bugzilla.tianocore.org/show_bug.cgi?id=2397 <https://bugzilla.tianocore.org/show_bug.cgi?id=2397>
>
>
> 
> 
> 4 февр. 2020 г., в 09:56, Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>> написал(а):
>
> Vitaly:
>   Yes. I think we should have better solution in OpenSSL to support EDK2 usage. But, this is a long term solution. For now, I will try the solution to remove -fms-compatibility option in CLANGPDB tool chain.
>
> Thanks
> Liming
> From: vit9696 <vit9696@protonmail.com <mailto:vit9696@protonmail.com>> 
> Sent: Monday, February 3, 2020 7:29 PM
> To: Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Marvin Häuser <marvin.haeuser@outlook.com <mailto:marvin.haeuser@outlook.com>>
> Subject: RE: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
>
> Liming,
>
> I believe it is reasonable to fix OpenSSL, but most likely it is faster to address EDK II code at first. In future we should still stick to _MSC_VER, but for now just ensuring that any toolchain by MSVC does not define _MSC_EXTENSIONS should work too.
>
> I believe that -fms-compatibility option is not needed for CLANGPDB, as CLANGPDB is literally using clang, and that worked fine before in CLANG38 and XCODE5. We may still have to update some preprocessor conditions to include __clang__ near __GNUC__ if __GNUC__ is left undefined even when we remove -fms-compatibility, but that sounds fine for me.
>
> We will investigate that on our own and submit a new patch, but help from other parties is strongly appreciated. We mostly use XCODE5 and are not well aware of other platforms.
>
> Best wishes,
> Vitaly
>
> On Mon, Feb 3, 2020 at 11:00, Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>> wrote:
> Vitaly:
> This change will cause CryptoPkg openssl build failure, because OpensslLib.inf undefines _MSC_VER macro to make sure openssl source code be built in edk2 project without using MS VS functions.
> 
> Now, I have no good solution to fix this issue. One way is to use defined(_MSC_EXTENSIONS) && !defined(__clang__), another way is to investigate whether we can remove -fms-compatibility option in CLANGPDB.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Vitaly Cheptsov via Groups.Io
> > Sent: Saturday, February 1, 2020 5:17 AM
> > To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > Subject: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
> >
> > Patch details are explained in https://bugzilla.tianocore.org/show_bug.cgi?id=2397 <https://bugzilla.tianocore.org/show_bug.cgi?id=2397>.
> > We request this to be merged in edk2-stable202002.
> >
> > Vitaly Cheptsov (1):
> > MdePkg: Use _MSC_VER to determine MSVC compiler
> >
> > MdePkg/Include/AArch64/ProcessorBind.h | 2 +-
> > MdePkg/Include/Arm/ProcessorBind.h | 8 ++++----
> > MdePkg/Include/Base.h | 10 +++++-----
> > MdePkg/Include/Ia32/ProcessorBind.h | 6 +++---
> > MdePkg/Include/X64/ProcessorBind.h | 6 +++---
> > 5 files changed, 16 insertions(+), 16 deletions(-)
> >
> > --
> > 2.21.1 (Apple Git-122.3)
> >
> >
> > -=-=-=-=-=-=
> > Groups.io <http://groups.io/> Links: You receive all messages sent to this group.
> >
> > View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623 <https://edk2.groups.io/g/devel/message/53623>
> > Mute This Topic: https://groups.io/mt/70882954/1759384 <https://groups.io/mt/70882954/1759384>
> > Group Owner: devel+owner@edk2.groups.io <mailto:devel+owner@edk2.groups.io>
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub <https://edk2.groups.io/g/devel/unsub> [liming.gao@intel.com <mailto:liming.gao@intel.com>]
> > -=-=-=-=-=-=
> 
>
> 


[-- Attachment #1.2: Type: text/html, Size: 23905 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]