Changeset 54736 in webkit


Ignore:
Timestamp:
Feb 12, 2010 2:22:09 PM (14 years ago)
Author:
barraclough@apple.com
Message:

​https://bugs.webkit.org/show_bug.cgi?id=33731
Remove uses of PtrAndFlags from WebCore::StringImpl.

Reviewed by Sam Weinig.

These break the OS X Leaks tool. Use a bits stolen from the refCount to hold the
'InTable' and 'HasTerminatingNullCharacter' flags, along with the string type
(fixes a leak where the string data is allocated at the address (this + 1), and is
misinterpreted as being an internal buffer).

  • WebCore.base.exp:
  • platform/text/StringImpl.cpp:

(WebCore::StringImpl::StringImpl):
(WebCore::StringImpl::~StringImpl):
(WebCore::StringImpl::create):
(WebCore::StringImpl::createWithTerminatingNullCharacter):
(WebCore::StringImpl::crossThreadString):
(WebCore::StringImpl::sharedBuffer):

  • platform/text/StringImpl.h:

(WebCore::StringImpl::):
(WebCore::StringImpl::hasTerminatingNullCharacter):
(WebCore::StringImpl::inTable):
(WebCore::StringImpl::setInTable):
(WebCore::StringImpl::ref):
(WebCore::StringImpl::deref):
(WebCore::StringImpl::hasOneRef):
(WebCore::StringImpl::operator new):
(WebCore::StringImpl::bufferOwnership):

  • storage/OriginUsageRecord.cpp:

(WebCore::OriginUsageRecord::addDatabase):
(WebCore::OriginUsageRecord::markDatabase):

Location:
trunk/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r54735 r54736  
     12010-02-12  Gavin Barraclough  <barraclough@apple.com>
     2
     3        Reviewed by Sam Weinig.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=33731
     6        Remove uses of PtrAndFlags from WebCore::StringImpl.
     7
     8        These break the OS X Leaks tool.  Use a bits stolen from the refCount to hold the
     9        'InTable' and 'HasTerminatingNullCharacter' flags, along with the string type
     10        (fixes a leak where the string data is allocated at the address (this + 1), and is
     11        misinterpreted as being an internal buffer).
     12
     13        * WebCore.base.exp:
     14        * platform/text/StringImpl.cpp:
     15        (WebCore::StringImpl::StringImpl):
     16        (WebCore::StringImpl::~StringImpl):
     17        (WebCore::StringImpl::create):
     18        (WebCore::StringImpl::createWithTerminatingNullCharacter):
     19        (WebCore::StringImpl::crossThreadString):
     20        (WebCore::StringImpl::sharedBuffer):
     21        * platform/text/StringImpl.h:
     22        (WebCore::StringImpl::):
     23        (WebCore::StringImpl::hasTerminatingNullCharacter):
     24        (WebCore::StringImpl::inTable):
     25        (WebCore::StringImpl::setInTable):
     26        (WebCore::StringImpl::ref):
     27        (WebCore::StringImpl::deref):
     28        (WebCore::StringImpl::hasOneRef):
     29        (WebCore::StringImpl::operator new):
     30        (WebCore::StringImpl::bufferOwnership):
     31        * storage/OriginUsageRecord.cpp:
     32        (WebCore::OriginUsageRecord::addDatabase):
     33        (WebCore::OriginUsageRecord::markDatabase):
     34
    1352010-02-12  Nikolas Zimmermann  <nzimmermann@rim.com>
    236

  • trunk/WebCore/WebCore.base.exp

    r54663 r54736  
    145145__ZN7WebCore10StringImplD1Ev
    146146__ZN7WebCore10StringImplcvP8NSStringEv
    147 __ZN7WebCore10StringImpldlEPv
    148147__ZN7WebCore10handCursorEv
    149148__ZN7WebCore10setCookiesEPNS_8DocumentERKNS_4KURLERKNS_6StringE

  • trunk/WebCore/platform/text/StringImpl.cpp

    r54460 r54736  
    5353}
    5454
    55 static inline void deleteUCharVector(const UChar* p)
    56 {
    57     fastFree(const_cast<UChar*>(p));
    58 }
    59 
    60 // Some of the factory methods create buffers using fastMalloc.
    61 // We must ensure that all allocations of StringImpl are allocated using
    62 // fastMalloc so that we don't have mis-matched frees. We accomplish
    63 // this by overriding the new and delete operators.
    64 void* StringImpl::operator new(size_t size, void* address)
    65 {
    66     if (address)
    67         return address;  // Allocating using an internal buffer
    68     return fastMalloc(size);
    69 }
    70 
    71 void* StringImpl::operator new(size_t size)
    72 {
    73     return fastMalloc(size);
    74 }
    75 
    76 void StringImpl::operator delete(void* address)
    77 {
    78     fastFree(address);
    79 }
    80 
    8155// This constructor is used only to create the empty string.
    8256StringImpl::StringImpl()
    8357    : m_data(0)
     58    , m_sharedBuffer(0)
    8459    , m_length(0)
     60    , m_refCountAndFlags(s_refCountIncrement | BufferInternal)
    8561    , m_hash(0)
    8662{
    … …  
    9167}
    9268
     69inline StringImpl::StringImpl(unsigned length)
     70    : m_data(reinterpret_cast<const UChar*>(this + 1))
     71    , m_sharedBuffer(0)
     72    , m_length(length)
     73    , m_refCountAndFlags(s_refCountIncrement | BufferInternal)
     74    , m_hash(0)
     75{
     76    ASSERT(m_data);
     77    ASSERT(m_length);
     78}
     79
    9380inline StringImpl::StringImpl(const UChar* characters, unsigned length)
    9481    : m_data(characters)
     82    , m_sharedBuffer(0)
    9583    , m_length(length)
     84    , m_refCountAndFlags(s_refCountIncrement | BufferOwned)
    9685    , m_hash(0)
    9786{
    98     ASSERT(characters);
    99     ASSERT(length);
    100     ASSERT(!bufferIsInternal());
    101 }
    102 
    103 inline StringImpl::StringImpl(unsigned length)
    104     : m_data(reinterpret_cast<const UChar*>(this + 1))
     87    ASSERT(m_data);
     88    ASSERT(m_length);
     89}
     90
     91inline StringImpl::StringImpl(const UChar* characters, unsigned length, PassRefPtr<SharedUChar> sharedBuffer)
     92    : m_data(characters)
     93    , m_sharedBuffer(sharedBuffer.releaseRef())
    10594    , m_length(length)
     95    , m_refCountAndFlags(s_refCountIncrement | BufferShared)
    10696    , m_hash(0)
    10797{
    108     ASSERT(length);
    109     ASSERT(bufferIsInternal());
     98    ASSERT(m_data);
     99    ASSERT(m_length);
    110100}
    111101
    … …  
    114104    if (inTable())
    115105        AtomicString::remove(this);
    116     if (!bufferIsInternal()) {
    117         SharedUChar* sharedBuffer = m_sharedBufferAndFlags.get();
    118         if (sharedBuffer)
    119             sharedBuffer->deref();
    120         else
    121             deleteUCharVector(m_data);
     106
     107    BufferOwnership ownership = bufferOwnership();
     108    if (ownership != BufferInternal) {
     109        if (ownership == BufferOwned) {
     110            ASSERT(!m_sharedBuffer);
     111            ASSERT(m_data);
     112            fastFree(const_cast<UChar*>(m_data));
     113        } else {
     114            ASSERT(ownership == BufferShared);
     115            ASSERT(m_sharedBuffer);
     116            m_sharedBuffer->deref();
     117        }
    122118    }
    123119}
    … …  
    977973PassRefPtr<StringImpl> StringImpl::create(const JSC::UString& str)
    978974{
    979     SharedUChar* sharedBuffer = const_cast<JSC::UString*>(&str)->rep()->sharedBuffer();
    980     if (sharedBuffer) {
    981         PassRefPtr<StringImpl> impl = adoptRef(new StringImpl(str.data(), str.size()));
    982         sharedBuffer->ref();
    983         impl->m_sharedBufferAndFlags.set(sharedBuffer);
    984         return impl;
    985     }
     975    if (SharedUChar* sharedBuffer = const_cast<JSC::UString*>(&str)->rep()->sharedBuffer())
     976        return adoptRef(new StringImpl(str.data(), str.size(), sharedBuffer));
    986977    return StringImpl::create(str.data(), str.size());
    987978}
    … …  
    1008999    terminatedString->m_length--;
    10091000    terminatedString->m_hash = string.m_hash;
    1010     terminatedString->m_sharedBufferAndFlags.setFlag(HasTerminatingNullCharacter);
     1001    terminatedString->m_refCountAndFlags |= s_refCountFlagHasTerminatingNullCharacter;
    10111002    return terminatedString.release();
    10121003}
    … …  
    10221013PassRefPtr<StringImpl> StringImpl::crossThreadString()
    10231014{
    1024     SharedUChar* shared = sharedBuffer();
    1025     if (shared) {
    1026         RefPtr<StringImpl> impl = adoptRef(new StringImpl(m_data, m_length));
    1027         impl->m_sharedBufferAndFlags.set(shared->crossThreadCopy().releaseRef());
    1028         return impl.release();
    1029     }
     1015    if (SharedUChar* sharedBuffer = this->sharedBuffer())
     1016        return adoptRef(new StringImpl(m_data, m_length, sharedBuffer->crossThreadCopy()));
    10301017
    10311018    // If no shared buffer is available, create a copy.
    … …  
    10351022StringImpl::SharedUChar* StringImpl::sharedBuffer()
    10361023{
    1037     if (m_length < minLengthToShare || bufferIsInternal())
     1024    if (m_length < minLengthToShare)
    10381025        return 0;
    10391026
    1040     if (!m_sharedBufferAndFlags.get())
    1041         m_sharedBufferAndFlags.set(SharedUChar::create(new OwnFastMallocPtr<UChar>(const_cast<UChar*>(m_data))).releaseRef());
    1042     return m_sharedBufferAndFlags.get();
    1043 }
    1044 
     1027    BufferOwnership ownership = bufferOwnership();
     1028
     1029    if (ownership == BufferInternal)
     1030        return 0;
     1031
     1032    if (ownership == BufferOwned) {
     1033        ASSERT(!m_sharedBuffer);
     1034        m_sharedBuffer = SharedUChar::create(new OwnFastMallocPtr<UChar>(const_cast<UChar*>(m_data))).releaseRef();
     1035        m_refCountAndFlags = (m_refCountAndFlags & ~s_refCountMaskBufferOwnership) | BufferShared;
     1036    }
     1037
     1038    ASSERT(bufferOwnership() == BufferShared);
     1039    ASSERT(m_sharedBuffer);
     1040    return m_sharedBuffer;
     1041}
    10451042
    10461043} // namespace WebCore

  • trunk/WebCore/platform/text/StringImpl.h

    r54460 r54736  
    2727#include <wtf/ASCIICType.h>
    2828#include <wtf/CrossThreadRefCounted.h>
     29#include <wtf/Noncopyable.h>
    2930#include <wtf/OwnFastMallocPtr.h>
    30 #include <wtf/PtrAndFlags.h>
    3131#include <wtf/RefCounted.h>
    3232#include <wtf/StringHashFunctions.h>
    … …  
    5959typedef bool (*CharacterMatchFunctionPtr)(UChar);
    6060
    61 class StringImpl : public RefCounted<StringImpl> {
     61class StringImpl : public Noncopyable {
    6262    friend struct CStringTranslator;
    6363    friend struct HashAndCharactersTranslator;
    … …  
    6565private:
    6666    friend class ThreadGlobalData;
     67
     68    enum BufferOwnership {
     69        BufferInternal,
     70        BufferOwned,
     71        BufferShared,
     72    };
     73
     74    typedef CrossThreadRefCounted<OwnFastMallocPtr<UChar> > SharedUChar;
     75
     76    // Used to create the empty string (""), automatically hashes.
    6777    StringImpl();
    68    
    69     // This constructor adopts the UChar* without copying the buffer.
     78    // Create a StringImpl with internal storage (BufferInternal)
     79    StringImpl(unsigned length);
     80    // Create a StringImpl adopting ownership of the provided buffer (BufferOwned)
    7081    StringImpl(const UChar*, unsigned length);
    71 
    72     // This constructor assumes that 'this' was allocated with a UChar buffer of size 'length' at the end.
    73     StringImpl(unsigned length);
     82    // Create a StringImpl using a shared buffer (BufferShared)
     83    StringImpl(const UChar*, unsigned length, PassRefPtr<SharedUChar>);
    7484
    7585    // For use only by AtomicString's XXXTranslator helpers.
    7686    void setHash(unsigned hash) { ASSERT(!m_hash); m_hash = hash; }
    7787   
    78     typedef CrossThreadRefCounted<OwnFastMallocPtr<UChar> > SharedUChar;
    79 
    8088public:
    8189    ~StringImpl();
    … …  
    100108    unsigned length() { return m_length; }
    101109
    102     bool hasTerminatingNullCharacter() const { return m_sharedBufferAndFlags.isFlagSet(HasTerminatingNullCharacter); }
    103 
    104     bool inTable() const { return m_sharedBufferAndFlags.isFlagSet(InTable); }
    105     void setInTable() { return m_sharedBufferAndFlags.setFlag(InTable); }
     110    bool hasTerminatingNullCharacter() const { return m_refCountAndFlags & s_refCountFlagHasTerminatingNullCharacter; }
     111
     112    bool inTable() const { return m_refCountAndFlags & s_refCountFlagInTable; }
     113    void setInTable() { m_refCountAndFlags |= s_refCountFlagInTable; }
    106114
    107115    unsigned hash() { if (m_hash == 0) m_hash = computeHash(m_data, m_length); return m_hash; }
    … …  
    109117    inline static unsigned computeHash(const UChar* data, unsigned length) { return WTF::stringHash(data, length); }
    110118    inline static unsigned computeHash(const char* data) { return WTF::stringHash(data); }
    111    
     119
     120    StringImpl* ref() { m_refCountAndFlags += s_refCountIncrement; return this; }
     121    ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) delete this; }
     122    ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & s_refCountMask) == s_refCountIncrement; }
     123
    112124    // Returns a StringImpl suitable for use on another thread.
    113125    PassRefPtr<StringImpl> crossThreadString();
    … …  
    179191#endif
    180192
    181     void operator delete(void*);
    182 
    183193private:
    184     // Allocation from a custom buffer is only allowed internally to avoid
    185     // mismatched allocators. Callers should use create().
    186     void* operator new(size_t size);
    187     void* operator new(size_t size, void* address);
     194    using Noncopyable::operator new;
     195    void* operator new(size_t, void* inPlace) { ASSERT(inPlace); return inPlace; }
    188196
    189197    static PassRefPtr<StringImpl> createStrippingNullCharactersSlowCase(const UChar*, unsigned length);
    … …  
    191199    // The StringImpl struct and its data may be allocated within a single heap block.
    192200    // In this case, the m_data pointer is an "internal buffer", and does not need to be deallocated.
    193     bool bufferIsInternal() { return m_data == reinterpret_cast<const UChar*>(this + 1); }
    194 
    195     enum StringImplFlags {
    196         HasTerminatingNullCharacter,
    197         InTable,
    198     };
     201    BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
     202
     203    static const unsigned s_refCountMask = 0xFFFFFFF0;
     204    static const unsigned s_refCountIncrement = 0x10;
     205    static const unsigned s_refCountFlagHasTerminatingNullCharacter = 0x8;
     206    static const unsigned s_refCountFlagInTable = 0x4;
     207    static const unsigned s_refCountMaskBufferOwnership = 0x3;
    199208
    200209    const UChar* m_data;
     210    SharedUChar* m_sharedBuffer;
    201211    unsigned m_length;
     212    unsigned m_refCountAndFlags;
    202213    mutable unsigned m_hash;
    203     PtrAndFlags<SharedUChar, StringImplFlags> m_sharedBufferAndFlags;
    204     // There is a fictitious variable-length UChar array at the end, which is used
    205     // as the internal buffer by the createUninitialized and create methods.
    206214};
    207215

  • trunk/WebCore/storage/OriginUsageRecord.cpp

    r53575 r54736  
    4343{
    4444    ASSERT(!m_databaseMap.contains(identifier));
    45     ASSERT_ARG(identifier, identifier.impl()->refCount() == 1);
    46     ASSERT_ARG(fullPath, fullPath.impl()->refCount() == 1);
     45    ASSERT_ARG(identifier, identifier.impl()->hasOneRef());
     46    ASSERT_ARG(fullPath, fullPath.impl()->hasOneRef());
    4747
    4848    m_databaseMap.set(identifier, DatabaseEntry(fullPath));
    … …  
    6464{
    6565    ASSERT(m_databaseMap.contains(identifier));
    66     ASSERT_ARG(identifier, identifier.impl()->refCount() == 1);
     66    ASSERT_ARG(identifier, identifier.impl()->hasOneRef());
    6767
    6868    m_unknownSet.add(identifier);
Note: See TracChangeset for help on using the changeset viewer.