Incorrect MOP operation sequences in builtins

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: anba, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
Incorrect MOP operation sequences are visible through Proxies, builtins should be audited to ensure they work as specified. 

For example Object.seal(), Array.prototype.pop(), Array.prototype.join() and Array.prototype.reverse() don't work as specified, to name just a few. A simple test script to log MOP operation sequences is available at https://gist.github.com/anba/fc8e71ed88f446663b0b .
Blocks: 694100
Doesn't seem like most of those examples in https://gist.github.com/anba/fc8e71ed88f446663b0b are still correct in the current ES6 version. But I don't doubt that we still have wrong behavior for some builtins.
So I think the only problem that is left here that we invoke [[HasProperty]] in Array.prototype.join.  André could you confirm?
Flags: needinfo?(andrebargull)
(Reporter)

Comment 3

3 years ago
(In reply to Tom Schuster [:evilpie] from comment #2)
> So I think the only problem that is left here that we invoke [[HasProperty]]
> in Array.prototype.join.  André could you confirm?

Yes, that's the only one left from the list in comment #0. But there are still other functions which need to be updated (I've only checked jsarray.cpp, builtin/Array.js, builtin/Object.cpp and builtin/Object.js):

- Object.create and Object.defineProperties (both use js::DefineProperties(...)): 

log = [];
Object.create(null, new Proxy({a: {value: 0}, b: {value: 1}}, new Proxy({}, {get(t,pk){log.push(pk)}}))); 
log.join();

Expected: "ownKeys,getOwnPropertyDescriptor,get,getOwnPropertyDescriptor,get"
Actual: "ownKeys,getOwnPropertyDescriptor,getOwnPropertyDescriptor,get,get"


log = [];
Object.defineProperties({}, new Proxy({a: {value: 0}, b: {value: 1}}, new Proxy({}, {get(t,pk){log.push(pk)}})));
log.join()

Expected: "ownKeys,getOwnPropertyDescriptor,get,getOwnPropertyDescriptor,get"
Actual: "ownKeys,getOwnPropertyDescriptor,getOwnPropertyDescriptor,get,get"


- Array.prototype.shift

log = [];
Array.prototype.shift.call(new Proxy([,], new Proxy({}, {get(t,pk){log.push(pk)}})));
log.join()

Expected: "get,get,deleteProperty,set,getOwnPropertyDescriptor,defineProperty"
Actual: "get,has,deleteProperty,set,getOwnPropertyDescriptor,defineProperty"
Flags: needinfo?(andrebargull)
(Reporter)

Comment 4

3 years ago
I didn't find any further built-ins method which need to be updated except the ones reported in comment #3.
(Reporter)

Updated

2 years ago
Duplicate of this bug: 1216640
Depends on: 1308482
Assignee: nobody → evilpies
Assignee: evilpies → nobody
(Reporter)

Updated

a year ago
Depends on: 1323825
We fixed all known issues here.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.