[edk2,v3,6/9] MdePkg: disallow open coded varargs implementation on optimizing GCC

Message ID 1468751686-28047-7-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel July 17, 2016, 10:34 a.m.
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

Comments

Ard Biesheuvel July 18, 2016, 6 a.m. | #1
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

Patch

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.