[PATCH] D35358: [scudo] Do not grab a cache for secondary allocation & per related changes

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/scudo/scudo_allocator.cpp:385
+        Salt = getPrng(ThreadContext)->getU8();
+        Ptr = BackendAllocator.Allocate(
+            getAllocatorCache(ThreadContext), AlignedSize, MinAlignment, true);
----------------
Now it feels like your combined allocator API should be AllocatePrimary/AllocateSecondary and the same for Deallocate/etc to do not pass useless params and branch on those. 


================
Comment at: lib/scudo/scudo_allocator.cpp:386
+        Ptr = BackendAllocator.Allocate(
+            getAllocatorCache(ThreadContext), AlignedSize, MinAlignment, true);
+        ThreadContext->unlock();
----------------
Yep, it is more compact now, but I'd still add /*parameter name*/ comments next to those literals. They do help readability.


================
Comment at: lib/scudo/scudo_allocator.cpp:390
+        SpinMutexLock l(&FallbackMutex);
+        Salt = FallbackPrng.getU8();
+        Ptr = BackendAllocator.Allocate(
----------------
I believe you want to {} two lines above too.


================
Comment at: lib/scudo/scudo_allocator.cpp:421
+    CHECK_LE(UserBeg + Size,
+             AllocBeg + (FromPrimary ? AlignedSize : NeededSize));
     Header.State = ChunkAllocated;
----------------
Nit: I get why you did this, but now there's a logical disconnect between the size passed to the allocator and this statement.


https://reviews.llvm.org/D35358