New issue
Advanced search Search tips

Issue 668414 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 668413



Sign in to add a comment

Difference between x64 and ia32: array concat

Project Member Reported by machenb...@chromium.org, Nov 24 2016

Issue description

Array.prototype[Symbol.isConcatSpreadable] = false;
print([].concat([-1073741825]));


# Compared x64 default with ia32 default

# Flags of x64:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging  --random-seed -1660780159
# Flags of ia32:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging  --random-seed -1660780159

Difference:
- ,-1073741825
+ -1073741825

### Start of configuration x64:
,-1073741825

### End of configuration x64

### Start of configuration ia32:
-1073741825

### End of configuration ia32

 
Cc: cbruni@chromium.org jkummerow@chromium.org
Status: Available (was: Untriaged)
The fast case hidden in {Slow_ArrayConcat} ignores the "concat spreadble protector". This only happens when the number in the first argument is a double-number, hence the decision differs between 32-bit and 64-bit architectures, but the bug is there in all architectures.

Comment 2 by cbruni@chromium.org, Nov 24 2016

Owner: cbruni@chromium.org
Status: Started (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 28 2016

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

commit a09e5eda26d9cfe4c71d42b34bc1944c527e60bf
Author: cbruni <cbruni@chromium.org>
Date: Mon Nov 28 10:06:07 2016

[runtime] Add missing @@IsConcatSpreadable check for FAST_DOUBLE_ELEMENTS

A missing @@IsConcatSpreadable check caused the fast path inside the slow path
to be incorrect and follow the default concat strategy when the arguments
arrays contain only doubles.

BUG= chromium:668414 

Review-Url: https://codereview.chromium.org/2527173002
Cr-Commit-Position: refs/heads/master@{#41301}

[modify] https://crrev.com/a09e5eda26d9cfe4c71d42b34bc1944c527e60bf/src/builtins/builtins-array.cc
[add] https://crrev.com/a09e5eda26d9cfe4c71d42b34bc1944c527e60bf/test/mjsunit/regress/regress-crbug-668414.js

Comment 4 by cbruni@chromium.org, Nov 29 2016

Status: Fixed (was: Started)
Woot! Thanks for taking this over Camillo, much appreciated!
Labels: v8-foozzie-failure

Sign in to add a comment