[PATCH] D20084: [sanitizer] Initial implementation of a Hardened Allocator

[Mike Aizatsky via llvm-commits]

aizatsky added inline comments.

================
Comment at: lib/hardened_allocator/scudo_allocator.cc:123
@@ +122,3 @@
+    crc = _mm_crc32_u64(crc, header_holder[1]);
+    return static_cast<u16>(crc ^ cookie);
+  }
----------------
cryptoad wrote:
> aizatsky wrote:
> > This uses this.cookie. Is this intended?
> I am not sure what you mean here, as there is no cookie member to the struct. Could you please clarify?
I had wrong idea where cookie is stored. NM.

================
Comment at: lib/hardened_allocator/scudo_allocator.cc:141
@@ +140,3 @@
+  // A different one would mean that another thread would have raced us.
+  void StoreHeader(UnpackedHeader *new_unpacked_header,
+                   UnpackedHeader *old_unpacked_header) {
----------------
cryptoad wrote:
> aizatsky wrote:
> > Instead of having optional parameter, maybe its better to split this function into two? One simple copying, another - verify and copy.
> I liked having a single one to avoid duplication, but I can see the interest in having two. I can change that if you feel strongly about it.
Besides making them much easier, this would clearly show where there is verification and where there isn't. These are totally two different functions, and you use null value as a special value logic switch.

================
Comment at: lib/hardened_allocator/scudo_allocator.h:68
@@ +67,3 @@
+
+const uptr kAllocatorSpace = ~0ULL;
+const uptr kAllocatorSize  =  0x10000000000ULL;
----------------
dvyukov wrote:
> aizatsky wrote:
> > static?
> constant variables are static by default.
Yes, but next two are static. I'm asking for consistency.


http://reviews.llvm.org/D20084