New issue
Advanced search Search tips

Issue 664506 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 661510



Sign in to add a comment

Difference between fullcode and ignition_staging: array toString and gc

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

Issue description

Flaky without --predictable. Very sensitive to the exact number of gc().

# Minimized program:
gc();
gc();
print({})
a = [];
gc();
print(Object.prototype.toString.call(a));

# Compared fullcode with ignition_turbo

# Flags of fullcode:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --predictable --random-seed -1109634722 --nocrankshaft --turbo-filter=~
# Flags of ignition_turbo:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --predictable --random-seed -1109634722 --ignition-staging --turbo

Difference:
- [object Null]
+ [object Array]

### Start of configuration fullcode:
[object Object]
[object Null]

### End of configuration fullcode

### Start of configuration ignition_turbo:
[object Object]
[object Array]

### End of configuration ignition_turbo

 
Cc: mythria@chromium.org leszeks@chromium.org
Owner: mstarzinger@chromium.org
Status: Assigned (was: Untriaged)
I have a fix in flight.
Project Member

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

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

commit 79aee39f24d6753ce6fbf36eb09f5fb63614e0d8
Author: mstarzinger <mstarzinger@chromium.org>
Date: Mon Nov 14 12:44:12 2016

[builtins] Fix pointer comparison in ToString builtin.

This fixes the bogus {Word32Equal} comparison in the ToString builtin
implementing Object.prototype.toString to be a pointer-size {WordEqual}
comparison instead. Comparing just the lower half-word is insufficient
on 64-bit architectures.

R=jgruber@chromium.org
TEST=mjsunit/regress/regress-crbug-664506
BUG= chromium:664506 

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

[modify] https://crrev.com/79aee39f24d6753ce6fbf36eb09f5fb63614e0d8/src/builtins/builtins-object.cc
[add] https://crrev.com/79aee39f24d6753ce6fbf36eb09f5fb63614e0d8/test/mjsunit/regress/regress-crbug-664506.js

Components: Blink>JavaScript
Status: Fixed (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 18 2016

Labels: merge-merged-5.6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/79aee39f24d6753ce6fbf36eb09f5fb63614e0d8

commit 79aee39f24d6753ce6fbf36eb09f5fb63614e0d8
Author: mstarzinger <mstarzinger@chromium.org>
Date: Mon Nov 14 12:44:12 2016

[builtins] Fix pointer comparison in ToString builtin.

This fixes the bogus {Word32Equal} comparison in the ToString builtin
implementing Object.prototype.toString to be a pointer-size {WordEqual}
comparison instead. Comparing just the lower half-word is insufficient
on 64-bit architectures.

R=jgruber@chromium.org
TEST=mjsunit/regress/regress-crbug-664506
BUG= chromium:664506 

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

[modify] https://crrev.com/79aee39f24d6753ce6fbf36eb09f5fb63614e0d8/src/builtins/builtins-object.cc
[add] https://crrev.com/79aee39f24d6753ce6fbf36eb09f5fb63614e0d8/test/mjsunit/regress/regress-crbug-664506.js

Cc: vogelheim@chromium.org
Why does this have the merge-merged-5.6 label? Which CL got merged?
OK - WAI. I'll write a PSA about that on v8-team.
Labels: v8-foozzie-failure

Sign in to add a comment