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:[email protected]] On Behalf Of >> Marvin H?user >> Sent: Friday, September 22, 2017 11:53 PM >> To: [email protected] >> Cc: Gao, Liming <[email protected]> >> 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:[email protected]] On Behalf Of >>> Gao, Liming >>> Sent: Monday, August 28, 2017 9:19 AM >>> To: Shi, Steven <[email protected]>; [email protected] >>> Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 >> in >>> CLANG38 toolchain >>> >>> Reviewed-by: Liming Gao <[email protected]> >>> >>>> -----Original Message----- >>>> From: Shi, Steven >>>> Sent: Wednesday, August 23, 2017 3:01 PM >>>> To: [email protected]; Gao, Liming <[email protected]> >>>> Cc: Zhu, Yonghong <[email protected]>; Shi, Steven >>>> <[email protected]> >>>> Subject: [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in >> CLANG38 >>>> toolchain >>>> >>>> From: "Shi, Steven" <[email protected]> >>>> >>>> Add LLVM39 and LLVM40 support in CLANG38 toolchain >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Steven Shi <[email protected]> >>>> --- >>>> 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 >>> [email protected] >>> https://lists.01.org/mailman/listinfo/edk2-devel >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
