Ard: This logic can't be met. I suggest to remove it and add comments to say this condition is invalid.
Thanks Liming From: edk2-devel [mailto:[email protected]] On Behalf Of Ard Biesheuvel Sent: Monday, July 18, 2016 2:01 PM To: Gao, Liming <[email protected]> Cc: [email protected]; Ye, Ting <[email protected]>; Justen, Jordan L <[email protected]>; [email protected]; [email protected]; Long, Qin <[email protected]>; Kinney, Michael D <[email protected]>; [email protected]; [email protected] Subject: Re: [edk2] [PATCH v3 6/9] MdePkg: disallow open coded varargs implementation on optimizing GCC On 18 July 2016 at 07:56, Gao, Liming wrote: > Ard: > > The below condition can't be trigged. It is in the else path of #elif > defined(__GNUC__). So, #if defined(__GNUC__) is always FALSE in below case. > Are they added for comment only? > This is only the case after the next patch removes the '&& !defined(NO_BUILTIN_VA_FUNCS)' from the __GNUC__ test. But you are right that at that point, the condition can no longer be met. I think it does make sense to keep it, since the combination is known to generate broken binaries, but I am happy to drop the patch if other people feel this is not necessary. -- Ard. >> -----Original Message----- >> From: edk2-devel [mailto:[email protected]] On Behalf Of >> Ard Biesheuvel >> Sent: Sunday, July 17, 2016 6:35 PM >> To: [email protected]<mailto:[email protected]>; >> [email protected]<mailto:[email protected]>; >> [email protected]<mailto:[email protected]>; Gao, >> Liming ; Shi, Steven ; Zhu, >> Yonghong ; Kinney, Michael D >> ; Justen, Jordan L >> Cc: [email protected]<mailto:[email protected]>; >> [email protected]<mailto:[email protected]>; Ard Biesheuvel >> ; Ye, Ting ; Long, Qin >> >> Subject: [edk2] [PATCH v3 6/9] MdePkg: disallow open coded varargs >> implementation on optimizing GCC >> >> The open coded varargs implementation that performs pointer arithmetic on >> the last named parameter of a function to calculate the addresses of >> variadic parameters and subsequently derefences them is fragile, and break >> spectacularly when used under GCC with optimization enabled. So explicitly >> disallow this combination. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> --- >> MdePkg/Include/Base.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h >> index e03fa8239284..95400f993e6b 100644 >> --- a/MdePkg/Include/Base.h >> +++ b/MdePkg/Include/Base.h >> @@ -635,6 +635,11 @@ typedef __builtin_va_list VA_LIST; >> #endif >> >> #else >> + >> +#if defined(__GNUC__) && defined(__OPTIMIZE__) >> +#error This VA_LIST implementation is incompatible with GCC optimization >> +#endif >> + >> /// >> /// Variable used to traverse the list of arguments. This type can vary by >> /// implementation and could be an array or structure. >> -- >> 1.9.1 >> >> _______________________________________________ >> edk2-devel mailing list >> [email protected]<mailto:[email protected]> >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected]<mailto:[email protected]> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
