graaljs icon indicating copy to clipboard operation
graaljs copied to clipboard

[Bug]: `Symbol.toPrimitive` in property acess

Open p51lee opened this issue 2 years ago • 1 comments

// input.js
null [ { [ Symbol . toPrimitive ] : () => { REF_ERR; } } ] ;

Hello,

Running input.js should throw ReferenceError. However, running it using GraalJS throws TypeError:

$ js --version
GraalVM JavaScript (GraalVM CE Native 22.2.0)
$ js input.js
TypeError: Cannot convert undefined or null to object: null
	at <js> :program(input.js:1:0-57)

According to ECMAScript 2022 spec section 13.3.2.1, EvaluatePropertyAccessWithIdentifierKey is called in line 4, where baseValue is null and Expression is { [ Symbol . toPrimitive ] : () => { REF_ERR; } }:

image

In line 3 of section 13.3.3, ToPropertyKey is called, where propertyNameValue is an evaluated value of expression( i.e. { [ Symbol . toPrimitive ] : () => { REF_ERR; } }). By the way, TypeError caused by reading a property of null can be thrown after line 4:

image

Then ToPrimitive in the first line of section 7.1.19 is executed, with argument { [ Symbol . toPrimitive ] : () => { REF_ERR; } }:

image

Inside the function ToPrimitive, now input is { [ Symbol . toPrimitive ] : () => { REF_ERR; } } so exoticToPrim in line 1-a becomes () => { REF_ERR; }. Finally in line 1-b-iv, Call ing exoticToPrim leads to ReferenceError since REF_ERR is not defined.

image

Interestingly, V8 has the same bug:

$ node input.js
input.js:1
null [ { [ Symbol . toPrimitive ] : () => { REF_ERR; } } ] ;
     ^

TypeError: Cannot read properties of null (reading '#<Object>')
    at Object.<anonymous> (input.js:1:6)
    at Module._compile (node:internal/modules/cjs/loader:1159:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
    at Module.load (node:internal/modules/cjs/loader:1037:32)
    at Module._load (node:internal/modules/cjs/loader:878:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.11.0

p51lee avatar Nov 10 '22 08:11 p51lee

You are right that we are not following the latest version of ECMAScript specification here. Unfortunately, this part of the specification is known for not matching the web reality (i.e. what the most popular JavaScript engines implement). Moreover, considering how base functionality this is, it is unlikely that the engines will change their behavior here singnificantly (as they would break some existing code very likely). So, the general direction here is not to push engines to implement what the specification says currently but to find the common intersection of the existing behavior of the most popular engines and to change the specification accordingly. Unfortunately, this process is very slow a buggy sometimes.

The behavior of the mentioned expression has changed recently due to some specification changes (there used to be RequireObjectCoercible(baseValue) in EvaluatePropertyAccessWithExpressionKey. I am not convinced that this particular consequence is expected/intentional. Anyway, it is a know discrepancy between the specification and web reality as you can see here.

iamstolis avatar Nov 10 '22 09:11 iamstolis