[PATCH] D35694: [scudo] Quarantine overhaul

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/scudo/scudo_allocator.cpp:164
 struct AllocatorOptions {
-  u32 QuarantineSizeMb;
+  u32 QuarantineSizeKb;
   u32 ThreadLocalQuarantineSizeKb;
----------------
Please remind me, what was the reason of switching to Kb? Do you need to go lower than 1Mb in real life or you need more granularity?


================
Comment at: lib/scudo/scudo_allocator.cpp:473
+      // GetActuallyAllocatedSize.
+      uptr LiableSize = Size + (Header->Offset << MinAlignmentLog);
       UnpackedHeader NewHeader = *Header;
----------------
EstimatedSize?


================
Comment at: lib/scudo/scudo_flags.cpp:113
+  if (f->QuarantineChunksUpToSize > (4 * 1024 * 1024)) {
+    dieWithMessage("ERROR: the chunk quarantine threshold is too large\n");
   }
----------------
Why do we care to check this?


================
Comment at: test/scudo/quarantine.cpp:33
+}
+
 int main(int argc, char **argv)
----------------
Those functions make the code more concise, but more fragile and less readable. Do you really need them? Inline asserts were just fine.


================
Comment at: test/scudo/quarantine.cpp:78
     }
     assert(found == true);
   }
----------------
 assert(found);


https://reviews.llvm.org/D35694