[scudo] Quarantine optimization

[cryptoad]

Summary

It turns out that the previous code construct was not optimizing the allocation
and deallocation of batches. The class id was read as a class member (even
though a precomputed one) and nothing else was optimized. By changing the
construct this way, the compiler actually optimizes most of the allocation and
deallocation away to only work with a single class id, which not only saves some
CPU but also some code footprint.

[dvyukov]

• const uptr BatchClassId = SizeClassMap::ClassID(sizeof(QuarantineBatch));

Shouldn't it be constexpr? Or at least static const?

[cryptoad]

In D46961#1101608, @dvyukov wrote:

• const uptr BatchClassId = SizeClassMap::ClassID(sizeof(QuarantineBatch));

Shouldn't it be constexpr? Or at least static const?

SizeClassMap::ClassID can't be made C++11 constexpr due to branches, so BatchClassId can't be made constexpr.
Making it static const makes the compiler use __cxa_guard_acquire & __cxa_guard_release which we don't want to be able to stay free of libc++ dependencies.

[dvyukov]

Making it static const makes the compiler use cxa_guard_acquire & cxa_guard_release which we don't want to be able to stay free of libc++ dependencies.

I meant keeping it as class member (not making it function-local static).

[cryptoad]

In D46961#1101650, @dvyukov wrote:

Making it static const makes the compiler use cxa_guard_acquire & cxa_guard_release which we don't want to be able to stay free of libc++ dependencies.

I meant keeping it as class member (not making it function-local static).

Sorry for the misunderstanding.
As a class member constexpr doesn't work for the previously mentioned reasons, and static const complains that the function SizeClassMap::ClassId is non-constexpr.

[dvyukov]

I see. Let's wait for C++14.

lib/scudo/scudo_allocator.cpp

void Recycle(void *Ptr) {
     else
       getBackendAllocator().deallocateSecondary(BackendPtr);
   }
 
   // Internal quarantine allocation and deallocation functions. We first check
   // that the batches are indeed serviced by the Primary.
   // TODO(kostyak): figure out the best way to protect the batches.
   void *Allocate(uptr Size) {
+    const uptr BatchClassId = SizeClassMap::ClassID(sizeof(QuarantineBatch));
     return getBackendAllocator().allocatePrimary(Cache_, BatchClassId);
   }
 
   void Deallocate(void *Ptr) {
+    const uptr BatchClassId = SizeClassMap::ClassID(sizeof(QuarantineBatch));
     getBackendAllocator().deallocatePrimary(Cache_, Ptr, BatchClassId);
   }
 
   AllocatorCache *Cache_;
   COMPILER_CHECK(sizeof(QuarantineBatch) < SizeClassMap::kMaxSize);
-  const uptr BatchClassId = SizeClassMap::ClassID(sizeof(QuarantineBatch));
 };
 
 typedef Quarantine<QuarantineCallback, void> ScudoQuarantine;
 typedef ScudoQuarantine::Cache ScudoQuarantineCache;
 COMPILER_CHECK(sizeof(ScudoQuarantineCache) <=
                sizeof(ScudoTSD::QuarantineCachePlaceHolder));
 
 ScudoQuarantineCache *getQuarantineCache(ScudoTSD *TSD) {