[WTF] Add WTF::unalignedLoad and WTF::unalignedStore
authoryusukesuzuki@slowstart.org <yusukesuzuki@slowstart.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 19 Aug 2018 22:50:05 +0000 (22:50 +0000)
committeryusukesuzuki@slowstart.org <yusukesuzuki@slowstart.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 19 Aug 2018 22:50:05 +0000 (22:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188716

Reviewed by Darin Adler.

Source/JavaScriptCore:

Use WTF::unalignedLoad and WTF::unalignedStore to avoid undefined behavior.
The compiler can emit appropriate mov operations in x86 even if we use these
helper functions.

* assembler/AssemblerBuffer.h:
(JSC::AssemblerBuffer::LocalWriter::putIntegralUnchecked):
(JSC::AssemblerBuffer::putIntegral):
(JSC::AssemblerBuffer::putIntegralUnchecked):
* assembler/MacroAssemblerX86.h:
(JSC::MacroAssemblerX86::readCallTarget):
* assembler/X86Assembler.h:
(JSC::X86Assembler::linkJump):
(JSC::X86Assembler::readPointer):
(JSC::X86Assembler::replaceWithHlt):
(JSC::X86Assembler::replaceWithJump):
(JSC::X86Assembler::setPointer):
(JSC::X86Assembler::setInt32):
(JSC::X86Assembler::setInt8):
* interpreter/InterpreterInlines.h:
(JSC::Interpreter::getOpcodeID): Embedded opcode may be misaligned. Actually UBSan detects misaligned accesses here.

Source/WTF:

While some CPUs allow unaligned accesses to memory, doing it in C++ with `reinterpret_cast<>` is
undefined behavior. This patch adds WTF::{unalignedLoad,unalignedStore} helper functions, which
can load from and store to the pointer in an unaligned manner.
Actual implementation uses `memcpy`. This can be optimized to direct unaligned access operations
in supported CPUs like x86. Even though a CPU does not support unaligned accesses, memcpy is still
safe and the compiler emits appropriate code.

We name these functions `unalignedLoad` and `unalignedStore` instead of `loadUnaligned` and `storeUnaligned`
in order to align them to `atomicLoad` and `atomicStore`.

* WTF.xcodeproj/project.pbxproj:
* wtf/CMakeLists.txt:
* wtf/UnalignedAccess.h: Added.
(WTF::unalignedLoad):
(WTF::unalignedStore):
* wtf/text/StringCommon.h:
(WTF::equal):
(WTF::loadUnaligned): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235018 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/AssemblerBuffer.h
Source/JavaScriptCore/assembler/MacroAssemblerX86.h
Source/JavaScriptCore/assembler/X86Assembler.h
Source/JavaScriptCore/interpreter/InterpreterInlines.h
Source/WTF/ChangeLog
Source/WTF/WTF.xcodeproj/project.pbxproj
Source/WTF/wtf/CMakeLists.txt
Source/WTF/wtf/UnalignedAccess.h [new file with mode: 0644]
Source/WTF/wtf/text/StringCommon.h

index a474cc1..54d7a35 100644 (file)
@@ -1,3 +1,31 @@
+2018-08-19  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
+        [WTF] Add WTF::unalignedLoad and WTF::unalignedStore
+        https://bugs.webkit.org/show_bug.cgi?id=188716
+
+        Reviewed by Darin Adler.
+
+        Use WTF::unalignedLoad and WTF::unalignedStore to avoid undefined behavior.
+        The compiler can emit appropriate mov operations in x86 even if we use these
+        helper functions.
+
+        * assembler/AssemblerBuffer.h:
+        (JSC::AssemblerBuffer::LocalWriter::putIntegralUnchecked):
+        (JSC::AssemblerBuffer::putIntegral):
+        (JSC::AssemblerBuffer::putIntegralUnchecked):
+        * assembler/MacroAssemblerX86.h:
+        (JSC::MacroAssemblerX86::readCallTarget):
+        * assembler/X86Assembler.h:
+        (JSC::X86Assembler::linkJump):
+        (JSC::X86Assembler::readPointer):
+        (JSC::X86Assembler::replaceWithHlt):
+        (JSC::X86Assembler::replaceWithJump):
+        (JSC::X86Assembler::setPointer):
+        (JSC::X86Assembler::setInt32):
+        (JSC::X86Assembler::setInt8):
+        * interpreter/InterpreterInlines.h:
+        (JSC::Interpreter::getOpcodeID): Embedded opcode may be misaligned. Actually UBSan detects misaligned accesses here.
+
 2018-08-17  Saam barati  <sbarati@apple.com>
 
         intersectionOfPastValuesAtHead must filter values after they've observed an invalidation point
index 7340952..54b2e9b 100644 (file)
@@ -34,6 +34,7 @@
 #include <wtf/Assertions.h>
 #include <wtf/FastMalloc.h>
 #include <wtf/StdLibExtras.h>
+#include <wtf/UnalignedAccess.h>
 
 namespace JSC {
 
@@ -239,7 +240,7 @@ namespace JSC {
             void putIntegralUnchecked(IntegralType value)
             {
                 ASSERT(m_index + sizeof(IntegralType) <= m_buffer.m_storage.capacity());
-                *reinterpret_cast_ptr<IntegralType*>(m_storageBuffer + m_index) = value;
+                WTF::unalignedStore<IntegralType>(m_storageBuffer + m_index, value);
                 m_index += sizeof(IntegralType);
             }
             AssemblerBuffer& m_buffer;
@@ -258,16 +259,14 @@ namespace JSC {
             unsigned nextIndex = m_index + sizeof(IntegralType);
             if (UNLIKELY(nextIndex > m_storage.capacity()))
                 outOfLineGrow();
-            ASSERT(isAvailable(sizeof(IntegralType)));
-            *reinterpret_cast_ptr<IntegralType*>(m_storage.buffer() + m_index) = value;
-            m_index = nextIndex;
+            putIntegralUnchecked<IntegralType>(value);
         }
 
         template<typename IntegralType>
         void putIntegralUnchecked(IntegralType value)
         {
             ASSERT(isAvailable(sizeof(IntegralType)));
-            *reinterpret_cast_ptr<IntegralType*>(m_storage.buffer() + m_index) = value;
+            WTF::unalignedStore<IntegralType>(m_storage.buffer() + m_index, value);
             m_index += sizeof(IntegralType);
         }
 
index 8bc023d..72def72 100644 (file)
@@ -302,7 +302,7 @@ public:
     template<PtrTag resultTag, PtrTag locationTag>
     static FunctionPtr<resultTag> readCallTarget(CodeLocationCall<locationTag> call)
     {
-        intptr_t offset = reinterpret_cast<int32_t*>(call.dataLocation())[-1];
+        intptr_t offset = WTF::unalignedLoad<int32_t>(bitwise_cast<int32_t*>(call.dataLocation()) - 1);
         return FunctionPtr<resultTag>(reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(call.dataLocation()) + offset));
     }
 
index f3a4083..c34cde9 100644 (file)
@@ -3680,7 +3680,7 @@ public:
         ASSERT(to.isSet());
 
         char* code = reinterpret_cast<char*>(m_formatter.data());
-        ASSERT(!reinterpret_cast<int32_t*>(code + from.m_offset)[-1]);
+        ASSERT(!WTF::unalignedLoad<int32_t>(bitwise_cast<int32_t*>(code + from.m_offset) - 1));
         setRel32(code + from.m_offset, code + to.m_offset);
     }
     
@@ -3739,22 +3739,21 @@ public:
     
     static void* readPointer(void* where)
     {
-        return reinterpret_cast<void**>(where)[-1];
+        return WTF::unalignedLoad<void*>(bitwise_cast<void**>(where) - 1);
     }
 
     static void replaceWithHlt(void* instructionStart)
     {
-        uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
-        ptr[0] = static_cast<uint8_t>(OP_HLT);
+        WTF::unalignedStore<uint8_t>(instructionStart, static_cast<uint8_t>(OP_HLT));
     }
 
     static void replaceWithJump(void* instructionStart, void* to)
     {
-        uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
-        uint8_t* dstPtr = reinterpret_cast<uint8_t*>(to);
+        uint8_t* ptr = bitwise_cast<uint8_t*>(instructionStart);
+        uint8_t* dstPtr = bitwise_cast<uint8_t*>(to);
         intptr_t distance = (intptr_t)(dstPtr - (ptr + 5));
-        ptr[0] = static_cast<uint8_t>(OP_JMP_rel32);
-        *reinterpret_cast<int32_t*>(ptr + 1) = static_cast<int32_t>(distance);
+        WTF::unalignedStore<uint8_t>(ptr, static_cast<uint8_t>(OP_JMP_rel32));
+        WTF::unalignedStore<int32_t>(ptr + 1, static_cast<int32_t>(distance));
     }
     
     static ptrdiff_t maxJumpReplacementSize()
@@ -3956,17 +3955,17 @@ private:
 
     static void setPointer(void* where, void* value)
     {
-        reinterpret_cast<void**>(where)[-1] = value;
+        WTF::unalignedStore<void*>(bitwise_cast<void**>(where) - 1, value);
     }
 
     static void setInt32(void* where, int32_t value)
     {
-        reinterpret_cast<int32_t*>(where)[-1] = value;
+        WTF::unalignedStore<int32_t>(bitwise_cast<int32_t*>(where) - 1, value);
     }
     
     static void setInt8(void* where, int8_t value)
     {
-        reinterpret_cast<int8_t*>(where)[-1] = value;
+        WTF::unalignedStore<int8_t>(bitwise_cast<int8_t*>(where) - 1, value);
     }
 
     static void setRel32(void* from, void* to)
index fc89a18..41a4551 100644 (file)
@@ -33,6 +33,7 @@
 #include "JSCPtrTag.h"
 #include "LLIntData.h"
 #include "UnlinkedCodeBlock.h"
+#include <wtf/UnalignedAccess.h>
 
 namespace JSC {
 
@@ -51,7 +52,7 @@ inline OpcodeID Interpreter::getOpcodeID(Opcode opcode)
     // in LowLevelInterpreter.cpp).
     auto codePtr = MacroAssemblerCodePtr<BytecodePtrTag>::createFromExecutableAddress(opcode);
     int32_t* opcodeIDAddress = codePtr.dataLocation<int32_t*>() - 1;
-    OpcodeID opcodeID = static_cast<OpcodeID>(*opcodeIDAddress);
+    OpcodeID opcodeID = static_cast<OpcodeID>(WTF::unalignedLoad<int32_t>(opcodeIDAddress));
     ASSERT(opcodeID < NUMBER_OF_BYTECODE_IDS);
     return opcodeID;
 #else
index 668e9b8..eac1685 100644 (file)
@@ -1,3 +1,29 @@
+2018-08-19  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
+
+        [WTF] Add WTF::unalignedLoad and WTF::unalignedStore
+        https://bugs.webkit.org/show_bug.cgi?id=188716
+
+        Reviewed by Darin Adler.
+
+        While some CPUs allow unaligned accesses to memory, doing it in C++ with `reinterpret_cast<>` is
+        undefined behavior. This patch adds WTF::{unalignedLoad,unalignedStore} helper functions, which
+        can load from and store to the pointer in an unaligned manner.
+        Actual implementation uses `memcpy`. This can be optimized to direct unaligned access operations
+        in supported CPUs like x86. Even though a CPU does not support unaligned accesses, memcpy is still
+        safe and the compiler emits appropriate code.
+
+        We name these functions `unalignedLoad` and `unalignedStore` instead of `loadUnaligned` and `storeUnaligned`
+        in order to align them to `atomicLoad` and `atomicStore`.
+
+        * WTF.xcodeproj/project.pbxproj:
+        * wtf/CMakeLists.txt:
+        * wtf/UnalignedAccess.h: Added.
+        (WTF::unalignedLoad):
+        (WTF::unalignedStore):
+        * wtf/text/StringCommon.h:
+        (WTF::equal):
+        (WTF::loadUnaligned): Deleted.
+
 2018-08-17  David Kilzer  <ddkilzer@apple.com>
 
         WTF's internal std::optional implementation should release assert on all bad accesses
index 704338a..45b5cea 100644 (file)
                E311FB161F0A568B003C08DE /* ThreadGroup.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ThreadGroup.h; sourceTree = "<group>"; };
                E3200AB41E9A536D003B59D2 /* PlatformRegisters.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PlatformRegisters.h; sourceTree = "<group>"; };
                E33D5F871FBED66700BF625E /* RecursableLambda.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RecursableLambda.h; sourceTree = "<group>"; };
+               E360C7642127B85B00C90F0E /* UnalignedAccess.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = UnalignedAccess.h; sourceTree = "<group>"; };
+               E360C7652127B85C00C90F0E /* Unexpected.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Unexpected.h; sourceTree = "<group>"; };
                E388886D20C9095100E632BC /* WorkerPool.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkerPool.cpp; sourceTree = "<group>"; };
                E388886E20C9095100E632BC /* WorkerPool.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WorkerPool.h; sourceTree = "<group>"; };
                E38C41241EB4E04C0042957D /* CPUTimeCocoa.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CPUTimeCocoa.cpp; sourceTree = "<group>"; };
                                0FED67B51B22D4D80066CE15 /* TinyPtrSet.h */,
                                149EF16216BBFE0D000A4331 /* TriState.h */,
                                83FBA93119DF459700F30ADB /* TypeCasts.h */,
+                               E360C7642127B85B00C90F0E /* UnalignedAccess.h */,
+                               E360C7652127B85C00C90F0E /* Unexpected.h */,
                                A8A4735C151A825B004123FF /* UnionFind.h */,
                                E300E521203D645F00DA79BE /* UniqueArray.h */,
                                5C7C88D31D0A3A0A009D2F6D /* UniqueRef.h */,
index 9798b9b..9adb555 100644 (file)
@@ -240,6 +240,7 @@ set(WTF_PUBLIC_HEADERS
     TriState.h
     TypeCasts.h
     UUID.h
+    UnalignedAccess.h
     Unexpected.h
     UniStdExtras.h
     UnionFind.h
diff --git a/Source/WTF/wtf/UnalignedAccess.h b/Source/WTF/wtf/UnalignedAccess.h
new file mode 100644 (file)
index 0000000..1a37b4e
--- /dev/null
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2018 Yusuke Suzuki <yusukesuzuki@slowstart.org>.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include <type_traits>
+#include <wtf/Platform.h>
+#include <wtf/StdLibExtras.h>
+
+namespace WTF {
+
+template<typename IntegralType>
+inline IntegralType unalignedLoad(const void* pointer)
+{
+    static_assert(std::is_integral<IntegralType>::value || std::is_pointer<IntegralType>::value, "");
+    IntegralType result { };
+    memcpy(&result, pointer, sizeof(IntegralType));
+    return result;
+}
+
+template<typename IntegralType>
+inline void unalignedStore(void* pointer, IntegralType value)
+{
+    static_assert(std::is_integral<IntegralType>::value || std::is_pointer<IntegralType>::value, "");
+    memcpy(pointer, &value, sizeof(IntegralType));
+}
+
+} // namespace WTF
index 5a49f8c..6883468 100644 (file)
@@ -30,6 +30,7 @@
 #include <unicode/uchar.h>
 #include <wtf/ASCIICType.h>
 #include <wtf/NotFound.h>
+#include <wtf/UnalignedAccess.h>
 
 namespace WTF {
 
@@ -48,19 +49,6 @@ template<typename StringClass, unsigned length> bool equalLettersIgnoringASCIICa
 bool equalIgnoringASCIICase(const char*, const char*);
 template<unsigned lowercaseLettersLength> bool equalLettersIgnoringASCIICase(const char*, const char (&lowercaseLetters)[lowercaseLettersLength]);
 
-template<typename T>
-inline T loadUnaligned(const char* s)
-{
-#if COMPILER(CLANG)
-    T tmp;
-    memcpy(&tmp, s, sizeof(T));
-    return tmp;
-#else
-    // This may result in undefined behavior due to unaligned access.
-    return *reinterpret_cast<const T*>(s);
-#endif
-}
-
 // Do comparisons 8 or 4 bytes-at-a-time on architectures where it's safe.
 #if (CPU(X86_64) || CPU(ARM64)) && !ASAN_ENABLED
 ALWAYS_INLINE bool equal(const LChar* aLChar, const LChar* bLChar, unsigned length)
@@ -72,7 +60,7 @@ ALWAYS_INLINE bool equal(const LChar* aLChar, const LChar* bLChar, unsigned leng
 
     if (dwordLength) {
         for (unsigned i = 0; i != dwordLength; ++i) {
-            if (loadUnaligned<uint64_t>(a) != loadUnaligned<uint64_t>(b))
+            if (unalignedLoad<uint64_t>(a) != unalignedLoad<uint64_t>(b))
                 return false;
 
             a += sizeof(uint64_t);
@@ -81,7 +69,7 @@ ALWAYS_INLINE bool equal(const LChar* aLChar, const LChar* bLChar, unsigned leng
     }
 
     if (length & 4) {
-        if (loadUnaligned<uint32_t>(a) != loadUnaligned<uint32_t>(b))
+        if (unalignedLoad<uint32_t>(a) != unalignedLoad<uint32_t>(b))
             return false;
 
         a += sizeof(uint32_t);
@@ -89,7 +77,7 @@ ALWAYS_INLINE bool equal(const LChar* aLChar, const LChar* bLChar, unsigned leng
     }
 
     if (length & 2) {
-        if (loadUnaligned<uint16_t>(a) != loadUnaligned<uint16_t>(b))
+        if (unalignedLoad<uint16_t>(a) != unalignedLoad<uint16_t>(b))
             return false;
 
         a += sizeof(uint16_t);
@@ -111,7 +99,7 @@ ALWAYS_INLINE bool equal(const UChar* aUChar, const UChar* bUChar, unsigned leng
 
     if (dwordLength) {
         for (unsigned i = 0; i != dwordLength; ++i) {
-            if (loadUnaligned<uint64_t>(a) != loadUnaligned<uint64_t>(b))
+            if (unalignedLoad<uint64_t>(a) != unalignedLoad<uint64_t>(b))
                 return false;
 
             a += sizeof(uint64_t);
@@ -120,7 +108,7 @@ ALWAYS_INLINE bool equal(const UChar* aUChar, const UChar* bUChar, unsigned leng
     }
 
     if (length & 2) {
-        if (loadUnaligned<uint32_t>(a) != loadUnaligned<uint32_t>(b))
+        if (unalignedLoad<uint32_t>(a) != unalignedLoad<uint32_t>(b))
             return false;
 
         a += sizeof(uint32_t);
@@ -140,7 +128,7 @@ ALWAYS_INLINE bool equal(const LChar* aLChar, const LChar* bLChar, unsigned leng
 
     unsigned wordLength = length >> 2;
     for (unsigned i = 0; i != wordLength; ++i) {
-        if (loadUnaligned<uint32_t>(a) != loadUnaligned<uint32_t>(b))
+        if (unalignedLoad<uint32_t>(a) != unalignedLoad<uint32_t>(b))
             return false;
         a += sizeof(uint32_t);
         b += sizeof(uint32_t);
@@ -168,7 +156,7 @@ ALWAYS_INLINE bool equal(const UChar* aUChar, const UChar* bUChar, unsigned leng
 
     unsigned wordLength = length >> 1;
     for (unsigned i = 0; i != wordLength; ++i) {
-        if (loadUnaligned<uint32_t>(a) != loadUnaligned<uint32_t>(b))
+        if (unalignedLoad<uint32_t>(a) != unalignedLoad<uint32_t>(b))
             return false;
         a += sizeof(uint32_t);
         b += sizeof(uint32_t);