New issue
Advanced search Search tips

Issue 670651 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

V8 doesn't throw an exception at v8::Object::SetAccessor.

Project Member Reported by yukishiino@chromium.org, Dec 2 2016

Issue description

Chrome Version: ToT
OS: All

V8 has a contract to throw an exception iff a V8 API returns an empty Maybe/MaybeLocal.  However, v8::Object::SetAccessor returns a Nothing without throwing an exception.  It's a violation of the contract between V8 and embedders.

https://cs.chromium.org/chromium/src/v8/src/api.cc?rcl=0&l=4570
https://cs.chromium.org/chromium/src/v8/src/api.cc?rcl=0&l=4576

 
Cc: yangguo@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>API
Owner: jochen@chromium.org
Status: Assigned (was: Untriaged)
can you give an example of how to trigger this?
Cc: jochen@chromium.org
Labels: Needs-Feedback
Owner: yukishiino@chromium.org
Owner: jochen@chromium.org
Define a property with {configurable: false}, then call the Maybe version of v8::Object::SetAccessor with the same property name.  The resulting Maybe is Nothing, but no exception is thrown.

The attached file is based on V8's samples/hello-world.cc with a little modification.  I can reproduce the case with this .cc file.
object-setaccessor-repro-case.cc
2.0 KB View Download
Labels: -Needs-Feedback
To be clear, you'll see the following on console.

Meybe::IsNothing = 1
TryCatch::HadException = 0

Owner: yukishiino@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d014b47fe595b1e119b6c177df0b3234bbc0cf24

commit d014b47fe595b1e119b6c177df0b3234bbc0cf24
Author: yukishiino <yukishiino@chromium.org>
Date: Fri Feb 10 05:56:00 2017

Fixes Object::SetAccessor to return false if the prop is unconfigurable.

http://www.ecma-international.org/ecma-262/7.0/#sec-validateandapplypropertydescriptor
says that [[DefineProperty]] should return false if the property is
already defined and it's unconfigurable (exactly speaking, the condition
in the spec is more complicated, but roughly speaking, it's when the
property is unconfigurable).

BUG= chromium:670651 

Review-Url: https://codereview.chromium.org/2680353004
Cr-Commit-Position: refs/heads/master@{#43080}

[modify] https://crrev.com/d014b47fe595b1e119b6c177df0b3234bbc0cf24/src/api.cc
[modify] https://crrev.com/d014b47fe595b1e119b6c177df0b3234bbc0cf24/test/cctest/test-api.cc
[modify] https://crrev.com/d014b47fe595b1e119b6c177df0b3234bbc0cf24/test/unittests/BUILD.gn
[add] https://crrev.com/d014b47fe595b1e119b6c177df0b3234bbc0cf24/test/unittests/api/v8-object-unittest.cc
[modify] https://crrev.com/d014b47fe595b1e119b6c177df0b3234bbc0cf24/test/unittests/unittests.gyp

Status: Fixed (was: Assigned)

Sign in to add a comment