object.entries return a weird answer
Reported by
toxic.jo...@gmail.com,
Jan 21 2018
|
|||||||||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3327.0 Safari/537.36
Steps to reproduce the problem:
1. open a blank page
2. input follow code
var config = {
flv: function flv() {}
};
function test(target, value, key) {
target[key] = value;
}
var target = {};
test(target, config.flv, 'flv');
console.log(target);
console.log(Object.entries(target));
3. You can see the object.entries return is werid
What is the expected behavior?
expect return this
What went wrong?
the return is werid
Did this work before? Yes chrome 63
Chrome version: 66.0.3327.0 Channel: canary
OS Version: OS X 10.13.2
Flash Version: Shockwave Flash 28.0 r0
,
Jan 21 2018
Bisect info: 528594 (good) - 528600 (bad) https://chromium.googlesource.com/chromium/src/+log/6f766682..6a83c2c4?pretty=fuller In r528595 "Update V8 to version 6.5.173" suspecting 03e9d415c2fbb51f6276b4abbb76d840f4106531 "Reland: Reimplement Object.entries/values as CSA to optimize performance" Landed in 65.0.3319.0
,
Jan 21 2018
set pri-1 for Regression
,
Jan 21 2018
,
Jan 22 2018
Reverting in https://crrev.com/c/877886.
,
Jan 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae commit e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae Author: Jakob Gruber <jgruber@chromium.org> Date: Mon Jan 22 09:47:43 2018 Revert "Reland: Reimplement Object.entries/values as CSA to optimize performance." This reverts commit 03e9d415c2fbb51f6276b4abbb76d840f4106531. Reason for revert: Correctness issues, see https://crbug.com/804159 . Bug: chromium:804159 Original change's description: > Reland: Reimplement Object.entries/values as CSA to optimize performance. > > Add Object.entries/values builtins to debug-evaluate.cc whitelist macro. > This fix revert commit of https://chromium-review.googlesource.com/c/v8/v8/+/859937 > Original is https://chromium-review.googlesource.com/c/v8/v8/+/810504 > >> Reimplements Object.entries/values as CSA to optimize performance. See more detail about https://bugs.chromium.org/p/v8/issues/ Issue 6804 . > > This reverts commit 1b49f725ac1674bc8a163c21ebcae9aa5f000fb7. > > Bug: v8:6804 > Change-Id: I57e8b66e1c4ece2abb52e1630a97fbfd4070d810 > Reviewed-on: https://chromium-review.googlesource.com/860679 > Commit-Queue: Yang Guo <yangguo@chromium.org> > Reviewed-by: Yang Guo <yangguo@chromium.org> > Cr-Commit-Position: refs/heads/master@{#50492} TBR=yangguo@chromium.org,cbruni@chromium.org,jgruber@chromium.org,ishell@chromium.org,brn@b6n.ch # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:6804 Change-Id: I39b1854ca7c2f57819ba377f84560356d3756bfb Reviewed-on: https://chromium-review.googlesource.com/877886 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#50746} [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/AUTHORS [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/bootstrapper.cc [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/builtins/builtins-definitions.h [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/builtins/builtins-object-gen.cc [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/builtins/builtins-object.cc [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/code-stub-assembler.cc [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/code-stub-assembler.h [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/debug/debug-evaluate.cc [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/objects.cc [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/objects.h [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/runtime/runtime-object.cc [modify] https://crrev.com/e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae/src/runtime/runtime.h
,
Jan 26 2018
Jakob -- Can you please add a regression test? Thanks!
,
Jan 29 2018
I'll leave that to the CL author when this is being relanded. I haven't investigated what exactly went wrong here.
,
Jan 29 2018
brn@b7n.ch:
Building chromium sounds like a good approach to investigate further. Another possibility (which might save you the loong compile times) could be to try reproducing this with inspector tests (see examples in test/inspector/).
FYI this is the sequence of inspector messages that occur when running the repro (you can probably ignore all up to Runtime.consoleAPICalled:
frontend: {"id":93,"method":"Runtime.compileScript","params":{"expression":"> var config = {\n> flv: function flv() {}\n> };\n> \n> function test(target, value, key) {\n> target[key] = function(){value};\n> }\n> \n> var target = {};\n> test(target, config.flv, 'flv');\n> \n> console.log(target); // this cause broken Object.entries result.\n> console.log(Object.entries(target));","sourceURL":"","persistScript":false,"executionContextId":3}}
inspector.js:3334 backend: {"id":93,"result":{"exceptionDetails":{"exceptionId":1,"text":"Uncaught","lineNumber":0,"columnNumber":0,"scriptId":"175","exception":{"type":"object","subtype":"error","className":"SyntaxError","description":"SyntaxError: Unexpected token >","objectId":"{\"injectedScriptId\":3,\"id\":1}"}}}}
inspector.js:3334 frontend: {"id":94,"method":"Runtime.compileScript","params":{"expression":" var config = {\n flv: function flv() {}\n };\n \n function test(target, value, key) {\n target[key] = function(){value};\n }\n \n var target = {};\n test(target, config.flv, 'flv');\n \n console.log(target); // this cause broken Object.entries result.\n console.log(Object.entries(target));","sourceURL":"","persistScript":false,"executionContextId":3}}
inspector.js:3334 backend: {"id":94,"result":{}}
inspector.js:3334 frontend: {"id":95,"method":"Network.getResponseBody","params":{"requestId":"39237.128"}}
inspector.js:3334 backend: {"id":95,"result":{"body":"","base64Encoded":true}}
inspector.js:3334 frontend: {"id":96,"method":"Runtime.compileScript","params":{"expression":" var config = {\n flv: function flv() {}\n };\n \n function test(target, value, key) {\n target[key] = function(){value};\n }\n \n var target = {};\n test(target, config.flv, 'flv');\n \n console.log(target); // this cause broken Object.entries result.\n console.log(Object.entries(target));","sourceURL":"","persistScript":true,"executionContextId":3}}
inspector.js:3334 backend: {"method":"Debugger.scriptParsed","params":{"scriptId":"177","url":"","startLine":0,"startColumn":0,"endLine":12,"endColumn":37,"executionContextId":3,"hash":"2ABCAED04BD4FEC201771FC5475442975A0AA4E2","executionContextAuxData":{"isDefault":true,"frameId":"BE0BFC20BD9916138D6BD632BE992BB3"},"isLiveEdit":false,"sourceMapURL":"","hasSourceURL":false,"isModule":false,"length":287}}
inspector.js:3334 backend: {"id":96,"result":{"scriptId":"177"}}
inspector.js:3334 frontend: {"id":97,"method":"Runtime.runScript","params":{"scriptId":"177","executionContextId":3,"objectGroup":"console","silent":false,"includeCommandLineAPI":true,"returnByValue":false,"generatePreview":true}}
inspector.js:3334 backend: {"method":"Debugger.scriptParsed","params":{"scriptId":"178","url":"","startLine":0,"startColumn":0,"endLine":0,"endColumn":41,"executionContextId":3,"hash":"DD4853B2B54D7428AB585EC2254D2BBC5C217635","executionContextAuxData":{"isDefault":true,"frameId":"BE0BFC20BD9916138D6BD632BE992BB3"},"isLiveEdit":false,"sourceMapURL":"","hasSourceURL":false,"isModule":false,"length":41}}
inspector.js:3334 backend: {"method":"Runtime.consoleAPICalled","params":{"type":"log","args":[{"type":"object","className":"Object","description":"Object","objectId":"{\"injectedScriptId\":3,\"id\":2}","preview":{"type":"object","description":"Object","overflow":false,"properties":[{"name":"flv","type":"function","value":""}]}}],"executionContextId":3,"timestamp":1517218012059.925,"stackTrace":{"callFrames":[{"functionName":"","scriptId":"177","url":"","lineNumber":11,"columnNumber":9}]}}}
inspector.js:3334 backend: {"method":"Runtime.consoleAPICalled","params":{"type":"log","args":[{"type":"object","subtype":"array","className":"Array","description":"Array(1)","objectId":"{\"injectedScriptId\":3,\"id\":3}","preview":{"type":"object","subtype":"array","description":"Array(1)","overflow":false,"properties":[{"name":"0","type":"object","value":"Array(2)","subtype":"array"}]}}],"executionContextId":3,"timestamp":1517218012063.3308,"stackTrace":{"callFrames":[{"functionName":"","scriptId":"177","url":"","lineNumber":12,"columnNumber":9}]}}}
inspector.js:3334 backend: {"id":97,"result":{"result":{"type":"undefined"}}}
,
Jan 29 2018
jgruber Thanks! I'll try to investigate this.
,
Jan 30 2018
I had fix this issue. This caused by incorrect descriptor array index. But only occur if enum_cache exists because that path only executed if that condition satisfied. More details see below CL. https://chromium-review.googlesource.com/c/v8/v8/+/892684/1
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/4455377fca84bd78274a87b6dce898000f7d5784 commit 4455377fca84bd78274a87b6dce898000f7d5784 Author: Taketoshi Aono <brn@b6n.ch> Date: Thu Feb 08 10:12:32 2018 Reland: Reimplement Object.entries/values as CSA to optimize performance. Original CL is https://chromium-review.googlesource.com/c/v8/v8/+/810504 Reverted issue is https://bugs.chromium.org/p/chromium/issues/detail?id=804159 Fix Object.entries descriptor array value index. This reverts commit e5ecb24859c379eea8b2d9ca0a7fbf9147bf07ae. Bug: v8:6804, chromium:804159 Change-Id: I73a5a5f670c5b36e0c5cc7984d5979ecec43d969 Reviewed-on: https://chromium-review.googlesource.com/892684 Commit-Queue: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#51170} [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/AUTHORS [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/bootstrapper.cc [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/builtins/builtins-definitions.h [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/builtins/builtins-object-gen.cc [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/builtins/builtins-object.cc [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/code-stub-assembler.cc [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/code-stub-assembler.h [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/debug/debug-evaluate.cc [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/objects.cc [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/objects.h [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/runtime/runtime-object.cc [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/src/runtime/runtime.h [modify] https://crrev.com/4455377fca84bd78274a87b6dce898000f7d5784/test/mjsunit/es8/object-entries.js
,
Mar 8 2018
The revert mentioned in comment#6 didn't make it to Chrome 66. This bug is present in Chrome 65. We have received a bug report from Facebook about this breakage in their production app. Adding merge-request-65 label.
,
Mar 8 2018
Also, this isn't terrible to work around on the client side but also the revert is straightforward and applies cleanly. Leaving it to the chrome release managers to make the call here
,
Mar 8 2018
This bug requires manual review: Request affecting a post-stable build Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 8 2018
+hablich@ for M65 merge review (pls note M65 already went out to stable yesterday).
,
Mar 8 2018
Please also set the bug to fixed.
,
Mar 8 2018
,
Mar 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6 commit ee390ca7f63f1f8251b64eb096605e5bfddcc8d6 Author: jgruber <jgruber@chromium.org> Date: Thu Mar 08 08:14:23 2018 Merged: Revert "Reland: Reimplement Object.entries/values as CSA to optimize performance." This reverts commit 03e9d415c2fbb51f6276b4abbb76d840f4106531. Reason for revert: Correctness issues, see https://crbug.com/804159 . Bug: chromium:804159 Original change's description: > Reland: Reimplement Object.entries/values as CSA to optimize performance. > > Add Object.entries/values builtins to debug-evaluate.cc whitelist macro. > This fix revert commit of https://chromium-review.googlesource.com/c/v8/v8/+/859937 > Original is https://chromium-review.googlesource.com/c/v8/v8/+/810504 > >> Reimplements Object.entries/values as CSA to optimize performance. See more detail about https://bugs.chromium.org/p/v8/issues/ Issue 6804 . > > This reverts commit 1b49f725ac1674bc8a163c21ebcae9aa5f000fb7. > > Bug: v8:6804 > Change-Id: I57e8b66e1c4ece2abb52e1630a97fbfd4070d810 > Reviewed-on: https://chromium-review.googlesource.com/860679 > Commit-Queue: Yang Guo <yangguo@chromium.org> > Reviewed-by: Yang Guo <yangguo@chromium.org> > Cr-Commit-Position: refs/heads/master@{#50492} TBR=yangguo@chromium.org,cbruni@chromium.org,jgruber@chromium.org,ishell@chromium.org,brn@b6n.ch NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true Bug: v8:6804 Change-Id: I39b1854ca7c2f57819ba377f84560356d3756bfb Reviewed-on: https://chromium-review.googlesource.com/877886 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#50746} Reviewed-on: https://chromium-review.googlesource.com/955083 Cr-Commit-Position: refs/branch-heads/6.5@{#63} Cr-Branched-From: 73c55f57fe8506011ff854b15026ca765b669700-refs/heads/6.5.254@{#1} Cr-Branched-From: 594a1a0b6e551397cfdf50870f6230da34db2dc8-refs/heads/master@{#50664} [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/AUTHORS [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/bootstrapper.cc [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/builtins/builtins-definitions.h [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/builtins/builtins-object-gen.cc [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/builtins/builtins-object.cc [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/code-stub-assembler.cc [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/code-stub-assembler.h [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/debug/debug-evaluate.cc [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/objects.cc [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/objects.h [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/runtime/runtime-object.cc [modify] https://crrev.com/ee390ca7f63f1f8251b64eb096605e5bfddcc8d6/src/runtime/runtime.h
,
Mar 8 2018
,
Mar 8 2018
I don't think M66 merge is needed for this bug just double checking based on comment #13. Pls request a merge to M66 if needed. Thank you. Comment #13 by gsathya@: The revert mentioned in comment#6 didn't make it to Chrome 66. This bug is present in Chrome 65. We have received a bug report from Facebook about this breakage in their production app. Adding merge-request-65 label.
,
Mar 8 2018
You're right. We don't need this for Chrome 66. I don't know what I was thinking when I wrote that comment.
,
Mar 8 2018
Np, thank you for confirmation.
,
Mar 10 2018
Issue v8:7545 has been merged into this issue.
,
Mar 12 2018
Issue v8:7550 has been merged into this issue.
,
Mar 12 2018
govind@, we're seeing a lot more bug reports. Is there going to be a respin for Chrome 65?
,
Mar 12 2018
We're planning M65 respin this week, merge listed at #19 will be included.
,
Mar 12 2018
gsathya@, pls apply appropriate OSs label. I think this should be applicable to all OSs except iOS, correct? +cmasso@ for visibility
,
Mar 12 2018
,
Mar 12 2018
Android is about to rollout to 100% today. How is this issue impacting Android?
,
Mar 12 2018
This is a bug in V8 which is part of the Chrome on Android. I don't have an Android device to double check, but I see no reason why this wouldn't repro on Android.
,
Mar 12 2018
Issue 821043 has been merged into this issue.
,
Mar 13 2018
Issue 820985 has been merged into this issue.
,
Mar 14 2018
,
Mar 14 2018
Able to reproduce this issue on 65.0.3325.146 hence verified on 66.0.3325.162 using Mac 10.13.3, Windows 10 and Ubuntu 14.04. Observed 0:(2) ["flv", ƒ] for first time only. Attaching screencast for reference. Hence adding TE-Verified labels. Thanks!
,
Apr 24 2018
Possibly related issue https://github.com/mobxjs/mobx/issues/1504#issuecomment-383906036
,
Apr 24 2018
#36, reported in issue 836145 |
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by toxic.jo...@gmail.com
, Jan 21 2018