AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[constant-tracking] Disable delete optimization for constant fields.

Browse Source 
When using the fast-properties optimization for `delete` with constant
fields we don't properly invalidate the constness on the original map
and might thereby just follow the same transition again later with the
same object, effectively violating the constness of that field. This
disables the fast-properties optimization for `delete` in case of a
field marked as "const" as a quick-fix. We might still want to change
the logic to properly invalidate the "const" bit later.

Bug: chromium:962588, v8:9233
Change-Id: I1d0a8649d117731a0cd5ebdb4b6d0b22a900f33d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1609796
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61484}
This commit is contained in:
Benedikt Meurer 2019-05-14 14:26:31 +02:00 committed by Commit Bot
parent ec2c299c42
commit f0e054c2c6
2 changed files with 29 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/runtime/runtime-object.cc
View File

@ -100,6 +100,13 @@ bool DeleteObjectPropertyFast(Isolate* isolate, Handle<JSReceiver> receiver,
// (3) The property to be deleted must be deletable.
PropertyDetails details = descriptors->GetDetails(descriptor);
if (!details.IsConfigurable()) return false;
// TODO(bmeurer): This optimization is unsound if the property is currently
// marked as constant, as there's no way that we can learn that it is not
// constant when we later follow the same transition again with a different
// value on the same object. As a quick-fix we just disable the optimization
// in case of constant fields. We might want to restructure the code here to
// update the {map} instead and deoptimize all code that depends on it.
if (details.constness() == PropertyConstness::kConst) return false;
// (4) The map must have a back pointer.
Object backpointer = map->GetBackPointer();
if (!backpointer->IsMap()) return false;

22
test/mjsunit/regress/regress-v8-9233.js Normal file
View File

@ -0,0 +1,22 @@
// Copyright 2019 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax
let o1 = { x: 999 };
o1.y = 999;
// o2 will share map with o1 in its initial state
var o2 = { x: 1 };
function f() { return o2.x; }
assertEquals(1, f());
assertEquals(1, f());
%OptimizeFunctionOnNextCall(f);
assertEquals(1, f());
delete o2.x;
o2.x = 2;
assertEquals(2, f());