Skip to content

Commit 0d26307

Browse files
camillobruniCommit Bot
camillobruni
authored and
Commit Bot
committed
[runtime] Properly calculate upper bound for NOF in_object_properties
Bug: chr:81499 Change-Id: I5a18b9ec061d426e21c08747a8c18a36bf5ca194 Reviewed-on: https://chromium-review.googlesource.com/950724 Commit-Queue: Camillo Bruni <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/master@{#51812}
1 parent 31f2a82 commit 0d26307

6 files changed

Lines changed: 121 additions & 3 deletions

File tree

‎src/field-index.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,8 @@ class FieldIndex final {
123123
};
124124
// Offset of first inobject property from beginning of object.
125125
class FirstInobjectPropertyOffsetBits
126-
: public BitField64<int, InObjectPropertyBits::kNext, 7> {};
126+
: public BitField64<int, InObjectPropertyBits::kNext,
127+
kFirstInobjectPropertyOffsetBitCount> {};
127128
class IsHiddenField
128129
: public BitField64<bool, FirstInobjectPropertyOffsetBits::kNext, 1> {};
129130
STATIC_ASSERT(IsHiddenField::kNext <= 64);

‎src/objects-inl.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2440,6 +2440,7 @@ int ObjectTemplateInfo::embedder_field_count() const {
24402440
}
24412441

24422442
void ObjectTemplateInfo::set_embedder_field_count(int count) {
2443+
DCHECK_LE(count, JSObject::kMaxEmbedderFields);
24432444
return set_data(
24442445
Smi::FromInt(EmbedderFieldCount::update(Smi::ToInt(data()), count)));
24452446
}

‎src/objects.cc‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13663,18 +13663,24 @@ void JSFunction::CalculateInstanceSizeHelper(InstanceType instance_type,
1366313663
int requested_in_object_properties,
1366413664
int* instance_size,
1366513665
int* in_object_properties) {
13666+
DCHECK_LE(static_cast<unsigned>(requested_embedder_fields),
13667+
JSObject::kMaxEmbedderFields);
1366613668
int header_size = JSObject::GetHeaderSize(instance_type, has_prototype_slot);
1366713669
int max_nof_fields =
1366813670
(JSObject::kMaxInstanceSize - header_size) >> kPointerSizeLog2;
1366913671
CHECK_LE(max_nof_fields, JSObject::kMaxInObjectProperties);
13670-
*in_object_properties = Min(requested_in_object_properties, max_nof_fields);
13671-
CHECK_LE(requested_embedder_fields, max_nof_fields - *in_object_properties);
13672+
CHECK_LE(static_cast<unsigned>(requested_embedder_fields),
13673+
static_cast<unsigned>(max_nof_fields));
13674+
*in_object_properties = Min(requested_in_object_properties,
13675+
max_nof_fields - requested_embedder_fields);
1367213676
*instance_size =
1367313677
header_size +
1367413678
((requested_embedder_fields + *in_object_properties) << kPointerSizeLog2);
1367513679
CHECK_EQ(*in_object_properties,
1367613680
((*instance_size - header_size) >> kPointerSizeLog2) -
1367713681
requested_embedder_fields);
13682+
CHECK_LE(static_cast<unsigned>(*instance_size),
13683+
static_cast<unsigned>(JSObject::kMaxInstanceSize));
1367813684
}
1367913685

1368013686
// static

‎src/objects.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2646,6 +2646,11 @@ class JSObject: public JSReceiver {
26462646
static const int kMaxInObjectProperties =
26472647
(kMaxInstanceSize - kHeaderSize) >> kPointerSizeLog2;
26482648
STATIC_ASSERT(kMaxInObjectProperties <= kMaxNumberOfDescriptors);
2649+
// TODO(cbruni): Revisit calculation of the max supported embedder fields.
2650+
static const int kMaxEmbedderFields =
2651+
((1 << kFirstInobjectPropertyOffsetBitCount) - 1 - kHeaderSize) >>
2652+
kPointerSizeLog2;
2653+
STATIC_ASSERT(kMaxEmbedderFields <= kMaxInObjectProperties);
26492654

26502655
class BodyDescriptor;
26512656
// No weak fields.

‎src/property-details.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ class Representation {
197197

198198

199199
static const int kDescriptorIndexBitCount = 10;
200+
static const int kFirstInobjectPropertyOffsetBitCount = 7;
200201
// The maximum number of descriptors we want in a descriptor array. It should
201202
// fit in a page and also the following should hold:
202203
// kMaxNumberOfDescriptors + kFieldsAdded <= PropertyArray::kMaxLength.

‎test/cctest/test-api.cc‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2730,6 +2730,110 @@ THREADED_TEST(InternalFields) {
27302730
CHECK_EQ(17, obj->GetInternalField(0)->Int32Value(env.local()).FromJust());
27312731
}
27322732

2733+
TEST(InternalFieldsSubclassing) {
2734+
LocalContext env;
2735+
v8::Isolate* isolate = env->GetIsolate();
2736+
v8::HandleScope scope(isolate);
2737+
for (int nof_embedder_fields = 0;
2738+
nof_embedder_fields < i::JSObject::kMaxEmbedderFields;
2739+
nof_embedder_fields++) {
2740+
Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
2741+
Local<v8::ObjectTemplate> instance_templ = templ->InstanceTemplate();
2742+
instance_templ->SetInternalFieldCount(nof_embedder_fields);
2743+
Local<Function> constructor =
2744+
templ->GetFunction(env.local()).ToLocalChecked();
2745+
// Check that instances have the correct NOF properties.
2746+
Local<v8::Object> obj =
2747+
constructor->NewInstance(env.local()).ToLocalChecked();
2748+
2749+
i::Handle<i::JSObject> i_obj =
2750+
i::Handle<i::JSObject>::cast(v8::Utils::OpenHandle(*obj));
2751+
CHECK_EQ(nof_embedder_fields, obj->InternalFieldCount());
2752+
CHECK_EQ(0, i_obj->map()->GetInObjectProperties());
2753+
// Check writing and reading internal fields.
2754+
for (int j = 0; j < nof_embedder_fields; j++) {
2755+
CHECK(obj->GetInternalField(j)->IsUndefined());
2756+
int value = 17 + j;
2757+
obj->SetInternalField(j, v8_num(value));
2758+
}
2759+
for (int j = 0; j < nof_embedder_fields; j++) {
2760+
int value = 17 + j;
2761+
CHECK_EQ(value,
2762+
obj->GetInternalField(j)->Int32Value(env.local()).FromJust());
2763+
}
2764+
CHECK(env->Global()
2765+
->Set(env.local(), v8_str("BaseClass"), constructor)
2766+
.FromJust());
2767+
// Create various levels of subclasses to stress instance size calculation.
2768+
const int kMaxNofProperties =
2769+
i::JSObject::kMaxInObjectProperties - nof_embedder_fields;
2770+
// Select only a few values to speed up the test.
2771+
int sizes[] = {0,
2772+
1,
2773+
2,
2774+
3,
2775+
4,
2776+
5,
2777+
6,
2778+
kMaxNofProperties / 4,
2779+
kMaxNofProperties / 2,
2780+
kMaxNofProperties - 2,
2781+
kMaxNofProperties - 1,
2782+
kMaxNofProperties + 1,
2783+
kMaxNofProperties + 2,
2784+
kMaxNofProperties * 2,
2785+
kMaxNofProperties * 2};
2786+
for (size_t i = 0; i < arraysize(sizes); i++) {
2787+
int nof_properties = sizes[i];
2788+
bool in_object_only = nof_properties <= kMaxNofProperties;
2789+
std::ostringstream src;
2790+
// Assembler source string for a subclass with {nof_properties}
2791+
// in-object properties.
2792+
src << "(function() {\n"
2793+
<< " class SubClass extends BaseClass {\n"
2794+
<< " constructor() {\n"
2795+
<< " super();\n";
2796+
// Set {nof_properties} instance properties in the constructor.
2797+
for (int j = 0; j < nof_properties; j++) {
2798+
src << " this.property" << j << " = " << j << ";\n";
2799+
}
2800+
src << " }\n"
2801+
<< " };\n"
2802+
<< " let instance;\n"
2803+
<< " for (let i = 0; i < 3; i++) {\n"
2804+
<< " instance = new SubClass();\n"
2805+
<< " }"
2806+
<< " return instance;\n"
2807+
<< "})();";
2808+
Local<v8::Object> value = CompileRun(src.str().c_str()).As<v8::Object>();
2809+
2810+
i::Handle<i::JSObject> i_value =
2811+
i::Handle<i::JSObject>::cast(v8::Utils::OpenHandle(*value));
2812+
#ifdef VERIFY_HEAP
2813+
i_value->HeapObjectVerify();
2814+
i_value->map()->HeapObjectVerify();
2815+
i_value->map()->FindRootMap()->HeapObjectVerify();
2816+
#endif
2817+
CHECK_EQ(nof_embedder_fields, value->InternalFieldCount());
2818+
if (in_object_only) {
2819+
CHECK_LE(nof_properties, i_value->map()->GetInObjectProperties());
2820+
} else {
2821+
CHECK_LE(kMaxNofProperties, i_value->map()->GetInObjectProperties());
2822+
}
2823+
2824+
// Make Sure we get the precise property count.
2825+
i_value->map()->FindRootMap()->CompleteInobjectSlackTracking();
2826+
// TODO(cbruni): fix accounting to make this condition true.
2827+
// CHECK_EQ(0, i_value->map()->UnusedPropertyFields());
2828+
if (in_object_only) {
2829+
CHECK_EQ(nof_properties, i_value->map()->GetInObjectProperties());
2830+
} else {
2831+
CHECK_LE(kMaxNofProperties, i_value->map()->GetInObjectProperties());
2832+
}
2833+
}
2834+
}
2835+
}
2836+
27332837
THREADED_TEST(InternalFieldsOfRegularObjects) {
27342838
LocalContext env;
27352839
v8::Isolate* isolate = env->GetIsolate();

0 commit comments

Comments
 (0)