diff --git a/lib/VM/JSError.cpp b/lib/VM/JSError.cpp index 478bfc25c8c..f012fc11b87 100644 --- a/lib/VM/JSError.cpp +++ b/lib/VM/JSError.cpp @@ -217,63 +217,6 @@ PseudoHandle JSError::create( return JSObjectInit::initToPseudoHandle(runtime, cell); } -ExecutionStatus JSError::setupStack( - Handle selfHandle, - Runtime &runtime) { - // Lazily allocate the accessor. - if (runtime.jsErrorStackAccessor.isUndefined()) { - // This code path allocates quite a few handles, so make sure we - // don't disturb the parent GCScope and free them. - GCScope gcScope{runtime}; - - auto getter = NativeFunction::create( - runtime, - Handle::vmcast(&runtime.functionPrototype), - nullptr, - errorStackGetter, - Predefined::getSymbolID(Predefined::emptyString), - 0, - Runtime::makeNullHandle()); - - auto setter = NativeFunction::create( - runtime, - Handle::vmcast(&runtime.functionPrototype), - nullptr, - errorStackSetter, - Predefined::getSymbolID(Predefined::emptyString), - 1, - Runtime::makeNullHandle()); - - runtime.jsErrorStackAccessor = - PropertyAccessor::create(runtime, getter, setter); - } - - auto accessor = - Handle::vmcast(&runtime.jsErrorStackAccessor); - - DefinePropertyFlags dpf{}; - dpf.setEnumerable = 1; - dpf.setConfigurable = 1; - dpf.setGetter = 1; - dpf.setSetter = 1; - dpf.enumerable = 0; - dpf.configurable = 1; - - auto res = JSObject::defineOwnProperty( - selfHandle, - runtime, - Predefined::getSymbolID(Predefined::stack), - dpf, - accessor); - - // Ignore failures to set the "stack" property as other engines do. - if (LLVM_UNLIKELY(res == ExecutionStatus::EXCEPTION)) { - runtime.clearThrownValue(); - } - - return ExecutionStatus::RETURNED; -} - CallResult> JSError::toString( Handle O, Runtime &runtime) { diff --git a/lib/VM/JSLib/Error.cpp b/lib/VM/JSLib/Error.cpp index 693f6071b76..8f2820dde08 100644 --- a/lib/VM/JSLib/Error.cpp +++ b/lib/VM/JSLib/Error.cpp @@ -14,6 +14,7 @@ #include "hermes/VM/CallResult.h" #include "hermes/VM/Operations.h" +#include "hermes/VM/PropertyAccessor.h" #include "hermes/VM/StringBuilder.h" #include "hermes/VM/StringPrimitive.h" @@ -36,7 +37,7 @@ Handle createErrorConstructor(Runtime &runtime) { 0); // Error.prototype.xxx properties. - // Error.prototype has two own properties: name and message. + // Error.prototype has three own properties: name, message, and stack. auto defaultName = runtime.getPredefinedString(Predefined::Error); defineProperty( runtime, @@ -51,6 +52,49 @@ Handle createErrorConstructor(Runtime &runtime) { Predefined::getSymbolID(Predefined::message), runtime.makeHandle(HermesValue::encodeStringValue(defaultMessage))); + auto getter = NativeFunction::create( + runtime, + Handle::vmcast(&runtime.functionPrototype), + nullptr, + errorStackGetter, + Predefined::getSymbolID(Predefined::emptyString), + 0, + Runtime::makeNullHandle()); + + auto setter = NativeFunction::create( + runtime, + Handle::vmcast(&runtime.functionPrototype), + nullptr, + errorStackSetter, + Predefined::getSymbolID(Predefined::emptyString), + 1, + Runtime::makeNullHandle()); + + // Save the accessors on the runtime so we can use them for captureStackTrace. + runtime.jsErrorStackAccessor = + PropertyAccessor::create(runtime, getter, setter); + + auto accessor = + Handle::vmcast(&runtime.jsErrorStackAccessor); + + DefinePropertyFlags dpf{}; + dpf.setEnumerable = 1; + dpf.setConfigurable = 1; + dpf.setGetter = 1; + dpf.setSetter = 1; + dpf.enumerable = 0; + dpf.configurable = 1; + + auto stackRes = JSObject::defineOwnProperty( + errorPrototype, + runtime, + Predefined::getSymbolID(Predefined::stack), + dpf, + accessor); + (void)stackRes; + + assert(*stackRes && "Failed to define stack accessor"); + auto cons = defineSystemConstructor( runtime, Predefined::getSymbolID(Predefined::Error), @@ -123,13 +167,6 @@ static CallResult constructErrorObject( return ExecutionStatus::EXCEPTION; } - // Initialize stack accessor. - if (LLVM_UNLIKELY( - JSError::setupStack(selfHandle, runtime) == - ExecutionStatus::EXCEPTION)) { - return ExecutionStatus::EXCEPTION; - } - // new Error(message). // Only proceed when 'typeof message' isn't undefined. if (!message->isUndefined()) { @@ -320,10 +357,24 @@ errorCaptureStackTrace(void *, Runtime &runtime, NativeArgs args) { } // Initialize the `stack` accessor on the target object. - if (LLVM_UNLIKELY( - JSError::setupStack(targetHandle, runtime) == - ExecutionStatus::EXCEPTION)) { - return ExecutionStatus::EXCEPTION; + DefinePropertyFlags dpf{}; + dpf.setEnumerable = 1; + dpf.setConfigurable = 1; + dpf.setGetter = 1; + dpf.setSetter = 1; + dpf.enumerable = 0; + dpf.configurable = 1; + + auto stackRes = JSObject::defineOwnProperty( + targetHandle, + runtime, + Predefined::getSymbolID(Predefined::stack), + dpf, + Handle<>(&runtime.jsErrorStackAccessor)); + + // Ignore failures to set the "stack" property as other engines do. + if (LLVM_UNLIKELY(stackRes == ExecutionStatus::EXCEPTION)) { + runtime.clearThrownValue(); } return HermesValue::encodeUndefinedValue(); diff --git a/lib/VM/Runtime.cpp b/lib/VM/Runtime.cpp index 85f9049049b..6143f86bb00 100644 --- a/lib/VM/Runtime.cpp +++ b/lib/VM/Runtime.cpp @@ -1281,7 +1281,6 @@ uint32_t Runtime::getCurrentFrameOffset() const { static ExecutionStatus raisePlaceholder(Runtime &runtime, Handle errorObj, Handle<> message) { JSError::recordStackTrace(errorObj, runtime); - JSError::setupStack(errorObj, runtime); JSError::setMessage(errorObj, runtime, message); return runtime.setThrownValue(errorObj.getHermesValue()); } diff --git a/utils/testsuite/testsuite_skiplist.py b/utils/testsuite/testsuite_skiplist.py index d0a520d389e..7edb49e1ca4 100644 --- a/utils/testsuite/testsuite_skiplist.py +++ b/utils/testsuite/testsuite_skiplist.py @@ -1287,6 +1287,8 @@ "mjsunit/regress/regress-prepare-break-while-recompile.js", # Fails when ASAN/UBSAN are limiting the max native stack depth. "mjsunit/compiler/regress-lazy-deopt.js", + # Error stack getter is on Error.prototype in Hermes. + "mjsunit/regress/regress-3404.js", # Uncategorized mjsunit failures "mjsunit/accessor-map-sharing.js", "mjsunit/accessors-on-global-object.js",