From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by mx.groups.io with SMTP id smtpd.web11.11066.1580989471974215866 for ; Thu, 06 Feb 2020 03:44:32 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@protonmail.com header.s=default header.b=N5II11lF; spf=pass (domain: protonmail.com, ip: 185.70.40.134, mailfrom: vit9696@protonmail.com) Date: Thu, 06 Feb 2020 11:44:22 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1580989466; bh=mIY33CkCkH3BNcnJiggfrV4oYqp4EHdYgD1n07nxwNQ=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References: Feedback-ID:From; b=N5II11lFcFMy4vdB9iPodg5huybvE1wwZUe6y1oyh2FchOoPyMR3M6lXLmRdQcaDm tL1SRsapM7gAT0R3o5A8qkZJqvfvWJ75tp3dt27sAU85SxLUB+WtCH5A47F7ay0BRc yg0mvraX0xU+Oh7QpSEpHWgPbgdahAuLXz8zNgD4= To: "Gao, Liming" From: "Vitaly Cheptsov" Cc: "devel@edk2.groups.io" , =?UTF-8?Q?Marvin_H=C3=A4user?= , Laszlo Ersek , "Kinney, Michael D" Reply-To: vit9696 Subject: Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler Message-ID: <994404A5-EFA4-4FF6-8CC0-67F194981846@protonmail.com> In-Reply-To: <0a4770a12ca148c4a94ab9263803726c@intel.com> References: <20200131211705.18511-1-vit9696@protonmail.com> <76f4dee34c9c4cf4b876dcd1c6776a73@intel.com> <71ZwJ4XFqzI7MI5QFePibWms3vLacIwHqgcxeat52NYQy-iNvZ6eiEqEYlgrJaYaVNUlYZ5sRm_CuOvlkHuUUs8_sqsgGg1rMpA1flr8OIs=@protonmail.com> <6c48ddd40a904bed967f45a6394f2b77@intel.com> <4C1625AC-3669-4ADE-BC33-DAD8DF05528F@protonmail.com> <0a4770a12ca148c4a94ab9263803726c@intel.com> Feedback-ID: p9QuX-L1wMgUm6nrSvNrf8juLupNs0VSnzXGVXuYDxlEahFdWtaedWDMB9zpwGDklGt7kzs1-RBc0cqz327Gcg==:Ext:ProtonMail MIME-Version: 1.0 X-Spam-Status: No, score=-0.7 required=7.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,FREEMAIL_REPLYTO_END_DIGIT,HTML_FONT_LOW_CONTRAST, HTML_MESSAGE shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mail.protonmail.ch X-Groupsio-MsgNum: 53860 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha256; boundary="---------------------a3e042a4faa04421e4eeffb8c8ffbd34"; charset=UTF-8 -----------------------a3e042a4faa04421e4eeffb8c8ffbd34 Cc: "devel@edk2.groups.io" , =?utf-8?Q?Marvin_H=C3=A4user?= , Laszlo Ersek , "Kinney, Michael D" Content-Type: multipart/alternative; boundary="Apple-Mail=_A253AAF1-928B-4BD4-9412-B6CEB407A10E" Date: Thu, 6 Feb 2020 14:43:17 +0300 From: vit9696 In-Reply-To: <0a4770a12ca148c4a94ab9263803726c@intel.com> Message-Id: <994404A5-EFA4-4FF6-8CC0-67F194981846@protonmail.com> Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) References: <20200131211705.18511-1-vit9696@protonmail.com> <76f4dee34c9c4cf4b876dcd1c6776a73@intel.com> <71ZwJ4XFqzI7MI5QFePibWms3vLacIwHqgcxeat52NYQy-iNvZ6eiEqEYlgrJaYaVNUlYZ5sRm_CuOvlkHuUUs8_sqsgGg1rMpA1flr8OIs=@protonmail.com> <6c48ddd40a904bed967f45a6394f2b77@intel.com> <4C1625AC-3669-4ADE-BC33-DAD8DF05528F@protonmail.com> <0a4770a12ca148c4a94ab9263803726c@intel.com> Subject: Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compiler To: "Gao, Liming" X-Mailer: Apple Mail (2.3608.60.0.2.5) --Apple-Mail=_A253AAF1-928B-4BD4-9412-B6CEB407A10E Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Liming, Correct, I believe it is not just Base.h but several other files. I plan t= o 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 thi= s as I do not have the hardware for validation. Best wishes, Vitaly > 6 =D1=84=D0=B5=D0=B2=D1=80. 2020 =D0=B3., =D0=B2 11:22, Gao, Liming =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >=20 > Vitaly: > We also find _MSC_VER is defined in Windows, but not in Linux. Your an= alysis explains it. When use i686-unknown-windows-gnu option, __GNUC__ macr= o will be defined. If so, we don=E2=80=99t need to append the check for def= ined (__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__) > =C3=A8 > #if defined (__GNUC__) > > Thanks > Liming > From: devel@edk2.groups.io > On Behalf Of Vitaly Cheptsov via Grou= ps.Io > Sent: Thursday, February 6, 2020 8:17 AM > To: Gao, Liming > > Cc: devel@edk2.groups.io ; Marvin H=C3=A4us= er >; Laszlo= Ersek >; Kinney, Michael D > > Subject: Re: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC com= piler > > 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 mean= s 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 ins= talled Visual Studio version. For example, for me it is implicitly passing = -fms-compatibility-version=3D19.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 re= sult the full triple will be x86_64-unknown-windows-msvc with _MSC_VER macr= o 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 ins= tallations. > > 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 U= INT32: > > 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 b= e used in structures. ANSI C integer promotion rules make INT32 or UINT32 i= nterchangeable when passed as an argument to a function. > > However, since I am not positive that no existing code relies on this, i= t is best to preserve the current behaviour. Supporting this is valid for G= NU flavour or as a Microsoft Extension. Disabling -fms-compatibility will r= esult 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 -targ= et argument due to a misconsideration of the clang triple implementation. N= ormally 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 re= solve the problem, we should use GNU targets i686-unknown-windows-gnu and x= 86_64-unknown-windows-gnu. This is basically the only and the least hurtful= solution, as using MSVC mode will define _MSC_EXTENSIONS, which already br= eaks many places and will require a heavy codebase refactoring, and randoml= y define _MSC_VER and use Visual Studio headers and configuration, which ma= kes reproducible builds on different operating systems questionable if not = impossible. >=20 >=20 > I will submit another patch that will replace this one later this week. = In addition to GNU targets I additionally pass -nostdlib and -nostdlibinc s= o that a freestanding target is used and only builtin headers are accessibl= e (like stdint.h, stddef.h, and stdbool.h). This is not required but an ext= ra safety measure. Our initial validation of the changes found no issues wi= th 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=3D2397 > > >=20 >=20 > 4 =D1=84=D0=B5=D0=B2=D1=80. 2020 =D0=B3., =D0=B2 09:56, Gao, Liming > =D0=BD=D0=B0=D0=BF=D0=B8= =D1=81=D0=B0=D0=BB(=D0=B0): > > 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 >= =20 > Sent: Monday, February 3, 2020 7:29 PM > To: Gao, Liming >; de= vel@edk2.groups.io ; Marvin H=C3=A4user > > Subject: RE: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC com= piler > > 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_VE= R, but for now just ensuring that any toolchain by MSVC does not define _MS= C_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 a= nd XCODE5. We may still have to update some preprocessor conditions to incl= ude __clang__ near __GNUC__ if __GNUC__ is left undefined even when we remo= ve -fms-compatibility, but that sounds fine for me. > > We will investigate that on our own and submit a new patch, but help fro= m other parties is strongly appreciated. We mostly use XCODE5 and are not w= ell aware of other platforms. > > Best wishes, > Vitaly > > On Mon, Feb 3, 2020 at 11:00, Gao, Liming > wrote: > Vitaly: > This change will cause CryptoPkg openssl build failure, because OpensslL= ib.inf undefines _MSC_VER macro to make sure openssl source code be built i= n edk2 project without using MS VS functions. >=20 > Now, I have no good solution to fix this issue. One way is to use define= d(_MSC_EXTENSIONS) && !defined(__clang__), another way is to investigate wh= ether we can remove -fms-compatibility option in CLANGPDB. >=20 > Thanks > Liming > > -----Original Message----- > > From: devel@edk2.groups.io > On Behalf Of Vitaly Cheptsov via Gr= oups.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 compi= ler > > > > Patch details are explained in https://bugzilla.tianocore.org/show_bug= .cgi?id=3D2397 . > > 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) > > > > > > -=3D-=3D-=3D-=3D-=3D-=3D > > Groups.io Links: You receive all messages sent to = this group. > > > > View/Reply Online (#53623): https://edk2.groups.io/g/devel/message/536= 23 > > 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 ] > > -=3D-=3D-=3D-=3D-=3D-=3D >=20 > >=20 --Apple-Mail=_A253AAF1-928B-4BD4-9412-B6CEB407A10E Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Liming,
Correct, I believe it is not just Base.h= but several other files. I plan to include the removal of __clang__ refere= nces in my patchset as well, since after the target change all the use of c= lang 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 wor= k on this as I do not have the hardware for validation.
=
Best wishes,
Vitaly

6 =D1=84=D0=B5= =D0=B2=D1=80. 2020 =D0=B3., =D0=B2 11:22, Gao, Liming <liming.gao@intel.com> =D0=BD=D0= =B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):

Vitaly:
  We also find _MSC_VER is = defined in Windows, but not in Linux. Your analysis explains it. When use i= 686-unknown-windows-gnu option, __GNUC__ macro will be defined. If so, we d= on=E2=80=99t need to append the check for defined (__clang__) in Base.h. An= d, this change can remove -fno-ms-extensions and -fms-compatibility option.= Then, CLANGPDB can keep the same behavior in Windows/Linux/MacOs host OS.<= o:p class=3D"">
&nbs= p;
#if defined (__GNUC__) || defined= (__clang__)
=C3=A8
#if defined (__GNUC__)
 
=
Thanks
Liming
From: = ;devel@edk2.groups.io <devel= @edk2.groups.io> <= b class=3D"">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.grou= ps.io; Marvin H=C3=A4user <marvin= .haeuser@outlook.com>; Laszlo Ersek <le= rsek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subje= ct: Re: [edk2-devel] = [PATCH 0/1] Use _MSC_VER to determine MSVC compiler
 
Liming,
&n= bsp;
We perfor= med the initial exploration of CLANGPDB toolchain issue on our end and beli= eve we can suggest a solid solution.
 
In addition to all the issu= es I mentioned in the BZ[1] there are several more.
 = ;
1. CLANGPDB = uses -target x86_64-unknown-windows, and this basically means different beh= aviour for Windows and other operating systems:
=
- On Windows it will "att= ach" to installed Visual Studio and will gather the parameters from this in= stallation, i.e. it will define _MSC_VER to installed Visual Studio version= . For example, for me it is implicitly passing -fms-compatibility-version= =3D19.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_V= ER macro not being defined.
=  
There basically is no control to it except -U_MSC_VER, which is ug= ly, as different include directories, other defines will still happen betwe= en installations.
 
2. EDK II relies on UINT32_MAX being a valid v= alue for enum. This is not the case in the specification, as it requires en= um to be either INT32 or UINT32:
 
=
Element of a standard ANSI C en= um 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 integ= er promotion rules make INT32 or UINT32 interchangeable when passed as an a= rgument 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. Dis= abling -fms-compatibility will result in a compile error for enums having 0= xFFFFFFFF values, like in Base.h.
 
All in all, we believe that CL= ANGPDB simply has an overlook in the -target argument due to a misconsidera= tion of the clang triple implementation. Normally for target only 3 words a= re provided, but for Windows it is crucial to have 4, as there are differen= t drivers with different automatics. To resolve the problem, we should use = GNU targets i686-unknown-windows-gnu and x86_64-unknown-windows-gnu. <= span style=3D"" class=3D"">This is basically the only and the least hurtful= solution, as using MSVC mode will define _MSC_EXTENSIONS, which already br= eaks many places and will require a heavy codebase refactoring, and randoml= y define _MSC_VER and use Visual Studio headers and configuration, which ma= kes reproducible builds on different operating systems questionable if= not impossible.


I will submit another patch that will replace this one l= ater this week. In addition to GNU targets I additionally pass -nostdlib an= d -nostdlibinc so that a freestanding target is used and only builtin heade= rs are accessible (like stdint.h, stddef.h, and stdbool.h). This is not req= uired but an extra safety measure. Our initial validation of the changes fo= und 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 th= eir 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=3D2397
 <= /o:p>
 


4 =D1= = =84=D0=B5=D0=B2=D1=80. 2020 =D0=B3., =D0=B2 09:56, Gao, Liming <liming.gao@intel.com> =D0=BD=D0=B0=D0=BF=D0= =B8=D1=81=D0=B0=D0=BB(=D0=B0):
 
Vitaly:
  Yes. I think we shou= ld have better solution in OpenSSL to support EDK2 usage. But, this is a lo= ng term solution. For now, I will try the solution to remove -fms-compatibi= lity option in CLANGPDB tool chain.
 
Thanks=
Liming
F= rom: vit9696 <vit9696@protonmail.com> 
Sent: Monday, February 3, 2020 7:2= 9 PM
To: Gao, Liming <liming.gao@int= el.com>; devel@edk2.groups.io; Marvin H=C3=A4user <marvin.haeuser@outlook.com>
Subject:&nbs= p;RE: [edk2-devel] [PATCH 0/1] Use _MSC_VER to determine MSVC compil= er
 
=
Liming,
 
I believe it is reasonable to fix OpenSSL, but most like= ly 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 doe= s not define _MSC_EXTENSIONS should work too.
 
I believe that -fms-compatibility option is not ne= eded 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&n= bsp;investigate that on our own and submit a new patch, but help from = other parties is strongly appreciated. We mostly use XCODE5 and are no= t well aware of other platforms.
 
Best wishes,
Vitaly
=
 
On Mon, Feb 3, 2= 020 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.

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

Thanks
Liming
> -----Origina= l Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly C= heptsov 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/sh= ow_bug.cgi?id=3D2397.
> We request this to be m= erged in edk2-stable202002.
>
> Vitaly Ch= eptsov (1):
> MdePkg: Use _MSC_VER to determine MSVC compi= ler
>
> MdePkg/Include/AArch64/ProcessorB= ind.h | 2 +-
> MdePkg/Include/Arm/ProcessorBind.h | 8 ++++= ----
> MdePkg/Include/Base.h | 10 +++++-----
> MdePkg/Include/Ia32/ProcessorBind.h | 6 +++---
> Mde= Pkg/Include/X64/ProcessorBind.h | 6 +++---
> 5 files chang= ed, 16 insertions(+), 16 deletions(-)
>
>= --
> 2.21.1 (Apple Git-122.3)
>
>
> -=3D-=3D-=3D-=3D-=3D-=3D
><= span class=3D"Apple-converted-space"> Groups= .io Links: You receiv= e all messages sent to this group.
>
> Vi= ew/Reply Online (#53623): https://edk2.groups.io/g/devel/message/53623
> Mute This Topic: <= /span>https://groups.io/mt/70882954/1759384
= > Group Owner: deve= l+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/d= evel/unsub [liming.gao@intel.com]
> -= =3D-=3D-=3D-=3D-=3D-=3D

=
=  
=

--Apple-Mail=_A253AAF1-928B-4BD4-9412-B6CEB407A10E-- -----------------------a3e042a4faa04421e4eeffb8c8ffbd34 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: ProtonMail wsBmBAEBCAAQBQJeO/wSCRBPsoxt7Hy0xQAKCRBPsoxt7Hy0xTNiB/9qaFbH hLdYHXXpQuQAr2fraIM3lhU2AxYUlDPmoA30Nk8MzwJoA2QvNyJWMlmSGeye HEcjyCaF3kt2PloLz8awyR4xTqBnKxV83FYLnmvhJT047Anc5zZRLluEs3r6 KhkfOK45xHtXjCac7G7RXnYxgUpukXDyqcyWVdmh4L2iKx4CwK80hQsr1Jbs 4i07zISy7YBmnrwBvMDKFrl+hGdUoM1/KpxOcuXKnsfr7H2IlFVPszSDpaPo Ss1Xmqeb/Q2JaxPoMWg+mbLK/Mk2+Rr5G1aNQe54gmXWmQpzRHjlZ8tm5d5a qMZQGbfkandHMAakEM0NX35C7zGJWIgsCSiJ =Q3pY -----END PGP SIGNATURE----- -----------------------a3e042a4faa04421e4eeffb8c8ffbd34--