Title: [276900] trunk/Source/_javascript_Core
Revision
276900
Author
mark....@apple.com
Date
2021-05-03 08:54:53 -0700 (Mon, 03 May 2021)

Log Message

Add some missing exception checks before some jsCasts.
https://bugs.webkit.org/show_bug.cgi?id=225264
rdar://77381608

Reviewed by Saam Barati.

Introducing JSObject::getAs() and JSValue::getAs() convenience methods that will
check for an exception before doing a jsCast on the get result.  We only need this
to placate the assertion in jsCast in the event a pending exception exists at the
time of the jsCast.  If ASSERTs are not enabled, this will be a no-op.  All clients
that use jsCast this way (and now use getAs instead) will already be doing an
exception check immediately after.  Hence, they are already doing the right thing
in terms of handling exceptions.  We're only placating the jsCast assertion here.

* runtime/AbstractModuleRecord.cpp:
(JSC::AbstractModuleRecord::hostResolveImportedModule):
* runtime/JSCJSValue.h:
* runtime/JSCJSValueInlines.h:
(JSC::JSValue::getAs const):
* runtime/JSInternalPromise.cpp:
(JSC::JSInternalPromise::then):
* runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::dependencyKeysIfEvaluated):
(JSC::JSModuleLoader::provideFetch):
(JSC::JSModuleLoader::loadAndEvaluateModule):
(JSC::JSModuleLoader::loadModule):
(JSC::JSModuleLoader::linkAndEvaluateModule):
(JSC::JSModuleLoader::requestImportModule):
* runtime/JSObject.h:
(JSC::JSObject::getAs const):
* runtime/JSPromise.cpp:
(JSC::JSPromise::createDeferredData):
* runtime/VM.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (276899 => 276900)


--- trunk/Source/_javascript_Core/ChangeLog	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-05-03 15:54:53 UTC (rev 276900)
@@ -1,3 +1,39 @@
+2021-05-03  Mark Lam  <mark....@apple.com>
+
+        Add some missing exception checks before some jsCasts.
+        https://bugs.webkit.org/show_bug.cgi?id=225264
+        rdar://77381608
+
+        Reviewed by Saam Barati.
+
+        Introducing JSObject::getAs() and JSValue::getAs() convenience methods that will
+        check for an exception before doing a jsCast on the get result.  We only need this
+        to placate the assertion in jsCast in the event a pending exception exists at the
+        time of the jsCast.  If ASSERTs are not enabled, this will be a no-op.  All clients
+        that use jsCast this way (and now use getAs instead) will already be doing an
+        exception check immediately after.  Hence, they are already doing the right thing
+        in terms of handling exceptions.  We're only placating the jsCast assertion here.
+
+        * runtime/AbstractModuleRecord.cpp:
+        (JSC::AbstractModuleRecord::hostResolveImportedModule):
+        * runtime/JSCJSValue.h:
+        * runtime/JSCJSValueInlines.h:
+        (JSC::JSValue::getAs const):
+        * runtime/JSInternalPromise.cpp:
+        (JSC::JSInternalPromise::then):
+        * runtime/JSModuleLoader.cpp:
+        (JSC::JSModuleLoader::dependencyKeysIfEvaluated):
+        (JSC::JSModuleLoader::provideFetch):
+        (JSC::JSModuleLoader::loadAndEvaluateModule):
+        (JSC::JSModuleLoader::loadModule):
+        (JSC::JSModuleLoader::linkAndEvaluateModule):
+        (JSC::JSModuleLoader::requestImportModule):
+        * runtime/JSObject.h:
+        (JSC::JSObject::getAs const):
+        * runtime/JSPromise.cpp:
+        (JSC::JSPromise::createDeferredData):
+        * runtime/VM.h:
+
 2021-05-03  Dmitry Bezhetskov  <dbezhets...@igalia.com>
 
         [WASM-Function-References] Add call_ref instruction

Modified: trunk/Source/_javascript_Core/runtime/AbstractModuleRecord.cpp (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/AbstractModuleRecord.cpp	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/AbstractModuleRecord.cpp	2021-05-03 15:54:53 UTC (rev 276900)
@@ -156,7 +156,7 @@
     JSValue moduleNameValue = identifierToJSValue(vm, moduleName);
     JSValue entry = m_dependenciesMap->JSMap::get(globalObject, moduleNameValue);
     RETURN_IF_EXCEPTION(scope, nullptr);
-    RELEASE_AND_RETURN(scope, jsCast<AbstractModuleRecord*>(entry.get(globalObject, Identifier::fromString(vm, "module"))));
+    RELEASE_AND_RETURN(scope, entry.getAs<AbstractModuleRecord*>(globalObject, Identifier::fromString(vm, "module")));
 }
 
 auto AbstractModuleRecord::resolveImport(JSGlobalObject* globalObject, const Identifier& localName) -> Resolution

Modified: trunk/Source/_javascript_Core/runtime/JSCJSValue.h (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/JSCJSValue.h	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValue.h	2021-05-03 15:54:53 UTC (rev 276900)
@@ -1,7 +1,7 @@
 /*
  *  Copyright (C) 1999-2001 Harri Porten (por...@kde.org)
  *  Copyright (C) 2001 Peter Kelly (p...@post.com)
- *  Copyright (C) 2003-2020 Apple Inc. All rights reserved.
+ *  Copyright (C) 2003-2021 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
@@ -316,6 +316,9 @@
     JSValue get(JSGlobalObject*, unsigned propertyName, PropertySlot&) const;
     JSValue get(JSGlobalObject*, uint64_t propertyName) const;
 
+    template<typename T, typename PropertyNameType>
+    T getAs(JSGlobalObject*, PropertyNameType) const;
+
     bool getPropertySlot(JSGlobalObject*, PropertyName, PropertySlot&) const;
     template<typename CallbackWhenNoException> typename std::result_of<CallbackWhenNoException(bool, PropertySlot&)>::type getPropertySlot(JSGlobalObject*, PropertyName, CallbackWhenNoException) const;
     template<typename CallbackWhenNoException> typename std::result_of<CallbackWhenNoException(bool, PropertySlot&)>::type getPropertySlot(JSGlobalObject*, PropertyName, PropertySlot&, CallbackWhenNoException) const;

Modified: trunk/Source/_javascript_Core/runtime/JSCJSValueInlines.h (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/JSCJSValueInlines.h	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValueInlines.h	2021-05-03 15:54:53 UTC (rev 276900)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011-2020 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2021 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -1045,6 +1045,18 @@
     return get(globalObject, Identifier::from(getVM(globalObject), static_cast<double>(propertyName)));
 }
 
+template<typename T, typename PropertyNameType>
+ALWAYS_INLINE T JSValue::getAs(JSGlobalObject* globalObject, PropertyNameType propertyName) const
+{
+    JSValue value = get(globalObject, propertyName);
+#if ASSERT_ENABLED || ENABLE(SECURITY_ASSERTIONS)
+    VM& vm = getVM(globalObject);
+    if (vm.exceptionForInspection())
+        return nullptr;
+#endif
+    return jsCast<T>(value);
+}
+
 inline bool JSValue::put(JSGlobalObject* globalObject, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
 {
     if (UNLIKELY(!isCell()))

Modified: trunk/Source/_javascript_Core/runtime/JSInternalPromise.cpp (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/JSInternalPromise.cpp	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/JSInternalPromise.cpp	2021-05-03 15:54:53 UTC (rev 276900)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2021 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -60,7 +60,7 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* function = jsCast<JSObject*>(get(globalObject, vm.propertyNames->builtinNames().thenPublicName()));
+    JSObject* function = getAs<JSObject*>(globalObject, vm.propertyNames->builtinNames().thenPublicName());
     RETURN_IF_EXCEPTION(scope, nullptr);
     auto callData = JSC::getCallData(vm, function);
     ASSERT(callData.type != CallData::Type::None);

Modified: trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp	2021-05-03 15:54:53 UTC (rev 276900)
@@ -126,7 +126,7 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* function = jsCast<JSObject*>(get(globalObject, vm.propertyNames->builtinNames().dependencyKeysIfEvaluatedPublicName()));
+    JSObject* function = getAs<JSObject*>(globalObject, vm.propertyNames->builtinNames().dependencyKeysIfEvaluatedPublicName());
     RETURN_IF_EXCEPTION(scope, nullptr);
     auto callData = JSC::getCallData(vm, function);
     ASSERT(callData.type != CallData::Type::None);
@@ -145,7 +145,7 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* function = jsCast<JSObject*>(get(globalObject, vm.propertyNames->builtinNames().provideFetchPublicName()));
+    JSObject* function = getAs<JSObject*>(globalObject, vm.propertyNames->builtinNames().provideFetchPublicName());
     RETURN_IF_EXCEPTION(scope, { });
     auto callData = JSC::getCallData(vm, function);
     ASSERT(callData.type != CallData::Type::None);
@@ -164,7 +164,7 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* function = jsCast<JSObject*>(get(globalObject, vm.propertyNames->builtinNames().loadAndEvaluateModulePublicName()));
+    JSObject* function = getAs<JSObject*>(globalObject, vm.propertyNames->builtinNames().loadAndEvaluateModulePublicName());
     RETURN_IF_EXCEPTION(scope, nullptr);
     auto callData = JSC::getCallData(vm, function);
     ASSERT(callData.type != CallData::Type::None);
@@ -185,7 +185,7 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* function = jsCast<JSObject*>(get(globalObject, vm.propertyNames->builtinNames().loadModulePublicName()));
+    JSObject* function = getAs<JSObject*>(globalObject, vm.propertyNames->builtinNames().loadModulePublicName());
     RETURN_IF_EXCEPTION(scope, nullptr);
     auto callData = JSC::getCallData(vm, function);
     ASSERT(callData.type != CallData::Type::None);
@@ -206,7 +206,7 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* function = jsCast<JSObject*>(get(globalObject, vm.propertyNames->builtinNames().linkAndEvaluateModulePublicName()));
+    JSObject* function = getAs<JSObject*>(globalObject, vm.propertyNames->builtinNames().linkAndEvaluateModulePublicName());
     RETURN_IF_EXCEPTION(scope, { });
     auto callData = JSC::getCallData(vm, function);
     ASSERT(callData.type != CallData::Type::None);
@@ -224,7 +224,7 @@
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    auto* function = jsCast<JSObject*>(get(globalObject, vm.propertyNames->builtinNames().requestImportModulePublicName()));
+    auto* function = getAs<JSObject*>(globalObject, vm.propertyNames->builtinNames().requestImportModulePublicName());
     RETURN_IF_EXCEPTION(scope, nullptr);
     auto callData = JSC::getCallData(vm, function);
     ASSERT(callData.type != CallData::Type::None);

Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/JSObject.h	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h	2021-05-03 15:54:53 UTC (rev 276900)
@@ -152,6 +152,9 @@
     JSValue get(JSGlobalObject*, unsigned propertyName) const;
     JSValue get(JSGlobalObject*, uint64_t propertyName) const;
 
+    template<typename T, typename PropertyNameType>
+    T getAs(JSGlobalObject*, PropertyNameType) const;
+
     template<bool checkNullStructure = false>
     bool getPropertySlot(JSGlobalObject*, PropertyName, PropertySlot&);
     bool getPropertySlot(JSGlobalObject*, unsigned propertyName, PropertySlot&);
@@ -1517,6 +1520,18 @@
     return jsUndefined();
 }
 
+template<typename T, typename PropertyNameType>
+inline T JSObject::getAs(JSGlobalObject* globalObject, PropertyNameType propertyName) const
+{
+    JSValue value = get(globalObject, propertyName);
+#if ASSERT_ENABLED || ENABLE(SECURITY_ASSERTIONS)
+    VM& vm = getVM(globalObject);
+    if (vm.exceptionForInspection())
+        return nullptr;
+#endif
+    return jsCast<T>(value);
+}
+
 inline bool JSObject::putDirect(VM& vm, PropertyName propertyName, JSValue value, unsigned attributes)
 {
     ASSERT(!value.isGetterSetter() && !(attributes & PropertyAttribute::Accessor));

Modified: trunk/Source/_javascript_Core/runtime/JSPromise.cpp (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/JSPromise.cpp	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/JSPromise.cpp	2021-05-03 15:54:53 UTC (rev 276900)
@@ -119,11 +119,11 @@
     RETURN_IF_EXCEPTION(scope, { });
 
     DeferredData result;
-    result.promise = jsCast<JSPromise*>(deferred.get(globalObject, vm.propertyNames->builtinNames().promisePrivateName()));
+    result.promise = deferred.getAs<JSPromise*>(globalObject, vm.propertyNames->builtinNames().promisePrivateName());
     RETURN_IF_EXCEPTION(scope, { });
-    result.resolve = jsCast<JSFunction*>(deferred.get(globalObject, vm.propertyNames->builtinNames().resolvePrivateName()));
+    result.resolve = deferred.getAs<JSFunction*>(globalObject, vm.propertyNames->builtinNames().resolvePrivateName());
     RETURN_IF_EXCEPTION(scope, { });
-    result.reject = jsCast<JSFunction*>(deferred.get(globalObject, vm.propertyNames->builtinNames().rejectPrivateName()));
+    result.reject = deferred.getAs<JSFunction*>(globalObject, vm.propertyNames->builtinNames().rejectPrivateName());
     RETURN_IF_EXCEPTION(scope, { });
 
     return result;

Modified: trunk/Source/_javascript_Core/runtime/VM.h (276899 => 276900)


--- trunk/Source/_javascript_Core/runtime/VM.h	2021-05-03 15:02:50 UTC (rev 276899)
+++ trunk/Source/_javascript_Core/runtime/VM.h	2021-05-03 15:54:53 UTC (rev 276900)
@@ -887,8 +887,8 @@
     Exception* lastException() const { return m_lastException; }
     JSCell** addressOfLastException() { return reinterpret_cast<JSCell**>(&m_lastException); }
 
-    // This should only be used for test or assertion code that wants to inspect
-    // the pending exception without interfering with Throw/CatchScopes.
+    // This should only be used for code that wants to check for any pending
+    // exception without interfering with Throw/CatchScopes.
     Exception* exceptionForInspection() const { return m_exception; }
 
     void setFailNextNewCodeBlock() { m_failNextNewCodeBlock = true; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to