Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/b51053d89ea340bda60baacfa5b654facf2c4ae8 (Reland: [builtins] Implement Array.prototype.slice in Torque).

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.

 Issue 899488  has been merged into this issue.

Reduced repro:

function __f_1(val) {
  this.value = val;
}
__v_1 = new Array();
function __f_2() {
  __v_1.splice(0, 1);
  return this.value.toString();
}
for (var __v_0 = 0; __v_0 < 7; __v_0++) {
  __v_1[__v_0] = new __f_1(__v_0);
  __v_1[__v_0].toString = __f_2;
}
__v_1.sort();
print(__v_1);

Looks like different sorting functions are used? Seems to depend on using the slow path. There seems to be a different side effect on the slow path when calling toString.

Had a quick look, this is related to a toString function with side-effects. The spec doesn't mandate how often the comparison function (and thus toString) is called, and the number of calls may differ on the slow/fast paths. AFAICT this is not a bug. 

Here's a deobfuscated repro:


function f(val) {
  this.value = val;
}

xs = new Array();

function g() {
  xs.splice(0, 1);                  // This modifies the array.
  return this.value.toString();
}

for (var i = 0; i < 7; i++) {
  xs[i] = new f(i);
  xs[i].toString = g;
}

xs.sort();

Alright, not a bug. But could we do something better to mock this out for correctness fuzzing? This case or similar cases might get detected in the future when comparing with the force-slow-path variant.

Not sure, did you have something in mind? 

A side-effectful toString or comparison function means that all bets are off and behavior may differ arbitrarily. Given that, what are our options?

No idea sadly. We can wait for more bugs like this to happen first. The ultimate hammer would be to completely stop comparisons between baseline and slow path.

Another possibility would be to ignore some results based on some (over-approximating) heuristics. Not sure what a good heuristics would be though. E.g. ignore all test cases using toString or using sort.

Until we find a solution, the two cases reported here will remain unfixed in clusterfuzz DB, and block two result hashes. Hence the issue should not just be closed. But we can keep it available to dedupe more things on it for now.

One solution might be to monkey patch the sort function in the fast/slow path comparison setting? I'd override sort in JS with my own function that should behave equally in all cases. We loose the coverage for the original sort in this setting.

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

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

ClusterFuzz has detected this issue as fixed in range 57854:57855.

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

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition:x64,slow_path
  sources: e5d
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=56828:56829
Fixed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=57854:57855

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

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 claims https://chromium-review.googlesource.com/c/v8/v8/+/1342929 fixed the issue. But I assume the general problem remains. Keeping this open.