[PATCH] D61214: [scudo][standalone] Add the memory reclaiming mechanism

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/release.h:20
+  ReleaseRecorder(uptr BaseAddress = 0, MapPlatformData *Data = nullptr)
+      : BaseAddress(BaseAddress), Data(Data) {}
+
----------------
vitalybuka wrote:
> Do you need default args here?
Leaving the Data one so that it can constructed without.
BaseAddress will always be specified by the 32 & the 64 bits primary.


================
Comment at: lib/scudo/standalone/release.h:59
+    CounterSizeBitsLog = getLog2(CounterSizeBits);
+    CounterMask = ~0UL >> (MaxCounterBits - CounterSizeBits);
+
----------------
vitalybuka wrote:
> with MSVC 64bit sizeof(~0UL) < sizeof(uptr)
> ~((uptr)0)
> 
> or just
> CounterMask = 0;
> CounterMask =  ~CounterMask >> (MaxCounterBits - CounterSizeBits);
Done. Sidenote: MSVC support is not in the books for any time soon.


================
Comment at: lib/scudo/standalone/release.h:66
+
+    BufferSize = (roundUpTo(N, 1UL << PackingRatioLog) >> PackingRatioLog) *
+                 sizeof(*Buffer);
----------------
vitalybuka wrote:
> same for 1UL
> Not sure why we use uptr for most members
> u64 and ULL should work
> 
That was one of the modifications I made from the initial code.
The reasoning behind using uptr is that u64 operation on 32-bit arm can end up being super costly.
I am trying to privilege 32-bit operations on arm32.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D61214