Repository navigation
vm: next branch (V8 4.3) has test failures #1774
Description
Activity
@jeisinger can you think of anything that might have changed between 4.2 and 4.3 regarding ObjectTemplate::SetNamedPropertyHandler and friends that would change a
foo.bar = 5; ^ ReferenceError: foo is not definedto a
foo.bar = 5; ^ TypeError: Cannot set property 'bar' of undefined?
Barest-bones repro on top of contextify:
const binding = process.binding('contextify'); const context = {}; binding.makeContext(context); const script = new binding.ContextifyScript('foo;'); var foo = script.runInContext(context); // should throw an exception console.log(foo); // logs undefined
My guess is that something in the Local -> MaybeLocal transition made it so that errors get swallowed unless you're more careful about things?
Another repro:
const binding = process.binding('contextify'); const context = {}; binding.makeContext(context); const script = new binding.ContextifyScript('"asdfasdfdf" in this;'); console.log(script.runInContext(context)); // logs true, not falseThis implies it's less about exceptions and more about screwing up the callbacks to somehow always signal something is present/undefined.
OK, I can fix the second test case, by ensuring GlobalPropertyQueryCallback returns an empty handle if there is no property, instead of returning None. (This appears to be a V8 change.)
The first test case (and the one in the OP) is less tractable. I am debugging through GlobalPropertyGetterCallback and it seems like while the sandbox gives back an empty handle, the proxy global does not. I wonder if there was some change in V8 semantics such that global objects for contexts now return undefined instead of empty handles when calling GetRealNamedProperty ??
At this point I am 90% sure this is a V8 bug caused by this commit v8/v8@6130b02
The new code for GetRealNamedProperty is as follows:
MaybeLocal<Value> v8::Object::GetRealNamedProperty(Local<Context> context, Local<Name> key) { PREPARE_FOR_EXECUTION( context, "v8::Object::GetRealNamedPropertyInPrototypeChain()", Value); auto self = Utils::OpenHandle(this); auto key_obj = Utils::OpenHandle(*key); i::LookupIterator it(self, key_obj, i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR); if (!it.IsFound()) return MaybeLocal<Value>(); Local<Value> result; has_pending_exception = !ToLocal<Value>(i::Object::GetProperty(&it), &result); RETURN_ON_FAILED_EXECUTION(Value); RETURN_ESCAPED(result); }
The line
if (!it.IsFound()) return MaybeLocal<Value>();is incorrect, becauseitis still in its initial state. It should be after the call toi::Object::GetProperty(&it).Reported at https://code.google.com/p/v8/issues/detail?id=4143. If someone else wants to give a patch a try, that'd be great, as I already took much of today to work on io.js and I should go back to Chrome soon.
It has just been fixed in master v8/v8@7b24219
I'll merge that soon to 4.4 and 4.3
See #1632 for details. Creating this to track.
What I've tried so far (no success):