Title: [271143] trunk/Source/_javascript_Core
Revision
271143
Author
tzaga...@apple.com
Date
2021-01-04 23:19:24 -0800 (Mon, 04 Jan 2021)

Log Message

Validate every instruction in AssemblerBuffer
https://bugs.webkit.org/show_bug.cgi?id=218104
<rdar://problem/69433094>

Reviewed by Saam Barati.

* assembler/AssemblerBuffer.cpp:
(JSC::threadSpecificAssemblerHashes):
* assembler/AssemblerBuffer.h:
(JSC::AssemblerBuffer::AssemblerBuffer):
(JSC::AssemblerBuffer::~AssemblerBuffer):
(JSC::AssemblerBuffer::releaseAssemblerData):
(JSC::AssemblerBuffer::releaseAssemblerHashes):
(JSC::AssemblerBuffer::putIntegralUnchecked):
(JSC::AssemblerBuffer::grow):
(JSC::AssemblerBuffer::outOfLineGrow):
(JSC::ARM64EHash::update): Deleted.
(JSC::ARM64EHash::finalHash const): Deleted.
(): Deleted.
(JSC::AssemblerBuffer::hash const): Deleted.
* assembler/LinkBuffer.cpp:
(JSC::LinkBuffer::copyCompactAndLinkCode):
* assembler/LinkBuffer.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (271142 => 271143)


--- trunk/Source/_javascript_Core/ChangeLog	2021-01-05 07:11:16 UTC (rev 271142)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-01-05 07:19:24 UTC (rev 271143)
@@ -1,3 +1,29 @@
+2020-11-17  Tadeu Zagallo  <tzaga...@apple.com>
+
+        Validate every instruction in AssemblerBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=218104
+        <rdar://problem/69433094>
+
+        Reviewed by Saam Barati.
+
+        * assembler/AssemblerBuffer.cpp:
+        (JSC::threadSpecificAssemblerHashes):
+        * assembler/AssemblerBuffer.h:
+        (JSC::AssemblerBuffer::AssemblerBuffer):
+        (JSC::AssemblerBuffer::~AssemblerBuffer):
+        (JSC::AssemblerBuffer::releaseAssemblerData):
+        (JSC::AssemblerBuffer::releaseAssemblerHashes):
+        (JSC::AssemblerBuffer::putIntegralUnchecked):
+        (JSC::AssemblerBuffer::grow):
+        (JSC::AssemblerBuffer::outOfLineGrow):
+        (JSC::ARM64EHash::update): Deleted.
+        (JSC::ARM64EHash::finalHash const): Deleted.
+        (): Deleted.
+        (JSC::AssemblerBuffer::hash const): Deleted.
+        * assembler/LinkBuffer.cpp:
+        (JSC::LinkBuffer::copyCompactAndLinkCode):
+        * assembler/LinkBuffer.h:
+
 2021-01-04  Dmitry Bezhetskov  <dbezhets...@igalia.com>
 
         [WASM-References] Fix data section parsing and add more tests from ref-types

Modified: trunk/Source/_javascript_Core/assembler/AssemblerBuffer.cpp (271142 => 271143)


--- trunk/Source/_javascript_Core/assembler/AssemblerBuffer.cpp	2021-01-05 07:11:16 UTC (rev 271142)
+++ trunk/Source/_javascript_Core/assembler/AssemblerBuffer.cpp	2021-01-05 07:19:24 UTC (rev 271143)
@@ -45,6 +45,20 @@
     return *threadSpecificAssemblerDataPtr;
 }
 
+#if CPU(ARM64E)
+static ThreadSpecificAssemblerData* threadSpecificAssemblerHashesPtr;
+ThreadSpecificAssemblerData& threadSpecificAssemblerHashes()
+{
+    static std::once_flag flag;
+    std::call_once(
+        flag,
+        [] () {
+            threadSpecificAssemblerHashesPtr = new ThreadSpecificAssemblerData();
+        });
+    return *threadSpecificAssemblerHashesPtr;
+}
+#endif // CPU(ARM64E)
+
 #endif // ENABLE(ASSEMBLER)
 
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/assembler/AssemblerBuffer.h (271142 => 271143)


--- trunk/Source/_javascript_Core/assembler/AssemblerBuffer.h	2021-01-05 07:11:16 UTC (rev 271142)
+++ trunk/Source/_javascript_Core/assembler/AssemblerBuffer.h	2021-01-05 07:19:24 UTC (rev 271143)
@@ -46,6 +46,7 @@
     typedef ThreadSpecific<AssemblerData, WTF::CanBeGCThread::True> ThreadSpecificAssemblerData;
 
     JS_EXPORT_PRIVATE ThreadSpecificAssemblerData& threadSpecificAssemblerData();
+    JS_EXPORT_PRIVATE ThreadSpecificAssemblerData& threadSpecificAssemblerHashes();
 
     class LinkBuffer;
 
@@ -183,23 +184,22 @@
 #if CPU(ARM64E)
     class ARM64EHash {
     public:
-        ARM64EHash() = default;
-        ALWAYS_INLINE void update(uint32_t value)
+        ARM64EHash(uint32_t initialHash)
+            : m_hash(initialHash)
         {
+        }
+
+        ALWAYS_INLINE uint32_t update(uint32_t value)
+        {
             uint64_t input = value ^ m_hash;
-            uint64_t a = static_cast<uint32_t>(tagInt<static_cast<PtrTag>(0)>(input) >> 39);
-            uint64_t b = tagInt<static_cast<PtrTag>(0xb7e151628aed2a6a)>(input) >> 23;
+            uint64_t a = static_cast<uint32_t>(tagInt(input, static_cast<PtrTag>(0)) >> 39);
+            uint64_t b = tagInt(input, static_cast<PtrTag>(0xb7e151628aed2a6a)) >> 23;
             m_hash = a ^ b;
+            return m_hash;
         }
-        uint32_t finalHash() const
-        {
-            uint64_t hash = m_hash;
-            uint64_t a = static_cast<uint32_t>(tagInt<static_cast<PtrTag>(0xbf7158809cf4f3c7)>(hash) >> 39);
-            uint64_t b = tagInt<static_cast<PtrTag>(0x62e7160f38b4da56)>(hash) >> 23;
-            return static_cast<uint32_t>(a ^ b);
-        }
+
     private:
-        uint32_t m_hash { 0 };
+        uint32_t m_hash;
     };
 #endif
 
@@ -208,15 +208,29 @@
         AssemblerBuffer()
             : m_storage()
             , m_index(0)
+#if CPU(ARM64E)
+            , m_hash(static_cast<uint32_t>(bitwise_cast<uint64_t>(this)))
+            , m_hashes()
+#endif
         {
-            auto& threadSpecific = threadSpecificAssemblerData();
-            m_storage.takeBufferIfLarger(WTFMove(*threadSpecific));
+            auto& threadSpecificData = threadSpecificAssemblerData();
+            m_storage.takeBufferIfLarger(WTFMove(*threadSpecificData));
+#if CPU(ARM64E)
+            auto& threadSpecificHashes = threadSpecificAssemblerHashes();
+            m_hashes.takeBufferIfLarger(WTFMove(*threadSpecificHashes));
+            ASSERT(m_storage.capacity() == m_hashes.capacity());
+#endif
         }
 
         ~AssemblerBuffer()
         {
-            auto& threadSpecific = threadSpecificAssemblerData();
-            threadSpecific->takeBufferIfLarger(WTFMove(m_storage));
+#if CPU(ARM64E)
+            ASSERT(m_storage.capacity() == m_hashes.capacity());
+            auto& threadSpecificHashes = threadSpecificAssemblerHashes();
+            threadSpecificHashes->takeBufferIfLarger(WTFMove(m_hashes));
+#endif
+            auto& threadSpecificData = threadSpecificAssemblerData();
+            threadSpecificData->takeBufferIfLarger(WTFMove(m_storage));
         }
 
         bool isAvailable(unsigned space)
@@ -269,8 +283,18 @@
 
         unsigned debugOffset() { return m_index; }
 
-        AssemblerData&& releaseAssemblerData() { return WTFMove(m_storage); }
+        AssemblerData&& releaseAssemblerData()
+        {
+            return WTFMove(m_storage);
+        }
 
+#if CPU(ARM64E)
+        AssemblerData&& releaseAssemblerHashes()
+        {
+            return WTFMove(m_hashes);
+        }
+#endif
+
         // LocalWriter is a trick to keep the storage buffer and the index
         // in memory while issuing multiple Stores.
         // It is created in a block scope and its attribute can stay live
@@ -323,15 +347,10 @@
         };
 #endif // !CPU(ARM64)
 
-#if CPU(ARM64E)
-        ARM64EHash hash() const { return m_hash; }
-#endif
-
 #if !CPU(ARM64) // If we were to define this on arm64e, we'd need a way to update the hash as we write directly into the buffer.
         void* data() const { return m_storage.buffer(); }
 #endif
 
-
     protected:
         template<typename IntegralType>
         void putIntegral(IntegralType value)
@@ -348,7 +367,8 @@
 #if CPU(ARM64)
             static_assert(sizeof(value) == 4, "");
 #if CPU(ARM64E)
-            m_hash.update(value);
+            uint32_t hash = m_hash.update(value);
+            WTF::unalignedStore<uint32_t>(m_hashes.buffer() + m_index, hash);
 #endif
 #endif
             ASSERT(isAvailable(sizeof(IntegralType)));
@@ -360,11 +380,17 @@
         void grow(int extraCapacity = 0)
         {
             m_storage.grow(extraCapacity);
+#if CPU(ARM64E)
+            m_hashes.grow(extraCapacity);
+#endif
         }
 
         NEVER_INLINE void outOfLineGrow()
         {
             m_storage.grow();
+#if CPU(ARM64E)
+            m_hashes.grow();
+#endif
         }
 
 #if !CPU(ARM64)
@@ -376,6 +402,7 @@
         unsigned m_index;
 #if CPU(ARM64E)
         ARM64EHash m_hash;
+        AssemblerData m_hashes;
 #endif
     };
 

Modified: trunk/Source/_javascript_Core/assembler/LinkBuffer.cpp (271142 => 271143)


--- trunk/Source/_javascript_Core/assembler/LinkBuffer.cpp	2021-01-05 07:11:16 UTC (rev 271142)
+++ trunk/Source/_javascript_Core/assembler/LinkBuffer.cpp	2021-01-05 07:19:24 UTC (rev 271143)
@@ -120,12 +120,6 @@
 
 #if ENABLE(BRANCH_COMPACTION)
 
-#if CPU(ARM64E)
-#define ENABLE_VERIFY_JIT_HASH 1
-#else
-#define ENABLE_VERIFY_JIT_HASH 0
-#endif
-
 class BranchCompactionLinkBuffer;
 
 using ThreadSpecificBranchCompactionLinkBuffer = ThreadSpecific<BranchCompactionLinkBuffer, WTF::CanBeGCThread::True>;
@@ -240,15 +234,15 @@
 
     Vector<LinkRecord, 0, UnsafeVectorOverflow>& jumpsToLink = macroAssembler.jumpsToLink();
     m_assemblerStorage = macroAssembler.m_assembler.buffer().releaseAssemblerData();
-    uint8_t* inData = reinterpret_cast<uint8_t*>(m_assemblerStorage.buffer());
+    uint8_t* inData = bitwise_cast<uint8_t*>(m_assemblerStorage.buffer());
+#if CPU(ARM64E)
+    ARM64EHash verifyUncompactedHash { static_cast<uint32_t>(bitwise_cast<uint64_t>(&macroAssembler.m_assembler.buffer())) };
+    m_assemblerHashesStorage = macroAssembler.m_assembler.buffer().releaseAssemblerHashes();
+    uint32_t* inHashes = bitwise_cast<uint32_t*>(m_assemblerHashesStorage.buffer());
+#endif
 
     uint8_t* codeOutData = m_code.dataLocation<uint8_t*>();
 
-#if ENABLE(VERIFY_JIT_HASH)
-    const uint32_t expectedFinalHash = macroAssembler.m_assembler.buffer().hash().finalHash();
-    ARM64EHash verifyUncompactedHash;
-#endif
-
     BranchCompactionLinkBuffer outBuffer(m_size, useFastJITPermissions() ? codeOutData : 0);
     uint8_t* outData = outBuffer.data();
 
@@ -261,6 +255,16 @@
     int writePtr = 0;
     unsigned jumpCount = jumpsToLink.size();
 
+    auto read = [&](const InstructionType* ptr) -> InstructionType {
+        InstructionType value = *ptr;
+#if CPU(ARM64E)
+        uint32_t hash = verifyUncompactedHash.update(value);
+        unsigned index = (bitwise_cast<uint8_t*>(ptr) - inData) / 4;
+        RELEASE_ASSERT(inHashes[index] == hash);
+#endif
+        return value;
+    };
+
     if (useFastJITPermissions())
         threadSelfRestrictRWXToRW();
 
@@ -278,11 +282,7 @@
             ASSERT(!(readPtr % 2));
             ASSERT(!(writePtr % 2));
             while (copySource != copyEnd) {
-                InstructionType insn = *copySource++;
-#if ENABLE(VERIFY_JIT_HASH)
-                static_assert(sizeof(InstructionType) == 4, "");
-                verifyUncompactedHash.update(insn);
-#endif
+                InstructionType insn = read(copySource++);
                 *copyDst++ = insn;
             }
             recordLinkOffsets(m_assemblerStorage, readPtr, jumpsToLink[i].from(), offset);
@@ -336,23 +336,11 @@
         RELEASE_ASSERT(bytes % sizeof(InstructionType) == 0);
 
         for (size_t i = 0; i < bytes; i += sizeof(InstructionType)) {
-            InstructionType insn = *src++;
-#if ENABLE(VERIFY_JIT_HASH)
-            verifyUncompactedHash.update(insn);
-#endif
+            InstructionType insn = read(src++);
             *dst++ = insn;
         }
     }
 
-#if ENABLE(VERIFY_JIT_HASH)
-    if (verifyUncompactedHash.finalHash() != expectedFinalHash) {
-#ifndef NDEBUG
-        dataLogLn("Hashes don't match: ", RawPointer(bitwise_cast<void*>(static_cast<uintptr_t>(verifyUncompactedHash.finalHash()))), " ", RawPointer(bitwise_cast<void*>(static_cast<uintptr_t>(expectedFinalHash))));
-        dataLogLn("Crashing!");
-#endif
-        CRASH();
-    }
-#endif
 
     recordLinkOffsets(m_assemblerStorage, readPtr, initialSize, readPtr - writePtr);
         

Modified: trunk/Source/_javascript_Core/assembler/LinkBuffer.h (271142 => 271143)


--- trunk/Source/_javascript_Core/assembler/LinkBuffer.h	2021-01-05 07:11:16 UTC (rev 271142)
+++ trunk/Source/_javascript_Core/assembler/LinkBuffer.h	2021-01-05 07:19:24 UTC (rev 271143)
@@ -360,6 +360,9 @@
     size_t m_size;
 #if ENABLE(BRANCH_COMPACTION)
     AssemblerData m_assemblerStorage;
+#if CPU(ARM64E)
+    AssemblerData m_assemblerHashesStorage;
+#endif
     bool m_shouldPerformBranchCompaction { true };
 #endif
     bool m_didAllocate;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to