[PATCH] D73824: scudo: Table driven size classes for Android allocator.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked 2 inline comments as done.
pcc added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/size_class_map.h:90
   static uptr getClassIdBySize(uptr Size) {
+    if (Size <= SizeDelta + (1 << Config::MinSizeLog))
+      return 1;
----------------
cryptoad wrote:
> Do we want to do this here or make sure that any call to this function will fall into the next if? I am not sure which one is better.
Hmm, it seems like we would need something like a conditional either way to make sure that getClassIdBySize(16) doesn't return an invalid class id. The way that I've done it seems more clear/explicit.


================
Comment at: compiler-rt/lib/scudo/standalone/tests/combined_test.cpp:167
     void *NewP = Allocator->reallocate(P, NewSize);
+    P = NewP;
     EXPECT_EQ(NewP, P);
----------------
cryptoad wrote:
> Why that line?
Sorry, this was just something that I added temporarily to keep the tests passing (because we're now moving between size classes in the test code here) and forgot to clean up. I'll fix this so that it's testing what is intended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73824