Skip to content

Commit

Permalink
Use PseudoHandle in JSObject API.
Browse files Browse the repository at this point in the history 
Summary:
Make most of the JSObject API functions return `PseudoHandle` instead of
`HermesValue`. This reduces the risk of using the result value across
allocations.

Reviewed By: tmikov

Differential Revision: D19746864

fbshipit-source-id: 8acf3f083adb567113529388b2f05a8333e83f01
  • Loading branch information
@avp @facebook-github-bot
avp authored and facebook-github-bot committed Jun 1, 2020
1 parent 47ce36a commit 8f95074
Show file tree
Hide file tree
Showing 47 changed files with 927 additions and 777 deletions.
20 changes: 10 additions & 10 deletions API/hermes/hermes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1566,7 +1566,7 @@ jsi::Value HermesRuntimeImpl::getProperty(
auto h = handle(obj);
auto res = h->getComputed_RJS(h, &runtime_, stringHandle(name));
checkStatus(res.getStatus());
return valueFromHermesValue(*res);
return valueFromHermesValue(res->get());
});
}

Expand All @@ -1579,7 +1579,7 @@ jsi::Value HermesRuntimeImpl::getProperty(
vm::SymbolID nameID = phv(name).getSymbol();
auto res = h->getNamedOrIndexed(h, &runtime_, nameID);
checkStatus(res.getStatus());
return valueFromHermesValue(*res);
return valueFromHermesValue(res->get());
});
}

Expand Down Expand Up @@ -1750,7 +1750,7 @@ jsi::Value HermesRuntimeImpl::getValueAtIndex(const jsi::Array &arr, size_t i) {
runtime_.makeHandle(vm::HermesValue::encodeNumberValue(i)));
checkStatus(res.getStatus());

return valueFromHermesValue(*res);
return valueFromHermesValue(res->get());
});
}

Expand Down Expand Up @@ -1850,7 +1850,7 @@ jsi::Value HermesRuntimeImpl::call(
auto callRes = vm::Callable::call(handle, &runtime_);
checkStatus(callRes.getStatus());

return valueFromHermesValue(*callRes);
return valueFromHermesValue(callRes->get());
});
}

Expand Down Expand Up @@ -1891,7 +1891,7 @@ jsi::Value HermesRuntimeImpl::callAsConstructor(
auto thisRes = vm::Callable::createThisForConstruct(funcHandle, &runtime_);
// We need to capture this in case the ctor doesn't return an object,
// we need to return this object.
auto objHandle = runtime_.makeHandle<vm::JSObject>(*thisRes);
auto objHandle = runtime_.makeHandle<vm::JSObject>(std::move(*thisRes));

// 13.2.2.8:
// Let result be the result of calling the [[Call]] internal property of
Expand Down Expand Up @@ -1920,7 +1920,7 @@ jsi::Value HermesRuntimeImpl::callAsConstructor(
// If Type(result) is Object then return result
// 13.2.2.10:
// Return obj
auto resultValue = *callRes;
auto resultValue = callRes->get();
vm::HermesValue resultHValue =
resultValue.isObject() ? resultValue : objHandle.getHermesValue();
return valueFromHermesValue(resultHValue);
Expand Down Expand Up @@ -2027,10 +2027,10 @@ size_t HermesRuntimeImpl::getLength(vm::Handle<vm::ArrayImpl> arr) {
auto res = vm::JSObject::getNamed_RJS(
arr, &runtime_, vm::Predefined::getSymbolID(vm::Predefined::length));
checkStatus(res.getStatus());
if (!res->isNumber()) {
if (!(*res)->isNumber()) {
throw jsi::JSError(*this, "getLength: property 'length' is not a number");
}
return static_cast<size_t>(res->getDouble());
return static_cast<size_t>((*res)->getDouble());
});
}

Expand All @@ -2041,11 +2041,11 @@ size_t HermesRuntimeImpl::getByteLength(vm::Handle<vm::JSArrayBuffer> arr) {
&runtime_,
vm::Predefined::getSymbolID(vm::Predefined::byteLength));
checkStatus(res.getStatus());
if (!res->isNumber()) {
if (!(*res)->isNumber()) {
throw jsi::JSError(
*this, "getLength: property 'byteLength' is not a number");
}
return static_cast<size_t>(res->getDouble());
return static_cast<size_t>((*res)->getDouble());
});
}

Expand Down
1 change: 0 additions & 1 deletion include/hermes/Support/Conversions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include "hermes/Support/OptValue.h"

#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/MathExtras.h"

Expand Down
10 changes: 5 additions & 5 deletions include/hermes/VM/ArrayLike.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ inline CallResult<uint64_t> getArrayLikeLength(
if (LLVM_UNLIKELY(propRes == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
return toLengthU64(runtime, runtime->makeHandle(*propRes));
return toLengthU64(runtime, runtime->makeHandle(std::move(*propRes)));
}

/// ES9 7.3.17 CreateListFromArrayLike
Expand Down Expand Up @@ -63,13 +63,13 @@ ExecutionStatus createListFromArrayLike(
// Slow path fallback: the actual getComputed on this,
// because the real value could be up the prototype chain.
iHandle = HermesValue::encodeNumberValue(elemIdx);
CallResult<HermesValue> propRes =
CallResult<PseudoHandle<>> propRes =
JSObject::getComputed_RJS(arrayLikeHandle, runtime, iHandle);
if (LLVM_UNLIKELY(propRes == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
if (LLVM_UNLIKELY(
elementCB(runtime, elemIdx, createPseudoHandle(*propRes)) ==
elementCB(runtime, elemIdx, std::move(*propRes)) ==
ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
Expand All @@ -79,13 +79,13 @@ ExecutionStatus createListFromArrayLike(
for (uint64_t elemIdx = 0; elemIdx < length; ++elemIdx) {
gcScope.flushToMarker(marker);
iHandle = HermesValue::encodeNumberValue(elemIdx);
CallResult<HermesValue> propRes =
CallResult<PseudoHandle<>> propRes =
JSObject::getComputed_RJS(arrayLikeHandle, runtime, iHandle);
if (LLVM_UNLIKELY(propRes == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
if (LLVM_UNLIKELY(
elementCB(runtime, elemIdx, createPseudoHandle(*propRes)) ==
elementCB(runtime, elemIdx, std::move(*propRes)) ==
ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
Expand Down
67 changes: 34 additions & 33 deletions include/hermes/VM/Callable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ struct CallableVTable {
/// Create a new object instance to be passed as the 'this' argument when
/// invoking the constructor. Overriding this method allows creation of
/// different underlying native objects.
CallResult<HermesValue> (*newObject)(
CallResult<PseudoHandle<JSObject>> (*newObject)(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<JSObject> parentHandle);

/// Call the callable with arguments already on the stack.
CallResult<HermesValue> (
CallResult<PseudoHandle<>> (
*call)(Handle<Callable> selfHandle, Runtime *runtime);
};

Expand Down Expand Up @@ -196,15 +196,15 @@ class Callable : public JSObject {

/// Execute this function with no arguments. This is just a convenience
/// helper method; it actually invokes the interpreter recursively.
static CallResult<HermesValue> executeCall0(
static CallResult<PseudoHandle<>> executeCall0(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<> thisArgHandle,
bool construct = false);

/// Execute this function with one argument. This is just a convenience
/// helper method; it actually invokes the interpreter recursively.
static CallResult<HermesValue> executeCall1(
static CallResult<PseudoHandle<>> executeCall1(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<> thisArgHandle,
Expand All @@ -213,7 +213,7 @@ class Callable : public JSObject {

/// Execute this function with two arguments. This is just a convenience
/// helper method; it actually invokes the interpreter recursively.
static CallResult<HermesValue> executeCall2(
static CallResult<PseudoHandle<>> executeCall2(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<> thisArgHandle,
Expand All @@ -223,7 +223,7 @@ class Callable : public JSObject {

/// Execute this function with three arguments. This is just a convenience
/// helper method; it actually invokes the interpreter recursively.
static CallResult<HermesValue> executeCall3(
static CallResult<PseudoHandle<>> executeCall3(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<> thisArgHandle,
Expand All @@ -234,7 +234,7 @@ class Callable : public JSObject {

/// Execute this function with four arguments. This is just a convenience
/// helper method; it actually invokes the interpreter recursively.
static CallResult<HermesValue> executeCall4(
static CallResult<PseudoHandle<>> executeCall4(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<> thisArgHandle,
Expand All @@ -247,23 +247,23 @@ class Callable : public JSObject {
/// Execute this function with given this and newTarget, and a
/// variable number of arguments taken from arrayLike. This invokes
/// the interpreter recursively.
static CallResult<HermesValue> executeCall(
static CallResult<PseudoHandle<>> executeCall(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<> newTarget,
Handle<> thisArgument,
Handle<JSObject> arrayLike);

/// Calls CallableVTable::newObject.
static CallResult<HermesValue> newObject(
static CallResult<PseudoHandle<JSObject>> newObject(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<JSObject> parentHandle) {
return selfHandle->getVT()->newObject(selfHandle, runtime, parentHandle);
}

/// Calls CallableVTable::call.
static CallResult<HermesValue> call(
static CallResult<PseudoHandle<>> call(
Handle<Callable> selfHandle,
Runtime *runtime) {
// Any call to a native or JS function could potentially allocate.
Expand All @@ -276,27 +276,29 @@ class Callable : public JSObject {
/// Call the callable in contruct mode with arguments already on the stack.
/// Checks the return value of the called function. If it is an object, then
/// it is returned, else the `this` value is returned.
static CallResult<HermesValue>
static CallResult<PseudoHandle<>>
construct(Handle<Callable> selfHandle, Runtime *runtime, Handle<> thisVal) {
auto result = call(selfHandle, runtime);
if (LLVM_UNLIKELY(result == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
return result->isObject() ? result : thisVal.getHermesValue();
return (*result)->isObject()
? std::move(result)
: CallResult<PseudoHandle<>>{PseudoHandle<>(thisVal)};
}

/// Create a new object and construct the new object of the given type
/// by invoking \p selfHandle with construct=true.
/// \param selfHandle the Callable from which to construct the new object.
static CallResult<HermesValue> executeConstruct0(
static CallResult<PseudoHandle<>> executeConstruct0(
Handle<Callable> selfHandle,
Runtime *runtime);

/// Create a new object and construct the new object of the given type
/// by invoking \p selfHandle with construct=true.
/// \param selfHandle the Callable from which to construct the new object.
/// \param param1 the first argument to the constructor.
static CallResult<HermesValue> executeConstruct1(
static CallResult<PseudoHandle<>> executeConstruct1(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<> param1);
Expand All @@ -319,7 +321,7 @@ class Callable : public JSObject {
/// Retrieves the "prototype" property from \p selfHandle,
/// and calls newObject() on it if it's an object,
/// else calls newObject() on the built-in Object prototype object.
static CallResult<HermesValue> createThisForConstruct(
static CallResult<PseudoHandle<JSObject>> createThisForConstruct(
Handle<Callable> selfHandle,
Runtime *runtime);

Expand All @@ -343,7 +345,7 @@ class Callable : public JSObject {

/// Create a an instance of Object to be passed as the 'this' argument when
/// invoking the constructor.
static CallResult<HermesValue> _newObjectImpl(
static CallResult<PseudoHandle<JSObject>> _newObjectImpl(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<JSObject> parentHandle);
Expand Down Expand Up @@ -389,7 +391,7 @@ class BoundFunction final : public Callable {
/// Other users of this class must use \c Callable::call().
/// \param ip the caller's IP at the point of the call (used for preserving
/// stack traces).
static CallResult<HermesValue>
static CallResult<PseudoHandle<>>
_boundCall(BoundFunction *self, const Inst *ip, Runtime *runtime);

/// Intialize the length and name and property of a lazily created bound
Expand Down Expand Up @@ -430,13 +432,13 @@ class BoundFunction final : public Callable {
}

/// Create an instance of the object using the bound constructor.
static CallResult<HermesValue> _newObjectImpl(
static CallResult<PseudoHandle<JSObject>> _newObjectImpl(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<JSObject> parentHandle);

/// Call the callable with arguments already on the stack.
static CallResult<HermesValue> _callImpl(
static CallResult<PseudoHandle<>> _callImpl(
Handle<Callable> selfHandle,
Runtime *runtime);
};
Expand Down Expand Up @@ -513,7 +515,7 @@ class NativeFunction : public Callable {
/// This is a lightweight and unsafe wrapper intended to be used only by the
/// interpreter. Its purpose is to avoid needlessly exposing the private
/// fields.
static CallResult<HermesValue> _nativeCall(
static CallResult<PseudoHandle<>> _nativeCall(
NativeFunction *self,
Runtime *runtime) {
ScopedNativeDepthTracker depthTracker{runtime};
Expand Down Expand Up @@ -541,7 +543,10 @@ class NativeFunction : public Callable {
++self->callCount_;
#endif
runtime->restoreStackAndPreviousFrame(newFrame);
return res;
if (LLVM_UNLIKELY(res == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
return createPseudoHandle(*res);
}

/// Create an instance of NativeFunction.
Expand Down Expand Up @@ -692,15 +697,15 @@ class NativeFunction : public Callable {
static std::string _snapshotNameImpl(GCCell *cell, GC *gc);

/// Call the native function with arguments already on the stack.
static CallResult<HermesValue> _callImpl(
static CallResult<PseudoHandle<>> _callImpl(
Handle<Callable> selfHandle,
Runtime *runtime);

/// We have to override this method because NativeFunction should not be
/// used as constructor.
/// Note: this may change in the future, in that case, we should create
/// a subclass of NativeFunction for this restriction.
static CallResult<HermesValue>
static CallResult<PseudoHandle<JSObject>>
_newObjectImpl(Handle<Callable>, Runtime *runtime, Handle<JSObject>);
};

Expand Down Expand Up @@ -861,23 +866,19 @@ class NativeConstructor final : public NativeFunction {

/// Create a an instance of an object from \c creator_ to be passed as the
/// 'this' argument when invoking the constructor.
static CallResult<HermesValue> _newObjectImpl(
static CallResult<PseudoHandle<JSObject>> _newObjectImpl(
Handle<Callable> selfHandle,
Runtime *runtime,
Handle<JSObject> parentHandle) {
auto nativeConsHandle = Handle<NativeConstructor>::vmcast(selfHandle);
auto res = nativeConsHandle->creator_(
return nativeConsHandle->creator_(
runtime, parentHandle, nativeConsHandle->getContext());
if (LLVM_UNLIKELY(res == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
return res->getHermesValue();
}

#ifndef NDEBUG
/// If construct=true, check that the constructor was called with a "this"
/// of the correct type.
static CallResult<HermesValue> _callImpl(
static CallResult<PseudoHandle<>> _callImpl(
Handle<Callable> selfHandle,
Runtime *runtime);
#endif
Expand Down Expand Up @@ -1023,7 +1024,7 @@ class JSFunction : public Callable {

protected:
/// Call the JavaScript function with arguments already on the stack.
static CallResult<HermesValue> _callImpl(
static CallResult<PseudoHandle<>> _callImpl(
Handle<Callable> selfHandle,
Runtime *runtime);

Expand Down Expand Up @@ -1170,15 +1171,15 @@ class GeneratorInnerFunction final : public JSFunction {
/// \param action the user-requested action, assigned to the action_ field.
/// Restores the stored arguments before actually calling the wrapped
/// function.
static CallResult<HermesValue> callInnerFunction(
static CallResult<PseudoHandle<>> callInnerFunction(
Handle<GeneratorInnerFunction> selfHandle,
Runtime *runtime,
Handle<> arg,
Action action);

/// GeneratorInnerFunction must not be called through _callImpl, but rather
/// directly by VM code only.
static CallResult<HermesValue> _callImpl(
static CallResult<PseudoHandle<>> _callImpl(
Handle<Callable> selfHandle,
Runtime *runtime) {
return runtime->raiseTypeError(
Expand Down
Loading

0 comments on commit 8f95074

Please sign in to comment.