Changeset 241991 in webkit


Ignore:
Timestamp:
Feb 23, 2019 10:15:41 AM (5 years ago)
Author:
mark.lam@apple.com
Message:

Add an exception check and some assertions in StringPrototype.cpp.
​https://bugs.webkit.org/show_bug.cgi?id=194962
<rdar://problem/48013416>

Reviewed by Yusuke Suzuki and Saam Barati.

Source/JavaScriptCore:

  • runtime/StringPrototype.cpp:

(JSC::jsSpliceSubstrings):
(JSC::jsSpliceSubstringsWithSeparators):
(JSC::operationStringProtoFuncReplaceRegExpEmptyStr):

Source/WTF:

Add an AssertNoOverflow overflow handler which allows us to do CheckedArithmetic
for assertion purpose only on debug builds but sacrifices no performance on
release builds.

  • wtf/CheckedArithmetic.h:

(WTF::AssertNoOverflow::overflowed):
(WTF::AssertNoOverflow::clearOverflow):
(WTF::AssertNoOverflow::crash):
(WTF::AssertNoOverflow::hasOverflowed const):
(WTF::observesOverflow):
(WTF::observesOverflow<AssertNoOverflow>):
(WTF::safeAdd):
(WTF::safeSub):
(WTF::safeMultiply):
(WTF::Checked::operator+=):
(WTF::Checked::operator-=):
(WTF::Checked::operator*=):
(WTF::operator+):
(WTF::operator-):
(WTF::operator*):

Location:
trunk/Source
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r241990 r241991  
     12019-02-23  Mark Lam  <mark.lam@apple.com>
     2
     3        Add an exception check and some assertions in StringPrototype.cpp.
     4        https://bugs.webkit.org/show_bug.cgi?id=194962
     5        <rdar://problem/48013416>
     6
     7        Reviewed by Yusuke Suzuki and Saam Barati.
     8
     9        * runtime/StringPrototype.cpp:
     10        (JSC::jsSpliceSubstrings):
     11        (JSC::jsSpliceSubstringsWithSeparators):
     12        (JSC::operationStringProtoFuncReplaceRegExpEmptyStr):
     13
    1142019-02-23  Keith Miller  <keith_miller@apple.com>
    215

  • trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp

    r240641 r241991  
    11/*
    22 *  Copyright (C) 1999-2001 Harri Porten (porten@kde.org)
    3  *  Copyright (C) 2004-2017 Apple Inc. All rights reserved.
     3 *  Copyright (C) 2004-2019 Apple Inc. All rights reserved.
    44 *  Copyright (C) 2009 Torch Mobile, Inc.
    55 *  Copyright (C) 2015 Jordan Harband (ljharb@gmail.com)
    … …  
    325325    }
    326326
    327     int totalLength = 0;
     327    // We know that the sum of substringRanges lengths cannot exceed length of
     328    // source because the substringRanges were computed from the source string
     329    // in removeUsingRegExpSearch(). Hence, totalLength cannot exceed
     330    // String::MaxLength, and therefore, cannot overflow.
     331    Checked<int, AssertNoOverflow> totalLength = 0;
    328332    for (int i = 0; i < rangeCount; i++)
    329333        totalLength += substringRanges[i].length;
     334    ASSERT(totalLength <= String::MaxLength);
    330335
    331336    if (!totalLength)
    … …  
    335340        LChar* buffer;
    336341        const LChar* sourceData = source.characters8();
    337         auto impl = StringImpl::tryCreateUninitialized(totalLength, buffer);
     342        auto impl = StringImpl::tryCreateUninitialized(totalLength.unsafeGet(), buffer);
    338343        if (!impl) {
    339344            throwOutOfMemoryError(exec, scope);
    … …  
    341346        }
    342347
    343         int bufferPos = 0;
     348        Checked<int, AssertNoOverflow> bufferPos = 0;
    344349        for (int i = 0; i < rangeCount; i++) {
    345350            if (int srcLen = substringRanges[i].length) {
    346                 StringImpl::copyCharacters(buffer + bufferPos, sourceData + substringRanges[i].position, srcLen);
     351                StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), sourceData + substringRanges[i].position, srcLen);
    347352                bufferPos += srcLen;
    348353            }
    … …  
    355360    const UChar* sourceData = source.characters16();
    356361
    357     auto impl = StringImpl::tryCreateUninitialized(totalLength, buffer);
     362    auto impl = StringImpl::tryCreateUninitialized(totalLength.unsafeGet(), buffer);
    358363    if (!impl) {
    359364        throwOutOfMemoryError(exec, scope);
    … …  
    361366    }
    362367
    363     int bufferPos = 0;
     368    Checked<int, AssertNoOverflow> bufferPos = 0;
    364369    for (int i = 0; i < rangeCount; i++) {
    365370        if (int srcLen = substringRanges[i].length) {
    366             StringImpl::copyCharacters(buffer + bufferPos, sourceData + substringRanges[i].position, srcLen);
     371            StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), sourceData + substringRanges[i].position, srcLen);
    367372            bufferPos += srcLen;
    368373        }
    … …  
    415420
    416421        int maxCount = std::max(rangeCount, separatorCount);
    417         int bufferPos = 0;
     422        Checked<int, AssertNoOverflow> bufferPos = 0;
    418423        for (int i = 0; i < maxCount; i++) {
    419424            if (i < rangeCount) {
    420425                if (int srcLen = substringRanges[i].length) {
    421                     StringImpl::copyCharacters(buffer + bufferPos, sourceData + substringRanges[i].position, srcLen);
     426                    StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), sourceData + substringRanges[i].position, srcLen);
    422427                    bufferPos += srcLen;
    423428                }
    … …  
    425430            if (i < separatorCount) {
    426431                if (int sepLen = separators[i].length()) {
    427                     StringImpl::copyCharacters(buffer + bufferPos, separators[i].characters8(), sepLen);
     432                    StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), separators[i].characters8(), sepLen);
    428433                    bufferPos += sepLen;
    429434                }
    … …  
    442447
    443448    int maxCount = std::max(rangeCount, separatorCount);
    444     int bufferPos = 0;
     449    Checked<int, AssertNoOverflow> bufferPos = 0;
    445450    for (int i = 0; i < maxCount; i++) {
    446451        if (i < rangeCount) {
    447452            if (int srcLen = substringRanges[i].length) {
    448453                if (source.is8Bit())
    449                     StringImpl::copyCharacters(buffer + bufferPos, source.characters8() + substringRanges[i].position, srcLen);
     454                    StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), source.characters8() + substringRanges[i].position, srcLen);
    450455                else
    451                     StringImpl::copyCharacters(buffer + bufferPos, source.characters16() + substringRanges[i].position, srcLen);
     456                    StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), source.characters16() + substringRanges[i].position, srcLen);
    452457                bufferPos += srcLen;
    453458            }
    … …  
    456461            if (int sepLen = separators[i].length()) {
    457462                if (separators[i].is8Bit())
    458                     StringImpl::copyCharacters(buffer + bufferPos, separators[i].characters8(), sepLen);
     463                    StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), separators[i].characters8(), sepLen);
    459464                else
    460                     StringImpl::copyCharacters(buffer + bufferPos, separators[i].characters16(), sepLen);
     465                    StringImpl::copyCharacters(buffer + bufferPos.unsafeGet(), separators[i].characters16(), sepLen);
    461466                bufferPos += sepLen;
    462467            }
    … …  
    730735        searchValue->setLastIndex(exec, 0);
    731736        RETURN_IF_EXCEPTION(scope, nullptr);
    732         RELEASE_AND_RETURN(scope, removeUsingRegExpSearch(vm, exec, thisValue, thisValue->value(exec), regExp));
     737        const String& source = thisValue->value(exec);
     738        RETURN_IF_EXCEPTION(scope, nullptr);
     739        RELEASE_AND_RETURN(scope, removeUsingRegExpSearch(vm, exec, thisValue, source, regExp));
    733740    }
    734741

  • trunk/Source/WTF/ChangeLog

    r241990 r241991  
     12019-02-23  Mark Lam  <mark.lam@apple.com>
     2
     3        Add an exception check and some assertions in StringPrototype.cpp.
     4        https://bugs.webkit.org/show_bug.cgi?id=194962
     5        <rdar://problem/48013416>
     6
     7        Reviewed by Yusuke Suzuki and Saam Barati.
     8
     9        Add an AssertNoOverflow overflow handler which allows us to do CheckedArithmetic
     10        for assertion purpose only on debug builds but sacrifices no performance on
     11        release builds.
     12
     13        * wtf/CheckedArithmetic.h:
     14        (WTF::AssertNoOverflow::overflowed):
     15        (WTF::AssertNoOverflow::clearOverflow):
     16        (WTF::AssertNoOverflow::crash):
     17        (WTF::AssertNoOverflow::hasOverflowed const):
     18        (WTF::observesOverflow):
     19        (WTF::observesOverflow<AssertNoOverflow>):
     20        (WTF::safeAdd):
     21        (WTF::safeSub):
     22        (WTF::safeMultiply):
     23        (WTF::Checked::operator+=):
     24        (WTF::Checked::operator-=):
     25        (WTF::Checked::operator*=):
     26        (WTF::operator+):
     27        (WTF::operator-):
     28        (WTF::operator*):
     29
    1302019-02-23  Keith Miller  <keith_miller@apple.com>
    231

  • trunk/Source/WTF/wtf/CheckedArithmetic.h

    r237577 r241991  
    11/*
    2  * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2011-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    7171};
    7272
     73class AssertNoOverflow {
     74public:
     75    static NO_RETURN_DUE_TO_ASSERT void overflowed()
     76    {
     77        ASSERT_NOT_REACHED();
     78    }
     79
     80    void clearOverflow() { }
     81
     82    static NO_RETURN_DUE_TO_CRASH void crash()
     83    {
     84        CRASH();
     85    }
     86
     87public:
     88    constexpr bool hasOverflowed() const { return false; }
     89};
     90
    7391class ConditionalCrashOnOverflow {
    7492public:
    … …  
    524542};
    525543
     544template <class OverflowHandler, typename = std::enable_if_t<!std::is_scalar<OverflowHandler>::value>>
     545inline constexpr bool observesOverflow() { return true; }
     546
     547template <>
     548inline constexpr bool observesOverflow<AssertNoOverflow>() { return !ASSERT_DISABLED; }
     549
    526550template <typename U, typename V, typename R> static inline bool safeAdd(U lhs, V rhs, R& result)
    527551{
    528552    return ArithmeticOperations<U, V, R>::add(lhs, rhs, result);
     553    return true;
     554}
     555
     556template <class OverflowHandler, typename U, typename V, typename R, typename = std::enable_if_t<!std::is_scalar<OverflowHandler>::value>>
     557static inline bool safeAdd(U lhs, V rhs, R& result)
     558{
     559    if (observesOverflow<OverflowHandler>())
     560        return safeAdd(lhs, rhs, result);
     561    result = lhs + rhs;
     562    return true;
    529563}
    530564
    … …  
    534568}
    535569
     570template <class OverflowHandler, typename U, typename V, typename R, typename = std::enable_if_t<!std::is_scalar<OverflowHandler>::value>>
     571static inline bool safeSub(U lhs, V rhs, R& result)
     572{
     573    if (observesOverflow<OverflowHandler>())
     574        return safeSub(lhs, rhs, result);
     575    result = lhs - rhs;
     576    return true;
     577}
     578
    536579template <typename U, typename V, typename R> static inline bool safeMultiply(U lhs, V rhs, R& result)
    537580{
    538581    return ArithmeticOperations<U, V, R>::multiply(lhs, rhs, result);
     582}
     583
     584template <class OverflowHandler, typename U, typename V, typename R, typename = std::enable_if_t<!std::is_scalar<OverflowHandler>::value>>
     585static inline bool safeMultiply(U lhs, V rhs, R& result)
     586{
     587    if (observesOverflow<OverflowHandler>())
     588        return safeMultiply(lhs, rhs, result);
     589    result = lhs * rhs;
     590    return true;
    539591}
    540592
    … …  
    677729    template <typename U> const Checked operator+=(U rhs)
    678730    {
    679         if (!safeAdd(m_value, rhs, m_value))
     731        if (!safeAdd<OverflowHandler>(m_value, rhs, m_value))
    680732            this->overflowed();
    681733        return *this;
    … …  
    684736    template <typename U> const Checked operator-=(U rhs)
    685737    {
    686         if (!safeSub(m_value, rhs, m_value))
     738        if (!safeSub<OverflowHandler>(m_value, rhs, m_value))
    687739            this->overflowed();
    688740        return *this;
    … …  
    691743    template <typename U> const Checked operator*=(U rhs)
    692744    {
    693         if (!safeMultiply(m_value, rhs, m_value))
     745        if (!safeMultiply<OverflowHandler>(m_value, rhs, m_value))
    694746            this->overflowed();
    695747        return *this;
    … …  
    815867    bool overflowed = lhs.safeGet(x) == CheckedState::DidOverflow || rhs.safeGet(y) == CheckedState::DidOverflow;
    816868    typename Result<U, V>::ResultType result = 0;
    817     overflowed |= !safeAdd(x, y, result);
     869    overflowed |= !safeAdd<OverflowHandler>(x, y, result);
    818870    if (overflowed)
    819871        return ResultOverflowed;
    … …  
    827879    bool overflowed = lhs.safeGet(x) == CheckedState::DidOverflow || rhs.safeGet(y) == CheckedState::DidOverflow;
    828880    typename Result<U, V>::ResultType result = 0;
    829     overflowed |= !safeSub(x, y, result);
     881    overflowed |= !safeSub<OverflowHandler>(x, y, result);
    830882    if (overflowed)
    831883        return ResultOverflowed;
    … …  
    839891    bool overflowed = lhs.safeGet(x) == CheckedState::DidOverflow || rhs.safeGet(y) == CheckedState::DidOverflow;
    840892    typename Result<U, V>::ResultType result = 0;
    841     overflowed |= !safeMultiply(x, y, result);
     893    overflowed |= !safeMultiply<OverflowHandler>(x, y, result);
    842894    if (overflowed)
    843895        return ResultOverflowed;
    … …  
    931983}
    932984
     985using WTF::AssertNoOverflow;
    933986using WTF::Checked;
    934987using WTF::CheckedState;
Note: See TracChangeset for help on using the changeset viewer.