[builtins] catch and rethrow the message together with the exception

This aligns the Torque semantics of catch with the JavaScript behavior:
When we catch an exception, we also reset the pending exception.
This also fixes a long-standing bug that we didn't restore the original
pending message after executing arbitrary JS in IteratorCloseOnException

Bug: v8:12439
Change-Id: I268d9d639d09023a424f352547cdce03428f983a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3303805
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78259}
This commit is contained in:
Tobias Tebbi 2021-12-06 21:32:23 +00:00 committed by V8 LUCI CQ
parent ab501b59a6
commit d3ba88a2ce
33 changed files with 149 additions and 57 deletions

View File

@ -110,9 +110,9 @@ ArrayFrom(js-implicit context: NativeContext, receiver: JSAny)(...arguments):
try {
mappedValue =
Call(context, UnsafeCast<Callable>(mapfn), thisArg, nextValue, k);
} catch (e) {
} catch (e, message) {
iterator::IteratorCloseOnException(iteratorRecord);
ReThrow(context, e);
ReThrowWithMessage(context, e, message);
}
} else {
mappedValue = nextValue;
@ -123,9 +123,9 @@ ArrayFrom(js-implicit context: NativeContext, receiver: JSAny)(...arguments):
// return ? IteratorClose(iteratorRecord, defineStatus).
try {
FastCreateDataProperty(a, k, mappedValue);
} catch (e) deferred {
} catch (e, message) deferred {
iterator::IteratorCloseOnException(iteratorRecord);
ReThrow(context, e);
ReThrowWithMessage(context, e, message);
}
// x. Set k to k + 1.
k += 1;

View File

@ -537,9 +537,9 @@ transitioning macro CycleProtectedArrayJoin<T: type>(
ArrayJoin<T>(useToLocaleString, o, sep, len, locales, options);
JoinStackPopInline(o);
return result;
} catch (e) deferred {
} catch (e, message) deferred {
JoinStackPopInline(o);
ReThrow(context, e);
ReThrowWithMessage(context, e, message);
}
} else {
return kEmptyString;

View File

@ -775,7 +775,9 @@ macro Equal(implicit context: Context)(left: JSAny, right: JSAny): Boolean {
extern macro StrictEqual(JSAny, JSAny): Boolean;
extern macro SmiLexicographicCompare(Smi, Smi): Smi;
extern runtime ReThrow(Context, JSAny): never;
extern runtime ReThrowWithMessage(
Context, JSAny, TheHole | JSMessageObject): never;
extern runtime Throw(implicit context: Context)(JSAny): never;
extern runtime ThrowInvalidStringLength(Context): never;

View File

@ -333,8 +333,11 @@ void BaseCollectionsAssembler::AddConstructorEntriesFromIterable(
}
BIND(&if_exception);
{
TNode<HeapObject> message = GetPendingMessage();
SetPendingMessage(TheHoleConstant());
IteratorCloseOnException(context, iterator);
CallRuntime(Runtime::kReThrow, context, var_exception.value());
CallRuntime(Runtime::kReThrowWithMessage, context, var_exception.value(),
message);
Unreachable();
}
BIND(&exit);

View File

@ -268,8 +268,11 @@ TNode<JSArray> IteratorBuiltinsAssembler::StringListFromIterable(
// 2. Return ? IteratorClose(iteratorRecord, error).
BIND(&if_exception);
TNode<HeapObject> message = GetPendingMessage();
SetPendingMessage(TheHoleConstant());
IteratorCloseOnException(context, iterator_record);
CallRuntime(Runtime::kReThrow, context, var_exception.value());
CallRuntime(Runtime::kReThrowWithMessage, context, var_exception.value(),
message);
Unreachable();
}
}

View File

@ -79,10 +79,10 @@ FinalizationRegistryCleanupLoop(implicit context: Context)(
case (weakCell: WeakCell): {
try {
Call(context, callback, Undefined, weakCell.holdings);
} catch (e) {
} catch (e, message) {
runtime::ShrinkFinalizationRegistryUnregisterTokenMap(
context, finalizationRegistry);
ReThrow(context, e);
ReThrowWithMessage(context, e, message);
}
}
}

View File

@ -130,7 +130,7 @@ transitioning macro IteratorCloseOnException(implicit context: Context)(
// c. Set innerResult to Call(return, iterator).
// If an exception occurs, the original exception remains bound
Call(context, method, iterator.object);
} catch (_e) {
} catch (_e, _message) {
// Swallow the exception.
}

View File

@ -69,9 +69,9 @@ ObjectFromEntries(
CreateDataProperty(result, pair.key, pair.value);
}
return result;
} catch (e) deferred {
} catch (e, message) deferred {
iterator::IteratorCloseOnException(i);
ReThrow(context, e);
ReThrowWithMessage(context, e, message);
}
} label Throw deferred {
ThrowTypeError(MessageTemplate::kNotIterable);

View File

@ -139,7 +139,7 @@ transitioning macro PerformPromiseAll<F1: type, F2: type>(
constructor: Constructor, capability: PromiseCapability,
promiseResolveFunction: JSAny, createResolveElementFunctor: F1,
createRejectElementFunctor: F2): JSAny labels
Reject(Object) {
Reject(JSAny) {
const promise = capability.promise;
const resolve = capability.resolve;
const reject = capability.reject;
@ -172,7 +172,7 @@ Reject(Object) {
// to true.
// ReturnIfAbrupt(nextValue).
nextValue = iterator::IteratorValue(next, fastIteratorResultMap);
} catch (e) {
} catch (e, _message) {
goto Reject(e);
}
@ -262,7 +262,7 @@ Reject(Object) {
// Set index to index + 1.
index += 1;
}
} catch (e) deferred {
} catch (e, _message) deferred {
iterator::IteratorCloseOnException(iter);
goto Reject(e);
} label Done {}
@ -354,11 +354,9 @@ transitioning macro GeneratePromiseAll<F1: type, F2: type>(
nativeContext, i, constructor, capability, promiseResolveFunction,
createResolveElementFunctor, createRejectElementFunctor)
otherwise Reject;
} catch (e) deferred {
} catch (e, _message) deferred {
goto Reject(e);
} label Reject(e: Object) deferred {
// Exception must be bound to a JS value.
const e = UnsafeCast<JSAny>(e);
} label Reject(e: JSAny) deferred {
const reject = UnsafeCast<JSAny>(capability.reject);
Call(context, reject, Undefined, e);
return capability.promise;

View File

@ -159,7 +159,7 @@ transitioning macro PerformPromiseAny(implicit context: Context)(
nativeContext: NativeContext, iteratorRecord: iterator::IteratorRecord,
constructor: Constructor, resultCapability: PromiseCapability,
promiseResolveFunction: JSAny): JSAny labels
Reject(Object) {
Reject(JSAny) {
// 1. Assert: ! IsConstructor(constructor) is true.
// 2. Assert: resultCapability is a PromiseCapability Record.
@ -198,7 +198,7 @@ Reject(Object) {
// g. ReturnIfAbrupt(nextValue).
nextValue = iterator::IteratorValue(next, fastIteratorResultMap);
} catch (e) {
} catch (e, _message) {
goto Reject(e);
}
@ -280,7 +280,7 @@ Reject(Object) {
context, rejectElement, kPromiseForwardingHandlerSymbol, True);
}
}
} catch (e) deferred {
} catch (e, _message) deferred {
iterator::IteratorCloseOnException(iteratorRecord);
goto Reject(e);
} label Done {}
@ -361,9 +361,9 @@ PromiseAny(
nativeContext, iteratorRecord, constructor, capability,
promiseResolveFunction)
otherwise Reject;
} catch (e) deferred {
} catch (e, _message) deferred {
goto Reject(e);
} label Reject(e: Object) deferred {
} label Reject(e: JSAny) deferred {
// Exception must be bound to a JS value.
dcheck(e != TheHole);
Call(

View File

@ -85,7 +85,7 @@ PromiseConstructor(
const reject = funcs.reject;
try {
Call(context, UnsafeCast<Callable>(executor), Undefined, resolve, reject);
} catch (e) {
} catch (e, _message) {
Call(context, reject, Undefined, e);
}

View File

@ -66,7 +66,7 @@ PromiseResolveThenableJob(implicit context: Context)(
try {
return Call(
context, UnsafeCast<Callable>(then), thenable, resolve, reject);
} catch (e) {
} catch (e, _message) {
return Call(context, UnsafeCast<Callable>(reject), Undefined, e);
}
}

View File

@ -112,7 +112,7 @@ transitioning macro RunContextPromiseHookInit(implicit context: Context)(
try {
Call(context, hook, Undefined, promise, parentObject);
} catch (e) {
} catch (e, _message) {
runtime::ReportMessageFromMicrotask(e);
}
}
@ -189,7 +189,7 @@ transitioning macro RunContextPromiseHook(implicit context: Context)(
try {
Call(context, hook, Undefined, promise);
} catch (e) {
} catch (e, _message) {
runtime::ReportMessageFromMicrotask(e);
}
}

View File

@ -47,7 +47,7 @@ PromiseRace(
// Let iterator be GetIterator(iterable).
// IfAbruptRejectPromise(iterator, promiseCapability).
i = iterator::GetIterator(iterable);
} catch (e) deferred {
} catch (e, _message) deferred {
goto Reject(e);
}
@ -69,7 +69,7 @@ PromiseRace(
// to true.
// ReturnIfAbrupt(nextValue).
nextValue = iterator::IteratorValue(next, fastIteratorResultMap);
} catch (e) {
} catch (e, _message) {
goto Reject(e);
}
// Let nextPromise be ? Call(constructor, _promiseResolve_, «
@ -91,14 +91,12 @@ PromiseRace(
context, thenResult, kPromiseHandledBySymbol, promise);
}
}
} catch (e) deferred {
} catch (e, _message) deferred {
iterator::IteratorCloseOnException(i);
goto Reject(e);
}
} label Reject(exception: Object) deferred {
Call(
context, UnsafeCast<JSAny>(reject), Undefined,
UnsafeCast<JSAny>(exception));
} label Reject(exception: JSAny) deferred {
Call(context, UnsafeCast<JSAny>(reject), Undefined, exception);
return promise;
}
unreachable;

View File

@ -60,7 +60,7 @@ macro FuflfillPromiseReactionJob(
const resolve = UnsafeCast<Callable>(capability.resolve);
try {
return Call(context, resolve, Undefined, result);
} catch (e) {
} catch (e, _message) {
return RejectPromiseReactionJob(
context, promiseOrCapability, e, reactionType);
}
@ -98,7 +98,7 @@ macro PromiseReactionJob(
return FuflfillPromiseReactionJob(
context, promiseOrCapability, result, reactionType);
}
} catch (e) {
} catch (e, _message) {
return RejectPromiseReactionJob(
context, promiseOrCapability, e, reactionType);
}

View File

@ -165,7 +165,7 @@ ResolvePromise(implicit context: Context)(
// 10. If then is an abrupt completion, then
try {
then = GetProperty(resolution, kThenString);
} catch (e) {
} catch (e, _message) {
// a. Return RejectPromise(promise, then.[[Value]]).
return RejectPromise(promise, e, False);
}

View File

@ -334,6 +334,16 @@ intrinsic %IndexedFieldLength<T: type>(o: T, f: constexpr string): intptr;
intrinsic %FieldSlice<T: type, TSlice: type>(
o: T, f: constexpr string): TSlice;
extern macro GetPendingMessage(): TheHole|JSMessageObject;
extern macro SetPendingMessage(TheHole | JSMessageObject): void;
// This is implicitly performed at the beginning of Torque catch-blocks.
macro GetAndResetPendingMessage(): TheHole|JSMessageObject {
const message = GetPendingMessage();
SetPendingMessage(TheHole);
return message;
}
} // namespace torque_internal
// Indicates that an array-field should not be initialized.

View File

@ -22,6 +22,7 @@
#include "src/objects/descriptor-array.h"
#include "src/objects/function-kind.h"
#include "src/objects/heap-number.h"
#include "src/objects/instance-type.h"
#include "src/objects/js-generator.h"
#include "src/objects/oddball.h"
#include "src/objects/ordered-hash-table-inl.h"
@ -6167,6 +6168,20 @@ void CodeStubAssembler::ThrowTypeError(TNode<Context> context,
Unreachable();
}
TNode<HeapObject> CodeStubAssembler::GetPendingMessage() {
TNode<ExternalReference> pending_message = ExternalConstant(
ExternalReference::address_of_pending_message(isolate()));
return UncheckedCast<HeapObject>(LoadFullTagged(pending_message));
}
void CodeStubAssembler::SetPendingMessage(TNode<HeapObject> message) {
CSA_DCHECK(this, Word32Or(IsTheHole(message),
InstanceTypeEqual(LoadInstanceType(message),
JS_MESSAGE_OBJECT_TYPE)));
TNode<ExternalReference> pending_message = ExternalConstant(
ExternalReference::address_of_pending_message(isolate()));
StoreFullTaggedNoWriteBarrier(pending_message, message);
}
TNode<BoolT> CodeStubAssembler::InstanceTypeEqual(TNode<Int32T> instance_type,
int type) {
return Word32Equal(instance_type, Int32Constant(type));

View File

@ -2461,6 +2461,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
base::Optional<TNode<Object>> arg1 = base::nullopt,
base::Optional<TNode<Object>> arg2 = base::nullopt);
TNode<HeapObject> GetPendingMessage();
void SetPendingMessage(TNode<HeapObject> message);
// Type checks.
// Check whether the map is for an object with special properties, such as a
// JSProxy or an object with interceptors.

View File

@ -276,6 +276,7 @@ bool Linkage::NeedsFrameStateInput(Runtime::FunctionId function) {
case Runtime::kPushBlockContext:
case Runtime::kPushCatchContext:
case Runtime::kReThrow:
case Runtime::kReThrowWithMessage:
case Runtime::kStringEqual:
case Runtime::kStringLessThan:
case Runtime::kStringLessThanOrEqual:

View File

@ -34,6 +34,7 @@ NativeContext Isolate::raw_native_context() {
}
void Isolate::set_pending_message(Object message_obj) {
DCHECK(message_obj.IsTheHole(this) || message_obj.IsJSMessageObject());
thread_local_top()->pending_message_ = message_obj;
}

View File

@ -1742,6 +1742,14 @@ Object Isolate::ReThrow(Object exception) {
return ReadOnlyRoots(heap()).exception();
}
Object Isolate::ReThrow(Object exception, Object message) {
DCHECK(!has_pending_exception());
DCHECK(!has_pending_message());
set_pending_message(message);
return ReThrow(exception);
}
namespace {
#if V8_ENABLE_WEBASSEMBLY
// This scope will set the thread-in-wasm flag after the execution of all

View File

@ -948,7 +948,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
// Re-throw an exception. This involves no error reporting since error
// reporting was handled when the exception was thrown originally.
// The first overload doesn't set the corresponding pending message, which
// has to be set separately or be guaranteed to not have changed.
Object ReThrow(Object exception);
Object ReThrow(Object exception, Object message);
// Find the correct handler for the current pending exception. This also
// clears and returns the current pending exception.

View File

@ -2613,12 +2613,8 @@ IGNITION_HANDLER(CreateRestParameter, InterpreterAssembler) {
// Sets the pending message to the value in the accumulator, and returns the
// previous pending message in the accumulator.
IGNITION_HANDLER(SetPendingMessage, InterpreterAssembler) {
TNode<ExternalReference> pending_message = ExternalConstant(
ExternalReference::address_of_pending_message(isolate()));
TNode<HeapObject> previous_message =
UncheckedCast<HeapObject>(LoadFullTagged(pending_message));
TNode<Object> new_message = GetAccumulator();
StoreFullTaggedNoWriteBarrier(pending_message, new_message);
TNode<HeapObject> previous_message = GetPendingMessage();
SetPendingMessage(CAST(GetAccumulator()));
SetAccumulator(previous_message);
Dispatch();
}

View File

@ -78,6 +78,12 @@ RUNTIME_FUNCTION(Runtime_ReThrow) {
return isolate->ReThrow(args[0]);
}
RUNTIME_FUNCTION(Runtime_ReThrowWithMessage) {
HandleScope scope(isolate);
DCHECK_EQ(2, args.length());
return isolate->ReThrow(args[0], args[1]);
}
RUNTIME_FUNCTION(Runtime_ThrowStackOverflow) {
SealHandleScope shs(isolate);
DCHECK_LE(0, args.length());

View File

@ -112,6 +112,7 @@ bool Runtime::NeedsExactContext(FunctionId id) {
case Runtime::kLoadPrivateGetter:
case Runtime::kLoadPrivateSetter:
case Runtime::kReThrow:
case Runtime::kReThrowWithMessage:
case Runtime::kThrow:
case Runtime::kThrowApplyNonFunction:
case Runtime::kThrowCalledNonCallable:
@ -154,6 +155,7 @@ bool Runtime::IsNonReturning(FunctionId id) {
case Runtime::kThrowSuperAlreadyCalledError:
case Runtime::kThrowSuperNotCalled:
case Runtime::kReThrow:
case Runtime::kReThrowWithMessage:
case Runtime::kThrow:
case Runtime::kThrowApplyNonFunction:
case Runtime::kThrowCalledNonCallable:

View File

@ -234,6 +234,7 @@ namespace internal {
F(PromoteScheduledException, 0, 1) \
F(ReportMessageFromMicrotask, 1, 1) \
F(ReThrow, 1, 1) \
F(ReThrowWithMessage, 2, 1) \
F(RunMicrotaskCallback, 2, 1) \
F(PerformMicrotaskCheckpoint, 0, 1) \
F(StackGuard, 0, 1) \

View File

@ -3421,12 +3421,21 @@ void ImplementationVisitor::GenerateCatchBlock(
if (catch_block) {
base::Optional<Binding<LocalLabel>*> catch_handler =
TryLookupLabel(kCatchLabelName);
// Reset the local scopes to prevent the macro calls below from using the
// current catch handler.
BindingsManagersScope bindings_managers_scope;
if (assembler().CurrentBlockIsComplete()) {
assembler().Bind(*catch_block);
assembler().Goto((*catch_handler)->block, 1);
GenerateCall(QualifiedName({TORQUE_INTERNAL_NAMESPACE_STRING},
"GetAndResetPendingMessage"),
Arguments{{}, {}}, {}, false);
assembler().Goto((*catch_handler)->block, 2);
} else {
CfgAssemblerScopedTemporaryBlock temp(&assembler(), *catch_block);
assembler().Goto((*catch_handler)->block, 1);
GenerateCall(QualifiedName({TORQUE_INTERNAL_NAMESPACE_STRING},
"GetAndResetPendingMessage"),
Arguments{{}, {}}, {}, false);
assembler().Goto((*catch_handler)->block, 2);
}
}
}

View File

@ -1740,16 +1740,32 @@ base::Optional<ParseResult> MakeLabelBlock(ParseResultIterator* child_results) {
}
base::Optional<ParseResult> MakeCatchBlock(ParseResultIterator* child_results) {
auto variable = child_results->NextAs<std::string>();
auto parameter_names = child_results->NextAs<std::vector<std::string>>();
auto body = child_results->NextAs<Statement*>();
if (!IsLowerCamelCase(variable)) {
NamingConventionError("Exception", variable, "lowerCamelCase");
for (const std::string& variable : parameter_names) {
if (!IsLowerCamelCase(variable)) {
NamingConventionError("Exception", variable, "lowerCamelCase");
}
}
if (parameter_names.size() != 2) {
ReportError(
"A catch clause needs to have exactly two parameters: The exception "
"and the message. How about: \"catch (exception, message) { ...\".");
}
ParameterList parameters;
parameters.names.push_back(MakeNode<Identifier>(variable));
parameters.names.push_back(MakeNode<Identifier>(parameter_names[0]));
parameters.types.push_back(MakeNode<BasicTypeExpression>(
std::vector<std::string>{}, MakeNode<Identifier>("JSAny"),
std::vector<TypeExpression*>{}));
parameters.names.push_back(MakeNode<Identifier>(parameter_names[1]));
parameters.types.push_back(MakeNode<UnionTypeExpression>(
MakeNode<BasicTypeExpression>(std::vector<std::string>{},
MakeNode<Identifier>("JSMessageObject"),
std::vector<TypeExpression*>{}),
MakeNode<BasicTypeExpression>(std::vector<std::string>{},
MakeNode<Identifier>("TheHole"),
std::vector<TypeExpression*>{})));
parameters.has_varargs = false;
TryHandler* result = MakeNode<TryHandler>(
TryHandler::HandlerKind::kCatch, MakeNode<Identifier>(kCatchLabelName),
@ -2573,7 +2589,8 @@ struct TorqueGrammar : Grammar {
Rule({Token("label"), &name,
TryOrDefault<ParameterList>(&parameterListNoVararg), &block},
MakeLabelBlock),
Rule({Token("catch"), Token("("), &identifier, Token(")"), &block},
Rule({Token("catch"), Token("("),
List<std::string>(&identifier, Token(",")), Token(")"), &block},
MakeCatchBlock)};
// Result: ExpressionWithSource

View File

@ -0,0 +1,10 @@
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
Object.prototype.return = function () {
try {
throw "";
} catch(e) {}
};
Object.fromEntries([0]);

View File

@ -0,0 +1,6 @@
*%(basename)s:10: TypeError: Iterator value 0 is not an entry object
Object.fromEntries([0]);
^
TypeError: Iterator value 0 is not an entry object
at Function.fromEntries (<anonymous>)
at *%(basename)s:10:8

View File

@ -637,7 +637,7 @@ macro TestCatch1(implicit context: Context)(): Smi {
let r: Smi = 0;
try {
ThrowTypeError(MessageTemplate::kInvalidArrayLength);
} catch (_e) {
} catch (_e, _message) {
r = 1;
return r;
}
@ -653,7 +653,7 @@ macro TestCatch2(implicit context: Context)(): Smi {
let r: Smi = 0;
try {
TestCatch2Wrapper();
} catch (_e) {
} catch (_e, _message) {
r = 2;
return r;
}
@ -670,7 +670,7 @@ macro TestCatch3(implicit context: Context)(): Smi {
let r: Smi = 0;
try {
TestCatch3WrapperWithLabel() otherwise Abort;
} catch (_e) {
} catch (_e, _message) {
r = 2;
return r;
} label Abort {

View File

@ -834,7 +834,7 @@ TEST(Torque, CatchFirstHandler) {
macro Test(): void {
try {
} label Foo {
} catch (e) {}
} catch (_e, _m) {}
}
)",
HasSubstr(