<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[205066] trunk/Source/WTF</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/205066">205066</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-08-26 19:39:39 -0700 (Fri, 26 Aug 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>bitwise_cast uses inactive member of union
https://bugs.webkit.org/show_bug.cgi?id=161244
Patch by JF Bastien <jfbastien@apple.com> 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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfCompilerh">trunk/Source/WTF/wtf/Compiler.h</a></li>
<li><a href="#trunkSourceWTFwtfStdLibExtrash">trunk/Source/WTF/wtf/StdLibExtras.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (205065 => 205066)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,59 @@
</span><ins>+2016-08-26 JF Bastien <jfbastien@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2016-08-26 Johan K. Jensen <johan_jensen@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: Frontend should have access to Resource Timing information
</span></span></pre></div>
<a id="trunkSourceWTFwtfCompilerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/Compiler.h (205065 => 205066)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -35,7 +35,7 @@
</span><span class="cx"> /* COMPILER_QUIRK() - whether the compiler being used to build the project requires a given quirk. */
</span><span class="cx"> #define COMPILER_QUIRK(WTF_COMPILER_QUIRK) (defined WTF_COMPILER_QUIRK_##WTF_COMPILER_QUIRK && WTF_COMPILER_QUIRK_##WTF_COMPILER_QUIRK)
</span><span class="cx">
</span><del>-/* COMPILER_HAS_CLANG_BUILTIN() - wether the compiler supports a particular clang builtin. */
</del><ins>+/* COMPILER_HAS_CLANG_BUILTIN() - whether the compiler supports a particular clang builtin. */
</ins><span class="cx"> #ifdef __has_builtin
</span><span class="cx"> #define COMPILER_HAS_CLANG_BUILTIN(x) __has_builtin(x)
</span><span class="cx"> #else
</span><span class="lines">@@ -42,6 +42,14 @@
</span><span class="cx"> #define COMPILER_HAS_CLANG_BUILTIN(x) 0
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+/* 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
+
</ins><span class="cx"> /* ==== COMPILER() - primary detection of the compiler being used to build the project, in alphabetical order ==== */
</span><span class="cx">
</span><span class="cx"> /* COMPILER(CLANG) - Clang */
</span><span class="lines">@@ -48,12 +56,13 @@
</span><span class="cx">
</span><span class="cx"> #if defined(__clang__)
</span><span class="cx"> #define WTF_COMPILER_CLANG 1
</span><del>-#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)
</del><ins>+#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)
</ins><span class="cx">
</span><span class="cx"> #ifdef __cplusplus
</span><span class="cx"> #if __cplusplus <= 201103L
</span><span class="lines">@@ -135,11 +144,7 @@
</span><span class="cx"> #define WTF_COMPILER_SUPPORTS_EABI 1
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>-#if defined(__has_feature)
-#define ASAN_ENABLED __has_feature(address_sanitizer)
-#else
-#define ASAN_ENABLED 0
-#endif
</del><ins>+#define ASAN_ENABLED COMPILER_HAS_CLANG_FEATURE(address_sanitizer)
</ins><span class="cx">
</span><span class="cx"> #if ASAN_ENABLED
</span><span class="cx"> #define SUPPRESS_ASAN __attribute__((no_sanitize_address))
</span></span></pre></div>
<a id="trunkSourceWTFwtfStdLibExtrash"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/StdLibExtras.h (205065 => 205066)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -28,10 +28,11 @@
</span><span class="cx"> #define WTF_StdLibExtras_h
</span><span class="cx">
</span><span class="cx"> #include <chrono>
</span><ins>+#include <cstring>
</ins><span class="cx"> #include <memory>
</span><del>-#include <string.h>
</del><span class="cx"> #include <wtf/Assertions.h>
</span><span class="cx"> #include <wtf/CheckedArithmetic.h>
</span><ins>+#include <wtf/Compiler.h>
</ins><span class="cx">
</span><span class="cx"> // This was used to declare and define a static local variable (static T;) so that
</span><span class="cx"> // it was leaked so that its destructors were not called at exit.
</span><span class="lines">@@ -145,12 +146,14 @@
</span><span class="cx"> inline ToType bitwise_cast(FromType from)
</span><span class="cx"> {
</span><span class="cx"> static_assert(sizeof(FromType) == sizeof(ToType), "bitwise_cast size of FromType and ToType must be equal!");
</span><del>- union {
- FromType from;
- ToType to;
- } u;
- u.from = from;
- return u.to;
</del><ins>+#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;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template<typename ToType, typename FromType>
</span></span></pre>
</div>
</div>
</body>
</html>