[PATCH] D26358: [scudo] 32-bit and hardware agnostic support

[Kostya Serebryany via llvm-commits]

kcc added inline comments.


================
Comment at: lib/scudo/scudo_allocator.cpp:45
 typedef SizeClassAllocator64<AP> PrimaryAllocator;
+#else
+static const uptr RegionSizeLog = 20;
----------------
Do you want to leave some comment saying that the 32-bit backend allocator itself is much less secure than the 64-bit one?


================
Comment at: lib/scudo/scudo_allocator.cpp:99
+    uptr Crc;
+#if SANITIZER_WORDSIZE == 64
+    u64 HeaderHolder;
----------------
I would recommend if (SANITIZER_WORD_SIZE == 64)
instead of #if
at least if that compiles. 

having #ifdefs inside a function makes maintenance harder. 


================
Comment at: lib/scudo/scudo_utils.cpp:182
+
+#if SANITIZER_WORDSIZE == 64
+u32 doCRC32u64(u32 Crc, u64 Data)
----------------
here too


================
Comment at: lib/scudo/scudo_utils.h:44
   Xorshift128Plus();
+#if SANITIZER_WORDSIZE == 64
   u64 Next() {
----------------
don't introduce #ifdefs, please. 

I'd make it two separate classes and then typedef to one or the other using FIRST_32_SECOND_64


https://reviews.llvm.org/D26358