Title: [205066] trunk/Source/WTF
- Revision
- 205066
- Author
- [email protected]
- Date
- 2016-08-26 19:39:39 -0700 (Fri, 26 Aug 2016)
Log Message
bitwise_cast uses inactive member of union
https://bugs.webkit.org/show_bug.cgi?id=161244
Patch by JF Bastien <[email protected]> on 2016-08-26
Reviewed by Benjamin Poulain.
* wtf/Compiler.h:
Add COMPILER_HAS_CLANG_FEATURE
* wtf/StdLibExtras.h:
(WTF::bitwise_cast):
Fix C++ UB, add trivially-copyable check.
bitwise_cast stores into a union with one type and reads with
another, which is technically C++ undefined behavior because it's
accessing the wrong active member of the union. The better way to
do this is through memcpy, which compilers optimize as well
because it's known-size in known-not-to-escape storage (for small
types they'll inline and then convert stack memory access to SSA
values which may be in-register if that makes sense, which would
be a move between int/FP registers at worst).
The C++ Standard's section [basic.types] explicitly blesses memcpy:
For any trivially copyable type T, if two pointers to T point to
distinct T objects obj1 and obj2, where neither obj1 nor obj2 is a
base-class subobject, if the underlying bytes (1.7) making up obj1
are copied into obj2, 42 obj2 shall subsequently hold the same
value as obj1.
[Example:
T* t1p;
T* t2p;
// provided that t2p points to an initialized object ...
std::memcpy(t1p, t2p, sizeof(T));
// at this point, every subobject of trivially copyable type in *t1p contains
// the same value as the corresponding subobject in *t2p
— end example ]
Whereas section [class.union] says:
In a union, at most one of the non-static data members can be
active at any time, that is, the value of at most one of the
non-static data members can be stored in a union at any time.
While we're at it, checking that sizeof(To) == sizeof(From) is
good, but we should also check that both types are trivially
copyable (can have a ctor, no dtor, and copy is defaulted as if by
memcpy for type and all subtypes). Unfortunately that trait isn't
implemented consistently in all recent compiler+stdlib
implementations, but recent clang has an equivalent builtin
(other compilers simply won't do the check, and will break on bots
with the right compilers which is better than the current silent
breakage). This builtin hack also avoids #include <type_traits>
which really doesn't save much.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (205065 => 205066)
--- trunk/Source/WTF/ChangeLog 2016-08-27 02:01:11 UTC (rev 205065)
+++ trunk/Source/WTF/ChangeLog 2016-08-27 02:39:39 UTC (rev 205066)
@@ -1,3 +1,59 @@
+2016-08-26 JF Bastien <[email protected]>
+
+ bitwise_cast uses inactive member of union
+ https://bugs.webkit.org/show_bug.cgi?id=161244
+
+ Reviewed by Benjamin Poulain.
+
+ * wtf/Compiler.h:
+ Add COMPILER_HAS_CLANG_FEATURE
+ * wtf/StdLibExtras.h:
+ (WTF::bitwise_cast):
+ Fix C++ UB, add trivially-copyable check.
+
+ bitwise_cast stores into a union with one type and reads with
+ another, which is technically C++ undefined behavior because it's
+ accessing the wrong active member of the union. The better way to
+ do this is through memcpy, which compilers optimize as well
+ because it's known-size in known-not-to-escape storage (for small
+ types they'll inline and then convert stack memory access to SSA
+ values which may be in-register if that makes sense, which would
+ be a move between int/FP registers at worst).
+
+ The C++ Standard's section [basic.types] explicitly blesses memcpy:
+
+ For any trivially copyable type T, if two pointers to T point to
+ distinct T objects obj1 and obj2, where neither obj1 nor obj2 is a
+ base-class subobject, if the underlying bytes (1.7) making up obj1
+ are copied into obj2, 42 obj2 shall subsequently hold the same
+ value as obj1.
+
+ [Example:
+ T* t1p;
+ T* t2p;
+ // provided that t2p points to an initialized object ...
+ std::memcpy(t1p, t2p, sizeof(T));
+ // at this point, every subobject of trivially copyable type in *t1p contains
+ // the same value as the corresponding subobject in *t2p
+ — end example ]
+
+ Whereas section [class.union] says:
+
+ In a union, at most one of the non-static data members can be
+ active at any time, that is, the value of at most one of the
+ non-static data members can be stored in a union at any time.
+
+ While we're at it, checking that sizeof(To) == sizeof(From) is
+ good, but we should also check that both types are trivially
+ copyable (can have a ctor, no dtor, and copy is defaulted as if by
+ memcpy for type and all subtypes). Unfortunately that trait isn't
+ implemented consistently in all recent compiler+stdlib
+ implementations, but recent clang has an equivalent builtin
+ (other compilers simply won't do the check, and will break on bots
+ with the right compilers which is better than the current silent
+ breakage). This builtin hack also avoids #include <type_traits>
+ which really doesn't save much.
+
2016-08-26 Johan K. Jensen <[email protected]>
Web Inspector: Frontend should have access to Resource Timing information
Modified: trunk/Source/WTF/wtf/Compiler.h (205065 => 205066)
--- trunk/Source/WTF/wtf/Compiler.h 2016-08-27 02:01:11 UTC (rev 205065)
+++ trunk/Source/WTF/wtf/Compiler.h 2016-08-27 02:39:39 UTC (rev 205066)
@@ -35,7 +35,7 @@
/* COMPILER_QUIRK() - whether the compiler being used to build the project requires a given quirk. */
#define COMPILER_QUIRK(WTF_COMPILER_QUIRK) (defined WTF_COMPILER_QUIRK_##WTF_COMPILER_QUIRK && WTF_COMPILER_QUIRK_##WTF_COMPILER_QUIRK)
-/* COMPILER_HAS_CLANG_BUILTIN() - wether the compiler supports a particular clang builtin. */
+/* COMPILER_HAS_CLANG_BUILTIN() - whether the compiler supports a particular clang builtin. */
#ifdef __has_builtin
#define COMPILER_HAS_CLANG_BUILTIN(x) __has_builtin(x)
#else
@@ -42,6 +42,14 @@
#define COMPILER_HAS_CLANG_BUILTIN(x) 0
#endif
+/* COMPILER_HAS_CLANG_HEATURE() - whether the compiler supports a particular language or library feature. */
+/* http://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension */
+#ifdef __has_feature
+#define COMPILER_HAS_CLANG_FEATURE(x) __has_feature(x)
+#else
+#define COMPILER_HAS_CLANG_FEATURE(x) 0
+#endif
+
/* ==== COMPILER() - primary detection of the compiler being used to build the project, in alphabetical order ==== */
/* COMPILER(CLANG) - Clang */
@@ -48,12 +56,13 @@
#if defined(__clang__)
#define WTF_COMPILER_CLANG 1
-#define WTF_COMPILER_SUPPORTS_BLOCKS __has_feature(blocks)
-#define WTF_COMPILER_SUPPORTS_C_STATIC_ASSERT __has_feature(c_static_assert)
-#define WTF_COMPILER_SUPPORTS_CXX_REFERENCE_QUALIFIED_FUNCTIONS __has_feature(cxx_reference_qualified_functions)
-#define WTF_COMPILER_SUPPORTS_CXX_USER_LITERALS __has_feature(cxx_user_literals)
-#define WTF_COMPILER_SUPPORTS_FALLTHROUGH_WARNINGS __has_feature(cxx_attributes) && __has_warning("-Wimplicit-fallthrough")
-#define WTF_COMPILER_SUPPORTS_CXX_EXCEPTIONS __has_feature(cxx_exceptions)
+#define WTF_COMPILER_SUPPORTS_BLOCKS COMPILER_HAS_CLANG_FEATURE(blocks)
+#define WTF_COMPILER_SUPPORTS_C_STATIC_ASSERT COMPILER_HAS_CLANG_FEATURE(c_static_assert)
+#define WTF_COMPILER_SUPPORTS_CXX_REFERENCE_QUALIFIED_FUNCTIONS COMPILER_HAS_CLANG_FEATURE(cxx_reference_qualified_functions)
+#define WTF_COMPILER_SUPPORTS_CXX_USER_LITERALS COMPILER_HAS_CLANG_FEATURE(cxx_user_literals)
+#define WTF_COMPILER_SUPPORTS_FALLTHROUGH_WARNINGS COMPILER_HAS_CLANG_FEATURE(cxx_attributes) && __has_warning("-Wimplicit-fallthrough")
+#define WTF_COMPILER_SUPPORTS_CXX_EXCEPTIONS COMPILER_HAS_CLANG_FEATURE(cxx_exceptions)
+#define WTF_COMPILER_SUPPORTS_BUILTIN_IS_TRIVIALLY_COPYABLE COMPILER_HAS_CLANG_FEATURE(is_trivially_copyable)
#ifdef __cplusplus
#if __cplusplus <= 201103L
@@ -135,11 +144,7 @@
#define WTF_COMPILER_SUPPORTS_EABI 1
#endif
-#if defined(__has_feature)
-#define ASAN_ENABLED __has_feature(address_sanitizer)
-#else
-#define ASAN_ENABLED 0
-#endif
+#define ASAN_ENABLED COMPILER_HAS_CLANG_FEATURE(address_sanitizer)
#if ASAN_ENABLED
#define SUPPRESS_ASAN __attribute__((no_sanitize_address))
Modified: trunk/Source/WTF/wtf/StdLibExtras.h (205065 => 205066)
--- trunk/Source/WTF/wtf/StdLibExtras.h 2016-08-27 02:01:11 UTC (rev 205065)
+++ trunk/Source/WTF/wtf/StdLibExtras.h 2016-08-27 02:39:39 UTC (rev 205066)
@@ -28,10 +28,11 @@
#define WTF_StdLibExtras_h
#include <chrono>
+#include <cstring>
#include <memory>
-#include <string.h>
#include <wtf/Assertions.h>
#include <wtf/CheckedArithmetic.h>
+#include <wtf/Compiler.h>
// This was used to declare and define a static local variable (static T;) so that
// it was leaked so that its destructors were not called at exit.
@@ -145,12 +146,14 @@
inline ToType bitwise_cast(FromType from)
{
static_assert(sizeof(FromType) == sizeof(ToType), "bitwise_cast size of FromType and ToType must be equal!");
- union {
- FromType from;
- ToType to;
- } u;
- u.from = from;
- return u.to;
+#if COMPILER_SUPPORTS(BUILTIN_IS_TRIVIALLY_COPYABLE)
+ // Not all recent STL implementations support the std::is_trivially_copyable type trait. Work around this by only checking on toolchains which have the equivalent compiler intrinsic.
+ static_assert(__is_trivially_copyable(ToType), "bitwise_cast of non-trivially-copyable type!");
+ static_assert(__is_trivially_copyable(FromType), "bitwise_cast of non-trivially-copyable type!");
+#endif
+ ToType to;
+ std::memcpy(&to, &from, sizeof(to));
+ return to;
}
template<typename ToType, typename FromType>
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
