Use span instead of unsized pointer for SampleVector counts
The PersistentMemoryAllocator::Get() method currently allocates some
memory and returns a pointer to the front of it. Instead, return a
span. Since the span should know the actual type, template the Get()
method and have it CHECK that the returned memory will be aligned
appropriately.
Then SampleVector subclasses pass along the span up to
SampleVectorBase so that when it iterates over the counts, it can
do so knowing the bounds of the array and preventing any possibility
of going out of bounds.
The null pointer atomic is converted to an optional<span> since it
null to encode whether the atomic had ever been set or not, but also
allows it to be set with an empty array.
[email protected]
Bug: 1490484
Change-Id: I79decf962a0a2d83cbaa5f7e24448bef477ec22c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5189273
Reviewed-by: Luc Nguyen <[email protected]>
Commit-Queue: Alexei Svitkine <[email protected]>
Reviewed-by: Tom Sepez <[email protected]>
Reviewed-by: Alexei Svitkine <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1252186}
diff --git a/base/metrics/persistent_memory_allocator.h b/base/metrics/persistent_memory_allocator.h
index 18401f9b..bbedaa5 100644
--- a/base/metrics/persistent_memory_allocator.h
+++ b/base/metrics/persistent_memory_allocator.h
@@ -15,6 +15,7 @@
#include "base/base_export.h"
#include "base/check.h"
#include "base/check_op.h"
+#include "base/containers/span.h"
#include "base/files/file_path.h"
#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
@@ -104,6 +105,18 @@
public:
typedef uint32_t Reference;
+ // All allocations and data-structures must be aligned to this byte boundary.
+ // Alignment as large as the physical bus between CPU and RAM is _required_
+ // for some architectures, is simply more efficient on other CPUs, and
+ // generally a Good Idea(tm) for all platforms as it reduces/eliminates the
+ // chance that a type will span cache lines. Alignment mustn't be less
+ // than 8 to ensure proper alignment for all types. The rest is a balance
+ // between reducing spans across multiple cache lines and wasted space spent
+ // padding out allocations. An alignment of 16 would ensure that the block
+ // header structure always sits in a single cache line. An average of about
+ // 1/2 this value will be wasted with every allocation.
+ static constexpr size_t kAllocAlignment = 8;
+
// These states are used to indicate the overall condition of the memory
// segment irrespective of what is stored within it. Because the data is
// often persistent and thus needs to be readable by different versions of
@@ -536,6 +549,7 @@
// an "object" interface similar to new and delete.
// Reserve space in the memory segment of the desired |size| and |type_id|.
+ //
// A return value of zero indicates the allocation failed, otherwise the
// returned reference can be used by any process to get a real pointer via
// the GetAsObject() or GetAsArray calls. The actual allocated size may be
@@ -550,6 +564,7 @@
// when the last field is actually variable length.
template <typename T>
T* New(size_t size) {
+ static_assert(alignof(T) <= kAllocAlignment);
if (size < sizeof(T))
size = sizeof(T);
Reference ref = Allocate(size, T::kPersistentTypeId);
@@ -682,17 +697,6 @@
private:
struct SharedMetadata;
struct BlockHeader;
- // All allocations and data-structures must be aligned to this byte boundary.
- // Alignment as large as the physical bus between CPU and RAM is _required_
- // for some architectures, is simply more efficient on other CPUs, and
- // generally a Good Idea(tm) for all platforms as it reduces/eliminates the
- // chance that a type will span cache lines. Alignment mustn't be less
- // than 8 to ensure proper alignment for all types. The rest is a balance
- // between reducing spans across multiple cache lines and wasted space spent
- // padding out allocations. An alignment of 16 would ensure that the block
- // header structure always sits in a single cache line. An average of about
- // 1/2 this value will be wasted with every allocation.
- static constexpr size_t kAllocAlignment = 8;
static const Reference kReferenceQueue;
// The shared metadata is always located at the top of the memory segment.
@@ -924,15 +928,26 @@
size_t offset = 0);
~DelayedPersistentAllocation();
- // Gets a pointer to the defined allocation. This will realize the request
+ // Gets a span to the defined allocation. This will realize the request
// and update the reference provided during construction. The memory will
// be zeroed the first time it is returned, after that it is shared with
// all other Get() requests and so shows any changes made to it elsewhere.
//
- // If the allocation fails for any reason, null will be returned. This works
- // even on "const" objects because the allocation is already defined, just
- // delayed.
- void* Get() const;
+ // If the allocation fails for any reason, an empty span will be returned.
+ // This works even on "const" objects because the allocation is already
+ // defined, just delayed.
+ template <typename T>
+ span<T> Get() const {
+ // PersistentMemoryAllocator only supports types with alignment at most
+ // kAllocAlignment.
+ static_assert(alignof(T) <= PersistentMemoryAllocator::kAllocAlignment);
+ // The offset must be a multiple of the alignment or misaligned pointers
+ // will result.
+ CHECK_EQ(offset_ % alignof(T), 0u);
+ span<uint8_t> untyped = GetUntyped();
+ return make_span(reinterpret_cast<T*>(untyped.data()),
+ untyped.size() / sizeof(T));
+ }
// Gets the internal reference value. If this returns a non-zero value then
// a subsequent call to Get() will do nothing but convert that reference into
@@ -943,6 +958,8 @@
}
private:
+ span<uint8_t> GetUntyped() const;
+
// The underlying object that does the actual allocation of memory. Its
// lifetime must exceed that of all DelayedPersistentAllocation objects
// that use it.
