array map function returns incorrect values
Reported by
davidasm...@gmail.com,
Jul 20 2017
|
|||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3162.0 Safari/537.36
Steps to reproduce the problem:
r =[14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14]
for(i=0;i<100000;i++){
r2 = r.map(function(y){return y/64})
if(r2[0]>1){console.log(r2); break;} // value should be 0.21875
}
What is the expected behavior?
You may have to run this multiple times. I think it is best to try it on a high load system. I ran it while running WebGL.
Should return array of 0.21875.
What went wrong?
Returns array of 2360 (or perhaps a different stupid number)
This seems to fail consistently.
Did this work before? Yes Last one
Chrome version: 61.0.3162.0 Channel: canary
OS Version: OS X 10.12.5
Flash Version:
I was using my live programming environment in WebGL to reproduce, but it seems like a replicatable Javascript error. You can reach me at davidasmith at gmail.com if you need more info.
,
Jul 20 2017
woxxom@, Thank you for the bisect. mvstanton@, can you please look into this change (https://chromium-review.googlesource.com/548639)? Thank you!
,
Jul 24 2017
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/d9b98f3d050da5c6fe2182bfc60fdfa3a397e2b8 commit d9b98f3d050da5c6fe2182bfc60fdfa3a397e2b8 Author: Mike Stanton <mvstanton@chromium.org> Date: Tue Jul 25 04:05:56 2017 [TurboFan] Array.prototype.map inlining error A Phi is necessary to carry the ElementsKind forward in case transitions are taken. Bug: chromium:747075 Change-Id: I9d9d66b0219fe3f67d08536f4d478ee300c76acb Reviewed-on: https://chromium-review.googlesource.com/583090 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Michael Stanton <mvstanton@chromium.org> Cr-Commit-Position: refs/heads/master@{#46852} [modify] https://crrev.com/d9b98f3d050da5c6fe2182bfc60fdfa3a397e2b8/src/compiler/effect-control-linearizer.cc [add] https://crrev.com/d9b98f3d050da5c6fe2182bfc60fdfa3a397e2b8/test/mjsunit/regress/regress-747075.js
,
Jul 25 2017
I'd like to offer a big thanks to David for reporting this, and my apologies for the inconvenience..!
,
Jul 25 2017
[Auto-generated comment by a script] We noticed that this issue is targeted for M-61; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-61 label, otherwise remove Merge-TBD label. Thanks.
,
Jul 25 2017
Yes, we should merge this to M61, correctness issue.
,
Jul 25 2017
* The merge should be clean. * The fix is localized to this one function (Array.prototype.map) when optimized. * Let's get a couple days of Canary coverage...
,
Jul 26 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f1996bb9e965e102fd2be28af9c9d2bb70abf4b1 commit f1996bb9e965e102fd2be28af9c9d2bb70abf4b1 Author: Mike Stanton <mvstanton@chromium.org> Date: Wed Jul 26 11:55:17 2017 Merged: [TurboFan] Array.prototype.map inlining error Approved merge to 6.1. Revision: d9b98f3d BUG= chromium:747075 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=jarin@chromium.org Change-Id: Ia59f078c7c8c1dfef3fe49bc0e493492f5f3d2ee Reviewed-on: https://chromium-review.googlesource.com/586689 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Cr-Commit-Position: refs/branch-heads/6.1@{#18} Cr-Branched-From: 1bf2e10ddb194d4c2871a87a4732613419de892d-refs/heads/6.1.534@{#1} Cr-Branched-From: e825c4318eb2065ffdf9044aa6a5278635c36427-refs/heads/master@{#46746} [modify] https://crrev.com/f1996bb9e965e102fd2be28af9c9d2bb70abf4b1/src/compiler/effect-control-linearizer.cc [add] https://crrev.com/f1996bb9e965e102fd2be28af9c9d2bb70abf4b1/test/mjsunit/regress/regress-747075.js
,
Jul 26 2017
Okay I merged to V8 6.1 (and I believe that runs into 3163). Thanks!
,
Jul 26 2017
Removing "Merge-Approved-61" label as this is already merged to M61 at comment #10.
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/950e4f4631a09f098076333585df3706c449f6ad commit 950e4f4631a09f098076333585df3706c449f6ad Author: Mike Stanton <mvstanton@chromium.org> Date: Thu Jul 27 12:39:44 2017 [TurboFan] Improved unit test for optimized Array.prototype.map. Test mjsunit/optimized-map walked an array through different ElementsKind transitions, but it failed to verify that the expected ElementsKind was in place. Although we have a regression test for the bug, it's a good idea to make sure the basic test covers all paths. Bug: chromium:747075 Change-Id: I1424880801857f3356bfd63839d351d6fd1521e0 Reviewed-on: https://chromium-review.googlesource.com/584837 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Michael Stanton <mvstanton@chromium.org> Cr-Commit-Position: refs/heads/master@{#46933} [modify] https://crrev.com/950e4f4631a09f098076333585df3706c449f6ad/test/mjsunit/optimized-map.js |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by woxxom@gmail.com
, Jul 20 2017