Changeset 223958 in webkit


Ignore:
Timestamp:
Oct 25, 2017 10:19:41 AM (6 years ago)
Author:
Chris Dumez
Message:

Make SharedStringHashTable less error prone
​https://bugs.webkit.org/show_bug.cgi?id=178764

Reviewed by Youenn Fablet.

SharedStringHashTable is backed by SharedMemory and this SharedMemory
may be readonly (and is when used in the WebContent process). As a result,
some of the operations on SharedStringHashTable that write to this shared
memory will crash if called and the SharedMemory is readonly.

To make this less error prone, introduce a new SharedStringHashTableReadOnly
base class for SharedStringHashTable and only keep the operations that
write to the shared memory on SharedStringHashTableReadOnly (namely, add() /
remove() / clear(). Update VisitedLinkTableController and WebSWOriginTable
to use SharedStringHashTableReadOnly since they are instantiated in the
WebContent process and use readonly shared memory.

  • Shared/SharedStringHashTable.cpp:

(WebKit::SharedStringHashTableReadOnly::SharedStringHashTableReadOnly):
(WebKit::SharedStringHashTableReadOnly::~SharedStringHashTableReadOnly):
(WebKit::SharedStringHashTableReadOnly::setSharedMemory):
(WebKit::doubleHash):
(WebKit::SharedStringHashTableReadOnly::contains const):
(WebKit::SharedStringHashTableReadOnly::findSlot const):
(WebKit::SharedStringHashTable::SharedStringHashTable):
(WebKit::SharedStringHashTable::~SharedStringHashTable):
(WebKit::SharedStringHashTable::add):
(WebKit::SharedStringHashTable::remove):
(WebKit::SharedStringHashTable::clear):

  • Shared/SharedStringHashTable.h:
  • WebProcess/Storage/WebSWOriginTable.h:
  • WebProcess/WebPage/VisitedLinkTableController.cpp:

(WebKit::VisitedLinkTableController::removeAllVisitedLinks):

  • WebProcess/WebPage/VisitedLinkTableController.h:
Location:
trunk/Source/WebKit
Files:
8 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/CMakeLists.txt

    r223951 r223958  
    182182    Shared/ShareableResource.cpp
    183183    Shared/SharedStringHashStore.cpp
     184    Shared/SharedStringHashTableReadOnly.cpp
    184185    Shared/SharedStringHashTable.cpp
    185186    Shared/StatisticsData.cpp

  • trunk/Source/WebKit/ChangeLog

    r223953 r223958  
     12017-10-25  Chris Dumez  <cdumez@apple.com>
     2
     3        Make SharedStringHashTable less error prone
     4        https://bugs.webkit.org/show_bug.cgi?id=178764
     5
     6        Reviewed by Youenn Fablet.
     7
     8        SharedStringHashTable is backed by SharedMemory and this SharedMemory
     9        may be readonly (and is when used in the WebContent process). As a result,
     10        some of the operations on SharedStringHashTable that write to this shared
     11        memory will crash if called and the SharedMemory is readonly.
     12
     13        To make this less error prone, introduce a new SharedStringHashTableReadOnly
     14        base class for SharedStringHashTable and only keep the operations that
     15        write to the shared memory on SharedStringHashTableReadOnly (namely, add() /
     16        remove() / clear(). Update VisitedLinkTableController and WebSWOriginTable
     17        to use SharedStringHashTableReadOnly since they are instantiated in the
     18        WebContent process and use readonly shared memory.
     19
     20        * Shared/SharedStringHashTable.cpp:
     21        (WebKit::SharedStringHashTableReadOnly::SharedStringHashTableReadOnly):
     22        (WebKit::SharedStringHashTableReadOnly::~SharedStringHashTableReadOnly):
     23        (WebKit::SharedStringHashTableReadOnly::setSharedMemory):
     24        (WebKit::doubleHash):
     25        (WebKit::SharedStringHashTableReadOnly::contains const):
     26        (WebKit::SharedStringHashTableReadOnly::findSlot const):
     27        (WebKit::SharedStringHashTable::SharedStringHashTable):
     28        (WebKit::SharedStringHashTable::~SharedStringHashTable):
     29        (WebKit::SharedStringHashTable::add):
     30        (WebKit::SharedStringHashTable::remove):
     31        (WebKit::SharedStringHashTable::clear):
     32        * Shared/SharedStringHashTable.h:
     33        * WebProcess/Storage/WebSWOriginTable.h:
     34        * WebProcess/WebPage/VisitedLinkTableController.cpp:
     35        (WebKit::VisitedLinkTableController::removeAllVisitedLinks):
     36        * WebProcess/WebPage/VisitedLinkTableController.h:
     37
    1382017-10-25  Adrian Perez de Castro  <aperez@igalia.com>
    239

  • trunk/Source/WebKit/Shared/SharedStringHashTable.cpp

    r223608 r223958  
    2929#include "SharedMemory.h"
    3030
     31namespace WebKit {
     32
    3133using namespace WebCore;
    32 
    33 namespace WebKit {
    3434
    3535SharedStringHashTable::SharedStringHashTable()
    … …  
    4141}
    4242
    43 #if !ASSERT_DISABLED
    44 static inline bool isPowerOf2(unsigned v)
    45 {
    46     // Taken from http://www.cs.utk.edu/~vose/c-stuff/bithacks.html
    47    
    48     return !(v & (v - 1)) && v;
    49 }
    50 #endif
    51 
    52 void SharedStringHashTable::setSharedMemory(Ref<SharedMemory>&& sharedMemory)
    53 {
    54     m_sharedMemory = WTFMove(sharedMemory);
    55    
    56     ASSERT(m_sharedMemory);
    57     ASSERT(!(m_sharedMemory->size() % sizeof(SharedStringHash)));
    58 
    59     m_table = static_cast<SharedStringHash*>(m_sharedMemory->data());
    60     m_tableSize = m_sharedMemory->size() / sizeof(SharedStringHash);
    61     ASSERT(isPowerOf2(m_tableSize));
    62    
    63     m_tableSizeMask = m_tableSize - 1;
    64 }
    65 
    66 static inline unsigned doubleHash(unsigned key)
    67 {
    68     key = ~key + (key >> 23);
    69     key ^= (key << 12);
    70     key ^= (key >> 7);
    71     key ^= (key << 2);
    72     key ^= (key >> 20);
    73     return key;
    74 }
    75    
    7643bool SharedStringHashTable::add(SharedStringHash sharedStringHash)
    7744{
    … …  
    9764}
    9865
    99 bool SharedStringHashTable::contains(SharedStringHash sharedStringHash) const
    100 {
    101     auto* slot = findSlot(sharedStringHash);
    102     return slot && *slot;
    103 }
    104 
    105 SharedStringHash* SharedStringHashTable::findSlot(SharedStringHash sharedStringHash) const
    106 {
    107     if (!m_sharedMemory)
    108         return nullptr;
    109 
    110     int k = 0;
    111     SharedStringHash* table = m_table;
    112     int sizeMask = m_tableSizeMask;
    113     unsigned h = static_cast<unsigned>(sharedStringHash);
    114     int i = h & sizeMask;
    115 
    116     SharedStringHash* entry;
    117     while (1) {
    118         entry = table + i;
    119 
    120         // Check if we've reached the end of the table.
    121         if (!*entry)
    122             return entry;
    123 
    124         if (*entry == sharedStringHash)
    125             return entry;
    126 
    127         if (!k)
    128             k = 1 | doubleHash(h);
    129         i = (i + k) & sizeMask;
    130     }
    131 }
    132 
    13366void SharedStringHashTable::clear()
    13467{
    … …  
    13770
    13871    memset(m_sharedMemory->data(), 0, m_sharedMemory->size());
    139     m_table = nullptr;
    140     m_tableSize = 0;
    141     m_tableSizeMask = 0;
    142     m_sharedMemory = nullptr;
     72    setSharedMemory(nullptr);
    14373}
    14474

  • trunk/Source/WebKit/Shared/SharedStringHashTable.h

    r222820 r223958  
    2626#pragma once
    2727
    28 #include <WebCore/SharedStringHash.h>
    29 #include <wtf/RefPtr.h>
     28#include "SharedStringHashTableReadOnly.h"
    3029
    3130namespace WebKit {
    3231
    33 class SharedMemory;
    34 
    35 class SharedStringHashTable {
     32class SharedStringHashTable : public SharedStringHashTableReadOnly {
    3633public:
    3734    SharedStringHashTable();
    3835    ~SharedStringHashTable();
    3936
    40     void setSharedMemory(Ref<SharedMemory>&&);
    41 
    42     // Can only be called if the underlying shared memory is in read / write mode.
    4337    bool add(WebCore::SharedStringHash);
    4438    bool remove(WebCore::SharedStringHash);
    45 
    46     bool contains(WebCore::SharedStringHash) const;
    47 
    48     SharedMemory* sharedMemory() const { return m_sharedMemory.get(); }
    4939    void clear();
    50 
    51 private:
    52     RefPtr<SharedMemory> m_sharedMemory;
    53     WebCore::SharedStringHash* findSlot(WebCore::SharedStringHash) const;
    54 
    55     unsigned m_tableSize { 0 };
    56     unsigned m_tableSizeMask { 0 };
    57     WebCore::SharedStringHash* m_table { nullptr };
    5840};
    5941

  • trunk/Source/WebKit/Shared/SharedStringHashTableReadOnly.cpp

    r223957 r223958  
    2525
    2626#include "config.h"
    27 #include "SharedStringHashTable.h"
     27#include "SharedStringHashTableReadOnly.h"
    2828
    2929#include "SharedMemory.h"
    3030
    31 using namespace WebCore;
    32 
    3331namespace WebKit {
    3432
    35 SharedStringHashTable::SharedStringHashTable()
    36 {
    37 }
    38 
    39 SharedStringHashTable::~SharedStringHashTable()
    40 {
    41 }
     33using namespace WebCore;
    4234
    4335#if !ASSERT_DISABLED
    … …  
    4537{
    4638    // Taken from http://www.cs.utk.edu/~vose/c-stuff/bithacks.html
    47    
     39
    4840    return !(v & (v - 1)) && v;
    4941}
    5042#endif
    51 
    52 void SharedStringHashTable::setSharedMemory(Ref<SharedMemory>&& sharedMemory)
    53 {
    54     m_sharedMemory = WTFMove(sharedMemory);
    55    
    56     ASSERT(m_sharedMemory);
    57     ASSERT(!(m_sharedMemory->size() % sizeof(SharedStringHash)));
    58 
    59     m_table = static_cast<SharedStringHash*>(m_sharedMemory->data());
    60     m_tableSize = m_sharedMemory->size() / sizeof(SharedStringHash);
    61     ASSERT(isPowerOf2(m_tableSize));
    62    
    63     m_tableSizeMask = m_tableSize - 1;
    64 }
    6543
    6644static inline unsigned doubleHash(unsigned key)
    … …  
    7351    return key;
    7452}
    75    
    76 bool SharedStringHashTable::add(SharedStringHash sharedStringHash)
     53
     54SharedStringHashTableReadOnly::SharedStringHashTableReadOnly()
    7755{
    78     auto* slot = findSlot(sharedStringHash);
    79     ASSERT(slot);
    80 
    81     // Check if the same link hash is in the table already.
    82     if (*slot)
    83         return false;
    84 
    85     *slot = sharedStringHash;
    86     return true;
    8756}
    8857
    89 bool SharedStringHashTable::remove(SharedStringHash sharedStringHash)
     58SharedStringHashTableReadOnly::~SharedStringHashTableReadOnly()
    9059{
    91     auto* slot = findSlot(sharedStringHash);
    92     if (!slot || !*slot)
    93         return false;
    94 
    95     *slot = 0;
    96     return true;
    9760}
    9861
    99 bool SharedStringHashTable::contains(SharedStringHash sharedStringHash) const
     62void SharedStringHashTableReadOnly::setSharedMemory(RefPtr<SharedMemory>&& sharedMemory)
     63{
     64    m_sharedMemory = WTFMove(sharedMemory);
     65
     66    if (m_sharedMemory) {
     67        ASSERT(!(m_sharedMemory->size() % sizeof(SharedStringHash)));
     68        m_table = static_cast<SharedStringHash*>(m_sharedMemory->data());
     69        m_tableSize = m_sharedMemory->size() / sizeof(SharedStringHash);
     70        ASSERT(isPowerOf2(m_tableSize));
     71        m_tableSizeMask = m_tableSize - 1;
     72    } else {
     73        m_table = nullptr;
     74        m_tableSize = 0;
     75        m_tableSizeMask = 0;
     76    }
     77}
     78
     79bool SharedStringHashTableReadOnly::contains(SharedStringHash sharedStringHash) const
    10080{
    10181    auto* slot = findSlot(sharedStringHash);
    … …  
    10383}
    10484
    105 SharedStringHash* SharedStringHashTable::findSlot(SharedStringHash sharedStringHash) const
     85SharedStringHash* SharedStringHashTableReadOnly::findSlot(SharedStringHash sharedStringHash) const
    10686{
    10787    if (!m_sharedMemory)
    … …  
    131111}
    132112
    133 void SharedStringHashTable::clear()
    134 {
    135     if (!m_sharedMemory)
    136         return;
    137 
    138     memset(m_sharedMemory->data(), 0, m_sharedMemory->size());
    139     m_table = nullptr;
    140     m_tableSize = 0;
    141     m_tableSizeMask = 0;
    142     m_sharedMemory = nullptr;
    143 }
    144 
    145113} // namespace WebKit

  • trunk/Source/WebKit/Shared/SharedStringHashTableReadOnly.h

    r223957 r223958  
    3333class SharedMemory;
    3434
    35 class SharedStringHashTable {
     35class SharedStringHashTableReadOnly {
    3636public:
    37     SharedStringHashTable();
    38     ~SharedStringHashTable();
    39 
    40     void setSharedMemory(Ref<SharedMemory>&&);
    41 
    42     // Can only be called if the underlying shared memory is in read / write mode.
    43     bool add(WebCore::SharedStringHash);
    44     bool remove(WebCore::SharedStringHash);
     37    SharedStringHashTableReadOnly();
     38    ~SharedStringHashTableReadOnly();
    4539
    4640    bool contains(WebCore::SharedStringHash) const;
    4741
    4842    SharedMemory* sharedMemory() const { return m_sharedMemory.get(); }
    49     void clear();
     43    void setSharedMemory(RefPtr<SharedMemory>&&);
    5044
    51 private:
    52     RefPtr<SharedMemory> m_sharedMemory;
     45protected:
    5346    WebCore::SharedStringHash* findSlot(WebCore::SharedStringHash) const;
    5447
     48    RefPtr<SharedMemory> m_sharedMemory;
    5549    unsigned m_tableSize { 0 };
    5650    unsigned m_tableSizeMask { 0 };
    … …  
    5852};
    5953
    60 }
     54} // namespace WebKit

  • trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj

    r223903 r223958  
    14021402                83F1A0791F96E7790045B94E /* WebSWOriginTable.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83F1A0771F96E7700045B94E /* WebSWOriginTable.cpp */; };
    14031403                83F1A07A1F96E7790045B94E /* WebSWOriginTable.h in Headers */ = {isa = PBXBuildFile; fileRef = 83F1A0781F96E7710045B94E /* WebSWOriginTable.h */; };
     1404                83F9644D1FA0F76E00C47750 /* SharedStringHashTableReadOnly.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83F9644B1FA0F76200C47750 /* SharedStringHashTableReadOnly.cpp */; };
     1405                83F9644E1FA0F76E00C47750 /* SharedStringHashTableReadOnly.h in Headers */ = {isa = PBXBuildFile; fileRef = 83F9644C1FA0F76300C47750 /* SharedStringHashTableReadOnly.h */; };
    14041406                84477853176FCC0800CDC7BB /* InjectedBundleHitTestResultMediaType.h in Headers */ = {isa = PBXBuildFile; fileRef = 84477851176FCAC100CDC7BB /* InjectedBundleHitTestResultMediaType.h */; };
    14051407                868160D0187645570021E79D /* WindowServerConnection.mm in Sources */ = {isa = PBXBuildFile; fileRef = 868160CF187645370021E79D /* WindowServerConnection.mm */; };
    … …  
    37203722                7CDE73A11F9DA41400390312 /* GeneratePreferences.rb */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.script.ruby; path = GeneratePreferences.rb; sourceTree = "<group>"; };
    37213723                7CDE73A21F9DA59700390312 /* PreferencesTemplates */ = {isa = PBXFileReference; lastKnownFileType = folder; path = PreferencesTemplates; sourceTree = "<group>"; };
    3722                 7CDE73A31F9DAB6500390312 /* WebPreferencesDefinitions.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = WebPreferencesDefinitions.h; path = WebPreferencesDefinitions.h; sourceTree = "<group>"; };
     3724                7CDE73A31F9DAB6500390312 /* WebPreferencesDefinitions.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WebPreferencesDefinitions.h; sourceTree = "<group>"; };
    37233725                7CE4D2061A46775700C7F152 /* APILegacyContextHistoryClient.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = APILegacyContextHistoryClient.h; sourceTree = "<group>"; };
    37243726                7CE4D2151A49148400C7F152 /* WebProcessPoolCocoa.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebProcessPoolCocoa.mm; sourceTree = "<group>"; };
    … …  
    37803782                83F1A0771F96E7700045B94E /* WebSWOriginTable.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebSWOriginTable.cpp; sourceTree = "<group>"; };
    37813783                83F1A0781F96E7710045B94E /* WebSWOriginTable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebSWOriginTable.h; sourceTree = "<group>"; };
     3784                83F9644B1FA0F76200C47750 /* SharedStringHashTableReadOnly.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SharedStringHashTableReadOnly.cpp; sourceTree = "<group>"; };
     3785                83F9644C1FA0F76300C47750 /* SharedStringHashTableReadOnly.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SharedStringHashTableReadOnly.h; sourceTree = "<group>"; };
    37823786                84477851176FCAC100CDC7BB /* InjectedBundleHitTestResultMediaType.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = InjectedBundleHitTestResultMediaType.h; sourceTree = "<group>"; };
    37833787                868160CD18763D4B0021E79D /* WindowServerConnection.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WindowServerConnection.h; sourceTree = "<group>"; };
    … …  
    50735077                                8313F7E81F7DAE0300B944EB /* SharedStringHashTable.cpp */,
    50745078                                8313F7E71F7DAE0300B944EB /* SharedStringHashTable.h */,
     5079                                83F9644B1FA0F76200C47750 /* SharedStringHashTableReadOnly.cpp */,
     5080                                83F9644C1FA0F76300C47750 /* SharedStringHashTableReadOnly.h */,
    50755081                                5272B2881406985D0096A5D0 /* StatisticsData.cpp */,
    50765082                                5272B2891406985D0096A5D0 /* StatisticsData.h */,
    … …  
    88358841                                8313F7EC1F7DAE0800B944EB /* SharedStringHashStore.h in Headers */,
    88368842                                8313F7EE1F7DAE0800B944EB /* SharedStringHashTable.h in Headers */,
     8843                                83F9644E1FA0F76E00C47750 /* SharedStringHashTableReadOnly.h in Headers */,
    88378844                                2DAF06D618BD1A470081CEB1 /* SmartMagnificationController.h in Headers */,
    88388845                                2DE6943E18BD2A68005C15E5 /* SmartMagnificationControllerMessages.h in Headers */,
    … …  
    1043010437                                8313F7EB1F7DAE0800B944EB /* SharedStringHashStore.cpp in Sources */,
    1043110438                                8313F7ED1F7DAE0800B944EB /* SharedStringHashTable.cpp in Sources */,
     10439                                83F9644D1FA0F76E00C47750 /* SharedStringHashTableReadOnly.cpp in Sources */,
    1043210440                                2DAF06D718BD1A470081CEB1 /* SmartMagnificationController.mm in Sources */,
    1043310441                                2DE6943D18BD2A68005C15E5 /* SmartMagnificationControllerMessageReceiver.cpp in Sources */,

  • trunk/Source/WebKit/WebProcess/Storage/WebSWOriginTable.h

    r223608 r223958  
    2929
    3030#include "SharedMemory.h"
    31 #include "SharedStringHashTable.h"
     31#include "SharedStringHashTableReadOnly.h"
    3232
    3333namespace WebCore {
    … …  
    4545
    4646private:
    47     SharedStringHashTable m_serviceWorkerOriginTable;
     47    SharedStringHashTableReadOnly m_serviceWorkerOriginTable;
    4848};
    4949

  • trunk/Source/WebKit/WebProcess/WebPage/VisitedLinkTableController.cpp

    r222664 r223958  
    113113void VisitedLinkTableController::removeAllVisitedLinks()
    114114{
    115     m_visitedLinkTable.clear();
     115    m_visitedLinkTable.setSharedMemory(nullptr);
    116116
    117117    invalidateStylesForAllLinks();

  • trunk/Source/WebKit/WebProcess/WebPage/VisitedLinkTableController.h

    r222664 r223958  
    2828#include "MessageReceiver.h"
    2929#include "SharedMemory.h"
    30 #include "SharedStringHashTable.h"
     30#include "SharedStringHashTableReadOnly.h"
    3131#include <WebCore/VisitedLinkStore.h>
    3232
    … …  
    5454
    5555    uint64_t m_identifier;
    56     SharedStringHashTable m_visitedLinkTable;
     56    SharedStringHashTableReadOnly m_visitedLinkTable;
    5757};
    5858
Note: See TracChangeset for help on using the changeset viewer.