[PATCH] D68471: [scudo][standalone] Correct releaseToOS behavior

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad marked 3 inline comments as done.
cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/tests/primary_test.cpp:195
+// test for an error in how the release criteria were computed.
+template <typename Primary> static void testReleaseToOS() {
+  auto Deleter = [](Primary *P) {
----------------
morehouse wrote:
> cryptoad wrote:
> > morehouse wrote:
> > > Does this test the two cases mentioned in the description?
> > > 
> > >   - `< 1` page in use, `> 1` page in free list (should release)
> > >   - `< 1` page in free list (shouldn't release)
> > > 
> > > 
> > > 
> > > 
> > Indeed, this tests the aforementioned first case, **not** the second one.
> > I don't think I have a way to test that from here. One of the indicators being `LastReleaseAtNs` being updated (without released bytes), and it's not accessible.
> > I'll see if I can toy with the prototype some more to bubble the information up the chain, unless you have an idea.
> > 
> > 
> Is there a way to glean unmapped/released bytes from `GlobalStats`?  If not, it seems like something worth adding to Scudo's telemetry.
> 
> I also see there's already a `printStats` method.  Perhaps we could modify it to print into a buffer where we could extract the released metadata.
So the plan of record on my side:
- Landing this as is, fixing the issue at hand takes precedence
- I am going to have to revisit how to fit the release stats in the mix: the primary's `releaseToOS` doesn't grab a cache, so we can't fit the stats in there; We could put the total of released bytes in regions data, but that departs from the model of all the other stats being the caches. Maybe do that globally indeed, that will require more thought.
- for the `printStats` suggestion: it is something I have to do as well, for `mallocz` purposes for example. Technically this could be leveraged through a lit test in it's current form I think, but right now I only have unit tests. So lit tests is also on my plate.



Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D68471