Simplify Footers in SkArenaAlloc to 64-bit values using pointers.
authorHerb Derby <herb@google.com>
Wed, 25 Jan 2017 22:05:05 +0000 (17:05 -0500)
committerSkia Commit-Bot <skia-commit-bot@chromium.org>
Wed, 25 Jan 2017 22:35:56 +0000 (22:35 +0000)
TBR=mtklein@google.com

Change-Id: I72c6cf6857b2bb00f4259cc9c4de2d51d454e6ab
Reviewed-on: https://skia-review.googlesource.com/7582
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>

src/core/SkArenaAlloc.cpp
src/core/SkArenaAlloc.h

index 787c5cab8c8a2443c72a28c99468c787f794e9df..90b703721ee804dc2e1038fd849c12eadf67ba02 100644 (file)
@@ -23,8 +23,8 @@ void SkArenaAlloc::RunDtorsOnBlock(char* footerEnd) {
         Footer footer;
         memcpy(&footer, footerEnd - sizeof(Footer), sizeof(Footer));
 
-        FooterAction* action = (FooterAction*)((char*)end_chain + (footer >> 5));
-        ptrdiff_t padding = footer & 31;
+        FooterAction* action = (FooterAction*)(footer >> 6);
+        ptrdiff_t padding = footer & 63;
 
         footerEnd = action(footerEnd) - padding;
     }
@@ -65,27 +65,22 @@ void SkArenaAlloc::reset() {
     new (this) SkArenaAlloc{fFirstBlock, fFirstSize, fExtraSize};
 }
 
-void SkArenaAlloc::installFooter(FooterAction* releaser, ptrdiff_t padding) {
-    ptrdiff_t releaserDiff = (char *)releaser - (char *)end_chain;
-    ptrdiff_t footerData = SkLeftShift((int64_t)releaserDiff, 5) | padding;
-    if (padding >= 32 || !SkTFitsIn<Footer>(footerData)) {
-        // Footer data will not fit.
-        SkFAIL("Constraints are busted.");
-    }
-
-    Footer footer = (Footer)(footerData);
-    memmove(fCursor, &footer, sizeof(Footer));
+void SkArenaAlloc::installFooter(FooterAction* action, uint32_t padding) {
+    SkASSERT(padding < 64);
+    int64_t actionInt = (int64_t)(intptr_t)action;
+    Footer encodedFooter = (actionInt << 6) | padding;
+    memmove(fCursor, &encodedFooter, sizeof(Footer));
     fCursor += sizeof(Footer);
     fDtorCursor = fCursor;
 }
 
-void SkArenaAlloc::installPtrFooter(FooterAction* action, char* ptr, ptrdiff_t padding) {
+void SkArenaAlloc::installPtrFooter(FooterAction* action, char* ptr, uint32_t padding) {
     memmove(fCursor, &ptr, sizeof(char*));
     fCursor += sizeof(char*);
     this->installFooter(action, padding);
 }
 
-void SkArenaAlloc::installUint32Footer(FooterAction* action, uint32_t value, ptrdiff_t padding) {
+void SkArenaAlloc::installUint32Footer(FooterAction* action, uint32_t value, uint32_t padding) {
     memmove(fCursor, &value, sizeof(uint32_t));
     fCursor += sizeof(uint32_t);
     this->installFooter(action, padding);
index e53ea996a441c9903348a476cbfd57cad843ea48..b2e81e2b4fba5ad68c02225370136ecd5c3895bf 100644 (file)
@@ -68,6 +68,7 @@ public:
 
     template <typename T, typename... Args>
     T* make(Args&&... args) {
+
         SkASSERT(SkTFitsIn<uint32_t>(sizeof(T)));
         char* objStart;
         if (skstd::is_trivially_destructible<T>::value) {
@@ -75,7 +76,8 @@ public:
             fCursor = objStart + sizeof(T);
         } else {
             objStart = this->allocObjectWithFooter(sizeof(T) + sizeof(Footer), alignof(T));
-            size_t padding = objStart - fCursor;
+            // Can never be UB because max value is alignof(T).
+            uint32_t padding = SkTo<uint32_t>(objStart - fCursor);
 
             // Advance to end of object to install footer.
             fCursor = objStart + sizeof(T);
@@ -125,9 +127,9 @@ private:
     static void RunDtorsOnBlock(char* footerEnd);
     static char* NextBlock(char* footerEnd);
 
-    void installFooter(FooterAction* releaser, ptrdiff_t padding);
-    void installUint32Footer(FooterAction* action, uint32_t value, ptrdiff_t padding);
-    void installPtrFooter(FooterAction* action, char* ptr, ptrdiff_t padding);
+    void installFooter(FooterAction* releaser, uint32_t padding);
+    void installUint32Footer(FooterAction* action, uint32_t value, uint32_t padding);
+    void installPtrFooter(FooterAction* action, char* ptr, uint32_t padding);
 
     void ensureSpace(size_t size, size_t alignment);
 
@@ -148,7 +150,9 @@ private:
         } else {
             size_t totalSize = arraySize + sizeof(Footer) + sizeof(uint32_t);
             objStart = this->allocObjectWithFooter(totalSize, alignof(T));
-            size_t padding = objStart - fCursor;
+
+            // Can never be UB because max value is alignof(T).
+            uint32_t padding = SkTo<uint32_t>(objStart - fCursor);
 
             // Advance to end of array to install footer.?
             fCursor = objStart + arraySize;