Changeset 203834 in webkit


Ignore:
Timestamp:
Jul 28, 2016 1:28:47 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

StringView should have an explicit m_is8Bit field.
https://bugs.webkit.org/show_bug.cgi?id=160282
<rdar://problem/27327943>

Reviewed by Benjamin Poulain.

Source/JavaScriptCore:

  • tests/stress/string-joining-long-strings-should-not-crash.js: Added.

(catch):

Source/WTF:

The current implementation reserves 1 bit in the 32-bit m_length field as an
is16Bit flag. As a result, a StringView is incapable of handling strings that
have a length of 32-bit in size. This results in a mismatch with the
expectations of String, StringImpl, and JavaScriptCore's JSString which all
support a 32-bit unsigned length.

This patch fixes this issue by introducing an explicit m_is8Bit field, thereby
allowing m_length to be a full 32-bit again.

We also introduced a clear() convenience method to set the fields of StringView
to empty values. Previously, we were duplicating the code for clearing those
fields. We now call clear() in all those places instead.

Note: in clear(), we set m_is8Bit to true because we want an empty StringView
to be 8-bit rather than 16-bit. This is consistent with what the empty() method
returns.

  • wtf/text/StringView.h:

(WTF::StringView::setUnderlyingString):
(WTF::StringView::StringView):
(WTF::StringView::operator=):
(WTF::StringView::initialize):
(WTF::StringView::clear):
(WTF::StringView::empty):
(WTF::StringView::length):
(WTF::StringView::operator bool):
(WTF::StringView::is8Bit):
(WTF::StringView::substring):
(WTF::StringView::getCharactersWithUpconvert):
(WTF::StringView::toString):
(WTF::StringView::toAtomicString):
(WTF::StringView::toFloat):
(WTF::StringView::toInt):
(WTF::StringView::toIntStrict):
(WTF::StringView::toStringWithoutCopying):
(WTF::StringView::find):

Location:
trunk/Source
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r203817 r203834  
     12016-07-28  Mark Lam  <mark.lam@apple.com>
     2
     3        StringView should have an explicit m_is8Bit field.
     4        https://bugs.webkit.org/show_bug.cgi?id=160282
     5        <rdar://problem/27327943>
     6
     7        Reviewed by Benjamin Poulain.
     8
     9        * tests/stress/string-joining-long-strings-should-not-crash.js: Added.
     10        (catch):
     11
    1122016-07-28  Csaba Osztrogonác  <ossy@webkit.org>
    213
  • trunk/Source/WTF/ChangeLog

    r203670 r203834  
     12016-07-28  Mark Lam  <mark.lam@apple.com>
     2
     3        StringView should have an explicit m_is8Bit field.
     4        https://bugs.webkit.org/show_bug.cgi?id=160282
     5        <rdar://problem/27327943>
     6
     7        Reviewed by Benjamin Poulain.
     8
     9        The current implementation reserves 1 bit in the 32-bit m_length field as an
     10        is16Bit flag.  As a result, a StringView is incapable of handling strings that
     11        have a length of 32-bit in size.  This results in a mismatch with the
     12        expectations of String, StringImpl, and JavaScriptCore's JSString which all
     13        support a 32-bit unsigned length.
     14
     15        This patch fixes this issue by introducing an explicit m_is8Bit field, thereby
     16        allowing m_length to be a full 32-bit again.
     17
     18        We also introduced a clear() convenience method to set the fields of StringView
     19        to empty values.  Previously, we were duplicating the code for clearing those
     20        fields.  We now call clear() in all those places instead.
     21
     22        Note: in clear(), we set m_is8Bit to true because we want an empty StringView
     23        to be 8-bit rather than 16-bit.  This is consistent with what the empty() method
     24        returns.
     25
     26        * wtf/text/StringView.h:
     27        (WTF::StringView::setUnderlyingString):
     28        (WTF::StringView::StringView):
     29        (WTF::StringView::operator=):
     30        (WTF::StringView::initialize):
     31        (WTF::StringView::clear):
     32        (WTF::StringView::empty):
     33        (WTF::StringView::length):
     34        (WTF::StringView::operator bool):
     35        (WTF::StringView::is8Bit):
     36        (WTF::StringView::substring):
     37        (WTF::StringView::getCharactersWithUpconvert):
     38        (WTF::StringView::toString):
     39        (WTF::StringView::toAtomicString):
     40        (WTF::StringView::toFloat):
     41        (WTF::StringView::toInt):
     42        (WTF::StringView::toIntStrict):
     43        (WTF::StringView::toStringWithoutCopying):
     44        (WTF::StringView::find):
     45
    1462016-07-24  Filip Pizlo  <fpizlo@apple.com>
    247
  • trunk/Source/WTF/wtf/text/StringView.h

    r203208 r203834  
    11/*
    2  * Copyright (C) 2014-2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2014-2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    157157    void setUnderlyingString(const StringView&) { }
    158158#endif
    159 
    160     static const unsigned is16BitStringFlag = 1u << 31;
     159    void clear();
    161160
    162161    const void* m_characters { nullptr };
    163162    unsigned m_length { 0 };
     163    bool m_is8Bit { true };
    164164
    165165#if CHECK_STRINGVIEW_LIFETIME
     
    211211    : m_characters(other.m_characters)
    212212    , m_length(other.m_length)
     213    , m_is8Bit(other.m_is8Bit)
    213214{
    214215    ASSERT(other.underlyingStringIsValid());
    215216
    216     other.m_characters = nullptr;
    217     other.m_length = 0;
     217    other.clear();
    218218
    219219    setUnderlyingString(other);
     
    224224    : m_characters(other.m_characters)
    225225    , m_length(other.m_length)
     226    , m_is8Bit(other.m_is8Bit)
    226227{
    227228    ASSERT(other.underlyingStringIsValid());
     
    236237    m_characters = other.m_characters;
    237238    m_length = other.m_length;
    238 
    239     other.m_characters = nullptr;
    240     other.m_length = 0;
     239    m_is8Bit = other.m_is8Bit;
     240
     241    other.clear();
    241242
    242243    setUnderlyingString(other);
     
    252253    m_characters = other.m_characters;
    253254    m_length = other.m_length;
     255    m_is8Bit = other.m_is8Bit;
    254256
    255257    setUnderlyingString(other);
     
    260262inline void StringView::initialize(const LChar* characters, unsigned length)
    261263{
    262     // FIXME: We need a better solution here, because there is no guarantee that
    263     // the length here won't be too long. Maybe at least a RELEASE_ASSERT?
    264     ASSERT(!(length & is16BitStringFlag));
    265264    m_characters = characters;
    266265    m_length = length;
     266    m_is8Bit = true;
    267267}
    268268
    269269inline void StringView::initialize(const UChar* characters, unsigned length)
    270270{
    271     // FIXME: We need a better solution here, because there is no guarantee that
    272     // the length here won't be too long. Maybe at least a RELEASE_ASSERT?
    273     ASSERT(!(length & is16BitStringFlag));
    274271    m_characters = characters;
    275     m_length = is16BitStringFlag | length;
     272    m_length = length;
     273    m_is8Bit = false;
    276274}
    277275
     
    311309    setUnderlyingString(string.impl());
    312310    if (!string.impl()) {
    313         m_characters = nullptr;
    314         m_length = 0;
     311        clear();
    315312        return;
    316313    }
     
    320317    }
    321318    initialize(string.characters16(), string.length());
     319}
     320
     321inline void StringView::clear()
     322{
     323    m_characters = nullptr;
     324    m_length = 0;
     325    m_is8Bit = true;
    322326}
    323327
     
    368372inline unsigned StringView::length() const
    369373{
    370     return m_length & ~is16BitStringFlag;
     374    return m_length;
    371375}
    372376
     
    378382inline bool StringView::is8Bit() const
    379383{
    380     return !(m_length & is16BitStringFlag);
     384    return m_is8Bit;
    381385}
    382386
     
    433437    }
    434438    auto characters16 = this->characters16();
    435     unsigned length = this->length();
    436     for (unsigned i = 0; i < length; ++i)
     439    for (unsigned i = 0; i < m_length; ++i)
    437440        destination[i] = characters16[i];
    438441}
     
    456459    if (is8Bit())
    457460        return String(characters8(), m_length);
    458     return String(characters16(), length());
     461    return String(characters16(), m_length);
    459462}
    460463
     
    463466    if (is8Bit())
    464467        return AtomicString(characters8(), m_length);
    465     return AtomicString(characters16(), length());
     468    return AtomicString(characters16(), m_length);
    466469}
    467470
     
    470473    if (is8Bit())
    471474        return charactersToFloat(characters8(), m_length, &isValid);
    472     return charactersToFloat(characters16(), length(), &isValid);
     475    return charactersToFloat(characters16(), m_length, &isValid);
    473476}
    474477
     
    483486    if (is8Bit())
    484487        return charactersToInt(characters8(), m_length, &isValid);
    485     return charactersToInt(characters16(), length(), &isValid);
     488    return charactersToInt(characters16(), m_length, &isValid);
    486489}
    487490
     
    490493    if (is8Bit())
    491494        return charactersToIntStrict(characters8(), m_length, &isValid);
    492     return charactersToIntStrict(characters16(), length(), &isValid);
     495    return charactersToIntStrict(characters16(), m_length, &isValid);
    493496}
    494497
     
    497500    if (is8Bit())
    498501        return StringImpl::createWithoutCopying(characters8(), m_length);
    499     return StringImpl::createWithoutCopying(characters16(), length());
     502    return StringImpl::createWithoutCopying(characters16(), m_length);
    500503}
    501504
     
    504507    if (is8Bit())
    505508        return WTF::find(characters8(), m_length, character, start);
    506     return WTF::find(characters16(), length(), character, start);
     509    return WTF::find(characters16(), m_length, character, start);
    507510}
    508511
     
    511514    if (is8Bit())
    512515        return WTF::find(characters8(), m_length, matchFunction, start);
    513     return WTF::find(characters16(), length(), matchFunction, start);
     516    return WTF::find(characters16(), m_length, matchFunction, start);
    514517}
    515518
Note: See TracChangeset for help on using the changeset viewer.