New issue
Advanced search Search tips

Issue 894864 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

V8 correctness failure in suppression: crbug.com/664068

Project Member Reported by ClusterFuzz, Oct 12

Issue description

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

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  suppression:  crbug.com/664068 
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=51672:51673

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Oct 12

Labels: Test-Predator-Auto-Owner
Owner: jkummerow@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/7c79a9fd1e35d5272b5dbe57af92c7e0d2120ca3 ([bigint] Stage BigInts).

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.
Cc: machenb...@chromium.org
Labels: -Pri-1 -Stability-Crash Pri-3
+machenbach

Do I understand correctly that the issue here is that ia32 and x64 builds produced different stdout? If so, I can confirm:

$ out/x64.release/d8 -e "new BigInt64Array(1073741824)"
unnamed:1: RangeError: Array buffer allocation failed
new BigInt64Array(1073741824)
^
RangeError: Array buffer allocation failed
    at new ArrayBuffer (<anonymous>)
    at new BigInt64Array (<anonymous>)
    at unnamed:1:1

$ out/ia32.release/d8 -e "new BigInt64Array(1073741824)"
unnamed:1: RangeError: Invalid typed array length: 1073741824
new BigInt64Array(1073741824)
^
RangeError: Invalid typed array length: 1073741824
    at new BigInt64Array (<anonymous>)
    at unnamed:1:1

This is due to there being different limits on typed array length (must fit into a Smi, which is 32/64 bit specific) and underlying array buffer byte length. Depending on which limit you run into, the RangeError has a different message. It probably doesn't make sense to adjust the limits or generated messages.

FWIW, with Float64Array or Int32Array (which we've had since forever) instead of BigInt64Array (which we've been shipping since March) the behavior is exactly the same as with BigInt64Array. I guess ClusterFuzz simply hasn't discovered variants of the testcase with other typed array kinds yet.

What do we do here? Does the regexp added here: https://chromium-review.googlesource.com/c/v8/v8/+/446363 need to include "Invalid typed array length" as well? What does "failure in suppression" even mean -- is this suppressed already or not? Why does ClusterFuzz file bugs for things that are already suppressed?

Please advise.
Ah, do we have to add BigInt64Array and BigUint64Array to tools/clusterfuzz/v8_mock_archs.js ?
Re 3: Yes, these types should be added to the mock file.
Re 2: Clusterfuzz dedupes all crash states named 'suppression:  crbug.com/664068 ' on the first test case it ever found with that state. I believe that the suppression regexp became obsolete (or remains as a second line of defense) after we added the v8_mock_archs.js logic. After that, clusterfuzz must have believed all old cases as fixed. Once the new types are added to the mock file I think clusterfuzz will auto-close this issue as well.

In general I don't think we should do anything to patch these architectural differences better (e.g. on V8 source side).
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 17

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

commit 109fec8ce07088aabf258278cbddf6e14e8c0e1f
Author: Jakob Kummerow <jkummerow@chromium.org>
Date: Wed Oct 17 06:24:06 2018

[foozzie] Add Big*64Array to mock file

To prevent correctness fuzzers from finding spurious differences
between architectures, we need to mock out the maximum length of
all TypedArrays. This patch adds the two new types BigInt64Array
and BigUint64Array to the existing list.

Bug:  chromium:894864 
Change-Id: I5cdeeafa597b09aee2d9b4d368c07f10008baf58
Reviewed-on: https://chromium-review.googlesource.com/c/1285399
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56710}
[modify] https://crrev.com/109fec8ce07088aabf258278cbddf6e14e8c0e1f/tools/clusterfuzz/v8_mock_archs.js

Project Member

Comment 6 by ClusterFuzz, Oct 17

ClusterFuzz has detected this issue as fixed in range 56709:56710.

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

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  suppression:  crbug.com/664068 
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=51672:51673
Fixed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=56709:56710

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

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.
Project Member

Comment 7 by ClusterFuzz, Oct 17

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 6341571520495616 is verified as fixed, so closing issue as verified.

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

Sign in to add a comment