New issue
Advanced search Search tips

Issue 910962 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

V8 correctness failure in configs: x64,ignition_turbo:ia32,ignition_turbo

Project Member Reported by ClusterFuzz, Dec 2

Issue description

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

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition_turbo:ia32,ignition_turbo
  sources: 5a5
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=49944:49945

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Dec 2

Labels: Test-Predator-Auto-Owner
Owner: bbudge@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/5679ab102df028a67916a736802943e232df12fd (Reland "[D8] Clean up ArrayBuffer Allocators in shell.").

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.
Looks like some x64/ia32 difference of something based on kMaxPointerLength? Maybe needs to honor the flag abort_on_stack_or_string_length_overflow?
Reduced repro:
try {
  var __v_11 = new Int32Array("1073741824");
} catch (e) {
  print("Caught: " + e.message)
}

Output:
### Start of configuration x64,ignition_turbo:
Caught: Array buffer allocation failed

### End of configuration x64,ignition_turbo
#
### Start of configuration ia32,ignition_turbo:
Caught: Invalid typed array length: 1073741824

### End of configuration ia32,ignition_turbo

Looks like ia32 and x64 handle it differently when the parameter passed to a typed array is a string. We maybe need to just monkey patch this as well as we did here?
https://cs.chromium.org/chromium/src/v8/tools/clusterfuzz/v8_mock_archs.js?q=mock_arch&sq=package:chromium&g=0&l=19

Cc: bbudge@chromium.org
Labels: -Pri-1 Pri-2
Owner: machenb...@chromium.org
I'll take this to investigate a better stub.
Cc: petermarshall@chromium.org
ia32 and x64 have different limits here - the length argument as a string bypasses the max length enforcement in v8_mock_archs.js. You could add string parsing/checking in there too. The constructor calls ToIndex (--> ToInteger --> ToNumber) on the length argument so you'd theoretically need to handle any string that can be converted to a number using https://tc39.github.io/ecma262/#sec-tonumber-applied-to-the-string-type

In practice you could probably do something simpler or check if the argument is an object and then call Number(args[0]) - this is basically what the typed array constructor does.

e.g. if (!(typeof args[0] === "object")) ... if (Number(args[0]) > ...)
Yea, sg. Will patch the mock.
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 11

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

commit fdcaa3d45216ea1cfb76423286caa498b60bc13e
Author: Michael Achenbach <machenbach@chromium.org>
Date: Tue Dec 11 09:14:40 2018

[foozzie] Properly stub out typed array constructor

When using correctness fuzzing, this makes sure all non-object
arguments to typed array constructors are bound by 1MiB when
interpreted as numbers.

NOTRY=true

Bug:  chromium:910962 
Change-Id: I66e87ece27aae7c5fa88429c5d1f1f478de702ae
Reviewed-on: https://chromium-review.googlesource.com/c/1369959
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58144}
[modify] https://crrev.com/fdcaa3d45216ea1cfb76423286caa498b60bc13e/tools/clusterfuzz/v8_mock_archs.js

Project Member

Comment 9 by ClusterFuzz, Dec 11

ClusterFuzz has detected this issue as fixed in range 58143:58144.

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

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition_turbo:ia32,ignition_turbo
  sources: 5a5
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=49944:49945
Fixed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=58143:58144

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

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 10 by ClusterFuzz, Dec 11

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5232262297419776 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