[vm] Attempt to avoid allocating non-Ptr fields with Object::null().

There have been multiple cases in the past where using Object::null()
as the initializer, even for non-Ptr fields, has caused nondeterministic
snapshot creation. In particular, this can happen if a non-Ptr field
is only used in some versions of an object but not others, and thus
there is no reason to worry about what the value of the field is during
runtime. Only when snapshotting can it become relevant that the field
value is a portion of the address of Object::null(), which can vary
over different runs.

Instead of initializing the entirety of allocated objects (outside
of a few rare cases) with Object::null(), only initialize the portion
of the object corresponding to object pointer fields (in most cases,
between from() and to() inclusively) to Object::null() and initialize
the rest of the object to 0.

With this change, the only special casing in Object::InitializeObject
that remains is to skip initialization for TypedData and Arrays when
the memory is guaranteed to be zero initialized and to initialize the
contents of Instructions objects with the break instruction, not 0.

Note that this behavior does not occur in the following cases, which
all involve copying an arbitrary object:

* Object::Clone(), which zero-initializes the contents of the object
  in all cases. However, the contents of the original object are then
  copied over before the thread can reach a safepoint.
* Inside the ObjectGraphCopier, which uses the old initialization
  behavior. This is safe, as any GC-important fields are immediately
  copied over, and the rest of the contents are eventually copied over
  before the caller receives the root of the copied object graph.

TEST=ci

Issue: https://github.com/dart-lang/sdk/issues/52876
Change-Id: Ib09fc562a8b6af97b509b493eb2d64109230ec35
Cq-Include-Trybots: luci.dart.try:vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-linux-release-x64-try,vm-aot-linux-product-x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312900
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This commit is contained in:
Tess Strickland 2023-07-11 10:15:58 +00:00 committed by Commit Queue
parent a0392698bf
commit 2f63acea22
5 changed files with 583 additions and 505 deletions

View file

@ -1918,20 +1918,41 @@ class Simd128MessageDeserializationCluster
: public MessageDeserializationCluster {
public:
explicit Simd128MessageDeserializationCluster(intptr_t cid)
: MessageDeserializationCluster("Simd128"), cid_(cid) {}
: MessageDeserializationCluster("Simd128"), cid_(cid) {
ASSERT(cid_ == kInt32x4Cid || cid_ == kFloat32x4Cid ||
cid_ == kFloat64x2Cid);
#if defined(DEBUG)
// If not for Int32x4, check that all the Int32x4-specific arguments used in
// ReadNodes match those for the actual class.
if (cid_ == kFloat32x4Cid) {
AssertSameStructure<Int32x4, Float32x4>();
} else if (cid_ == kFloat64x2Cid) {
AssertSameStructure<Int32x4, Float64x2>();
}
#endif
}
~Simd128MessageDeserializationCluster() {}
#if defined(DEBUG)
template <typename Expected, typename Got>
static void AssertSameStructure() {
ASSERT_EQUAL(Got::InstanceSize(), Expected::InstanceSize());
ASSERT_EQUAL(Got::ContainsCompressedPointers(),
Expected::ContainsCompressedPointers());
ASSERT_EQUAL(Object::from_offset<Got>(), Object::from_offset<Expected>());
ASSERT_EQUAL(Object::to_offset<Got>(), Object::to_offset<Expected>());
ASSERT_EQUAL(Got::value_offset(), Expected::value_offset());
}
#endif
void ReadNodes(MessageDeserializer* d) {
const intptr_t count = d->ReadUnsigned();
for (intptr_t i = 0; i < count; i++) {
ASSERT_EQUAL(Int32x4::InstanceSize(), Float32x4::InstanceSize());
ASSERT_EQUAL(Int32x4::InstanceSize(), Float64x2::InstanceSize());
ObjectPtr vector =
Object::Allocate(cid_, Int32x4::InstanceSize(), Heap::kNew,
Int32x4::ContainsCompressedPointers());
ObjectPtr vector = Object::Allocate(
cid_, Int32x4::InstanceSize(), Heap::kNew,
Int32x4::ContainsCompressedPointers(), Object::from_offset<Int32x4>(),
Object::to_offset<Int32x4>());
d->AssignRef(vector);
ASSERT_EQUAL(Int32x4::value_offset(), Float32x4::value_offset());
ASSERT_EQUAL(Int32x4::value_offset(), Float64x2::value_offset());
d->ReadBytes(&(static_cast<Int32x4Ptr>(vector)->untag()->value_),
sizeof(simd128_value_t));
}

File diff suppressed because it is too large Load diff

View file

@ -88,15 +88,20 @@ class BaseTextBuffer;
// ContainsCompressedPointers() returns the same value for AllStatic class and
// class used for handles.
#define ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(object, handle) \
static_assert(std::is_base_of<dart::handle##Ptr, dart::object##Ptr>::value, \
public: /* NOLINT */ \
using UntaggedObjectType = dart::Untagged##object; \
using ObjectPtrType = dart::object##Ptr; \
static_assert(std::is_base_of<dart::handle##Ptr, ObjectPtrType>::value, \
#object "Ptr must be a subtype of " #handle "Ptr"); \
static_assert(dart::handle::ContainsCompressedPointers() == \
dart::Untagged##object::kContainsCompressedPointers, \
UntaggedObjectType::kContainsCompressedPointers, \
"Pointer compression in Untagged" #object \
" must match pointer compression in Untagged" #handle); \
static constexpr bool ContainsCompressedPointers() { \
return dart::Untagged##object::kContainsCompressedPointers; \
}
return UntaggedObjectType::kContainsCompressedPointers; \
} \
\
private: /* NOLINT */
#define BASE_OBJECT_IMPLEMENTATION(object, super) \
public: /* NOLINT */ \
@ -701,7 +706,49 @@ class Object {
static ObjectPtr Allocate(intptr_t cls_id,
intptr_t size,
Heap::Space space,
bool compressed);
bool compressed,
uword ptr_field_start_offset,
uword ptr_field_end_offset);
// Templates of Allocate that retrieve the appropriate values to pass from
// the class.
template <typename T>
DART_FORCE_INLINE static typename T::ObjectPtrType Allocate(
Heap::Space space) {
return static_cast<typename T::ObjectPtrType>(Allocate(
T::kClassId, T::InstanceSize(), space, T::ContainsCompressedPointers(),
Object::from_offset<T>(), Object::to_offset<T>()));
}
template <typename T>
DART_FORCE_INLINE static typename T::ObjectPtrType Allocate(
Heap::Space space,
intptr_t elements) {
return static_cast<typename T::ObjectPtrType>(
Allocate(T::kClassId, T::InstanceSize(elements), space,
T::ContainsCompressedPointers(), Object::from_offset<T>(),
Object::to_offset<T>(elements)));
}
// Additional versions that also take a class_id for types like Array, Map,
// and Set that have more than one possible class id.
template <typename T>
DART_FORCE_INLINE static typename T::ObjectPtrType AllocateVariant(
intptr_t class_id,
Heap::Space space) {
return static_cast<typename T::ObjectPtrType>(Allocate(
class_id, T::InstanceSize(), space, T::ContainsCompressedPointers(),
Object::from_offset<T>(), Object::to_offset<T>()));
}
template <typename T>
DART_FORCE_INLINE static typename T::ObjectPtrType
AllocateVariant(intptr_t class_id, Heap::Space space, intptr_t elements) {
return static_cast<typename T::ObjectPtrType>(
Allocate(class_id, T::InstanceSize(elements), space,
T::ContainsCompressedPointers(), Object::from_offset<T>(),
Object::to_offset<T>(elements)));
}
static constexpr intptr_t RoundedAllocationSize(intptr_t size) {
return Utils::RoundUp(size, kObjectAlignment);
@ -818,6 +865,30 @@ class Object {
ObjectPtr ptr_; // The raw object reference.
protected:
// The first offset in an allocated object of the given type that contains a
// (possibly compressed) object pointer. Used to initialize object pointer
// fields to Object::null() instead of 0.
//
// Always returns an offset after the object header tags.
template <typename T>
DART_FORCE_INLINE static uword from_offset() {
return UntaggedObject::from_offset<typename T::UntaggedObjectType>();
}
// The last offset in an allocated object of the given type that contains a
// (possibly compressed) object pointer. Used to initialize object pointer
// fields to Object::null() instead of 0.
//
// Takes an optional argument that is the number of elements in the payload,
// which is ignored if the object never contains a payload.
//
// If there are no pointer fields in the object, then
// to_offset<T>() < from_offset<T>().
template <typename T>
DART_FORCE_INLINE static uword to_offset(intptr_t length = 0) {
return UntaggedObject::to_offset<typename T::UntaggedObjectType>(length);
}
void AddCommonObjectProperties(JSONObject* jsobj,
const char* protocol_type,
bool ref) const;
@ -831,7 +902,47 @@ class Object {
static void InitializeObject(uword address,
intptr_t id,
intptr_t size,
bool compressed);
bool compressed,
uword ptr_field_start_offset,
uword ptr_field_end_offset);
// Templates of InitializeObject that retrieve the appropriate values to pass
// from the class.
template <typename T>
DART_FORCE_INLINE static void InitializeObject(uword address) {
return InitializeObject(address, T::kClassId, T::InstanceSize(),
T::ContainsCompressedPointers(),
Object::from_offset<T>(), Object::to_offset<T>());
}
template <typename T>
DART_FORCE_INLINE static void InitializeObject(uword address,
intptr_t elements) {
return InitializeObject(address, T::kClassId, T::InstanceSize(elements),
T::ContainsCompressedPointers(),
Object::from_offset<T>(),
Object::to_offset<T>(elements));
}
// Additional versions that also take a class_id for types like Array, Map,
// and Set that have more than one possible class id.
template <typename T>
DART_FORCE_INLINE static void InitializeObjectVariant(uword address,
intptr_t class_id) {
return InitializeObject(address, class_id, T::InstanceSize(),
T::ContainsCompressedPointers(),
Object::from_offset<T>(), Object::to_offset<T>());
}
template <typename T>
DART_FORCE_INLINE static void InitializeObjectVariant(uword address,
intptr_t class_id,
intptr_t elements) {
return InitializeObject(address, class_id, T::InstanceSize(elements),
T::ContainsCompressedPointers(),
Object::from_offset<T>(),
Object::to_offset<T>(elements));
}
static void RegisterClass(const Class& cls,
const String& name,
@ -10309,8 +10420,6 @@ class StringHasher : public ValueObject {
class OneByteString : public AllStatic {
public:
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(OneByteString, String);
static uint16_t CharAt(const String& str, intptr_t index) {
ASSERT(str.IsOneByteString());
return OneByteString::CharAt(static_cast<OneByteStringPtr>(str.ptr()),
@ -10434,6 +10543,8 @@ class OneByteString : public AllStatic {
return &str.UnsafeMutableNonPointer(untag(str)->data())[0];
}
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(OneByteString, String);
friend class Class;
friend class ExternalOneByteString;
friend class FlowGraphSerializer;
@ -10449,8 +10560,6 @@ class OneByteString : public AllStatic {
class TwoByteString : public AllStatic {
public:
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(TwoByteString, String);
static uint16_t CharAt(const String& str, intptr_t index) {
ASSERT(str.IsTwoByteString());
return TwoByteString::CharAt(static_cast<TwoByteStringPtr>(str.ptr()),
@ -10558,6 +10667,8 @@ class TwoByteString : public AllStatic {
return &str.UnsafeMutableNonPointer(untag(str)->data())[0];
}
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(TwoByteString, String);
friend class Class;
friend class FlowGraphSerializer;
friend class ImageWriter;
@ -10570,8 +10681,6 @@ class TwoByteString : public AllStatic {
class ExternalOneByteString : public AllStatic {
public:
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(ExternalOneByteString, String);
static uint16_t CharAt(const String& str, intptr_t index) {
ASSERT(str.IsExternalOneByteString());
return ExternalOneByteString::CharAt(
@ -10653,6 +10762,8 @@ class ExternalOneByteString : public AllStatic {
return -kWordSize;
}
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(ExternalOneByteString, String);
friend class Class;
friend class String;
friend class StringHasher;
@ -10663,8 +10774,6 @@ class ExternalOneByteString : public AllStatic {
class ExternalTwoByteString : public AllStatic {
public:
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(ExternalTwoByteString, String);
static uint16_t CharAt(const String& str, intptr_t index) {
ASSERT(str.IsExternalTwoByteString());
return ExternalTwoByteString::CharAt(
@ -10742,6 +10851,8 @@ class ExternalTwoByteString : public AllStatic {
return -kWordSize;
}
ALLSTATIC_CONTAINS_COMPRESSED_IMPLEMENTATION(ExternalTwoByteString, String);
friend class Class;
friend class String;
friend class StringHasher;

View file

@ -255,7 +255,24 @@ ObjectPtr AllocateObject(intptr_t cid,
const intptr_t kLargeMessageThreshold = 16 * MB;
const Heap::Space space =
allocated_bytes > kLargeMessageThreshold ? Heap::kOld : Heap::kNew;
return Object::Allocate(cid, size, space, compressed);
// Mimic the old initialization behavior of Object::InitializeObject where
// the contents are initialized to Object::null(), except for TypedDataBase
// subclasses which are initialized to 0, as the contents of the original
// are translated and copied over prior to returning the object graph root.
if (IsTypedDataBaseClassId(cid)) {
return Object::Allocate(cid, size, space, compressed,
Object::from_offset<TypedDataBase>(),
Object::to_offset<TypedDataBase>());
} else {
// Remember that ptr_field_end_offset is the offset to the last Ptr
// field, not the offset just past it.
const uword ptr_field_end_offset =
size - (compressed ? kCompressedWordSize : kWordSize);
return Object::Allocate(cid, size, space, compressed,
Object::from_offset<Object>(),
ptr_field_end_offset);
}
}
DART_FORCE_INLINE
@ -2458,8 +2475,28 @@ class ObjectGraphCopier : public StackResource {
#else
const bool compressed = false;
#endif
Object::InitializeObject(reinterpret_cast<uword>(to.untag()), cid,
from.untag()->HeapSize(), compressed);
// Mimic the old initialization behavior of Object::InitializeObject
// where the contents are initialized to Object::null(), except for
// TypedDataBase subclasses which are initialized to 0, as the contents
// of the original are translated and copied over prior to returning
// the object graph root.
if (IsTypedDataBaseClassId(cid)) {
Object::InitializeObject(reinterpret_cast<uword>(to.untag()), cid,
from.untag()->HeapSize(), compressed,
Object::from_offset<TypedDataBase>(),
Object::to_offset<TypedDataBase>());
} else {
// Remember that ptr_field_end_offset is the offset to the last Ptr
// field, not the offset just past it.
const uword ptr_field_end_offset =
from.untag()->HeapSize() -
(compressed ? kCompressedWordSize : kWordSize);
Object::InitializeObject(reinterpret_cast<uword>(to.untag()), cid,
from.untag()->HeapSize(), compressed,
Object::from_offset<Object>(),
ptr_field_end_offset);
}
UpdateLengthField(cid, from, to);
}
}

View file

@ -59,6 +59,7 @@ class StackFrame;
#define VISIT_FROM(first) \
DEFINE_CONTAINS_COMPRESSED(decltype(first##_)) \
static constexpr bool kContainsPointerFields = true; \
base_ptr_type<decltype(first##_)>::type* from() { \
return reinterpret_cast<base_ptr_type<decltype(first##_)>::type*>( \
&first##_); \
@ -69,6 +70,7 @@ class StackFrame;
is_compressed_ptr<elem_type>::value, \
"Payload elements must be object pointers"); \
DEFINE_CONTAINS_COMPRESSED(elem_type) \
static constexpr bool kContainsPointerFields = true; \
base_ptr_type<elem_type>::type* from() { \
const uword payload_start = reinterpret_cast<uword>(this) + sizeof(*this); \
ASSERT(Utils::IsAligned(payload_start, sizeof(elem_type))); \
@ -77,6 +79,8 @@ class StackFrame;
#define VISIT_TO(last) \
CHECK_CONTAIN_COMPRESSED(decltype(last##_)); \
static_assert(kContainsPointerFields, \
"Must have a corresponding VISIT_FROM"); \
base_ptr_type<decltype(last##_)>::type* to(intptr_t length = 0) { \
return reinterpret_cast<base_ptr_type<decltype(last##_)>::type*>( \
&last##_); \
@ -86,6 +90,8 @@ class StackFrame;
static_assert(is_uncompressed_ptr<elem_type>::value || \
is_compressed_ptr<elem_type>::value, \
"Payload elements must be object pointers"); \
static_assert(kContainsPointerFields, \
"Must have a corresponding VISIT_FROM"); \
CHECK_CONTAIN_COMPRESSED(elem_type); \
base_ptr_type<elem_type>::type* to(intptr_t length) { \
const uword payload_start = reinterpret_cast<uword>(this) + sizeof(*this); \
@ -508,6 +514,52 @@ class UntaggedObject {
protected:
// Automatically inherited by subclasses unless overridden.
static constexpr bool kContainsCompressedPointers = false;
// Automatically inherited by subclasses unless overridden.
static constexpr bool kContainsPointerFields = false;
// The first offset in an allocated object of the given type that contains a
// (possibly compressed) object pointer. Used to initialize object pointer
// fields to Object::null() instead of 0.
//
// Always returns an offset after the object header tags.
template <typename T>
static constexpr std::enable_if_t<!T::kContainsPointerFields, uword>
from_offset();
// The first offset in an allocated object of the given untagged type that
// contains a (possibly compressed) object pointer. Used to initialize object
// pointer fields to Object::null() instead of 0.
//
// Always returns an offset after the object header tags.
template <typename T>
DART_FORCE_INLINE static std::enable_if_t<T::kContainsPointerFields, uword>
from_offset();
// The last offset in an allocated object of the given untagged type that
// contains a (possibly compressed) object pointer. Used to initialize object
// pointer fields to Object::null() instead of 0.
//
// Takes an optional argument that is the number of elements in the payload,
// which is ignored if the object never contains a payload.
//
// If there are no pointer fields in the object, then
// to_offset<T>() < from_offset<T>().
template <typename T>
static constexpr std::enable_if_t<!T::kContainsPointerFields, uword>
to_offset(intptr_t length = 0);
// The last offset in an allocated object of the given untagged type that
// contains a (possibly compressed) object pointer. Used to initialize object
// pointer fields to Object::null() instead of 0.
//
// Takes an optional argument that is the number of elements in the payload,
// which is ignored if the object never contains a payload.
//
// If there are no pointer fields in the object, then
// to_offset<T>() < from_offset<T>().
template <typename T>
DART_FORCE_INLINE static std::enable_if_t<T::kContainsPointerFields, uword>
to_offset(intptr_t length = 0);
// All writes to heap objects should ultimately pass through one of the
// methods below or their counterparts in Object, to ensure that the
@ -796,6 +848,38 @@ class UntaggedObject {
DISALLOW_IMPLICIT_CONSTRUCTORS(UntaggedObject);
};
// Note that the below templates for from_offset and to_offset for objects
// with pointer fields assume that the range from from() and to() cover all
// pointer fields. If this is not the case (e.g., the next_seen_by_gc_ field
// in WeakArray/WeakProperty/WeakReference), then specialize the definitions.
template <typename T>
constexpr std::enable_if_t<!T::kContainsPointerFields, uword>
UntaggedObject::from_offset() {
// Non-zero to ensure to_offset() < from_offset() in this case, as
// to_offset() is the offset to the last pointer field, not past it.
return sizeof(UntaggedObject);
}
template <typename T>
DART_FORCE_INLINE std::enable_if_t<T::kContainsPointerFields, uword>
UntaggedObject::from_offset() {
return reinterpret_cast<uword>(reinterpret_cast<T*>(kOffsetOfPtr)->from()) -
kOffsetOfPtr;
}
template <typename T>
constexpr std::enable_if_t<!T::kContainsPointerFields, uword>
UntaggedObject::to_offset(intptr_t length) {
return 0;
}
template <typename T>
DART_FORCE_INLINE std::enable_if_t<T::kContainsPointerFields, uword>
UntaggedObject::to_offset(intptr_t length) {
return reinterpret_cast<uword>(
reinterpret_cast<T*>(kOffsetOfPtr)->to(length)) -
kOffsetOfPtr;
}
inline intptr_t ObjectPtr::GetClassId() const {
return untag()->GetClassId();
}
@ -1748,6 +1832,14 @@ class UntaggedWeakArray : public UntaggedObject {
friend class ScavengerVisitorBase;
};
// WeakArray is special in that it has a pointer field which is not
// traversed by pointer visitors, and thus not in the range [from(),to()]:
// next_seen_by_gc, which is before the other fields.
template <>
DART_FORCE_INLINE uword UntaggedObject::from_offset<UntaggedWeakArray>() {
return OFFSET_OF(UntaggedWeakArray, next_seen_by_gc_);
}
class UntaggedCode : public UntaggedObject {
RAW_HEAP_OBJECT_IMPLEMENTATION(Code);
@ -2388,9 +2480,8 @@ class UntaggedContextScope : public UntaggedObject {
OPEN_ARRAY_START(CompressedObjectPtr, CompressedObjectPtr);
}
const VariableDesc* VariableDescAddr(intptr_t index) const {
ASSERT((index >= 0) && (index < num_variables_ + 1));
// data() points to the first component of the first descriptor.
return &(reinterpret_cast<const VariableDesc*>(data())[index]);
return reinterpret_cast<const VariableDesc*>(data()) + index;
}
#define DEFINE_ACCESSOR(type, name) \
@ -3495,6 +3586,15 @@ class UntaggedWeakProperty : public UntaggedInstance {
friend class SlowObjectCopy; // For OFFSET_OF
};
// WeakProperty is special in that it has a pointer field which is not
// traversed by pointer visitors, and thus not in the range [from(),to()]:
// next_seen_by_gc, which is after the other fields.
template <>
DART_FORCE_INLINE uword
UntaggedObject::to_offset<UntaggedWeakProperty>(intptr_t length) {
return OFFSET_OF(UntaggedWeakProperty, next_seen_by_gc_);
}
class UntaggedWeakReference : public UntaggedInstance {
RAW_HEAP_OBJECT_IMPLEMENTATION(WeakReference);
@ -3521,6 +3621,15 @@ class UntaggedWeakReference : public UntaggedInstance {
friend class SlowObjectCopy; // For OFFSET_OF
};
// WeakReference is special in that it has a pointer field which is not
// traversed by pointer visitors, and thus not in the range [from(),to()]:
// next_seen_by_gc, which is after the other fields.
template <>
DART_FORCE_INLINE uword
UntaggedObject::to_offset<UntaggedWeakReference>(intptr_t length) {
return OFFSET_OF(UntaggedWeakReference, next_seen_by_gc_);
}
class UntaggedFinalizerBase : public UntaggedInstance {
RAW_HEAP_OBJECT_IMPLEMENTATION(FinalizerBase);
@ -3632,6 +3741,15 @@ class UntaggedFinalizerEntry : public UntaggedInstance {
friend class ObjectGraph;
};
// FinalizerEntry is special in that it has a pointer field which is not
// traversed by pointer visitors, and thus not in the range [from(),to()]:
// next_seen_by_gc, which is after the other fields.
template <>
DART_FORCE_INLINE uword
UntaggedObject::to_offset<UntaggedFinalizerEntry>(intptr_t length) {
return OFFSET_OF(UntaggedFinalizerEntry, next_seen_by_gc_);
}
// MirrorReferences are used by mirrors to hold reflectees that are VM
// internal objects, such as libraries, classes, functions or types.
class UntaggedMirrorReference : public UntaggedInstance {