This is an archive of the discontinued LLVM Phabricator instance.

[scudo][standalone] Add the memory reclaiming mechanism
ClosedPublic

Authored by cryptoad on Apr 26 2019, 3:14 PM.

Details

Summary

This CL implements the memory reclaiming function releaseFreeMemoryToOS
and its associated classes. Most of this code was originally written by
Aleksey for the Primary64 in sanitizer_common, and I made some changes to
be able to implement 32-bit reclaiming as well. The code has be restructured
a bit to accomodate for freelist of batches instead of the freearray used
in the current sanitizer_common code.

Diff Detail

Repository
rL LLVM

Event Timeline

cryptoad created this revision.Apr 26 2019, 3:14 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 26 2019, 3:14 PM
Herald added subscribers: Restricted Project, delcypher, mgorny, srhines. · View Herald Transcript
cryptoad updated this revision to Diff 196919.Apr 26 2019, 3:15 PM

Correct typo.

vitalybuka added inline comments.Apr 29 2019, 12:47 PM
lib/scudo/standalone/release.h
20 ↗(On Diff #196919)

Do you need default args here?

37 ↗(On Diff #196919)

Initialize everything for consistency?

59 ↗(On Diff #196919)

with MSVC 64bit sizeof(~0UL) < sizeof(uptr)
~((uptr)0)

or just
CounterMask = 0;
CounterMask = ~CounterMask >> (MaxCounterBits - CounterSizeBits);

66 ↗(On Diff #196919)

same for 1UL
Not sure why we use uptr for most members
u64 and ULL should work

159 ↗(On Diff #196919)

I think the following way it's more readable and easier to see that UNREACHABLE is not possible

if (BlockSize <= PageSize) {
  if (PageSize % BlockSize == 0) {
    // Same number of chunks per page, no cross overs.
    FullPagesBlockCountMax = PageSize / BlockSize;
    SameBlockCountPerPage = true;
  } else if (BlockSize % (PageSize % BlockSize) == 0) {
    // Some chunks are crossing page boundaries, which means that the page
    // contains one or two partial chunks, but all pages contain the same
    // number of chunks.
    FullPagesBlockCountMax = PageSize / BlockSize + 1;
    SameBlockCountPerPage = true;
  } else {
    FullPagesBlockCountMax = PageSize / BlockSize + 2;
    SameBlockCountPerPage = false;
  }
} else {
  if (BlockSize % PageSize == 0) {
    // One chunk covers multiple pages, no cross overs.
    FullPagesBlockCountMax = 1;
    SameBlockCountPerPage = true;
  } else {
    // One chunk covers multiple pages, Some chunks are crossing page
    // boundaries. Some pages contain one chunk, some contain two.
    FullPagesBlockCountMax = 2;
    SameBlockCountPerPage = false;
  }
}
cryptoad updated this revision to Diff 197174.Apr 29 2019, 1:12 PM
cryptoad marked 6 inline comments as done.

Addressing Vitaly's comments.

lib/scudo/standalone/release.h
20 ↗(On Diff #196919)

Leaving the Data one so that it can constructed without.
BaseAddress will always be specified by the 32 & the 64 bits primary.

59 ↗(On Diff #196919)

Done. Sidenote: MSVC support is not in the books for any time soon.

66 ↗(On Diff #196919)

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.

vitalybuka accepted this revision.Apr 29 2019, 1:43 PM
This revision is now accepted and ready to land.Apr 29 2019, 1:43 PM
This revision was automatically updated to reflect the committed changes.