Re: [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in CLANG38 toolchain

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

Laszlo:
  This is a better way. The bug https://bugzilla.tianocore.org/show_bug.cgi?id=410 only lists GetBestLanguage() API. I am not sure whether they are similar cases in edk2. If have, we had better fix them together. 

Thanks
Liming
>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Monday, September 25, 2017 4:13 PM
>To: Gao, Liming <liming.gao@intel.com>; Marvin Häuser
><Marvin.Haeuser@outlook.com>; edk2-devel@lists.01.org
>Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in
>CLANG38 toolchain
>
>On 09/25/17 06:59, Gao, Liming wrote:
>> Marvin:
>>   My concern is that the fix is an incompatible change. It requires to modify
>library API. In fact, this is an undefined behavior. But, current compiler (VS,
>GCC and Clang) makes it work.  So, I prefer to keep the code as-is, and disable
>this warning first. If you find any real issue, we can return back and figure out
>the solution.
>
>I think we can draw a parallel here to GetVariable() and GetVariable2().
>GetVariable() is now unavailable if DISABLE_NEW_DEPRECATED_INTERFACES
>is
>defined.
>
>I think the right solution would be to
>- introduce GetBestLanguage2(),
>- migrate all the current call sites to GetBestLanguage2() -- I counted
>  18, and the updates should be easy --,
>- and then make GetVariable() conditional on
>  not-DISABLE_NEW_DEPRECATED_INTERFACES.
>
>
>CHAR8 *
>EFIAPI
>GetBestLanguage2 (
>  IN CONST CHAR8  *SupportedLanguages,
>  IN INTN         Iso639Language,
>  ...
>  );
>
>I don't feel strongly about this question, I just think technically this
>would be best. CLANG warnings are valuable.
>
>Thanks
>Laszlo
>
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>>> Marvin H?user
>>> Sent: Friday, September 22, 2017 11:53 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Gao, Liming <liming.gao@intel.com>
>>> Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40
>in
>>> CLANG38 toolchain
>>>
>>> Hey,
>>>
>>> I just noticed this patch as it recently has been pushed. I found this has
>been a
>>> reaction to https://bugzilla.tianocore.org/show_bug.cgi?id=410
>>> Though as Clang correctly detected, this is Undefined Behavior per the C
>>> specification, so why was the warning hidden?
>>> In context of the issue in UefiLib, providing the first element of the VA list
>as a
>>> prototyped argument, would have solved the issue without UB.
>>>
>>> Do you wish such a patch?
>>>
>>> Thanks,
>>> Marvin.
>>>
>>>> -----Original Message-----
>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>>>> Gao, Liming
>>>> Sent: Monday, August 28, 2017 9:19 AM
>>>> To: Shi, Steven <steven.shi@intel.com>; edk2-devel@lists.01.org
>>>> Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and
>LLVM40
>>> in
>>>> CLANG38 toolchain
>>>>
>>>> Reviewed-by: Liming Gao <liming.gao@intel.com>
>>>>
>>>>> -----Original Message-----
>>>>> From: Shi, Steven
>>>>> Sent: Wednesday, August 23, 2017 3:01 PM
>>>>> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
>>>>> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Shi, Steven
>>>>> <steven.shi@intel.com>
>>>>> Subject: [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in
>>> CLANG38
>>>>> toolchain
>>>>>
>>>>> From: "Shi, Steven" <steven.shi@intel.com>
>>>>>
>>>>> Add LLVM39 and LLVM40 support in CLANG38 toolchain
>>>>>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>> Signed-off-by: Steven Shi <steven.shi@intel.com>
>>>>> ---
>>>>> BaseTools/Conf/tools_def.template | 5 +++--
>>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/BaseTools/Conf/tools_def.template
>>>>> b/BaseTools/Conf/tools_def.template
>>>>> index 1fa3ca3..2f83341 100755
>>>>> --- a/BaseTools/Conf/tools_def.template
>>>>> +++ b/BaseTools/Conf/tools_def.template
>>>>> @@ -380,7 +380,8 @@ DEFINE SOURCERY_CYGWIN_TOOLS =
>>>> /cygdrive/c/Program
>>>>> Files/CodeSourcery/Sourcery G
>>>>> #                               Intel(r) ACPI Compiler from
>>>>> #                               https://acpica.org/downloads
>>>>> #   CLANG38  -Linux-  Requires:
>>>>> -#                             Clang v3.8 or later, LLVMgold plugin and GNU binutils
>2.26
>>>>> targeting x86_64-linux-gnu
>>>>> +#                             Clang v3.8, LLVMgold plugin and GNU binutils 2.26
>>>> targeting
>>>>> x86_64-linux-gnu
>>>>> +#                             Clang v3.9 or later, LLVMgold plugin and GNU binutils
>2.28
>>>>> targeting x86_64-linux-gnu
>>>>> #                        Optional:
>>>>> #                             Required to build platforms or ACPI tables:
>>>>> #                               Intel(r) ACPI Compiler from
>>>>> @@ -5512,7 +5513,7 @@ DEFINE CLANG38_X64_PREFIX           =
>>>>> ENV(CLANG38_BIN)
>>>>> DEFINE CLANG38_IA32_TARGET          = -target i686-pc-linux-gnu
>>>>> DEFINE CLANG38_X64_TARGET           = -target x86_64-pc-linux-gnu
>>>>>
>>>>> -DEFINE CLANG38_ALL_CC_FLAGS         = DEF(GCC44_ALL_CC_FLAGS) -
>>> Wno-
>>>>> empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno-
>>> shift-
>>>>> negative-value -Wno-parentheses-equality -Wno-unknown-pragmas -
>>>> Wno-
>>>>> tautological-constant-out-of-range-compare -Wno-incompatible-library-
>>>>> redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -
>>>> msoft-
>>>>> float -mno-implicit-float  -ftrap-
>>>>> function=undefined_behavior_has_been_optimized_away_by_clang -
>>>>> funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-
>>>>> tautological-compare -Wno-unknown-warning-option
>>>>> +DEFINE CLANG38_ALL_CC_FLAGS         = DEF(GCC44_ALL_CC_FLAGS) -
>>>> Wno-
>>>>> empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno-
>>> shift-
>>>>> negative-value -Wno-parentheses-equality -Wno-unknown-pragmas -
>>>> Wno-
>>>>> tautological-constant-out-of-range-compare -Wno-incompatible-library-
>>>>> redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -
>>>> msoft-
>>>>> float -mno-implicit-float  -ftrap-
>>>>> function=undefined_behavior_has_been_optimized_away_by_clang -
>>>>> funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-
>>>>> tautological-compare -Wno-unknown-warning-option -Wno-varargs
>>>>>
>>>>> ###########################
>>>>> # CLANG38 IA32 definitions
>>>>> --
>>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> 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
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>