Skip to content

deps: V8: cherry-pick e06ace6b5cdb #34673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
addaleax wants to merge 3 commits into from
Closed

deps: V8: cherry-pick e06ace6b5cdb #34673

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b0a451
deps: V8: cherry-pick e06ace6b5cdb
addaleax Aug 7, 2020
bd0ccf2
test: add vm crash regression test
addaleax Aug 7, 2020
37e4ec2
fixup! test: add vm crash regression test
addaleax Aug 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
deps: V8: cherry-pick e06ace6b5cdb 
Original commit message:

    [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes

    `Object::GetRealNamedPropertyAttributes()` can crash if an empty
    `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because
    it was not checking for that. Fix that.

    Refs: #34606
    Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#69258}

Refs: v8/v8@e06ace6
  • Loading branch information
@addaleax
addaleax committed Aug 7, 2020
commit 2b0a4514fbda7745f640f65e56c7b0752ebc0234
2 changes: 1 addition & 1 deletion common.gypi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.13',
'v8_embedder_string': '-node.14',

##### V8 defaults for Node.js #####

Expand Down
12 changes: 7 additions & 5 deletions deps/v8/src/api/api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4653,9 +4653,9 @@ Maybe<PropertyAttribute>
v8::Object::GetRealNamedPropertyAttributesInPrototypeChain(
Local<Context> context, Local<Name> key) {
auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
ENTER_V8_NO_SCRIPT(isolate, context, Object,
GetRealNamedPropertyAttributesInPrototypeChain,
Nothing<PropertyAttribute>(), i::HandleScope);
ENTER_V8(isolate, context, Object,
GetRealNamedPropertyAttributesInPrototypeChain,
Nothing<PropertyAttribute>(), i::HandleScope);
i::Handle<i::JSReceiver> self = Utils::OpenHandle(this);
if (!self->IsJSObject()) return Nothing<PropertyAttribute>();
i::Handle<i::Name> key_obj = Utils::OpenHandle(*key);
Expand All @@ -4668,6 +4668,7 @@ v8::Object::GetRealNamedPropertyAttributesInPrototypeChain(
i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
Maybe<i::PropertyAttributes> result =
i::JSReceiver::GetPropertyAttributes(&it);
has_pending_exception = result.IsNothing();
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(PropertyAttribute);
if (!it.IsFound()) return Nothing<PropertyAttribute>();
if (result.FromJust() == i::ABSENT) return Just(None);
Expand All @@ -4692,14 +4693,15 @@ MaybeLocal<Value> v8::Object::GetRealNamedProperty(Local<Context> context,
Maybe<PropertyAttribute> v8::Object::GetRealNamedPropertyAttributes(
Local<Context> context, Local<Name> key) {
auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
ENTER_V8_NO_SCRIPT(isolate, context, Object, GetRealNamedPropertyAttributes,
Nothing<PropertyAttribute>(), i::HandleScope);
ENTER_V8(isolate, context, Object, GetRealNamedPropertyAttributes,
Nothing<PropertyAttribute>(), i::HandleScope);
i::Handle<i::JSReceiver> self = Utils::OpenHandle(this);
i::Handle<i::Name> key_obj = Utils::OpenHandle(*key);
i::LookupIterator::Key lookup_key(isolate, key_obj);
i::LookupIterator it(isolate, self, lookup_key, self,
i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
auto result = i::JSReceiver::GetPropertyAttributes(&it);
has_pending_exception = result.IsNothing();
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(PropertyAttribute);
if (!it.IsFound()) return Nothing<PropertyAttribute>();
if (result.FromJust() == i::ABSENT) {
Expand Down
42 changes: 42 additions & 0 deletions deps/v8/test/cctest/test-api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11959,6 +11959,48 @@ THREADED_TEST(VariousGetPropertiesAndThrowingCallbacks) {
CHECK(result.IsEmpty());
}

THREADED_TEST(GetRealNamedPropertyAttributes_With_Proxy) {
LocalContext context;
HandleScope scope(context->GetIsolate());

{
Local<Object> proxy =
CompileRun(
"new Proxy({ p: 1 }, { getOwnPropertyDescriptor: _ => { "
" throw new Error('xyz'); } });")
.As<Object>();
TryCatch try_catch(context->GetIsolate());
v8::Maybe<v8::PropertyAttribute> result =
proxy->GetRealNamedPropertyAttributes(context.local(), v8_str("p"));
CHECK(result.IsNothing());
CHECK(try_catch.HasCaught());
CHECK(try_catch.Exception()
.As<Object>()
->Get(context.local(), v8_str("message"))
.ToLocalChecked()
->StrictEquals(v8_str("xyz")));
}

{
Local<Object> proxy =
CompileRun(
"Object.create("
" new Proxy({ p: 1 }, { getOwnPropertyDescriptor: _ => { "
" throw new Error('abc'); } }))")
.As<Object>();
TryCatch try_catch(context->GetIsolate());
v8::Maybe<v8::PropertyAttribute> result =
proxy->GetRealNamedPropertyAttributesInPrototypeChain(context.local(),
v8_str("p"));
CHECK(result.IsNothing());
CHECK(try_catch.HasCaught());
CHECK(try_catch.Exception()
.As<Object>()
->Get(context.local(), v8_str("message"))
.ToLocalChecked()
->StrictEquals(v8_str("abc")));
}
}

static void ThrowingCallbackWithTryCatch(
const v8::FunctionCallbackInfo<v8::Value>& args) {
Expand Down