Undefined values in objects cause ReferenceErrors through various operations

[KevOrr]

Related: #1691

Investigating this a bit more, @djeis97 had an idea that this might be a bigger problem. Indeed, the following expression incorrectly raises a ReferenceError:

[undefined][0]

This is because [undefined] is a temporary value, so it has no references. jspeFactorFunctionCall correctly evaluates this expression to a JsVar NAME whose varData is the integer 0, and whose firstChild is undefined ([r1,l1] Name Integer 0 undefined). But at the end of that function, it unlocks the parent value, which in this case is the array [undefined].

When jsvUnLock unlocks this array, the array's lock count is 0, so its ref count becomes 0, so it is freed. This in turn decrements our name var's refcount to 0 ([r0,l1] Name Integer 0 undefined).

Later, jspParse calls jsvCheckReferenceError, which sees [r0,l1] Name Integer 0 undefined and treats it as a ReferenceError.

Fundamentally, this seems to be an issue because [r0,l_] Name _ undefined are used both for indicating a ReferenceError, and for representing the reference obtained by indexing into an object with no references itself.

Other examples of this error:

x = [undefined]
x.pop()

({a: undefined})['a']

cc @kiranshila

[gfwilliams]

Thanks for this - I was a bit worried about this myself but the one-liner for #1691 seemed too good to pass up on :)

x = [undefined]
x.pop()

This one especially looks big and is something I'm very surprised hasn't been found before.

It seems like the solution may be to add a new variable type that causes a referenceerror when accessed, and to then use that in jspGetNamedVariable if the variable is not found. It's not ideal as it causes another allocation every time a new variable is written to, but I guess that is actually reasonable rare in well-written code anyway.

[KevOrr]

I think fixing specifically the arguments case solved the majority of real cases of this bug though, so thanks for that fix.

Btw the way ES handles this is by having the named reference be a pair of name and parent value, instead of name and child value. This way, the parent can't be freed until the reference is forced to a value. But the approach of introducing a new kind of var for ReferenceErrors sounds easier to integrate with existing code.

What about just calling jsExceptionHere at the site where a reference error would be created, instead of using jsvCheckReferenceError?

[gfwilliams]

What about just calling jsExceptionHere at the site where a reference error would be created

I'm not sure I understand? Because of the parsing, you get an ID, for instance foo, and you don't know if it's from: print(foo) or print(foo=3) when you hit it. It's not as easy as just creating the error as soon as you come across the ID

[KevOrr]

Ah, that makes sense. Thanks for explaining

[gfwilliams]

Also seen with g.theme: http://forum.espruino.com/conversations/367936/#comment16178913

[gfwilliams]

Looks like this got mostly fixed.

({a: undefined})['a']

Would seem to still be a problem though