From bc5641f30c30ba8940abc78fb794afde877c7bdc Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Mon, 7 Oct 2019 15:23:11 -0700 Subject: [PATCH] src: v12.x-compatible DescriptorArray V8 changed DescriptorArray from a FixedArray to a proper HeapObject. These changes update accessors for DescriptorArray fields to make them compatible with FixedArray-like and HeapObject-like access. Ref: https://github.com/nodejs/llnode/issues/255 --- src/llscan.cc | 6 ++-- src/llv8-constants.cc | 15 ++++++---- src/llv8-constants.h | 12 ++++---- src/llv8-inl.h | 48 +++++++++++++++++++++---------- src/llv8.cc | 51 ++++++++++++++++++++------------ src/llv8.h | 9 ++++-- src/printer.cc | 67 +++++++++++++++++++++++++++++-------------- 7 files changed, 137 insertions(+), 71 deletions(-) diff --git a/src/llscan.cc b/src/llscan.cc index 6755d57a..7162878c 100644 --- a/src/llscan.cc +++ b/src/llscan.cc @@ -1563,7 +1563,7 @@ bool FindJSObjectsVisitor::MapCacheEntry::Load(v8::Map map, if (is_histogram) type_name = heap_object.GetTypeName(err); v8::HeapObject descriptors_obj = map.InstanceDescriptors(err); - if (err.Fail()) return false; + RETURN_IF_INVALID(descriptors_obj, false); v8::DescriptorArray descriptors(descriptors_obj); own_descriptors_count_ = map.NumberOfOwnDescriptors(err); @@ -1579,8 +1579,8 @@ bool FindJSObjectsVisitor::MapCacheEntry::Load(v8::Map map, } for (uint64_t i = 0; i < own_descriptors_count_; i++) { - v8::Value key = descriptors.GetKey(i, err); - if (err.Fail()) continue; + v8::Value key = descriptors.GetKey(i); + if (!key.Check()) continue; properties_.emplace_back(key.ToString(err)); } diff --git a/src/llv8-constants.cc b/src/llv8-constants.cc index 2f1f4684..4ee16fe8 100644 --- a/src/llv8-constants.cc +++ b/src/llv8-constants.cc @@ -95,6 +95,7 @@ void Map::Load() { "class_Map__constructor__Object"); kInstanceDescriptorsOffset = LoadConstant({ "class_Map__instance_descriptors__DescriptorArray", + "class_Map__instance_descriptors_offset", }); kBitField3Offset = LoadConstant("class_Map__bit_field3__int", "class_Map__bit_field3__SMI"); @@ -403,9 +404,9 @@ void JSArrayBufferView::Load() { void DescriptorArray::Load() { - kDetailsOffset = LoadConstant("prop_desc_details"); - kKeyOffset = LoadConstant("prop_desc_key"); - kValueOffset = LoadConstant("prop_desc_value"); + kDetailsOffset = LoadConstant({"prop_desc_details"}); + kKeyOffset = LoadConstant({"prop_desc_key"}); + kValueOffset = LoadConstant({"prop_desc_value"}); kPropertyIndexMask = LoadConstant("prop_index_mask"); kPropertyIndexShift = LoadConstant("prop_index_shift"); @@ -455,8 +456,12 @@ void DescriptorArray::Load() { kRepresentationDouble = 7; } - kFirstIndex = LoadConstant("prop_idx_first"); - kSize = LoadConstant("prop_desc_size"); + // NOTE(mmarchini): removed from V8 7.2. + // https://github.com/v8/v8/commit/1ad0cd5 + kFirstIndex = LoadOptionalConstant({"prop_idx_first"}, 0); + kSize = LoadConstant({"prop_desc_size"}); + kHeaderSize = LoadOptionalConstant( + {"class_DescriptorArray__header_size__uintptr_t"}, 0); } diff --git a/src/llv8-constants.h b/src/llv8-constants.h index cf297419..bfecf2f1 100644 --- a/src/llv8-constants.h +++ b/src/llv8-constants.h @@ -406,9 +406,9 @@ class DescriptorArray : public Module { public: CONSTANTS_DEFAULT_METHODS(DescriptorArray); - int64_t kDetailsOffset; - int64_t kKeyOffset; - int64_t kValueOffset; + Constant kDetailsOffset; + Constant kKeyOffset; + Constant kValueOffset; int64_t kPropertyIndexMask; int64_t kPropertyIndexShift; @@ -417,8 +417,10 @@ class DescriptorArray : public Module { int64_t kRepresentationDouble; - int64_t kFirstIndex; - int64_t kSize; + Constant kFirstIndex; + Constant kHeaderSize; + Constant kSize; + Constant kEntrySize; // node.js <= 7 int64_t kPropertyTypeMask = -1; diff --git a/src/llv8-inl.h b/src/llv8-inl.h index beda40cb..31ba309f 100644 --- a/src/llv8-inl.h +++ b/src/llv8-inl.h @@ -271,6 +271,7 @@ inline bool Context::IsContext(LLV8* v8, HeapObject heap_object, Error& err) { } inline int64_t Map::InObjectProperties(Error& err) { + RETURN_IF_THIS_INVALID(-1); if (!IsJSObjectMap(err)) { err = Error::Failure( "Invalid call to Map::InObjectProperties with a non-JsObject type"); @@ -724,25 +725,42 @@ inline T FixedArray::Get(int index, Error& err) { return LoadFieldValue(off, err); } -inline Smi DescriptorArray::GetDetails(int index, Error& err) { - return Get(v8()->descriptor_array()->kFirstIndex + - index * v8()->descriptor_array()->kSize + - v8()->descriptor_array()->kDetailsOffset, - err); +template +inline T DescriptorArray::Get(int index, int64_t offset) { + // TODO(mmarchini): shouldn't need Error here. + Error err; + RETURN_IF_INVALID(v8()->descriptor_array()->kSize, T()); + + index = index * *(v8()->descriptor_array()->kSize); + if (v8()->descriptor_array()->kFirstIndex.Loaded()) { + return FixedArray::Get( + *(v8()->descriptor_array()->kFirstIndex) + index + offset, err); + } else if (v8()->descriptor_array()->kHeaderSize.Loaded()) { + index *= v8()->common()->kPointerSize; + index += *(v8()->descriptor_array()->kHeaderSize); + index += (v8()->common()->kPointerSize * offset); + return LoadFieldValue(index, err); + } else { + PRINT_DEBUG( + "Missing FirstIndex and HeaderSize constants, can't get key from " + "DescriptorArray"); + return T(); + } +} + +inline Smi DescriptorArray::GetDetails(int index) { + RETURN_IF_INVALID(v8()->descriptor_array()->kDetailsOffset, Smi()); + return Get(index, *v8()->descriptor_array()->kDetailsOffset); } -inline Value DescriptorArray::GetKey(int index, Error& err) { - return Get(v8()->descriptor_array()->kFirstIndex + - index * v8()->descriptor_array()->kSize + - v8()->descriptor_array()->kKeyOffset, - err); +inline Value DescriptorArray::GetKey(int index) { + RETURN_IF_INVALID(v8()->descriptor_array()->kKeyOffset, Value()); + return Get(index, *v8()->descriptor_array()->kKeyOffset); } -inline Value DescriptorArray::GetValue(int index, Error& err) { - return Get(v8()->descriptor_array()->kFirstIndex + - index * v8()->descriptor_array()->kSize + - v8()->descriptor_array()->kValueOffset, - err); +inline Value DescriptorArray::GetValue(int index) { + RETURN_IF_INVALID(v8()->descriptor_array()->kValueOffset, Value()); + return Get(index, *v8()->descriptor_array()->kValueOffset); } inline bool DescriptorArray::IsDescriptorDetails(Smi details) { diff --git a/src/llv8.cc b/src/llv8.cc index b783a2d2..9bf9c358 100644 --- a/src/llv8.cc +++ b/src/llv8.cc @@ -571,6 +571,8 @@ std::string Value::GetTypeName(Error& err) { std::string Value::ToString(Error& err) { + RETURN_IF_THIS_INVALID(std::string()); + Smi smi(this); if (smi.Check()) return smi.ToString(err); @@ -922,7 +924,7 @@ std::vector> JSObject::DictionaryEntries(Error& err) { std::vector> JSObject::DescriptorEntries(Map map, Error& err) { HeapObject descriptors_obj = map.InstanceDescriptors(err); - if (err.Fail()) return {}; + RETURN_IF_INVALID(descriptors_obj, {}); DescriptorArray descriptors(descriptors_obj); @@ -942,18 +944,22 @@ std::vector> JSObject::DescriptorEntries(Map map, std::vector> entries; for (int64_t i = 0; i < own_descriptors_count; i++) { - Smi details = descriptors.GetDetails(i, err); - if (err.Fail()) continue; + Smi details = descriptors.GetDetails(i); + if (!details.Check()) { + PRINT_DEBUG("Failed to get details for index %ld", i); + entries.push_back(std::pair(Value(), Value())); + continue; + } - Value key = descriptors.GetKey(i, err); - if (err.Fail()) continue; + Value key = descriptors.GetKey(i); + if (!key.Check()) continue; if (descriptors.IsConstFieldDetails(details) || descriptors.IsDescriptorDetails(details)) { Value value; - value = descriptors.GetValue(i, err); - if (err.Fail()) continue; + value = descriptors.GetValue(i); + if (!value.Check()) continue; entries.push_back(std::pair(key, value)); continue; @@ -1037,18 +1043,22 @@ void JSObject::DictionaryKeys(std::vector& keys, Error& err) { void JSObject::DescriptorKeys(std::vector& keys, Map map, Error& err) { HeapObject descriptors_obj = map.InstanceDescriptors(err); - if (err.Fail()) return; + RETURN_IF_INVALID(descriptors_obj, ); DescriptorArray descriptors(descriptors_obj); int64_t own_descriptors_count = map.NumberOfOwnDescriptors(err); if (err.Fail()) return; for (int64_t i = 0; i < own_descriptors_count; i++) { - Smi details = descriptors.GetDetails(i, err); - if (err.Fail()) return; + Smi details = descriptors.GetDetails(i); + if (!details.Check()) { + PRINT_DEBUG("Failed to get details for index %ld", i); + keys.push_back("???"); + continue; + } - Value key = descriptors.GetKey(i, err); - if (err.Fail()) return; + Value key = descriptors.GetKey(i); + RETURN_IF_INVALID(key, ); // Skip non-fields for now, Object.keys(obj) does // not seem to return these (for example the "length" @@ -1124,7 +1134,7 @@ Value JSObject::GetDictionaryProperty(std::string key_name, Error& err) { Value JSObject::GetDescriptorProperty(std::string key_name, Map map, Error& err) { HeapObject descriptors_obj = map.InstanceDescriptors(err); - if (err.Fail()) return Value(); + RETURN_IF_INVALID(descriptors_obj, Value()); DescriptorArray descriptors(descriptors_obj); int64_t own_descriptors_count = map.NumberOfOwnDescriptors(err); @@ -1142,11 +1152,14 @@ Value JSObject::GetDescriptorProperty(std::string key_name, Map map, FixedArray extra_properties(extra_properties_obj); for (int64_t i = 0; i < own_descriptors_count; i++) { - Smi details = descriptors.GetDetails(i, err); - if (err.Fail()) return Value(); + Smi details = descriptors.GetDetails(i); + if (!details.Check()) { + PRINT_DEBUG("Failed to get details for index %ld", i); + continue; + } - Value key = descriptors.GetKey(i, err); - if (err.Fail()) return Value(); + Value key = descriptors.GetKey(i); + RETURN_IF_INVALID(key, Value()); if (key.ToString(err) != key_name) { continue; @@ -1159,8 +1172,8 @@ Value JSObject::GetDescriptorProperty(std::string key_name, Map map, descriptors.IsDescriptorDetails(details)) { Value value; - value = descriptors.GetValue(i, err); - if (err.Fail()) return Value(); + value = descriptors.GetValue(i); + RETURN_IF_INVALID(value, Value()); continue; } diff --git a/src/llv8.h b/src/llv8.h index 5c8706c6..f54ef323 100644 --- a/src/llv8.h +++ b/src/llv8.h @@ -455,11 +455,14 @@ class DescriptorArray : public FixedArray { public: V8_VALUE_DEFAULT_METHODS(DescriptorArray, FixedArray) - inline Smi GetDetails(int index, Error& err); - inline Value GetKey(int index, Error& err); + template + inline T Get(int index, int64_t offset); + + inline Smi GetDetails(int index); + inline Value GetKey(int index); // NOTE: Only for DATA_CONSTANT - inline Value GetValue(int index, Error& err); + inline Value GetValue(int index); inline bool IsFieldDetails(Smi details); inline bool IsDescriptorDetails(Smi details); diff --git a/src/printer.cc b/src/printer.cc index 507e5c3b..58d1f848 100644 --- a/src/printer.cc +++ b/src/printer.cc @@ -467,9 +467,7 @@ std::string Printer::Stringify(v8::JSArrayBufferView js_array_buffer_view, template <> std::string Printer::Stringify(v8::Map map, Error& err) { - v8::HeapObject descriptors_obj = map.InstanceDescriptors(err); - if (err.Fail()) return std::string(); - + // TODO(mmarchini): don't fail if can't load NumberOfOwnDescriptors int64_t own_descriptors_count = map.NumberOfOwnDescriptors(err); if (err.Fail()) return std::string(); @@ -492,25 +490,41 @@ std::string Printer::Stringify(v8::Map map, Error& err) { char tmp[256]; std::stringstream ss; ss << rang::fg::yellow - << "(own_descriptors_count), in_object_properties_or_constructor.c_str(), static_cast(in_object_properties_or_constructor_index), - static_cast(instance_size), descriptors_obj.raw()); + static_cast(instance_size)); if (!options_.detailed) { return std::string(tmp) + ">"; } - v8::DescriptorArray descriptors(descriptors_obj); - if (err.Fail()) return std::string(); + if (descriptors_obj.Check()) { + v8::DescriptorArray descriptors(descriptors_obj); + if (err.Fail()) return std::string(); - return std::string(tmp) + ":" + Stringify(descriptors, err) + - ">"; + return std::string(tmp) + ":" + + Stringify(descriptors, err) + ">"; + } else { + std::string(tmp) + ">"; + } } template <> @@ -965,7 +979,7 @@ std::string Printer::StringifyDictionary(v8::JSObject js_object, Error& err) { std::string Printer::StringifyDescriptors(v8::JSObject js_object, v8::Map map, Error& err) { v8::HeapObject descriptors_obj = map.InstanceDescriptors(err); - if (err.Fail()) return std::string(); + RETURN_IF_INVALID(descriptors_obj, std::string()); v8::DescriptorArray descriptors(descriptors_obj); int64_t own_descriptors_count = map.NumberOfOwnDescriptors(err); @@ -987,26 +1001,37 @@ std::string Printer::StringifyDescriptors(v8::JSObject js_object, v8::Map map, std::string res; std::stringstream ss; for (int64_t i = 0; i < own_descriptors_count; i++) { - v8::Smi details = descriptors.GetDetails(i, err); - if (err.Fail()) return std::string(); - - v8::Value key = descriptors.GetKey(i, err); - if (err.Fail()) return std::string(); - if (!res.empty()) res += ",\n"; + v8::Value key = descriptors.GetKey(i); + ss.str(""); ss.clear(); - ss << rang::style::bold << rang::fg::yellow << " ." + key.ToString(err) - << rang::fg::reset << rang::style::reset; + ss << rang::style::bold << rang::fg::yellow << " ."; + if (key.Check()) { + ss << key.ToString(err); + } else { + PRINT_DEBUG("Failed to get key for index %ld", i); + ss << "???"; + } + ss << rang::fg::reset << rang::style::reset; + res += ss.str() + "="; if (err.Fail()) return std::string(); + v8::Smi details = descriptors.GetDetails(i); + if (!details.Check()) { + PRINT_DEBUG("Failed to get details for index %ld", i); + res += "???"; + continue; + } + if (descriptors.IsConstFieldDetails(details) || descriptors.IsDescriptorDetails(details)) { v8::Value value; - value = descriptors.GetValue(i, err); + value = descriptors.GetValue(i); + RETURN_IF_INVALID(value, std::string()); if (err.Fail()) return std::string(); res += printer.Stringify(value, err);