Changeset 47092 in webkit


Ignore:
Timestamp:
Aug 11, 2009 11:22:41 PM (15 years ago)
Author:
oliver@apple.com
Message:

Make it harder to misuse try* allocation routines
​https://bugs.webkit.org/show_bug.cgi?id=27469

Reviewed by Gavin Barraclough

Jump through a few hoops to make it much harder to accidentally
miss null-checking of values returned by the try-* allocation
routines.

Location:
trunk
Files:
2 added
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r47091 r47092  
    7575        (JSC::FunctionBodyNode::make):
    7676            Make this method inline (was FuncDeclNode::makeFunction).
     77
     782009-08-11  Oliver Hunt  <oliver@apple.com>
     79
     80        Reviewed by Gavin Barraclough.
     81
     82        Make it harder to misuse try* allocation routines
     83        https://bugs.webkit.org/show_bug.cgi?id=27469
     84
     85        Jump through a few hoops to make it much harder to accidentally
     86        miss null-checking of values returned by the try-* allocation
     87        routines.
     88
     89        * JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.def:
     90        * JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore_debug.def:
     91        * JavaScriptCore.xcodeproj/project.pbxproj:
     92        * runtime/JSArray.cpp:
     93        (JSC::JSArray::putSlowCase):
     94        (JSC::JSArray::increaseVectorLength):
     95        * runtime/StringPrototype.cpp:
     96        (JSC::stringProtoFuncFontsize):
     97        (JSC::stringProtoFuncLink):
     98        * runtime/UString.cpp:
     99        (JSC::allocChars):
     100        (JSC::reallocChars):
     101        (JSC::expandCapacity):
     102        (JSC::UString::Rep::reserveCapacity):
     103        (JSC::UString::expandPreCapacity):
     104        (JSC::createRep):
     105        (JSC::concatenate):
     106        (JSC::UString::spliceSubstringsWithSeparators):
     107        (JSC::UString::replaceRange):
     108        (JSC::UString::append):
     109        (JSC::UString::operator=):
     110        * runtime/UString.h:
     111        (JSC::UString::Rep::createEmptyBuffer):
     112        * wtf/FastMalloc.cpp:
     113        (WTF::tryFastZeroedMalloc):
     114        (WTF::tryFastMalloc):
     115        (WTF::tryFastCalloc):
     116        (WTF::tryFastRealloc):
     117        (WTF::TCMallocStats::tryFastMalloc):
     118        (WTF::TCMallocStats::tryFastCalloc):
     119        (WTF::TCMallocStats::tryFastRealloc):
     120        * wtf/FastMalloc.h:
     121        (WTF::TryMallocReturnValue::TryMallocReturnValue):
     122        (WTF::TryMallocReturnValue::~TryMallocReturnValue):
     123        (WTF::TryMallocReturnValue::operator PossiblyNull<T>):
     124        (WTF::TryMallocReturnValue::getValue):
     125        * wtf/Platform.h:
     126        * wtf/PossiblyNull.h: Added.
     127        (WTF::PossiblyNull::PossiblyNull):
     128        (WTF::PossiblyNull::~PossiblyNull):
     129        (WTF::::getValue):
    77130
    781312009-08-11  Oliver Hunt  <oliver@apple.com>

  • trunk/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.def

    r47091 r47092  
    265265    ?toUInt32@UString@JSC@@QBEIPA_N_N@Z
    266266    ?toUInt32SlowCase@JSC@@YAINAA_N@Z
    267     ?tryFastCalloc@WTF@@YAPAXII@Z
     267    ?tryFastCalloc@WTF@@YA?AUTryMallocReturnValue@1@II@Z
    268268    ?tryLock@Mutex@WTF@@QAE_NXZ
    269269    ?type@DebuggerCallFrame@JSC@@QBE?AW4Type@12@XZ

  • trunk/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore_debug.def

    r47091 r47092  
    264264    ?toUInt32@UString@JSC@@QBEIPA_N_N@Z
    265265    ?toUInt32SlowCase@JSC@@YAINAA_N@Z
    266     ?tryFastCalloc@WTF@@YAPAXII@Z
     266    ?tryFastCalloc@WTF@@YA?AUTryMallocReturnValue@1@II@Z
    267267    ?tryLock@Mutex@WTF@@QAE_NXZ
    268268    ?type@DebuggerCallFrame@JSC@@QBE?AW4Type@12@XZ

  • trunk/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r47022 r47092  
    208208                A7B48F490EE8936F00DCBDB6 /* ExecutableAllocator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A7B48DB60EE74CFC00DCBDB6 /* ExecutableAllocator.cpp */; };
    209209                A7C530E4102A3813005BC741 /* MarkStackPosix.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A7C530E3102A3813005BC741 /* MarkStackPosix.cpp */; };
     210                A7D649AA1015224E009B2E1B /* PossiblyNull.h in Headers */ = {isa = PBXBuildFile; fileRef = A7D649A91015224E009B2E1B /* PossiblyNull.h */; settings = {ATTRIBUTES = (Private, ); }; };
    210211                A7E2EA6B0FB460CF00601F06 /* LiteralParser.h in Headers */ = {isa = PBXBuildFile; fileRef = A7E2EA690FB460CF00601F06 /* LiteralParser.h */; };
    211212                A7E2EA6C0FB460CF00601F06 /* LiteralParser.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A7E2EA6A0FB460CF00601F06 /* LiteralParser.cpp */; };
    … …  
    753754                A7B48DB60EE74CFC00DCBDB6 /* ExecutableAllocator.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ExecutableAllocator.cpp; sourceTree = "<group>"; };
    754755                A7C530E3102A3813005BC741 /* MarkStackPosix.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MarkStackPosix.cpp; sourceTree = "<group>"; };
     756                A7D649A91015224E009B2E1B /* PossiblyNull.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PossiblyNull.h; sourceTree = "<group>"; };
    755757                A7E2EA690FB460CF00601F06 /* LiteralParser.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LiteralParser.h; sourceTree = "<group>"; };
    756758                A7E2EA6A0FB460CF00601F06 /* LiteralParser.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LiteralParser.cpp; sourceTree = "<group>"; };
    … …  
    12431245                                6580F795094070560082C219 /* PassRefPtr.h */,
    12441246                                65D6D87E09B5A32E0002E4D7 /* Platform.h */,
     1247                                A7D649A91015224E009B2E1B /* PossiblyNull.h */,
    12451248                                0B1F921B0F17502D0036468E /* PtrAndFlags.h */,
    12461249                                088FA5B90EF76D4300578E6F /* RandomNumber.cpp */,
    … …  
    19031906                                9688CB160ED12B4E001D649F /* X86Assembler.h in Headers */,
    19041907                                A7795590101A74D500114E55 /* MarkStack.h in Headers */,
     1908                                A7D649AA1015224E009B2E1B /* PossiblyNull.h in Headers */,
    19051909                        );
    19061910                        runOnlyForDeploymentPostprocessing = 0;

  • trunk/JavaScriptCore/runtime/JSArray.cpp

    r47022 r47092  
    349349    }
    350350
    351     storage = static_cast<ArrayStorage*>(tryFastRealloc(storage, storageSize(newVectorLength)));
    352     if (!storage) {
     351    if (!tryFastRealloc(storage, storageSize(newVectorLength)).getValue(storage)) {
    353352        throwOutOfMemoryError(exec);
    354353        return;
    … …  
    468467    unsigned newVectorLength = increasedVectorLength(newLength);
    469468
    470     storage = static_cast<ArrayStorage*>(tryFastRealloc(storage, storageSize(newVectorLength)));
    471     if (!storage)
     469    if (!tryFastRealloc(storage, storageSize(newVectorLength)).getValue(storage))
    472470        return false;
    473471

  • trunk/JavaScriptCore/runtime/StringPrototype.cpp

    r46598 r47092  
    822822        unsigned stringSize = s.size();
    823823        unsigned bufferSize = 22 + stringSize;
    824         UChar* buffer = static_cast<UChar*>(tryFastMalloc(bufferSize * sizeof(UChar)));
    825         if (!buffer)
     824        UChar* buffer;
     825        if (!tryFastMalloc(bufferSize * sizeof(UChar)).getValue(buffer))
    826826            return jsUndefined();
    827827        buffer[0] = '<';
    … …  
    870870    unsigned stringSize = s.size();
    871871    unsigned bufferSize = 15 + linkTextSize + stringSize;
    872     UChar* buffer = static_cast<UChar*>(tryFastMalloc(bufferSize * sizeof(UChar)));
    873     if (!buffer)
     872    UChar* buffer;
     873    if (!tryFastMalloc(bufferSize * sizeof(UChar)).getValue(buffer))
    874874        return jsUndefined();
    875875    buffer[0] = '<';

  • trunk/JavaScriptCore/runtime/UString.cpp

    r46180 r47092  
    6969static inline size_t maxUChars() { return std::numeric_limits<size_t>::max() / sizeof(UChar); }
    7070
    71 static inline UChar* allocChars(size_t length)
     71static inline PossiblyNull<UChar*> allocChars(size_t length)
    7272{
    7373    ASSERT(length);
    7474    if (length > maxUChars())
    7575        return 0;
    76     return static_cast<UChar*>(tryFastMalloc(sizeof(UChar) * length));
    77 }
    78 
    79 static inline UChar* reallocChars(UChar* buffer, size_t length)
     76    return tryFastMalloc(sizeof(UChar) * length);
     77}
     78
     79static inline PossiblyNull<UChar*> reallocChars(UChar* buffer, size_t length)
    8080{
    8181    ASSERT(length);
    8282    if (length > maxUChars())
    8383        return 0;
    84     return static_cast<UChar*>(tryFastRealloc(buffer, sizeof(UChar) * length));
     84    return tryFastRealloc(buffer, sizeof(UChar) * length);
    8585}
    8686
    … …  
    481481        size_t newCapacity = expandedSize(requiredLength, base->preCapacity);
    482482        UChar* oldBuf = base->buf;
    483         base->buf = reallocChars(base->buf, newCapacity);
    484         if (!base->buf) {
     483        if (!reallocChars(base->buf, newCapacity).getValue(base->buf)) {
    485484            base->buf = oldBuf;
    486485            return false;
    … …  
    513512    size_t newCapacity = expandedSize(capacity, base->preCapacity);
    514513    UChar* oldBuf = base->buf;
    515     base->buf = reallocChars(base->buf, newCapacity);
    516     if (!base->buf) {
     514    if (!reallocChars(base->buf, newCapacity).getValue(base->buf)) {
    517515        base->buf = oldBuf;
    518516        return false;
    … …  
    541539        int delta = newCapacity - base->capacity - base->preCapacity;
    542540
    543         UChar* newBuf = allocChars(newCapacity);
    544         if (!newBuf) {
     541        UChar* newBuf;
     542        if (!allocChars(newCapacity).getValue(newBuf)) {
    545543            makeNull();
    546544            return;
    … …  
    567565
    568566    size_t length = strlen(c);
    569     UChar* d = allocChars(length);
    570     if (!d)
     567    UChar* d;
     568    if (!allocChars(length).getValue(d))
    571569        return &UString::Rep::null();
    572570    else {
    … …  
    657655        // This is shared in some way that prevents us from modifying base, so we must make a whole new string.
    658656        size_t newCapacity = expandedSize(length, 0);
    659         UChar* d = allocChars(newCapacity);
    660         if (!d)
     657        UChar* d;
     658        if (!allocChars(newCapacity).getValue(d))
    661659            rep = &UString::Rep::null();
    662660        else {
    … …  
    713711        // This is shared in some way that prevents us from modifying base, so we must make a whole new string.
    714712        size_t newCapacity = expandedSize(length, 0);
    715         UChar* d = allocChars(newCapacity);
    716         if (!d)
     713        UChar* d;
     714        if (!allocChars(newCapacity).getValue(d))
    717715            rep = &UString::Rep::null();
    718716        else {
    … …  
    801799    // a does not qualify for append, and b does not qualify for prepend, gotta make a whole new string
    802800    size_t newCapacity = expandedSize(length, 0);
    803     UChar* d = allocChars(newCapacity);
    804     if (!d)
     801    UChar* d;
     802    if (!allocChars(newCapacity).getValue(d))
    805803        return 0;
    806804    copyChars(d, a->data(), aSize);
    … …  
    10771075        return "";
    10781076
    1079     UChar* buffer = allocChars(totalLength);
    1080     if (!buffer)
     1077    UChar* buffer;
     1078    if (!allocChars(totalLength).getValue(buffer))
    10811079        return null();
    10821080
    … …  
    11061104        return "";
    11071105
    1108     UChar* buffer = allocChars(totalLength);
    1109     if (!buffer)
     1106    UChar* buffer;
     1107    if (!allocChars(totalLength).getValue(buffer))
    11101108        return null();
    11111109
    … …  
    11541152        // This is shared in some way that prevents us from modifying base, so we must make a whole new string.
    11551153        size_t newCapacity = expandedSize(length, 0);
    1156         UChar* d = allocChars(newCapacity);
    1157         if (!d)
     1154        UChar* d;
     1155        if (!allocChars(newCapacity).getValue(d))
    11581156            makeNull();
    11591157        else {
    … …  
    12071205        // this is empty - must make a new m_rep because we don't want to pollute the shared empty one
    12081206        size_t newCapacity = expandedSize(1, 0);
    1209         UChar* d = allocChars(newCapacity);
    1210         if (!d)
     1207        UChar* d;
     1208        if (!allocChars(newCapacity).getValue(d))
    12111209            makeNull();
    12121210        else {
    … …  
    12351233        // This is shared in some way that prevents us from modifying base, so we must make a whole new string.
    12361234        size_t newCapacity = expandedSize(length + 1, 0);
    1237         UChar* d = allocChars(newCapacity);
    1238         if (!d)
     1235        UChar* d;
     1236        if (!allocChars(newCapacity).getValue(d))
    12391237            makeNull();
    12401238        else {
    … …  
    13141312        m_rep->len = l;
    13151313    } else {
    1316         d = allocChars(l);
    1317         if (!d) {
     1314        if (!allocChars(l).getValue(d)) {
    13181315            makeNull();
    13191316            return *this;

  • trunk/JavaScriptCore/runtime/UString.h

    r46180 r47092  
    9292                // Guard against integer overflow
    9393                if (size < (std::numeric_limits<size_t>::max() / sizeof(UChar))) {
    94                     if (void * buf = tryFastMalloc(size * sizeof(UChar)))
     94                    void * buf = 0;
     95                    if (tryFastMalloc(size * sizeof(UChar)).getValue(buf))
    9596                        return adoptRef(new BaseString(static_cast<UChar*>(buf), 0, size));
    9697                }

  • trunk/JavaScriptCore/wtf/FastMalloc.cpp

    r46999 r47092  
    179179}
    180180   
    181 void* tryFastZeroedMalloc(size_t n)
    182 {
    183     void* result = tryFastMalloc(n);
    184     if (!result)
     181TryMallocReturnValue tryFastZeroedMalloc(size_t n)
     182{
     183    void* result;
     184    if (!tryFastMalloc(n).getValue(result))
    185185        return 0;
    186186    memset(result, 0, n);
    … …  
    201201namespace WTF {
    202202
    203 void* tryFastMalloc(size_t n)
     203TryMallocReturnValue tryFastMalloc(size_t n)
    204204{
    205205    ASSERT(!isForbidden());
    … …  
    237237}
    238238
    239 void* tryFastCalloc(size_t n_elements, size_t element_size)
     239TryMallocReturnValue tryFastCalloc(size_t n_elements, size_t element_size)
    240240{
    241241    ASSERT(!isForbidden());
    … …  
    292292}
    293293
    294 void* tryFastRealloc(void* p, size_t n)
     294TryMallocReturnValue tryFastRealloc(void* p, size_t n)
    295295{
    296296    ASSERT(!isForbidden());
    … …  
    35773577}
    35783578
    3579 void* tryFastMalloc(size_t size)
     3579TryMallocReturnValue tryFastMalloc(size_t size)
    35803580{
    35813581    return malloc<false>(size);
    … …  
    36383638}
    36393639
    3640 void* tryFastCalloc(size_t n, size_t elem_size)
     3640TryMallocReturnValue tryFastCalloc(size_t n, size_t elem_size)
    36413641{
    36423642    return calloc<false>(n, elem_size);
    … …  
    37023702}
    37033703
    3704 void* tryFastRealloc(void* old_ptr, size_t new_size)
     3704TryMallocReturnValue tryFastRealloc(void* old_ptr, size_t new_size)
    37053705{
    37063706    return realloc<false>(old_ptr, new_size);

  • trunk/JavaScriptCore/wtf/FastMalloc.h

    r47010 r47092  
    2323
    2424#include "Platform.h"
     25#include "PossiblyNull.h"
    2526#include <stdlib.h>
    2627#include <new>
    … …  
    3435    void* fastRealloc(void*, size_t);
    3536
    36     // These functions return 0 if an allocation fails.
    37     void* tryFastMalloc(size_t);
    38     void* tryFastZeroedMalloc(size_t);
    39     void* tryFastCalloc(size_t numElements, size_t elementSize);
    40     void* tryFastRealloc(void*, size_t);
     37    struct TryMallocReturnValue {
     38        TryMallocReturnValue(void* data)
     39            : m_data(data)
     40        {
     41        }
     42        TryMallocReturnValue(const TryMallocReturnValue& source)
     43            : m_data(source.m_data)
     44        {
     45            source.m_data = 0;
     46        }
     47        ~TryMallocReturnValue() { ASSERT(!m_data); }
     48        template <typename T> bool getValue(T& data) WARN_UNUSED_RETURN;
     49        template <typename T> operator PossiblyNull<T>()
     50        {
     51            T value;
     52            getValue(value);
     53            return PossiblyNull<T>(value);
     54        }
     55    private:
     56        mutable void* m_data;
     57    };
     58   
     59    template <typename T> bool TryMallocReturnValue::getValue(T& data) {
     60        union u { void* data; T target; } res;
     61        res.data = m_data;
     62        data = res.target;
     63        bool returnValue = !!m_data;
     64        m_data = 0;
     65        return returnValue;
     66    }
     67
     68    TryMallocReturnValue tryFastMalloc(size_t n);
     69    TryMallocReturnValue tryFastZeroedMalloc(size_t n);
     70    TryMallocReturnValue tryFastCalloc(size_t n_elements, size_t element_size);
     71    TryMallocReturnValue tryFastRealloc(void* p, size_t n);
    4172
    4273    void fastFree(void*);

  • trunk/JavaScriptCore/wtf/Platform.h

    r47056 r47092  
    733733#endif
    734734
     735#if COMPILER(GCC)
     736#define WARN_UNUSED_RETURN __attribute__ ((warn_unused_result))
     737#else
     738#define WARN_UNUSED_RETURN
     739#endif
     740
    735741#endif /* WTF_Platform_h */

  • trunk/WebCore/ChangeLog

    r47089 r47092  
     12009-08-11  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Gavin Barraclough.
     4
     5        Make it harder to misuse try* allocation routines
     6        https://bugs.webkit.org/show_bug.cgi?id=27469
     7
     8        Add forwarding header for PossiblyNull type, and add missing null check
     9        to ImageBuffer creation.
     10
     11        * ForwardingHeaders/wtf/PossiblyNull.h: Added.
     12        * platform/graphics/cg/ImageBufferCG.cpp:
     13        (WebCore::ImageBuffer::ImageBuffer):
     14
    1152009-08-11  Gavin Barraclough  <barraclough@apple.com>
    216

  • trunk/WebCore/platform/graphics/cg/ImageBufferCG.cpp

    r46957 r47092  
    6666    }
    6767
    68     m_data.m_data = tryFastCalloc(size.height(), bytesPerRow);
     68    if (!tryFastCalloc(size.height(), bytesPerRow).getValue(m_data.m_data))
     69        return;
     70
    6971    ASSERT((reinterpret_cast<size_t>(m_data.m_data) & 2) == 0);
    7072
Note: See TracChangeset for help on using the changeset viewer.