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

[map] Move Map::IsInRetainedMapListBit out of Map::bit_field2.

Browse Source 
The invariant is that Map::bit_field2 shouldn't change, and the
IsInRetainedMapListBit apparently changes when the map is held
weakly from optimized code. This causes TurboFan compilations to
change the Map::Hash() result, which in turn causes lookups on
the normalized map cache to miss (and maybe other bad consequences).

With this change we swap Map::IsInRetainedMapListBit (previously in
bit_field2) and Map::HasHiddenPrototypeBit (previously in bit_field3)
to address this problem.

Bug: chromium:963411, v8:9114, v8:9267
Change-Id: I040a27c37305fa602649750bd93bee40c91fca78
Cq-Include-Trybots: luci.chromium.try:linux-rel,win7-rel
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1619747
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61657}
This commit is contained in:
Benedikt Meurer 2019-05-20 15:00:57 +02:00 committed by Commit Bot
parent 2f8a7538ce
commit 437d710fc5
10 changed files with 77 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/builtins/builtins-call-gen.cc
View File

@ -478,7 +478,7 @@ TNode<JSReceiver> CallOrConstructBuiltinsAssembler::GetCompatibleReceiver(
// the receiver did not pass the {signature} check.
TNode<Map> holder_map = LoadMap(holder);
var_holder = LoadMapPrototype(holder_map);
GotoIf(IsSetWord32(LoadMapBitField3(holder_map),
GotoIf(IsSetWord32(LoadMapBitField2(holder_map),
Map::HasHiddenPrototypeBit::kMask),
&holder_loop);
ThrowTypeError(context, MessageTemplate::kIllegalInvocation);

4
src/builtins/builtins-object-gen.cc
View File

@ -187,8 +187,8 @@ TNode<BoolT> ObjectEntriesValuesBuiltinsAssembler::IsPropertyKindData(
TNode<Uint32T> ObjectEntriesValuesBuiltinsAssembler::HasHiddenPrototype(
TNode<Map> map) {
TNode<Uint32T> bit_field3 = LoadMapBitField3(map);
return DecodeWord32<Map::HasHiddenPrototypeBit>(bit_field3);
TNode<Uint32T> bit_field2 = Unsigned(LoadMapBitField2(map));
return DecodeWord32<Map::HasHiddenPrototypeBit>(bit_field2);
}
void ObjectEntriesValuesBuiltinsAssembler::GetOwnValuesOrEntries(

7
src/code-stub-assembler.cc
View File

@ -1699,11 +1699,12 @@ TNode<Uint32T> CodeStubAssembler::EnsureOnlyHasSimpleProperties(
// This check can have false positives, since it applies to any JSValueType.
GotoIf(IsCustomElementsReceiverInstanceType(instance_type), bailout);
TNode<Uint32T> bit_field3 = LoadMapBitField3(map);
GotoIf(IsSetWord32(bit_field3, Map::IsDictionaryMapBit::kMask |
Map::HasHiddenPrototypeBit::kMask),
GotoIf(IsSetWord32(LoadMapBitField2(map), Map::HasHiddenPrototypeBit::kMask),
bailout);
TNode<Uint32T> bit_field3 = LoadMapBitField3(map);
GotoIf(IsSetWord32(bit_field3, Map::IsDictionaryMapBit::kMask), bailout);
return bit_field3;
}

4
src/compiler/js-heap-broker.cc
View File

@ -2637,12 +2637,12 @@ BIMODAL_ACCESSOR(JSTypedArray, HeapObject, buffer)
BIMODAL_ACCESSOR_B(Map, bit_field2, elements_kind, Map::ElementsKindBits)
BIMODAL_ACCESSOR_B(Map, bit_field2, is_extensible, Map::IsExtensibleBit)
BIMODAL_ACCESSOR_B(Map, bit_field2, has_hidden_prototype,
Map::HasHiddenPrototypeBit)
BIMODAL_ACCESSOR_B(Map, bit_field3, is_deprecated, Map::IsDeprecatedBit)
BIMODAL_ACCESSOR_B(Map, bit_field3, is_dictionary_map, Map::IsDictionaryMapBit)
BIMODAL_ACCESSOR_B(Map, bit_field3, NumberOfOwnDescriptors,
Map::NumberOfOwnDescriptorsBits)
BIMODAL_ACCESSOR_B(Map, bit_field3, has_hidden_prototype,
Map::HasHiddenPrototypeBit)
BIMODAL_ACCESSOR_B(Map, bit_field3, is_migration_target,
Map::IsMigrationTargetBit)
BIMODAL_ACCESSOR_B(Map, bit_field, has_prototype_slot, Map::HasPrototypeSlotBit)

2
src/heap/factory.cc
View File

@ -2006,11 +2006,11 @@ Map Factory::InitializeMap(Map map, InstanceType type, int instance_size,
map->set_visitor_id(Map::GetVisitorId(map));
map->set_bit_field(0);
map->set_bit_field2(Map::IsExtensibleBit::kMask);
DCHECK(!map->is_in_retained_map_list());
int bit_field3 = Map::EnumLengthBits::encode(kInvalidEnumCacheSentinel) |
Map::OwnsDescriptorsBit::encode(true) |
Map::ConstructionCounterBits::encode(Map::kNoSlackTracking);
map->set_bit_field3(bit_field3);
DCHECK(!map->is_in_retained_map_list());
map->clear_padding();
map->set_elements_kind(elements_kind);
map->set_new_target_is_base(true);

8
src/objects/map-inl.h
View File

@ -76,14 +76,14 @@ BIT_FIELD_ACCESSORS(Map, relaxed_bit_field, has_prototype_slot,
// |bit_field2| fields.
BIT_FIELD_ACCESSORS(Map, bit_field2, is_extensible, Map::IsExtensibleBit)
BIT_FIELD_ACCESSORS(Map, bit_field2, is_prototype_map, Map::IsPrototypeMapBit)
BIT_FIELD_ACCESSORS(Map, bit_field2, is_in_retained_map_list,
Map::IsInRetainedMapListBit)
BIT_FIELD_ACCESSORS(Map, bit_field2, has_hidden_prototype,
Map::HasHiddenPrototypeBit)
// |bit_field3| fields.
BIT_FIELD_ACCESSORS(Map, bit_field3, owns_descriptors, Map::OwnsDescriptorsBit)
BIT_FIELD_ACCESSORS(Map, bit_field3, has_hidden_prototype,
Map::HasHiddenPrototypeBit)
BIT_FIELD_ACCESSORS(Map, bit_field3, is_deprecated, Map::IsDeprecatedBit)
BIT_FIELD_ACCESSORS(Map, bit_field3, is_in_retained_map_list,
Map::IsInRetainedMapListBit)
BIT_FIELD_ACCESSORS(Map, bit_field3, is_migration_target,
Map::IsMigrationTargetBit)
BIT_FIELD_ACCESSORS(Map, bit_field3, is_immutable_proto,

14
src/objects/map.cc
View File

@ -1466,6 +1466,7 @@ Handle<Map> Map::RawCopy(Isolate* isolate, Handle<Map> map, int instance_size,
new_bit_field3 =
EnumLengthBits::update(new_bit_field3, kInvalidEnumCacheSentinel);
new_bit_field3 = IsDeprecatedBit::update(new_bit_field3, false);
new_bit_field3 = IsInRetainedMapListBit::update(new_bit_field3, false);
if (!map->is_dictionary_map()) {
new_bit_field3 = IsUnstableBit::update(new_bit_field3, false);
}
@ -1501,8 +1502,17 @@ Handle<Map> Map::Normalize(Isolate* isolate, Handle<Map> fast_map,
Map::kDependentCodeOffset + kTaggedSize);
DCHECK_EQ(0, memcmp(reinterpret_cast<void*>(fresh->address()),
reinterpret_cast<void*>(new_map->address()),
Map::kDependentCodeOffset));
int offset = Map::kPrototypeValidityCellOffset + kTaggedSize;
Map::kBitField3Offset));
// The IsInRetainedMapListBit might be different if the {new_map}
// that we got from the {cache} was already embedded into optimized
// code somewhere.
DCHECK_EQ(fresh->bit_field3() & ~IsInRetainedMapListBit::kMask,
new_map->bit_field3() & ~IsInRetainedMapListBit::kMask);
int offset = Map::kBitField3Offset + kInt32Size;
DCHECK_EQ(0, memcmp(reinterpret_cast<void*>(fresh->address() + offset),
reinterpret_cast<void*>(new_map->address() + offset),
Map::kDependentCodeOffset - offset));
offset = Map::kPrototypeValidityCellOffset + kTaggedSize;
if (new_map->is_prototype_map()) {
// For prototype maps, the PrototypeInfo is not copied.
STATIC_ASSERT(Map::kTransitionsOrPrototypeInfoOffset ==

14
src/objects/map.h
View File

@ -142,7 +142,7 @@ using MapHandles = std::vector<Handle<Map>>;
// | Byte | [bit_field2] |
// | | - is_extensible (bit 0) |
// | | - is_prototype_map (bit 1) |
// | | - is_in_retained_map_list (bit 2) |
// | | - has_hidden_prototype (bit 2) |
// | | - elements_kind (bits 3..7) |
// +----+----------+---------------------------------------------+
// | Int | [bit_field3] |
@ -150,7 +150,7 @@ using MapHandles = std::vector<Handle<Map>>;
// | | - number_of_own_descriptors (bit 10..19) |
// | | - is_dictionary_map (bit 20) |
// | | - owns_descriptors (bit 21) |
// | | - has_hidden_prototype (bit 22) |
// | | - is_in_retained_map_list (bit 22) |
// | | - is_deprecated (bit 23) |
// | | - is_unstable (bit 24) |
// | | - is_migration_target (bit 25) |
@ -267,10 +267,10 @@ class Map : public HeapObject {
DECL_PRIMITIVE_ACCESSORS(bit_field2, byte)
// Bit positions for |bit_field2|.
#define MAP_BIT_FIELD2_FIELDS(V, _) \
V(IsExtensibleBit, bool, 1, _) \
V(IsPrototypeMapBit, bool, 1, _) \
V(IsInRetainedMapListBit, bool, 1, _) \
#define MAP_BIT_FIELD2_FIELDS(V, _) \
V(IsExtensibleBit, bool, 1, _) \
V(IsPrototypeMapBit, bool, 1, _) \
V(HasHiddenPrototypeBit, bool, 1, _) \
V(ElementsKindBits, ElementsKind, 5, _)
DEFINE_BIT_FIELDS(MAP_BIT_FIELD2_FIELDS)
@ -291,7 +291,7 @@ class Map : public HeapObject {
V(NumberOfOwnDescriptorsBits, int, kDescriptorIndexBitCount, _) \
V(IsDictionaryMapBit, bool, 1, _) \
V(OwnsDescriptorsBit, bool, 1, _) \
V(HasHiddenPrototypeBit, bool, 1, _) \
V(IsInRetainedMapListBit, bool, 1, _) \
V(IsDeprecatedBit, bool, 1, _) \
V(IsUnstableBit, bool, 1, _) \
V(IsMigrationTargetBit, bool, 1, _) \

22
test/mjsunit/regress/regress-v8-9267-1.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
function bar(a) {
return Object.defineProperty(a, 'x', {get() { return 1; }});
}
function foo() {
return Array(1);
}
%NeverOptimizeFunction(bar);
%PrepareFunctionForOptimization(foo);
bar(foo());
const a = bar(foo());
%OptimizeFunctionOnNextCall(foo);
const b = bar(foo());
assertTrue(%HaveSameMap(a, b));

22
test/mjsunit/regress/regress-v8-9267-2.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
function bar(a) {
return Object.defineProperty(a, 'x', {get() { return 1; }});
}
function foo() {
return {};
}
%NeverOptimizeFunction(bar);
%PrepareFunctionForOptimization(foo);
bar(foo());
const a = bar(foo());
%OptimizeFunctionOnNextCall(foo);
const b = bar(foo());
assertTrue(%HaveSameMap(a, b));