Changeset 173326 in webkit


Ignore:
Timestamp:
Sep 5, 2014 12:33:29 PM (10 years ago)
Author:
ddkilzer@apple.com
Message:

JavaScriptCore should build with newer clang
<http://webkit.org/b/136002>
<rdar://problem/18020616>

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Other than the JSC::SourceProvider::asID() change (which simply
removes code that the optimizing compiler would have discarded
in Release builds), we move the |this| checks in OpaqueJSString
to NULL checks in to JSBase, JSObjectRef, JSScriptRef,
JSStringRef{CF} and JSValueRef.

Note that the following function arguments are _not_ NULL-checked
since doing so would just cover up bugs (and were not needed to
prevent any tests from failing):

  • |script| in JSEvaluateScript(), JSCheckScriptSyntax();
  • |body| in JSObjectMakeFunction();
  • |source| in JSScriptCreateReferencingImmortalASCIIText() (which is a const char* anyway);
  • |source| in JSScriptCreateFromString().
  • API/JSBase.cpp:

(JSEvaluateScript): Add NULL check for |sourceURL|.
(JSCheckScriptSyntax): Ditto.

  • API/JSObjectRef.cpp:

(JSObjectMakeFunction): Ditto.

  • API/JSScriptRef.cpp:

(JSScriptCreateReferencingImmortalASCIIText): Ditto.
(JSScriptCreateFromString): Add NULL check for |url|.

  • API/JSStringRef.cpp:

(JSStringGetLength): Return early if NULL pointer is passed in.
(JSStringGetCharactersPtr): Ditto.
(JSStringGetUTF8CString): Ditto. Also check |buffer| parameter.

  • API/JSStringRefCF.cpp:

(JSStringCopyCFString): Ditto.

  • API/JSValueRef.cpp:

(JSValueMakeString): Add NULL check for |string|.

  • API/OpaqueJSString.cpp:

(OpaqueJSString::string): Remove code that checks |this|.
(OpaqueJSString::identifier): Ditto.
(OpaqueJSString::characters): Ditto.

  • API/OpaqueJSString.h:

(OpaqueJSString::is8Bit): Remove code that checks |this|.
(OpaqueJSString::characters8): Ditto.
(OpaqueJSString::characters16): Ditto.
(OpaqueJSString::length): Ditto.

  • parser/SourceProvider.h:

(JSC::SourceProvider::asID): Remove code that checks |this|.

Source/WebKit2:

  • Shared/API/c/WKString.cpp:

(WKStringCreateWithJSString): Add NULL check to prevent
WebKitTestRunner crashes that relied on the previous |this|
behavior where NULL values were allowed.

Location:
trunk/Source
Files:
12 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/API/JSBase.cpp

    r173263 r173326  
    6161    // evaluate sets "this" to the global object if it is NULL
    6262    JSGlobalObject* globalObject = exec->vmEntryGlobalObject();
    63     SourceCode source = makeSource(script->string(), sourceURL->string(), TextPosition(OrdinalNumber::fromOneBasedInt(startingLineNumber), OrdinalNumber::first()));
     63    SourceCode source = makeSource(script->string(), sourceURL ? sourceURL->string() : String(), TextPosition(OrdinalNumber::fromOneBasedInt(startingLineNumber), OrdinalNumber::first()));
    6464
    6565    JSValue evaluationException;
     
    9898    startingLineNumber = std::max(1, startingLineNumber);
    9999
    100     SourceCode source = makeSource(script->string(), sourceURL->string(), TextPosition(OrdinalNumber::fromOneBasedInt(startingLineNumber), OrdinalNumber::first()));
     100    SourceCode source = makeSource(script->string(), sourceURL ? sourceURL->string() : String(), TextPosition(OrdinalNumber::fromOneBasedInt(startingLineNumber), OrdinalNumber::first()));
    101101   
    102102    JSValue syntaxException;
  • trunk/Source/JavaScriptCore/API/JSObjectRef.cpp

    r171691 r173326  
    148148    args.append(jsString(exec, body->string()));
    149149
    150     JSObject* result = constructFunction(exec, exec->lexicalGlobalObject(), args, nameID, sourceURL->string(), TextPosition(OrdinalNumber::fromOneBasedInt(startingLineNumber), OrdinalNumber::first()));
     150    JSObject* result = constructFunction(exec, exec->lexicalGlobalObject(), args, nameID, sourceURL ? sourceURL->string() : String(), TextPosition(OrdinalNumber::fromOneBasedInt(startingLineNumber), OrdinalNumber::first()));
    151151    if (exec->hadException()) {
    152152        JSValue exceptionValue = exec->exception();
  • trunk/Source/JavaScriptCore/API/JSScriptRef.cpp

    r173263 r173326  
    8585    startingLineNumber = std::max(1, startingLineNumber);
    8686
    87     RefPtr<OpaqueJSScript> result = OpaqueJSScript::create(vm, url->string(), startingLineNumber, String(StringImpl::createFromLiteral(source, length)));
     87    RefPtr<OpaqueJSScript> result = OpaqueJSScript::create(vm, url ? url->string() : String(), startingLineNumber, String(StringImpl::createFromLiteral(source, length)));
    8888
    8989    ParserError error;
     
    106106    startingLineNumber = std::max(1, startingLineNumber);
    107107
    108     RefPtr<OpaqueJSScript> result = OpaqueJSScript::create(vm, url->string(), startingLineNumber, source->string());
     108    RefPtr<OpaqueJSScript> result = OpaqueJSScript::create(vm, url ? url->string() : String(), startingLineNumber, source->string());
    109109
    110110    ParserError error;
  • trunk/Source/JavaScriptCore/API/JSStringRef.cpp

    r173263 r173326  
    7979size_t JSStringGetLength(JSStringRef string)
    8080{
     81    if (!string)
     82        return 0;
    8183    return string->length();
    8284}
     
    8486const JSChar* JSStringGetCharactersPtr(JSStringRef string)
    8587{
     88    if (!string)
     89        return nullptr;
    8690    return string->characters();
    8791}
     
    9599size_t JSStringGetUTF8CString(JSStringRef string, char* buffer, size_t bufferSize)
    96100{
    97     if (!bufferSize)
     101    if (!string || !buffer || !bufferSize)
    98102        return 0;
    99103
  • trunk/Source/JavaScriptCore/API/JSStringRefCF.cpp

    r173263 r173326  
    5858CFStringRef JSStringCopyCFString(CFAllocatorRef allocator, JSStringRef string)
    5959{
    60     if (!string->length())
     60    if (!string || !string->length())
    6161        return CFSTR("");
    6262
  • trunk/Source/JavaScriptCore/API/JSValueRef.cpp

    r173263 r173326  
    319319    JSLockHolder locker(exec);
    320320
    321     return toRef(exec, jsString(exec, string->string()));
     321    return toRef(exec, jsString(exec, string ? string->string() : String()));
    322322}
    323323
  • trunk/Source/JavaScriptCore/API/OpaqueJSString.cpp

    r173263 r173326  
    5757String OpaqueJSString::string() const
    5858{
    59     if (!this)
    60         return String();
    61 
    6259    // Return a copy of the wrapped string, because the caller may make it an Identifier.
    6360    return m_string.isolatedCopy();
     
    6663Identifier OpaqueJSString::identifier(VM* vm) const
    6764{
    68     if (!this || m_string.isNull())
     65    if (m_string.isNull())
    6966        return Identifier();
    7067
     
    8077const UChar* OpaqueJSString::characters()
    8178{
    82     if (!this)
    83         return nullptr;
    84 
    8579    // m_characters is put in a local here to avoid an extra atomic load.
    8680    UChar* characters = m_characters;
  • trunk/Source/JavaScriptCore/API/OpaqueJSString.h

    r173263 r173326  
    5656    JS_EXPORT_PRIVATE ~OpaqueJSString();
    5757
    58     bool is8Bit() { return this ? m_string.is8Bit() : false; }
    59     const LChar* characters8() { return this ? m_string.characters8() : nullptr; }
    60     const UChar* characters16() { return this ? m_string.characters16() : nullptr; }
    61     unsigned length() { return this ? m_string.length() : 0; }
     58    bool is8Bit() { return m_string.is8Bit(); }
     59    const LChar* characters8() { return m_string.characters8(); }
     60    const UChar* characters16() { return m_string.characters16(); }
     61    unsigned length() { return m_string.length(); }
    6262
    6363    const UChar* characters();
  • trunk/Source/JavaScriptCore/ChangeLog

    r173318 r173326  
     12014-09-05  David Kilzer  <ddkilzer@apple.com>
     2
     3        JavaScriptCore should build with newer clang
     4        <http://webkit.org/b/136002>
     5        <rdar://problem/18020616>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Other than the JSC::SourceProvider::asID() change (which simply
     10        removes code that the optimizing compiler would have discarded
     11        in Release builds), we move the |this| checks in OpaqueJSString
     12        to NULL checks in to JSBase, JSObjectRef, JSScriptRef,
     13        JSStringRef{CF} and JSValueRef.
     14
     15        Note that the following function arguments are _not_ NULL-checked
     16        since doing so would just cover up bugs (and were not needed to
     17        prevent any tests from failing):
     18        - |script| in JSEvaluateScript(), JSCheckScriptSyntax();
     19        - |body| in JSObjectMakeFunction();
     20        - |source| in JSScriptCreateReferencingImmortalASCIIText()
     21          (which is a const char* anyway);
     22        - |source| in JSScriptCreateFromString().
     23
     24        * API/JSBase.cpp:
     25        (JSEvaluateScript): Add NULL check for |sourceURL|.
     26        (JSCheckScriptSyntax): Ditto.
     27        * API/JSObjectRef.cpp:
     28        (JSObjectMakeFunction): Ditto.
     29        * API/JSScriptRef.cpp:
     30        (JSScriptCreateReferencingImmortalASCIIText): Ditto.
     31        (JSScriptCreateFromString): Add NULL check for |url|.
     32        * API/JSStringRef.cpp:
     33        (JSStringGetLength): Return early if NULL pointer is passed in.
     34        (JSStringGetCharactersPtr): Ditto.
     35        (JSStringGetUTF8CString): Ditto.  Also check |buffer| parameter.
     36        * API/JSStringRefCF.cpp:
     37        (JSStringCopyCFString): Ditto.
     38        * API/JSValueRef.cpp:
     39        (JSValueMakeString): Add NULL check for |string|.
     40
     41        * API/OpaqueJSString.cpp:
     42        (OpaqueJSString::string): Remove code that checks |this|.
     43        (OpaqueJSString::identifier): Ditto.
     44        (OpaqueJSString::characters): Ditto.
     45        * API/OpaqueJSString.h:
     46        (OpaqueJSString::is8Bit): Remove code that checks |this|.
     47        (OpaqueJSString::characters8): Ditto.
     48        (OpaqueJSString::characters16): Ditto.
     49        (OpaqueJSString::length): Ditto.
     50
     51        * parser/SourceProvider.h:
     52        (JSC::SourceProvider::asID): Remove code that checks |this|.
     53
    1542014-06-06  Jer Noble  <jer.noble@apple.com>
    255
  • trunk/Source/JavaScriptCore/parser/SourceProvider.h

    r173263 r173326  
    5555        intptr_t asID()
    5656        {
    57             ASSERT(this);
    58             if (!this) // Be defensive in release mode.
    59                 return nullID;
    6057            if (!m_id)
    6158                getID();
  • trunk/Source/WebKit2/ChangeLog

    r173320 r173326  
     12014-09-05  David Kilzer  <ddkilzer@apple.com>
     2
     3        JavaScriptCore should build with newer clang
     4        <http://webkit.org/b/136002>
     5        <rdar://problem/18020616>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        * Shared/API/c/WKString.cpp:
     10        (WKStringCreateWithJSString): Add NULL check to prevent
     11        WebKitTestRunner crashes that relied on the previous |this|
     12        behavior where NULL values were allowed.
     13
    1142014-09-05  Beth Dakin  <bdakin@apple.com>
    215
  • trunk/Source/WebKit2/Shared/API/c/WKString.cpp

    r173263 r173326  
    8686WKStringRef WKStringCreateWithJSString(JSStringRef jsStringRef)
    8787{
    88     RefPtr<API::String> apiString = API::String::create(jsStringRef);
     88    RefPtr<API::String> apiString = jsStringRef ? API::String::create(jsStringRef) : API::String::createNull();
    8989    return toAPI(apiString.release().leakRef());
    9090}
Note: See TracChangeset for help on using the changeset viewer.