[PATCH] D35694: [scudo] Quarantine overhaul

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl accepted this revision.
alekseyshl added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/scudo/scudo_flags.cpp:83
+  if (f->QuarantineSizeMb >= 0)
+    f->QuarantineSizeKb = f->QuarantineSizeMb * 1024;
+  // We enforce an upper limit for the quarantine size of 32Mb.
----------------
Move it up, before if (f->QuarantineSizeKb < 0) check. With my QuarantineChunksUpToSize suggestion (below), all legacy QuarantineSizeMb business will be handled right here, not spoiling the rest of the code.


================
Comment at: lib/scudo/scudo_flags.cpp:110
+  if (f->QuarantineSizeMb >= 0)
+    f->QuarantineChunksUpToSize = -1;
+  // We enforce an upper limit for the chunk quarantine threshold of 4Mb.
----------------
I think we should check that QuarantineChunksUpToSize is not set when QuarantineSizeMb is set and dieWithMessage (up abive, where we do the same for QuarantineSizeKb) instead of setting it to -1. Otherwise we silently ignore the flag value.


================
Comment at: test/scudo/quarantine.cpp:58
     bool found = false;
-    for (int i = 0; i < 0x100 && found == false; i++) {
+    for (int i = 0; i < 0x200 && found == false; i++) {
       p = malloc(size);
----------------
!found


https://reviews.llvm.org/D35694