Object.freeze doesn't deoptimize properly |
||||||||
Issue description
# Minimized program:
var v = "";
function foo(a) {
while (a > 0) { v += 1; a--; }
return v + 0;
}
foo(20000);
Object.freeze(this);
foo(6);
print(v.length);
# Compared default with noturbo_opt
# Flags of default:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=360 --random-seed -92266327
# Flags of noturbo_opt:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=360 --random-seed -92266327 --always-opt --turbo-filter=~ --noturbo-asm
Difference:
- 20000
+ 20006
### Start of configuration default:
20000
### End of configuration default
### Start of configuration noturbo_opt:
20006
### End of configuration noturbo_opt
,
Nov 10 2016
It's a lot easier than that. The bug seems to be that Object.freeze doesn't deopt optimized code objects (both TurboFan and Crankshaft) that do store to the property cell when the cell changes from writable to read-only. I.e. the following test case fails in all configurations:
=======================================
// Flags: --allow-natives-syntax
var v = 0;
function foo(a) {
v = a;
}
foo(1);
foo(2);
%OptimizeFunctionOnNextCall(foo);
foo(3);
assertEquals(3, v);
Object.freeze(this);
foo(4);
foo(5);
%OptimizeFunctionOnNextCall(foo);
foo(6);
assertEquals(3, v);
=======================================
Assigning to ishell@ for runtime investigation.
,
Nov 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/6aa16edf36ae0f0fbfaf8eedccbcf1f5a7e801c1 commit 6aa16edf36ae0f0fbfaf8eedccbcf1f5a7e801c1 Author: ishell <ishell@chromium.org> Date: Thu Nov 10 10:36:40 2016 [runtime] Ensure Object.freeze() deoptimizes code that depends on global property cells. BUG= chromium:663750 Review-Url: https://codereview.chromium.org/2488223002 Cr-Commit-Position: refs/heads/master@{#40882} [modify] https://crrev.com/6aa16edf36ae0f0fbfaf8eedccbcf1f5a7e801c1/src/objects.cc [add] https://crrev.com/6aa16edf36ae0f0fbfaf8eedccbcf1f5a7e801c1/test/mjsunit/regress/regress-crbug-663750.js
,
Nov 10 2016
,
Nov 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/45b9f15f4489a93896d6d8131058607ee85bf4a4 commit 45b9f15f4489a93896d6d8131058607ee85bf4a4 Author: ishell <ishell@chromium.org> Date: Thu Nov 10 16:00:43 2016 [runtime] Treat empty property cells properly when doing Object.freeze() on a global object. BUG= chromium:663750 , chromium:664123 Review-Url: https://codereview.chromium.org/2495563002 Cr-Commit-Position: refs/heads/master@{#40902} [modify] https://crrev.com/45b9f15f4489a93896d6d8131058607ee85bf4a4/src/objects.cc [modify] https://crrev.com/45b9f15f4489a93896d6d8131058607ee85bf4a4/test/mjsunit/regress/regress-crbug-663750.js
,
Nov 11 2016
# A slightly different program still reproes. Is this another bug?
var v = {};
function foo(a) {
v = a;
}
foo();
%OptimizeFunctionOnNextCall(foo);
foo();
Object.freeze(this);
foo(4);
print(v);
# Compared fullcode with ignition_turbo
# Flags of fullcode:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 1352700209 --nocrankshaft --turbo-filter=~
# Flags of ignition_turbo:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 1352700209 --ignition-staging --turbo
Difference:
- undefined
+ 4
### Start of configuration fullcode:
undefined
### End of configuration fullcode
### Start of configuration ignition_turbo:
4
### End of configuration ignition_turbo
,
Nov 15 2016
TurboFan does not install a code dependency on a property cell.
,
Nov 16 2016
Just do the appropriate fix in JSGlobalObjectSpecialization.
,
Nov 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/446d6a067890b2f8310ce95885b41b2410193e0e commit 446d6a067890b2f8310ce95885b41b2410193e0e Author: ishell <ishell@chromium.org> Date: Wed Nov 16 12:02:06 2016 [turbofan] Always install code dependency when optimizing a store to global property. The reason is that non-configurability still allows a writable property to become read-only. BUG= chromium:663750 Review-Url: https://codereview.chromium.org/2508873002 Cr-Commit-Position: refs/heads/master@{#41029} [modify] https://crrev.com/446d6a067890b2f8310ce95885b41b2410193e0e/src/compiler/js-global-object-specialization.cc [modify] https://crrev.com/446d6a067890b2f8310ce95885b41b2410193e0e/test/mjsunit/regress/regress-crbug-663750.js
,
Nov 16 2016
,
Dec 13 2016
,
Jan 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/c4a35ed7e831448e1fd2a2ca1f79d5fefae0655e commit c4a35ed7e831448e1fd2a2ca1f79d5fefae0655e Author: machenbach <machenbach@chromium.org> Date: Mon Jan 16 09:01:51 2017 [foozzie] Remove suppressions for fixed bugs BUG= chromium:663750 , chromium:662907 , chromium:663340 , chromium:666308 , chromium:669017 NOTRY=true TBR=jarin@chromium.org, bmeurer@chromium.org Review-Url: https://codereview.chromium.org/2632153002 Cr-Commit-Position: refs/heads/master@{#42356} [modify] https://crrev.com/c4a35ed7e831448e1fd2a2ca1f79d5fefae0655e/tools/foozzie/v8_suppressions.js [modify] https://crrev.com/c4a35ed7e831448e1fd2a2ca1f79d5fefae0655e/tools/foozzie/v8_suppressions.py |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by bmeu...@chromium.org
, Nov 10 2016Status: Started (was: Untriaged)
Summary: Bug in Crankshaft: Object.freeze on global object (was: Difference between turbofan and crankshaft: Object.freeze)