[PATCH] D91106: [scudo][standalone] Correct min/max region indices

[Mitch Phillips via Phabricator via llvm-commits]

hctim accepted this revision.
hctim added a comment.
This revision is now accepted and ready to land.

LGTM w/ nits



================
Comment at: compiler-rt/lib/scudo/standalone/primary32.h:161
+    uptr MinRegionIndex = NumRegions, MaxRegionIndex = 0;
+    for (uptr I = 0; I < NumClasses; I++) {
+      SizeClassInfo *Sci = getSizeClassInfo(I);
----------------
If it's substantially cheaper to walk each class and get the region bounds, can you do the same thing for `unmapTestOnly`? If it's not a whole bunch cheaper, can you change this to just walk all the regions?


================
Comment at: compiler-rt/lib/scudo/standalone/primary32.h:299
 
   uptr allocateRegion(uptr ClassId) {
     DCHECK_LT(ClassId, NumClasses);
----------------
Instead of rematerializing, can you pass the size class info as a parameter from `populateFreeList`.


================
Comment at: compiler-rt/lib/scudo/standalone/primary32.h:312
+      SizeClassInfo *Sci = getSizeClassInfo(ClassId);
+      if (RegionIndex < Sci->MinRegionIndex)
+        Sci->MinRegionIndex = RegionIndex;
----------------
Probably didn't help because of the rematerialization, but can you add a comment here that `Sci->Mutex` is already locked by the caller?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91106/new/

https://reviews.llvm.org/D91106