Changeset 275049 in webkit

Timestamp:

Mar 25, 2021, 11:47:17 AM (5 years ago)

Author:

Chris Dumez

Message:

Address some alignment issues found by UBSan in contentextensions code
​https://bugs.webkit.org/show_bug.cgi?id=223748

Reviewed by Darin Adler.

Address some alignment issues found by UBSan in contentextensions code:

• contentextensions/SerializedNFA.h:55:20: runtime error: reference binding to misaligned address 0xnnnnnnnnn for type 'const unsigned long long', which requires 8 byte alignment
• contentextensions/NFAToDFA.cpp:254:29: runtime error: reference binding to misaligned address 0xnnnnnnnnn for type 'const unsigned long long', which requires 8 byte alignment
• contentextensions/DFABytecodeCompiler.cpp:43:5: runtime error: store to misaligned address 0xnnnnnnnnn for type 'unsigned short', which requires 2 byte alignment

Use memcpy() to store/load to/from misaligned address.

• contentextensions/DFABytecodeCompiler.cpp:

(WebCore::ContentExtensions::append):

• contentextensions/NFAToDFA.cpp:

(WebCore::ContentExtensions::epsilonClosureExcludingSelf):
(WebCore::ContentExtensions::NodeIdSetToUniqueNodeIdSetTranslator::translate):

• contentextensions/SerializedNFA.h:

(WebCore::ContentExtensions::SerializedNFA::Range::pointerAt const):
(WebCore::ContentExtensions::SerializedNFA::Range::valueAt const):
(WebCore::ContentExtensions::SerializedNFA::ConstTargetIterator::operator* const):
(WebCore::ContentExtensions::SerializedNFA::ConstTargetIterator::operator-> const):
(WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::first const):
(WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::last const):
(WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::data const):
(WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::range const):
(WebCore::ContentExtensions::SerializedNFA::transitionsForNode const):
(WebCore::ContentExtensions::SerializedNFA::Range::operator[] const): Deleted.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• contentextensions/DFABytecodeCompiler.cpp (modified) (1 diff)
• contentextensions/NFAToDFA.cpp (modified) (2 diffs)
• contentextensions/SerializedNFA.h (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-25  Chris Dumez  <cdumez@apple.com>
+
+        Address some alignment issues found by UBSan in contentextensions code
+        https://bugs.webkit.org/show_bug.cgi?id=223748
+
+        Reviewed by Darin Adler.
+
+        Address some alignment issues found by UBSan in contentextensions code:
+        - contentextensions/SerializedNFA.h:55:20: runtime error: reference binding to misaligned address 0xnnnnnnnnn for type 'const unsigned long long', which requires 8 byte alignment
+        - contentextensions/NFAToDFA.cpp:254:29: runtime error: reference binding to misaligned address 0xnnnnnnnnn for type 'const unsigned long long', which requires 8 byte alignment
+        - contentextensions/DFABytecodeCompiler.cpp:43:5: runtime error: store to misaligned address 0xnnnnnnnnn for type 'unsigned short', which requires 2 byte alignment
+
+        Use memcpy() to store/load to/from misaligned address.
+
+        * contentextensions/DFABytecodeCompiler.cpp:
+        (WebCore::ContentExtensions::append):
+        * contentextensions/NFAToDFA.cpp:
+        (WebCore::ContentExtensions::epsilonClosureExcludingSelf):
+        (WebCore::ContentExtensions::NodeIdSetToUniqueNodeIdSetTranslator::translate):
+        * contentextensions/SerializedNFA.h:
+        (WebCore::ContentExtensions::SerializedNFA::Range::pointerAt const):
+        (WebCore::ContentExtensions::SerializedNFA::Range::valueAt const):
+        (WebCore::ContentExtensions::SerializedNFA::ConstTargetIterator::operator* const):
+        (WebCore::ContentExtensions::SerializedNFA::ConstTargetIterator::operator-> const):
+        (WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::first const):
+        (WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::last const):
+        (WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::data const):
+        (WebCore::ContentExtensions::SerializedNFA::ConstRangeIterator::range const):
+        (WebCore::ContentExtensions::SerializedNFA::transitionsForNode const):
+        (WebCore::ContentExtensions::SerializedNFA::Range::operator[] const): Deleted.
+
 2021-03-25  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/contentextensions/DFABytecodeCompiler.cpp

@@ -41 +41 @@
 {
     bytecode.grow(bytecode.size() + sizeof(IntType));
-    *reinterpret_cast<IntType*>(&bytecode[bytecode.size() - sizeof(IntType)]) = value;
+    memcpy(&bytecode[bytecode.size() - sizeof(IntType)], &value, sizeof(IntType));
 }
 

trunk/Source/WebCore/contentextensions/NFAToDFA.cpp

@@ -60 +60 @@
     do {
         unsigned unprocessedNodeId = unprocessedNodes.takeLast();
-        const auto& node = nfa.nodes()[unprocessedNodeId];
-
-        for (uint32_t epsilonTargetIndex = node.epsilonTransitionTargetsStart; epsilonTargetIndex < node.epsilonTransitionTargetsEnd; ++epsilonTargetIndex) {
-            uint32_t targetNodeId = nfa.epsilonTransitionsTargets()[epsilonTargetIndex];
+        const auto* node = nfa.nodes().pointerAt(unprocessedNodeId);
+
+        for (uint32_t epsilonTargetIndex = node->epsilonTransitionTargetsStart; epsilonTargetIndex < node->epsilonTransitionTargetsEnd; ++epsilonTargetIndex) {
+            uint32_t targetNodeId = nfa.epsilonTransitionsTargets().valueAt(epsilonTargetIndex);
             auto addResult = closure.add(targetNodeId);
             if (addResult.isNewEntry) {
@@ -250 +250 @@
 
         for (unsigned nfaNodeId : source.nodeIdSet) {
-            const auto& nfaNode = source.nfa.nodes()[nfaNodeId];
-            for (unsigned actionIndex = nfaNode.actionStart; actionIndex < nfaNode.actionEnd; ++actionIndex)
-                actions.add(source.nfa.actions()[actionIndex]);
+            const auto* nfaNode = source.nfa.nodes().pointerAt(nfaNodeId);
+            for (unsigned actionIndex = nfaNode->actionStart; actionIndex < nfaNode->actionEnd; ++actionIndex)
+                actions.add(source.nfa.actions().valueAt(actionIndex));
         }
 

trunk/Source/WebCore/contentextensions/SerializedNFA.h

@@ -50 +50 @@
         const T* end() const { return m_begin + m_size; }
         size_t size() const { return m_size; }
-        const T& operator[](size_t i) const
+        const T* pointerAt(size_t i) const
         {
             RELEASE_ASSERT(i < m_size);
-            return begin()[i];
+            return begin() + i;
+        }
+        T valueAt(size_t i) const
+        {
+            RELEASE_ASSERT(i < m_size);
+            T value;
+            memcpy(&value, begin() + i, sizeof(T));
+            return value;
         }
     private:
@@ -76 +83 @@
         uint32_t position;
 
-        const uint32_t& operator*() const { return serializedNFA.targets()[position]; }
-        const uint32_t* operator->() const { return &serializedNFA.targets()[position]; }
+        uint32_t operator*() const { return serializedNFA.targets().valueAt(position); }
+        const uint32_t* operator->() const { return serializedNFA.targets().pointerAt(position); }
 
         bool operator==(const ConstTargetIterator& other) const
@@ -121 +128 @@
         char first() const
         {
-            return range().first;
+            return range()->first;
         }
 
         char last() const
         {
-            return range().last;
+            return range()->last;
         }
 
         IterableConstTargets data() const
         {
-            const ImmutableRange<char>& range = this->range();
-            return { serializedNFA, range.targetStart, range.targetEnd };
+            const ImmutableRange<char>* range = this->range();
+            return { serializedNFA, range->targetStart, range->targetEnd };
         };
 
     private:
-        const ImmutableRange<char>& range() const
-        {
-            return serializedNFA.transitions()[position];
+        const ImmutableRange<char>* range() const
+        {
+            return serializedNFA.transitions().pointerAt(position);
         }
     };
@@ -161 +168 @@
     IterableConstRange transitionsForNode(uint32_t nodeId) const
     {
-        const auto& node = nodes()[nodeId];
-        return { *this, node.rangesStart, node.rangesEnd };
+        const auto* node = nodes().pointerAt(nodeId);
+        return { *this, node->rangesStart, node->rangesEnd };
     }