Align all SkRecord::alloc() calls up to at least a pointer size.
This should make the LSAN bots able to see all our pointers.
BUG=skia:
R=reed@google.com, robertphillips@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/517073002
diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index 4d590c2..bdd2e93 100644
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -300,6 +300,9 @@
#define SkAlign8(x) (((x) + 7) >> 3 << 3)
#define SkIsAlign8(x) (0 == ((x) & 7))
+#define SkAlignPtr(x) (sizeof(void*) == 8 ? SkAlign8(x) : SkAlign4(x))
+#define SkIsAlignPtr(x) (sizeof(void*) == 8 ? SkIsAlign8(x) : SkIsAlign4(x))
+
typedef uint32_t SkFourByteTag;
#define SkSetFourByteTag(a, b, c, d) (((a) << 24) | ((b) << 16) | ((c) << 8) | (d))
diff --git a/src/core/SkRecord.h b/src/core/SkRecord.h
index 96da69b..203a16c 100644
--- a/src/core/SkRecord.h
+++ b/src/core/SkRecord.h
@@ -65,7 +65,8 @@
// Here T can be any class, not just those from SkRecords. Throws on failure.
template <typename T>
T* alloc(size_t count = 1) {
- return (T*)fAlloc.allocThrow(sizeof(T) * count);
+ // Bump up to the next pointer width if needed, so all allocations start pointer-aligned.
+ return (T*)fAlloc.allocThrow(SkAlignPtr(sizeof(T) * count));
}
// Add a new command of type T to the end of this SkRecord.
diff --git a/tests/RecordTest.cpp b/tests/RecordTest.cpp
index 2240ae9..2a0e615 100644
--- a/tests/RecordTest.cpp
+++ b/tests/RecordTest.cpp
@@ -78,3 +78,28 @@
#undef APPEND
+template <typename T>
+static bool is_aligned(const T* p) {
+ return (((uintptr_t)p) & (sizeof(T) - 1)) == 0;
+}
+
+DEF_TEST(Record_Alignment, r) {
+ SkRecord record;
+
+ // Of course a byte's always aligned.
+ REPORTER_ASSERT(r, is_aligned(record.alloc<uint8_t>()));
+
+ // (If packed tightly, the rest below here would be off by one.)
+
+ // It happens that the first implementation always aligned to 4 bytes,
+ // so these two were always correct.
+ REPORTER_ASSERT(r, is_aligned(record.alloc<uint16_t>()));
+ REPORTER_ASSERT(r, is_aligned(record.alloc<uint32_t>()));
+
+ // These two are regression tests (void* only on 64-bit machines).
+ REPORTER_ASSERT(r, is_aligned(record.alloc<uint64_t>()));
+ REPORTER_ASSERT(r, is_aligned(record.alloc<void*>()));
+
+ // We're not testing beyond sizeof(void*), which is where the current implementation will break.
+}
+