Skip to content

Commit 61bf2cc

Browse files
ulanCommit Bot
ulan
authored and
Commit Bot
committed
[runtime] Make layout descriptor helper safe for concurrent marking.
The layout descriptor helper computes the object header size using map->instance_size() and map->GetInObjectProperties(). It races with finalization of slack tracking, which changes both the instance size and the in-object properties count. This patch replaces the in-object properties count byte in the map with the byte that stores the start offset of in-object properties. The new byte can be used in the layout descriptor to compute the object header size and it is immutable. This patch also renames InstanceSize to InstanceSizeInWords where the instance size is represented in words. Bug: chromium:786069, chromium:694255 Change-Id: I4b48c6944d3fe8a950bd7b0ba43d75216b177a78 Reviewed-on: https://chromium-review.googlesource.com/776720 Commit-Queue: Ulan Degenbaev <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/master@{#49461}
1 parent ed53f05 commit 61bf2cc

File tree

12 files changed

+101
-75
lines changed

12 files changed

+101
-75
lines changed

‎src/bootstrapper.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,9 @@ Handle<Map> CreateNonConstructorMap(Handle<Map> source_map,
805805
// TODO(ulan): Do not change instance size after map creation.
806806
int unused_property_fields = map->UnusedPropertyFields();
807807
map->set_instance_size(map->instance_size() + kPointerSize);
808+
// The prototype slot shifts the in-object properties area by one slot.
809+
map->SetInObjectPropertiesStartInWords(
810+
map->GetInObjectPropertiesStartInWords() + 1);
808811
map->set_has_prototype_slot(true);
809812
map->SetInObjectUnusedPropertyFields(unused_property_fields);
810813
}
@@ -3369,6 +3372,9 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
33693372
// TODO(ulan): Do not change instance size after map creation.
33703373
int unused_property_fields = proxy_function_map->UnusedPropertyFields();
33713374
proxy_function_map->set_instance_size(JSFunction::kSizeWithPrototype);
3375+
// The prototype slot shifts the in-object properties area by one slot.
3376+
proxy_function_map->SetInObjectPropertiesStartInWords(
3377+
proxy_function_map->GetInObjectPropertiesStartInWords() + 1);
33723378
proxy_function_map->set_has_prototype_slot(true);
33733379
proxy_function_map->set_is_constructor(true);
33743380
proxy_function_map->SetInObjectUnusedPropertyFields(unused_property_fields);

‎src/builtins/builtins-async-gen.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Node* AsyncBuiltinsAssembler::Await(
5757
LoadObjectField(promise_fun, JSFunction::kPrototypeOrInitialMapOffset);
5858
// Assert that the JSPromise map has an instance size is
5959
// JSPromise::kSizeWithEmbedderFields.
60-
CSA_ASSERT(this, WordEqual(LoadMapInstanceSize(promise_map),
60+
CSA_ASSERT(this, WordEqual(LoadMapInstanceSizeInWords(promise_map),
6161
IntPtrConstant(JSPromise::kSizeWithEmbedderFields /
6262
kPointerSize)));
6363
Node* const wrapped_value = InnerAllocate(base, kWrappedPromiseOffset);
@@ -158,7 +158,7 @@ void AsyncBuiltinsAssembler::InitializeNativeClosure(Node* context,
158158
native_context, Context::STRICT_FUNCTION_WITHOUT_PROTOTYPE_MAP_INDEX);
159159
// Ensure that we don't have to initialize prototype_or_initial_map field of
160160
// JSFunction.
161-
CSA_ASSERT(this, WordEqual(LoadMapInstanceSize(function_map),
161+
CSA_ASSERT(this, WordEqual(LoadMapInstanceSizeInWords(function_map),
162162
IntPtrConstant(JSFunction::kSizeWithoutPrototype /
163163
kPointerSize)));
164164
StoreMapNoWriteBarrier(function, function_map);

‎src/builtins/builtins-constructor-gen.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Node* ConstructorBuiltinsAssembler::EmitFastNewClosure(Node* shared_info,
8484

8585
// Create a new closure from the given function info in new space
8686
Node* instance_size_in_bytes =
87-
TimesPointerSize(LoadMapInstanceSize(function_map));
87+
TimesPointerSize(LoadMapInstanceSizeInWords(function_map));
8888
Node* result = Allocate(instance_size_in_bytes);
8989
StoreMapNoWriteBarrier(result, function_map);
9090
InitializeJSObjectBodyNoSlackTracking(result, function_map,
@@ -508,7 +508,8 @@ Node* ConstructorBuiltinsAssembler::EmitCreateShallowObjectLiteral(
508508
// Ensure new-space allocation for a fresh JSObject so we can skip write
509509
// barriers when copying all object fields.
510510
STATIC_ASSERT(JSObject::kMaxInstanceSize < kMaxRegularHeapObjectSize);
511-
Node* instance_size = TimesPointerSize(LoadMapInstanceSize(boilerplate_map));
511+
Node* instance_size =
512+
TimesPointerSize(LoadMapInstanceSizeInWords(boilerplate_map));
512513
Node* allocation_size = instance_size;
513514
bool needs_allocation_memento = FLAG_allocation_site_pretenuring;
514515
if (needs_allocation_memento) {

‎src/code-stub-assembler.cc‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,19 +1362,20 @@ TNode<PrototypeInfo> CodeStubAssembler::LoadMapPrototypeInfo(
13621362
return CAST(prototype_info);
13631363
}
13641364

1365-
TNode<IntPtrT> CodeStubAssembler::LoadMapInstanceSize(SloppyTNode<Map> map) {
1365+
TNode<IntPtrT> CodeStubAssembler::LoadMapInstanceSizeInWords(
1366+
SloppyTNode<Map> map) {
13661367
CSA_SLOW_ASSERT(this, IsMap(map));
1367-
return ChangeInt32ToIntPtr(
1368-
LoadObjectField(map, Map::kInstanceSizeOffset, MachineType::Uint8()));
1368+
return ChangeInt32ToIntPtr(LoadObjectField(
1369+
map, Map::kInstanceSizeInWordsOffset, MachineType::Uint8()));
13691370
}
13701371

1371-
TNode<IntPtrT> CodeStubAssembler::LoadMapInobjectProperties(
1372+
TNode<IntPtrT> CodeStubAssembler::LoadMapInobjectPropertiesStartInWords(
13721373
SloppyTNode<Map> map) {
13731374
CSA_SLOW_ASSERT(this, IsMap(map));
1374-
// See Map::GetInObjectProperties() for details.
1375+
// See Map::GetInObjectPropertiesStartInWords() for details.
13751376
CSA_ASSERT(this, IsJSObjectMap(map));
13761377
return ChangeInt32ToIntPtr(LoadObjectField(
1377-
map, Map::kInObjectPropertiesOrConstructorFunctionIndexOffset,
1378+
map, Map::kInObjectPropertiesStartOrConstructorFunctionIndexOffset,
13781379
MachineType::Uint8()));
13791380
}
13801381

@@ -1384,7 +1385,7 @@ TNode<IntPtrT> CodeStubAssembler::LoadMapConstructorFunctionIndex(
13841385
// See Map::GetConstructorFunctionIndex() for details.
13851386
CSA_ASSERT(this, IsPrimitiveInstanceType(LoadMapInstanceType(map)));
13861387
return ChangeInt32ToIntPtr(LoadObjectField(
1387-
map, Map::kInObjectPropertiesOrConstructorFunctionIndexOffset,
1388+
map, Map::kInObjectPropertiesStartOrConstructorFunctionIndexOffset,
13881389
MachineType::Uint8()));
13891390
}
13901391

@@ -2464,7 +2465,7 @@ Node* CodeStubAssembler::CopyNameDictionary(Node* dictionary,
24642465
Node* CodeStubAssembler::AllocateStruct(Node* map, AllocationFlags flags) {
24652466
Comment("AllocateStruct");
24662467
CSA_ASSERT(this, IsMap(map));
2467-
Node* size = TimesPointerSize(LoadMapInstanceSize(map));
2468+
Node* size = TimesPointerSize(LoadMapInstanceSizeInWords(map));
24682469
Node* object = Allocate(size, flags);
24692470
StoreMapNoWriteBarrier(object, map);
24702471
InitializeStructBody(object, map, size, Struct::kHeaderSize);
@@ -2492,7 +2493,7 @@ Node* CodeStubAssembler::AllocateJSObjectFromMap(
24922493
CSA_ASSERT(this, Word32BinaryNot(IsJSFunctionMap(map)));
24932494
CSA_ASSERT(this, Word32BinaryNot(InstanceTypeEqual(LoadMapInstanceType(map),
24942495
JS_GLOBAL_OBJECT_TYPE)));
2495-
Node* instance_size = TimesPointerSize(LoadMapInstanceSize(map));
2496+
Node* instance_size = TimesPointerSize(LoadMapInstanceSizeInWords(map));
24962497
Node* object = AllocateInNewSpace(instance_size, flags);
24972498
StoreMapNoWriteBarrier(object, map);
24982499
InitializeJSObjectFromMap(object, map, instance_size, properties, elements,
@@ -6616,19 +6617,19 @@ void CodeStubAssembler::LoadPropertyFromFastObject(Node* object, Node* map,
66166617
Node* representation =
66176618
DecodeWord32<PropertyDetails::RepresentationField>(details);
66186619

6619-
Node* inobject_properties = LoadMapInobjectProperties(map);
6620+
field_index =
6621+
IntPtrAdd(field_index, LoadMapInobjectPropertiesStartInWords(map));
6622+
Node* instance_size_in_words = LoadMapInstanceSizeInWords(map);
66206623

66216624
Label if_inobject(this), if_backing_store(this);
66226625
VARIABLE(var_double_value, MachineRepresentation::kFloat64);
66236626
Label rebox_double(this, &var_double_value);
6624-
Branch(UintPtrLessThan(field_index, inobject_properties), &if_inobject,
6627+
Branch(UintPtrLessThan(field_index, instance_size_in_words), &if_inobject,
66256628
&if_backing_store);
66266629
BIND(&if_inobject);
66276630
{
66286631
Comment("if_inobject");
6629-
Node* field_offset = TimesPointerSize(
6630-
IntPtrAdd(IntPtrSub(LoadMapInstanceSize(map), inobject_properties),
6631-
field_index));
6632+
Node* field_offset = TimesPointerSize(field_index);
66326633

66336634
Label if_double(this), if_tagged(this);
66346635
Branch(Word32NotEqual(representation,
@@ -6655,7 +6656,7 @@ void CodeStubAssembler::LoadPropertyFromFastObject(Node* object, Node* map,
66556656
{
66566657
Comment("if_backing_store");
66576658
Node* properties = LoadFastProperties(object);
6658-
field_index = IntPtrSub(field_index, inobject_properties);
6659+
field_index = IntPtrSub(field_index, instance_size_in_words);
66596660
Node* value = LoadFixedArrayElement(properties, field_index);
66606661

66616662
Label if_double(this), if_tagged(this);

‎src/code-stub-assembler.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -505,9 +505,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
505505
TNode<PrototypeInfo> LoadMapPrototypeInfo(SloppyTNode<Map> map,
506506
Label* if_has_no_proto_info);
507507
// Load the instance size of a Map.
508-
TNode<IntPtrT> LoadMapInstanceSize(SloppyTNode<Map> map);
509-
// Load the inobject properties count of a Map (valid only for JSObjects).
510-
TNode<IntPtrT> LoadMapInobjectProperties(SloppyTNode<Map> map);
508+
TNode<IntPtrT> LoadMapInstanceSizeInWords(SloppyTNode<Map> map);
509+
// Load the inobject properties start of a Map (valid only for JSObjects).
510+
TNode<IntPtrT> LoadMapInobjectPropertiesStartInWords(SloppyTNode<Map> map);
511511
// Load the constructor function index of a Map (only for primitive maps).
512512
TNode<IntPtrT> LoadMapConstructorFunctionIndex(SloppyTNode<Map> map);
513513
// Load the constructor of a Map (equivalent to Map::GetConstructor()).

‎src/heap/heap.cc‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2398,7 +2398,8 @@ AllocationResult Heap::AllocatePartialMap(InstanceType instance_type,
23982398
}
23992399
// GetVisitorId requires a properly initialized LayoutDescriptor.
24002400
map->set_visitor_id(Map::GetVisitorId(map));
2401-
map->set_inobject_properties_or_constructor_function_index(0);
2401+
map->set_inobject_properties_start_or_constructor_function_index(0);
2402+
DCHECK(!map->IsJSObjectMap());
24022403
map->SetInObjectUnusedPropertyFields(0);
24032404
map->set_bit_field(0);
24042405
map->set_bit_field2(0);
@@ -2425,8 +2426,14 @@ AllocationResult Heap::AllocateMap(InstanceType instance_type,
24252426
map->set_prototype(null_value(), SKIP_WRITE_BARRIER);
24262427
map->set_constructor_or_backpointer(null_value(), SKIP_WRITE_BARRIER);
24272428
map->set_instance_size(instance_size);
2428-
map->set_inobject_properties_or_constructor_function_index(
2429-
inobject_properties);
2429+
if (map->IsJSObjectMap()) {
2430+
map->SetInObjectPropertiesStartInWords(instance_size / kPointerSize -
2431+
inobject_properties);
2432+
DCHECK_EQ(map->GetInObjectProperties(), inobject_properties);
2433+
} else {
2434+
DCHECK_EQ(inobject_properties, 0);
2435+
map->set_inobject_properties_start_or_constructor_function_index(0);
2436+
}
24302437
map->set_dependent_code(DependentCode::cast(empty_fixed_array()),
24312438
SKIP_WRITE_BARRIER);
24322439
map->set_weak_cell_cache(Smi::kZero);

‎src/ic/keyed-store-generic.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -686,17 +686,17 @@ void KeyedStoreGenericAssembler::OverwriteExistingFastProperty(
686686
slow);
687687
Node* field_index =
688688
DecodeWordFromWord32<PropertyDetails::FieldIndexField>(details);
689-
Node* inobject_properties = LoadMapInobjectProperties(object_map);
689+
field_index =
690+
IntPtrAdd(field_index, LoadMapInobjectPropertiesStartInWords(object_map));
691+
Node* instance_size_in_words = LoadMapInstanceSizeInWords(object_map);
690692

691693
Label inobject(this), backing_store(this);
692-
Branch(UintPtrLessThan(field_index, inobject_properties), &inobject,
694+
Branch(UintPtrLessThan(field_index, instance_size_in_words), &inobject,
693695
&backing_store);
694696

695697
BIND(&inobject);
696698
{
697-
Node* field_offset = TimesPointerSize(IntPtrAdd(
698-
IntPtrSub(LoadMapInstanceSize(object_map), inobject_properties),
699-
field_index));
699+
Node* field_offset = TimesPointerSize(field_index);
700700
Label tagged_rep(this), double_rep(this);
701701
Branch(Word32Equal(representation, Int32Constant(Representation::kDouble)),
702702
&double_rep, &tagged_rep);
@@ -722,7 +722,7 @@ void KeyedStoreGenericAssembler::OverwriteExistingFastProperty(
722722

723723
BIND(&backing_store);
724724
{
725-
Node* backing_store_index = IntPtrSub(field_index, inobject_properties);
725+
Node* backing_store_index = IntPtrSub(field_index, instance_size_in_words);
726726
Label tagged_rep(this), double_rep(this);
727727
Branch(Word32Equal(representation, Int32Constant(Representation::kDouble)),
728728
&double_rep, &tagged_rep);

‎src/layout-descriptor-inl.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,7 @@ LayoutDescriptorHelper::LayoutDescriptorHelper(Map* map)
219219
return;
220220
}
221221

222-
int inobject_properties = map->GetInObjectProperties();
223-
DCHECK_GT(inobject_properties, 0);
224-
header_size_ = map->instance_size() - (inobject_properties * kPointerSize);
222+
header_size_ = map->GetInObjectPropertiesStartInWords() * kPointerSize;
225223
DCHECK_GE(header_size_, 0);
226224

227225
all_fields_tagged_ = false;

‎src/objects-inl.h‎

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2980,51 +2980,68 @@ void Map::set_visitor_id(VisitorId id) {
29802980
WRITE_BYTE_FIELD(this, kVisitorIdOffset, static_cast<byte>(id));
29812981
}
29822982

2983+
int Map::instance_size_in_words() const {
2984+
return RELAXED_READ_BYTE_FIELD(this, kInstanceSizeInWordsOffset);
2985+
}
2986+
2987+
void Map::set_instance_size_in_words(int value) {
2988+
RELAXED_WRITE_BYTE_FIELD(this, kInstanceSizeInWordsOffset,
2989+
static_cast<byte>(value));
2990+
}
2991+
29832992
int Map::instance_size() const {
2984-
return RELAXED_READ_BYTE_FIELD(this, kInstanceSizeOffset) << kPointerSizeLog2;
2993+
return instance_size_in_words() << kPointerSizeLog2;
29852994
}
29862995

2987-
int Map::inobject_properties_or_constructor_function_index() const {
2988-
return RELAXED_READ_BYTE_FIELD(
2989-
this, kInObjectPropertiesOrConstructorFunctionIndexOffset);
2996+
void Map::set_instance_size(int value) {
2997+
DCHECK_EQ(0, value & (kPointerSize - 1));
2998+
value >>= kPointerSizeLog2;
2999+
DCHECK(0 <= value && value < 256);
3000+
set_instance_size_in_words(value);
29903001
}
29913002

3003+
int Map::inobject_properties_start_or_constructor_function_index() const {
3004+
return RELAXED_READ_BYTE_FIELD(
3005+
this, kInObjectPropertiesStartOrConstructorFunctionIndexOffset);
3006+
}
29923007

2993-
void Map::set_inobject_properties_or_constructor_function_index(int value) {
3008+
void Map::set_inobject_properties_start_or_constructor_function_index(
3009+
int value) {
29943010
DCHECK_LE(0, value);
29953011
DCHECK_LT(value, 256);
2996-
RELAXED_WRITE_BYTE_FIELD(this,
2997-
kInObjectPropertiesOrConstructorFunctionIndexOffset,
2998-
static_cast<byte>(value));
3012+
RELAXED_WRITE_BYTE_FIELD(
3013+
this, kInObjectPropertiesStartOrConstructorFunctionIndexOffset,
3014+
static_cast<byte>(value));
29993015
}
30003016

3001-
int Map::GetInObjectProperties() const {
3017+
int Map::GetInObjectPropertiesStartInWords() const {
30023018
DCHECK(IsJSObjectMap());
3003-
return inobject_properties_or_constructor_function_index();
3019+
return inobject_properties_start_or_constructor_function_index();
30043020
}
30053021

3022+
void Map::SetInObjectPropertiesStartInWords(int value) {
3023+
DCHECK(IsJSObjectMap());
3024+
set_inobject_properties_start_or_constructor_function_index(value);
3025+
}
30063026

3007-
void Map::SetInObjectProperties(int value) {
3027+
int Map::GetInObjectProperties() const {
30083028
DCHECK(IsJSObjectMap());
3009-
set_inobject_properties_or_constructor_function_index(value);
3029+
return instance_size_in_words() - GetInObjectPropertiesStartInWords();
30103030
}
30113031

30123032
int Map::GetConstructorFunctionIndex() const {
30133033
DCHECK(IsPrimitiveMap());
3014-
return inobject_properties_or_constructor_function_index();
3034+
return inobject_properties_start_or_constructor_function_index();
30153035
}
30163036

30173037

30183038
void Map::SetConstructorFunctionIndex(int value) {
30193039
DCHECK(IsPrimitiveMap());
3020-
set_inobject_properties_or_constructor_function_index(value);
3040+
set_inobject_properties_start_or_constructor_function_index(value);
30213041
}
30223042

30233043
int Map::GetInObjectPropertyOffset(int index) const {
3024-
// Adjust for the number of properties stored in the object.
3025-
index -= GetInObjectProperties();
3026-
DCHECK_LE(index, 0);
3027-
return instance_size() + (index * kPointerSize);
3044+
return (GetInObjectPropertiesStartInWords() + index) * kPointerSize;
30283045
}
30293046

30303047

@@ -3099,15 +3116,6 @@ int HeapObject::SizeFromMap(Map* map) const {
30993116
return reinterpret_cast<const Code*>(this)->CodeSize();
31003117
}
31013118

3102-
3103-
void Map::set_instance_size(int value) {
3104-
DCHECK_EQ(0, value & (kPointerSize - 1));
3105-
value >>= kPointerSizeLog2;
3106-
DCHECK(0 <= value && value < 256);
3107-
RELAXED_WRITE_BYTE_FIELD(this, kInstanceSizeOffset, static_cast<byte>(value));
3108-
}
3109-
3110-
31113119
InstanceType Map::instance_type() const {
31123120
return static_cast<InstanceType>(READ_BYTE_FIELD(this, kInstanceTypeOffset));
31133121
}
@@ -3195,7 +3203,7 @@ void Map::AccountAddedPropertyField() {
31953203
// Update used_instance_size_in_words.
31963204
int value = used_instance_size_in_words();
31973205
if (value >= JSObject::kFieldsAdded) {
3198-
if (value == instance_size() / kPointerSize) {
3206+
if (value == instance_size_in_words()) {
31993207
AccountAddedOutOfObjectPropertyField(0);
32003208
} else {
32013209
// The property is added in-object, so simply increment the counter.
@@ -3225,7 +3233,7 @@ void Map::VerifyUnusedPropertyFields() const {
32253233
int value = used_instance_size_in_words();
32263234
int unused;
32273235
if (value >= JSObject::kFieldsAdded) {
3228-
unused = instance_size() / kPointerSize - value;
3236+
unused = instance_size_in_words() - value;
32293237
} else {
32303238
// For out of object properties "used_instance_size_in_words" byte encodes
32313239
// the slack in the property array.

‎src/objects.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9579,7 +9579,8 @@ Handle<Map> Map::Create(Isolate* isolate, int inobject_properties) {
95799579

95809580
// Adjust the map with the extra inobject properties.
95819581
copy->set_instance_size(new_instance_size);
9582-
copy->SetInObjectProperties(inobject_properties);
9582+
copy->SetInObjectPropertiesStartInWords(JSObject::kHeaderSize / kPointerSize);
9583+
DCHECK_EQ(copy->GetInObjectProperties(), inobject_properties);
95839584
copy->SetInObjectUnusedPropertyFields(inobject_properties);
95849585
copy->set_visitor_id(Map::GetVisitorId(*copy));
95859586
return copy;
@@ -12322,7 +12323,6 @@ static void ShrinkInstanceSize(Map* map, void* data) {
1232212323
#endif
1232312324
int slack = *reinterpret_cast<int*>(data);
1232412325
DCHECK_GE(slack, 0);
12325-
map->SetInObjectProperties(map->GetInObjectProperties() - slack);
1232612326
map->set_unused_property_fields(map->unused_property_fields() - slack);
1232712327
map->set_instance_size(map->instance_size() - slack * kPointerSize);
1232812328
map->set_construction_counter(Map::kNoSlackTracking);

0 commit comments

Comments
 (0)