Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/0d4ed06cac780d71ce1bd6445351921a4fce7312 (Replace JS version of Array.of with a CSA version).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Simpler repro:

a = [1]; function f() { return a; }; b = Array.of.call(f); b.unshift(1);

Ross CCing you in case you think we should revert today. I'll work on it tomorrow otherwise.

Given this is a DCHECK, let's leave it till tomorrow and if you can't easily fix we can revert then.

Not sure if this is related, but there's some other changed behaviour:

Current v8 in chrome stable:
> a = [1]; function f() { return a; }; b = Array.of.call(f);
> b[1] = 2
> b
(2) [empty, 2]

d8 with the new Array.of
d8> a = [1]; function f() { return a; }; b = Array.of.call(f);
d8> b[1] = 2
d8> b
[1, 2]

Once more:

d8> a = [9,9,9,9]; function f() { return a; }; b = Array.of.call(f); b[7] = 1; b
[9, 9, 9, 9, , , , 1]

Previously V8 would have just inserted holes into the un-assigned parts, but now the array "returned" from the constructor is leaking into the new array.

I think the problem is actually in the way the length is set in the CSA implementation. Performing array.length = 0 followed by array.length = N > 0 should restore the original values, but that is what's happening here.

In this case, the constructor is actually returning a normal array and then we're calling: 

StoreObjectFieldNoWriteBarrier(array, JSArray::kLengthOffset, length);

Presumably this just sets the property without doing any of the other things it should do with an array.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d5dca89b6063ec65b8aae93f87973d5153438ab2

commit d5dca89b6063ec65b8aae93f87973d5153438ab2
Author: Dan Elphick <delphick@chromium.org>
Date: Tue Jan 23 21:59:16 2018

[builtins] Fix Array.of crashes by setting length correctly

Before we can set the length of the created array in CSA, first check
that it's possible and will do what we want. I.e. check
a) that the length is writable
b) the backing store is not copy-on-write and
c) the old length is not greater than the new length (as otherwise later
insertion past the end could restore values from the original
constructor).

If not then fall back on Runtime::kSetProperty.

Bug:  chromium:804177 
Change-Id: Id0e452f9d160704bbd71e87a075ba4e3983729a7
Reviewed-on: https://chromium-review.googlesource.com/880922
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50818}
[modify] https://crrev.com/d5dca89b6063ec65b8aae93f87973d5153438ab2/src/builtins/builtins-array-gen.cc
[add] https://crrev.com/d5dca89b6063ec65b8aae93f87973d5153438ab2/test/mjsunit/regress/regress-804177.js

ClusterFuzz has detected this issue as fixed in range 50817:50818.

Detailed report: https://clusterfuzz.com/testcase?key=6033054794776576

Fuzzer: ochang_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  map() != GetHeap()->fixed_cow_array_map() in fixed-array-inl.h
  v8::internal::FixedArray::set
  SetImpl
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=50679:50680
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=50817:50818

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6033054794776576

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 6033054794776576 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

+awhalley@ for M67 merge review. 

I'm confused by this. The change went into M66: https://chromiumdash-staging.googleplex.com/commit/d5dca89b6063ec65b8aae93f87973d5153438ab2

Why do we need to merge to M67?

Agree, delphick@. Thank you.
Cl listed #23 is already in M67 branch. Removing "Merge-Review-67" label.

awhalley@, sheriffbot is wrongly requested merge here at #15 and few other bugs too. Could you pls check?

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot