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

[Kostya Kortchinsky via llvm-commits]

cryptoad added inline comments.

================
Comment at: docs/HardenedAllocator.rst:89
@@ +88,3 @@
+Your linked binary should now make use of the Scudo allocation and deallocation
+functions.
+
----------------
kcc wrote:
> Did you? 
I removed the part about the preinit_array as I do not use that anymore.
Whatever LIT is using requires the whole-archive flag, if using gcc to link the static library against a project, it doesn't.

================
Comment at: docs/HardenedAllocator.rst:94
@@ +93,3 @@
+Several aspects of the allocator can be configured through environment options,
+following the usual ASan options syntax, through the variable SCUDO_OPTIONS.
+
----------------
kcc wrote:
> Give an example instead of referring to "usual ASan syntax".
> Scudo users don't have to be asan experts. 
I didn't realize that I hadn't updated the options names below as well. Also added ThreadLocalQuarantineSizeKb.

================
Comment at: projects/compiler-rt/lib/hardened_allocator/scudo_allocator.cc:109
@@ +108,3 @@
+        (NewUnpackedHeader->Checksum != Checksum(NewUnpackedHeader))) {
+      Printf("ERROR: corrupted chunk header at address %p\n", this);
+      Die();
----------------
kcc wrote:
> I suggest to replace all cases of 
>    if (!cond) {
>      Printf()
>      Die()
>    }
> With 
>     if (!cond)
>       DieWithMessage();
> 
> 
> This is using the Printf from sanitizer_common, right? 
> It might be worth replacing it with your own, simpler one. 
> If you agree, just leave a TODO near DieWithMessage and address it later. 
There is also a PrintfAndReportCallback callback that I just noticed.
I will have to address that later as well.


http://reviews.llvm.org/D20084