Changeset 206281 in webkit


Ignore:
Timestamp:
Sep 22, 2016 4:41:56 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

Array.prototype.join should do overflow checks on string joins.
​https://bugs.webkit.org/show_bug.cgi?id=162459

Reviewed by Saam Barati.

JSTests:

  • stress/array-join-on-strings-need-overflow-checks.js: Added.

(assert):
(catch):

Source/JavaScriptCore:

Change the 2 JSRopeString::create() functions that do joins to be private, and
force all clients of it to go through the jsString() utility functions that do
overflow checks before creating the ropes.

  • dfg/DFGOperations.cpp:
  • runtime/ArrayPrototype.cpp:

(JSC::slowJoin):

  • runtime/JSString.h:
  • runtime/Operations.h:

(JSC::jsString):

Location:
trunk
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r206268 r206281  
     12016-09-22  Mark Lam  <mark.lam@apple.com>
     2
     3        Array.prototype.join should do overflow checks on string joins.
     4        https://bugs.webkit.org/show_bug.cgi?id=162459
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/array-join-on-strings-need-overflow-checks.js: Added.
     9        (assert):
     10        (catch):
     11
    1122016-09-22  Joseph Pecoraro  <pecoraro@apple.com>
    213

  • trunk/Source/JavaScriptCore/ChangeLog

    r206274 r206281  
     12016-09-22  Mark Lam  <mark.lam@apple.com>
     2
     3        Array.prototype.join should do overflow checks on string joins.
     4        https://bugs.webkit.org/show_bug.cgi?id=162459
     5
     6        Reviewed by Saam Barati.
     7
     8        Change the 2 JSRopeString::create() functions that do joins to be private, and
     9        force all clients of it to go through the jsString() utility functions that do
     10        overflow checks before creating the ropes.
     11
     12        * dfg/DFGOperations.cpp:
     13        * runtime/ArrayPrototype.cpp:
     14        (JSC::slowJoin):
     15        * runtime/JSString.h:
     16        * runtime/Operations.h:
     17        (JSC::jsString):
     18
    1192016-09-22  Filip Pizlo  <fpizlo@apple.com>
    220

  • trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

    r206267 r206281  
    5353#include "JSSet.h"
    5454#include "ObjectConstructor.h"
     55#include "Operations.h"
    5556#include "RegExpObject.h"
    5657#include "Repatch.h"
    … …  
    15011502    VM& vm = exec->vm();
    15021503    NativeCallFrameTracer tracer(&vm, exec);
    1503     auto scope = DECLARE_THROW_SCOPE(vm);
    1504 
    1505     if (sumOverflows<int32_t>(left->length(), right->length())) {
    1506         throwOutOfMemoryError(exec, scope);
    1507         return nullptr;
    1508     }
    1509 
    1510     return JSRopeString::create(vm, left, right);
     1504
     1505    return jsString(exec, left, right);
    15111506}
    15121507
    … …  
    15151510    VM& vm = exec->vm();
    15161511    NativeCallFrameTracer tracer(&vm, exec);
    1517     auto scope = DECLARE_THROW_SCOPE(vm);
    1518 
    1519     if (sumOverflows<int32_t>(a->length(), b->length(), c->length())) {
    1520         throwOutOfMemoryError(exec, scope);
    1521         return nullptr;
    1522     }
    1523 
    1524     return JSRopeString::create(vm, a, b, c);
     1512
     1513    return jsString(exec, a, b, c);
    15251514}
    15261515
    … …  
    15361525    ASSERT(!scope.exception());
    15371526
    1538     if (sumOverflows<int32_t>(str1->length(), str2->length())) {
    1539         throwOutOfMemoryError(exec, scope);
    1540         return nullptr;
    1541     }
    1542 
    1543     return JSRopeString::create(vm, str1, str2);
     1527    scope.release();
     1528    return jsString(exec, str1, str2);
    15441529}
    15451530   
    … …  
    15571542    ASSERT(!scope.exception());
    15581543
    1559     if (sumOverflows<int32_t>(str1->length(), str2->length(), str3->length())) {
    1560         throwOutOfMemoryError(exec, scope);
    1561         return nullptr;
    1562     }
    1563 
    1564     return JSRopeString::create(vm, str1, str2, str3);
     1544    scope.release();
     1545    return jsString(exec, str1, str2, str3);
    15651546}
    15661547

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

    r205910 r206281  
    4242#include "ObjectConstructor.h"
    4343#include "ObjectPrototype.h"
     44#include "Operations.h"
    4445#include "StringRecursionChecker.h"
    4546#include <algorithm>
    … …  
    547548        // a. Let S be the String value produced by concatenating R and sep.
    548549        // d. Let R be a String value produced by concatenating S and next.
    549         r = JSRopeString::create(vm, r, separator, next);
     550        r = jsString(&exec, r, separator, next);
     551        if (UNLIKELY(scope.exception()))
     552            return JSValue();
    550553    }
    551554    // 10. Return R.

  • trunk/Source/JavaScriptCore/runtime/JSString.h

    r204485 r206281  
    220220    StringView unsafeView(ExecState&) const;
    221221
    222     friend JSValue jsString(ExecState*, JSString*, JSString*);
     222    friend JSString* jsString(ExecState*, JSString*, JSString*);
    223223    friend JSString* jsSubstring(ExecState*, JSString*, unsigned offset, unsigned length);
    224224};
    … …  
    278278    {
    279279        Base::finishCreation(vm);
     280        ASSERT(!sumOverflows<int32_t>(s1->length(), s2->length()));
    280281        m_length = s1->length() + s2->length();
    281282        setIs8Bit(s1->is8Bit() && s2->is8Bit());
    … …  
    289290    {
    290291        Base::finishCreation(vm);
     292        ASSERT(!sumOverflows<int32_t>(s1->length(), s2->length(), s3->length()));
    291293        m_length = s1->length() + s2->length() + s3->length();
    292294        setIs8Bit(s1->is8Bit() && s2->is8Bit() &&  s3->is8Bit());
    … …  
    358360
    359361public:
     362    static JSString* create(VM& vm, ExecState* exec, JSString* base, unsigned offset, unsigned length)
     363    {
     364        JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
     365        newString->finishCreation(vm, exec, base, offset, length);
     366        return newString;
     367    }
     368
     369    ALWAYS_INLINE static JSString* createSubstringOfResolved(VM& vm, JSString* base, unsigned offset, unsigned length)
     370    {
     371        JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
     372        newString->finishCreationSubstringOfResolved(vm, base, offset, length);
     373        return newString;
     374    }
     375
     376    void visitFibers(SlotVisitor&);
     377
     378    static ptrdiff_t offsetOfFibers() { return OBJECT_OFFSETOF(JSRopeString, u); }
     379
     380    static const unsigned s_maxInternalRopeLength = 3;
     381
     382private:
    360383    static JSString* create(VM& vm, JSString* s1, JSString* s2)
    361384    {
    … …  
    371394    }
    372395
    373     static JSString* create(VM& vm, ExecState* exec, JSString* base, unsigned offset, unsigned length)
    374     {
    375         JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
    376         newString->finishCreation(vm, exec, base, offset, length);
    377         return newString;
    378     }
    379 
    380     ALWAYS_INLINE static JSString* createSubstringOfResolved(VM& vm, JSString* base, unsigned offset, unsigned length)
    381     {
    382         JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
    383         newString->finishCreationSubstringOfResolved(vm, base, offset, length);
    384         return newString;
    385     }
    386 
    387     void visitFibers(SlotVisitor&);
    388 
    389     static ptrdiff_t offsetOfFibers() { return OBJECT_OFFSETOF(JSRopeString, u); }
    390 
    391     static const unsigned s_maxInternalRopeLength = 3;
    392 
    393 private:
    394396    friend JSValue jsStringFromRegisterArray(ExecState*, Register*, unsigned);
    395397    friend JSValue jsStringFromArguments(ExecState*, JSValue);
    … …  
    450452        WriteBarrierBase<JSString> string;
    451453    } u[s_maxInternalRopeLength];
     454
     455
     456    friend JSString* jsString(ExecState*, JSString*, JSString*);
     457    friend JSString* jsString(ExecState*, JSString*, JSString*, JSString*);
     458    friend JSString* jsString(ExecState*, const String&, const String&, const String&);
    452459};
    453460

  • trunk/Source/JavaScriptCore/runtime/Operations.h

    r205462 r206281  
    3838size_t normalizePrototypeChain(CallFrame*, Structure*);
    3939
    40 ALWAYS_INLINE JSValue jsString(ExecState* exec, JSString* s1, JSString* s2)
     40ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2)
    4141{
    4242    VM& vm = exec->vm();
    … …  
    4949    if (!length2)
    5050        return s1;
    51     if (sumOverflows<int32_t>(length1, length2))
    52         return throwOutOfMemoryError(exec, scope);
     51    if (sumOverflows<int32_t>(length1, length2)) {
     52        throwOutOfMemoryError(exec, scope);
     53        return nullptr;
     54    }
    5355
    5456    return JSRopeString::create(vm, s1, s2);
    5557}
    5658
    57 ALWAYS_INLINE JSValue jsString(ExecState* exec, const String& u1, const String& u2, const String& u3)
     59ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2, JSString* s3)
     60{
     61    VM& vm = exec->vm();
     62    auto scope = DECLARE_THROW_SCOPE(vm);
     63
     64    int32_t length1 = s1->length();
     65    if (!length1) {
     66        scope.release();
     67        return jsString(exec, s2, s3);
     68    }
     69    int32_t length2 = s2->length();
     70    if (!length2) {
     71        scope.release();
     72        return jsString(exec, s1, s3);
     73    }
     74    int32_t length3 = s3->length();
     75    if (!length3) {
     76        scope.release();
     77        return jsString(exec, s1, s2);
     78    }
     79
     80    if (sumOverflows<int32_t>(length1, length2, length3)) {
     81        throwOutOfMemoryError(exec, scope);
     82        return nullptr;
     83    }
     84    return JSRopeString::create(vm, s1, s2, s3);
     85}
     86
     87ALWAYS_INLINE JSString* jsString(ExecState* exec, const String& u1, const String& u2, const String& u3)
    5888{
    5989    VM* vm = &exec->vm();
    … …  
    6494    int32_t length3 = u3.length();
    6595   
    66     if (length1 < 0 || length2 < 0 || length3 < 0)
    67         return throwOutOfMemoryError(exec, scope);
     96    if (length1 < 0 || length2 < 0 || length3 < 0) {
     97        throwOutOfMemoryError(exec, scope);
     98        return nullptr;
     99    }
    68100   
    69     if (!length1)
     101    if (!length1) {
     102        scope.release();
    70103        return jsString(exec, jsString(vm, u2), jsString(vm, u3));
    71     if (!length2)
     104    }
     105    if (!length2) {
     106        scope.release();
    72107        return jsString(exec, jsString(vm, u1), jsString(vm, u3));
    73     if (!length3)
     108    }
     109    if (!length3) {
     110        scope.release();
    74111        return jsString(exec, jsString(vm, u1), jsString(vm, u2));
    75 
    76     if (sumOverflows<int32_t>(length1, length2, length3))
    77         return throwOutOfMemoryError(exec, scope);
    78 
    79     return JSRopeString::create(exec->vm(), jsString(vm, u1), jsString(vm, u2), jsString(vm, u3));
     112    }
     113
     114    if (sumOverflows<int32_t>(length1, length2, length3)) {
     115        throwOutOfMemoryError(exec, scope);
     116        return nullptr;
     117    }
     118
     119    return JSRopeString::create(*vm, jsString(vm, u1), jsString(vm, u2), jsString(vm, u3));
    80120}
    81121
Note: See TracChangeset for help on using the changeset viewer.