| Message ID | 1468751686-28047-7-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 18 July 2016 at 07:56, Gao, Liming <liming.gao@intel.com> 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:edk2-devel-bounces@lists.01.org] On Behalf Of >> Ard Biesheuvel >> Sent: Sunday, July 17, 2016 6:35 PM >> To: edk2-devel@lists.01.org; lersek@redhat.com; afish@apple.com; Gao, >> Liming <liming.gao@intel.com>; Shi, Steven <steven.shi@intel.com>; Zhu, >> Yonghong <yonghong.zhu@intel.com>; Kinney, Michael D >> <michael.d.kinney@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com> >> Cc: bruce@cran.org.uk; pbonzini@redhat.com; Ard Biesheuvel >> <ard.biesheuvel@linaro.org>; Ye, Ting <ting.ye@intel.com>; Long, Qin >> <qin.long@intel.com> >> 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 <ard.biesheuvel@linaro.org> >> --- >> 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 >> 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
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.
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 <ard.biesheuvel@linaro.org> --- MdePkg/Include/Base.h | 5 +++++ 1 file changed, 5 insertions(+) -- 1.9.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel