New issue
Advanced search Search tips

Issue 737645 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

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

Project Member Reported by ClusterFuzz, Jun 28 2017

Issue description

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

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: f97
  
Sanitizer: address (ASAN)

Regressed: V8: 46103:46104

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


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: cbruni@chromium.org
Owner: verwa...@chromium.org
Status: Assigned (was: Untriaged)
// Bisects to https://chromium-review.googlesource.com/c/542895/
// Maybe I have too little JS foo, but I don't really understand either of the two versions, ia32 and x64. PTAL Toon.

// Repro:

a = { length: 12, 1: 58, 1073741824: 16 };
Array.prototype.sort.call(a);
Array.prototype.sort.call(a);
print(Object.keys(a).map(function(key) {print(key)}))

// Output:

# Compared x64,ignition with ia32,ignition
#
# Flags of x64,ignition:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 123 --ignition --turbo-filter=~ --hydrogen-filter=~ --noopt
# Flags of ia32,ignition:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 123 --ignition --turbo-filter=~ --hydrogen-filter=~ --noopt
#
# Difference:
- 1
+ 1073741824
#
### Start of configuration x64,ignition:
0
1
length
,,

### End of configuration x64,ignition
#
### Start of configuration ia32,ignition:
0
1073741824
length
,,

### End of configuration ia32,ignition

Even before Toon's commit, I find the behavior surprising. E.g.:

a = { length: 12, 1: 58, 1073741824: 16 };
print(Object.keys(a))
Array.prototype.sort.call(a);
print(Object.keys(a))

outputs:
1,1073741824,length
0,1073741824,length

Where is the 0 coming from?
Cc: verwa...@chromium.org
Owner: cbruni@chromium.org
Status: Started (was: Assigned)
Sorting is rather confusing in this case :)

1. Array.prototype.sort only looks at elements < length (in this case 12)
2. Any element bigger than that is left untouched
3. Holes are ignored

So in this case the only valid index to look at is 1, which when sorted get's pushed to the front (index = 0) while the large index is left untouched.
Hence the correct result should be: {length: 12, 0:58, 1073741824:16}
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 6 2017

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

commit 78c74e68f7d195fcc28451b7d170931849aaecb1
Author: Camillo Bruni <cbruni@chromium.org>
Date: Thu Jul 06 10:45:52 2017

[runtime] Fix Array.prototype.sort for large entries

Bug:  chromium:737645 
Cq-Include-Trybots: master.tryserver.v8:v8_linux_noi18n_rel_ng
Change-Id: Ib02b3082cec82dfbbc48b21609dde7499e87042e
Reviewed-on: https://chromium-review.googlesource.com/558868
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46438}
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/builtins/builtins-intl.cc
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/elements.cc
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/elements.h
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/factory.cc
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/objects-inl.h
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/objects-printer.cc
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/objects.cc
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/objects.h
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/runtime/runtime-array.cc
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/runtime/runtime-intl.cc
[modify] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/src/runtime/runtime-object.cc
[add] https://crrev.com/78c74e68f7d195fcc28451b7d170931849aaecb1/test/mjsunit/regress/regress-crbug-737645.js

Project Member

Comment 6 by ClusterFuzz, Jul 7 2017

ClusterFuzz has detected this issue as fixed in range 46437:46438.

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

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: f97
  
Sanitizer: address (ASAN)

Regressed: V8: 46103:46104
Fixed: V8: 46437:46438

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


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Status: Fixed (was: Started)

Sign in to add a comment