Closed Bug 1681677 Opened 4 years ago Closed 4 years ago

Incorrect behavior on Array.slice with WARP on in Vue + DecimalJs setup

Categories

(Core :: JavaScript Engine: JIT, defect, P2)

Firefox 83
defect

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
firefox85 --- fixed

People

(Reporter: wc3-enter, Assigned: iain)

Details

Attachments

(2 files)

Attached file test project

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.111 Safari/537.36

Steps to reproduce:

Launch attached index.html
Press 'GO' button

Actual results:

An error message in console.

Expected results:

No error message, data is processed correctly.

Problem is with Array.slice method, when WARP is turned on.

Problem consists of two parts:

  1. Vue adds reactivity to DecimalJS's object (the 'd' property - it is an array);
  2. On Decimal's constructor the 'slice' method is frequently used like this:

x.d = y.d.slice()

Vue adds a modification to Array's prototype chain like this:

Client code -> ObservedArray(aka mutator) by Vue -> Array.prototype

At start (while there is no 'mutator' object) everything is ok, and

y.d.slice() returns an Array as expected.

But when Vue takes part during calculation process, Array.slice suddenly starts to return an object, inherited from 'mutator' prototype. At this point, code brakes.

I want to make it bold: this occurs only with WARP on, if I switch it off evrything is ok. I tried to replicate this behavior using pure JS, but as of now failed. Obviously, it is important to have an overhead for the bug to show up.

We have no problem in Chrome(ium).

I admit, that Vue injection is not something that is correct, but our project setup is not something nobody does =)

In case you need any additional info or comments or data or whatever, please, let me know! Thanks!

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → JavaScript Engine: JIT
Product: Firefox → Core

Simplified test case:

var a = [1];
var p = {__proto__: Array.prototype};
Object.setPrototypeOf(a, p);
for (var i = 0; i < 100_000; ++i) {
  var x = a.slice(0);
  assertEq(x.__proto__, Array.prototype);
}

CodeGenerator::visitArraySlice copies over the group, which implies the prototype. This looks like an old bug, it should also be reproducible with Ion.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Iain, can you look at this issue?
This sounds like an issue which fix might be backported to release/beta.

Severity: -- → S2
Flags: needinfo?(iireland)
Priority: -- → P2

Looks like anba did all the heavy lifting here. Thanks!

I haven't managed to replicate the same failure in Ion. inlineArraySlice doesn't inline the call. I think the ultimate reason is this code.

I believe the fix is just to remove the group-copying code in visitArraySlice. With TI gone, there's no reason for it.

Flags: needinfo?(iireland)
Assignee: nobody → iireland
Status: NEW → ASSIGNED
Pushed by iireland@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c484aab51bf2 Don't copy group in visitArraySlice r=jandem
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: