Besides the GetBestLanguage() , there are several other lib functions have the same problem which are listed in attachment https://bugzilla.tianocore.org/attachment.cgi?id=97 of the Bug410. These functions can also cause the build fails with CLANG38 in my side with clang version 4.0 or clang 5.0.
I'm OK to enable the -Wvarargs again if we think these lib ABI updates are acceptable. Steven Shi Intel\SSG\STO\UEFI Firmware Tel: +86 021-61166522 iNet: 821-6522 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Marvin H?user > Sent: Monday, September 25, 2017 8:30 PM > To: edk2-devel@lists.01.org > Cc: Laszlo Ersek <ler...@redhat.com>; Gao, Liming <liming....@intel.com> > Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in > CLANG38 toolchain > > Hey Liming and Laszlo, > > Thanks for your answers! > > I rather thought of this (I didn't check whether Language1 should be CONST, > but doesn't matter for now): > > CHAR8 * > EFIAPI > GetBestLanguage ( > IN CONST CHAR8 *SupportedLanguages, > IN BOOLEAN Iso639Language, > IN CHAR8 *Language1, > ... > ); > > This would be compatible with all existing code because the VA list solely > consists of CHAR8 pointers. > The only difference would be that one cannot pass just NULL after > Iso639Language, but this scenario makes no sense anyway. > One would just use Langauge1 on the first iteration and, as part of a do- > while-loop, assign the VA args via VA_ARG(). > > Best regards, > Marvin. > > > -----Original Message----- > > From: Gao, Liming [mailto:liming....@intel.com] > > Sent: Monday, September 25, 2017 11:58 AM > > To: Laszlo Ersek <ler...@redhat.com>; Marvin H?user > > <marvin.haeu...@outlook.com>; edk2-devel@lists.01.org > > Subject: RE: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 > in > > CLANG38 toolchain > > > > 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:ler...@redhat.com] > > >Sent: Monday, September 25, 2017 4:13 PM > > >To: Gao, Liming <liming....@intel.com>; Marvin Häuser > > ><marvin.haeu...@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-boun...@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....@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-boun...@lists.01.org] On > Behalf > > >>>> Of Gao, Liming > > >>>> Sent: Monday, August 28, 2017 9:19 AM > > >>>> To: Shi, Steven <steven....@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....@intel.com> > > >>>> > > >>>>> -----Original Message----- > > >>>>> From: Shi, Steven > > >>>>> Sent: Wednesday, August 23, 2017 3:01 PM > > >>>>> To: edk2-devel@lists.01.org; Gao, Liming <liming....@intel.com> > > >>>>> Cc: Zhu, Yonghong <yonghong....@intel.com>; Shi, Steven > > >>>>> <steven....@intel.com> > > >>>>> Subject: [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in > > >>> CLANG38 > > >>>>> toolchain > > >>>>> > > >>>>> From: "Shi, Steven" <steven....@intel.com> > > >>>>> > > >>>>> Add LLVM39 and LLVM40 support in CLANG38 toolchain > > >>>>> > > >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 > > >>>>> Signed-off-by: Steven Shi <steven....@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 > > >> > > _______________________________________________ > 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
