[edk2] [Patch 1/2] BaseTools: Add -D NO_MSABI_VARGS to X64 XCODE5 CC_FLAGS

Michael Kinney posted 2 patches 7 years, 7 months ago
[edk2] [Patch 1/2] BaseTools: Add -D NO_MSABI_VARGS to X64 XCODE5 CC_FLAGS
Posted by Michael Kinney 7 years, 7 months ago
https://bugzilla.tianocore.org/show_bug.cgi?id=561

Update BaseTools/Conf/tools_def.template to add the define

-D NO_MSABI_VAARGS

To CC_FLAGS for X64 XCODE5 builds.

The llvm/clang compiler used in XCODE5 builds supports the
_ms_ versions of the vararg builtins, but the compiler
generates build errors.

The recommendation from the XCODE5 experts is to never use
the _ms_ version of the vararg builtins.  The define
NO_MSABI_VARARGS is already supported in MdePkg/Include/Base.h
and forces the use the standard vararg builtins.

Cc: Andrew Fish <afish@apple.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 BaseTools/Conf/tools_def.template | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 427ef1b..ed9e834 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -7494,9 +7494,9 @@ RELEASE_XCODE5_X64_ASM_FLAGS  = -arch x86_64
 *_XCODE5_*_VFRPP_FLAGS      = -x c -E -P -DVFRCOMPILE -include $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h 
 
 
-  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
-  NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
-RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
+  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -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 -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
+RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
 
 *_XCODE5_*_ASLCC_FLAGS      = -x c -save-temps -g -O0 -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-missing-braces -c -include AutoGen.h 
 *_XCODE5_*_ASLDLINK_FLAGS   = -e _ReferenceAcpiTable -preload -segalign 0x20  -pie -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
-- 
2.6.3.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/2] BaseTools: Add -D NO_MSABI_VARGS to X64 XCODE5 CC_FLAGS
Posted by Ard Biesheuvel 7 years, 7 months ago
On 19 May 2017 at 07:52, Michael Kinney <michael.d.kinney@intel.com> wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=561
>
> Update BaseTools/Conf/tools_def.template to add the define
>
> -D NO_MSABI_VAARGS
>
> To CC_FLAGS for X64 XCODE5 builds.
>
> The llvm/clang compiler used in XCODE5 builds supports the
> _ms_ versions of the vararg builtins, but the compiler
> generates build errors.
>
> The recommendation from the XCODE5 experts is to never use
> the _ms_ version of the vararg builtins.  The define
> NO_MSABI_VARARGS is already supported in MdePkg/Include/Base.h
> and forces the use the standard vararg builtins.
>

Are such builds compliant with the UEFI spec?

> Cc: Andrew Fish <afish@apple.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  BaseTools/Conf/tools_def.template | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
> index 427ef1b..ed9e834 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -7494,9 +7494,9 @@ RELEASE_XCODE5_X64_ASM_FLAGS  = -arch x86_64
>  *_XCODE5_*_VFRPP_FLAGS      = -x c -E -P -DVFRCOMPILE -include $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
>
>
> -  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
> -  NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
> -RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
> +  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -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 -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
> +RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
>
>  *_XCODE5_*_ASLCC_FLAGS      = -x c -save-temps -g -O0 -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-missing-braces -c -include AutoGen.h
>  *_XCODE5_*_ASLDLINK_FLAGS   = -e _ReferenceAcpiTable -preload -segalign 0x20  -pie -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
> --
> 2.6.3.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/2] BaseTools: Add -D NO_MSABI_VARGS to X64 XCODE5 CC_FLAGS
Posted by Andrew Fish 7 years, 7 months ago
> On May 19, 2017, at 5:49 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> On 19 May 2017 at 07:52, Michael Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>> wrote:
>> https://bugzilla.tianocore.org/show_bug.cgi?id=561
>> 
>> Update BaseTools/Conf/tools_def.template to add the define
>> 
>> -D NO_MSABI_VAARGS
>> 
>> To CC_FLAGS for X64 XCODE5 builds.
>> 
>> The llvm/clang compiler used in XCODE5 builds supports the
>> _ms_ versions of the vararg builtins, but the compiler
>> generates build errors.
>> 
>> The recommendation from the XCODE5 experts is to never use
>> the _ms_ version of the vararg builtins.  The define
>> NO_MSABI_VARARGS is already supported in MdePkg/Include/Base.h
>> and forces the use the standard vararg builtins.
>> 
> 
> Are such builds compliant with the UEFI spec?
> 

Ard I think you are confusing implementation and architecture. The UEFI Spec quotes an ABI, and __builtin_ms_* is a non standard compiler extension. 

Clang was designed to be a cross compiler and for X64 (x86_64) we set the arch to clang via -target x86_64-pc-win32-macho. Basically this tells clang it is an x86_64 (X64) arch, with a Windows ABI (EFI ABI), and produce a Mach-O Executable (for debugging). So the default var args builtin does the right thing for the EFI ABI, and __builtin_ms_* is not supported. The x86_64-pc-win32-macho triple is only used for EFI and when I talked to the clang folks they were like why do you need __builtin_ms_* when it is the same as __builtin_*, please fix your include files. 

Thanks,

Andrew Fish

>> Cc: Andrew Fish <afish@apple.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
>> ---
>> BaseTools/Conf/tools_def.template | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
>> index 427ef1b..ed9e834 100755
>> --- a/BaseTools/Conf/tools_def.template
>> +++ b/BaseTools/Conf/tools_def.template
>> @@ -7494,9 +7494,9 @@ RELEASE_XCODE5_X64_ASM_FLAGS  = -arch x86_64
>> *_XCODE5_*_VFRPP_FLAGS      = -x c -E -P -DVFRCOMPILE -include $(DEST_DIR_DEBUG)/$(MODULE_NAME)StrDefs.h
>> 
>> 
>> -  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
>> -  NOOPT_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
>> -RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang $(PLATFORM_FLAGS)
>> +  DEBUG_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c -g -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 -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 -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
>> +RELEASE_XCODE5_X64_CC_FLAGS   = -target x86_64-pc-win32-macho -c    -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 -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -D NO_MSABI_VA_FUNCS $(PLATFORM_FLAGS)
>> 
>> *_XCODE5_*_ASLCC_FLAGS      = -x c -save-temps -g -O0 -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-missing-braces -c -include AutoGen.h
>> *_XCODE5_*_ASLDLINK_FLAGS   = -e _ReferenceAcpiTable -preload -segalign 0x20  -pie -seg1addr 0x240 -read_only_relocs suppress -map $(DEST_DIR_DEBUG)/$(BASE_NAME).map
>> --
>> 2.6.3.windows.1
>> 
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/2] BaseTools: Add -D NO_MSABI_VARGS to X64 XCODE5 CC_FLAGS
Posted by Ard Biesheuvel 7 years, 7 months ago
On 19 May 2017 at 17:46, Andrew Fish <afish@apple.com> wrote:
>
> On May 19, 2017, at 5:49 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
>
> On 19 May 2017 at 07:52, Michael Kinney <michael.d.kinney@intel.com> wrote:
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=561
>
> Update BaseTools/Conf/tools_def.template to add the define
>
> -D NO_MSABI_VAARGS
>
> To CC_FLAGS for X64 XCODE5 builds.
>
> The llvm/clang compiler used in XCODE5 builds supports the
> _ms_ versions of the vararg builtins, but the compiler
> generates build errors.
>
> The recommendation from the XCODE5 experts is to never use
> the _ms_ version of the vararg builtins.  The define
> NO_MSABI_VARARGS is already supported in MdePkg/Include/Base.h
> and forces the use the standard vararg builtins.
>
>
> Are such builds compliant with the UEFI spec?
>
>
> Ard I think you are confusing implementation and architecture. The UEFI Spec
> quotes an ABI, and __builtin_ms_* is a non standard compiler extension.
>
> Clang was designed to be a cross compiler and for X64 (x86_64) we set the
> arch to clang via -target x86_64-pc-win32-macho. Basically this tells clang
> it is an x86_64 (X64) arch, with a Windows ABI (EFI ABI), and produce a
> Mach-O Executable (for debugging). So the default var args builtin does the
> right thing for the EFI ABI, and __builtin_ms_* is not supported. The
> x86_64-pc-win32-macho triple is only used for EFI and when I talked to the
> clang folks they were like why do you need __builtin_ms_* when it is the
> same as __builtin_*, please fix your include files.
>

Thanks for clarifying. What confused me is the use of NO_MSABI_VAARGS,
which to a casual reader may seem to force a varargs ABI different
from the MS one, which is exactly what it is used for in libraries
such as OpensslLib IIRC.

In any case, if the clang target setting already fully defines the
varargs flavor of __builtin_va_xxx calls, then I suppose the resulting
code should be compliant. It may still be worthwhile to rename the
preprocessor macro to something more intuitive, but I won't insist.

Thanks,
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel