This is an archive of the discontinued LLVM Phabricator instance.

[scudo][standalone] Adding a stats class
ClosedPublic

Authored by cryptoad on Mar 6 2019, 9:50 AM.

Details

Summary

This adds simple local & global stats classes to be used by the Primary
and Secondary, and associated test. Note that we don't need the strict
atomicity of the addition & subtraction (as is in sanitizer_common) so
we just use load & store.

Diff Detail

Event Timeline

cryptoad created this revision.Mar 6 2019, 9:50 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 6 2019, 9:50 AM
Herald added subscribers: Restricted Project, jfb, delcypher, mgorny. · View Herald Transcript
vitalybuka accepted this revision.Mar 6 2019, 11:31 AM
vitalybuka added inline comments.
lib/scudo/standalone/stats.h
32

If this is per-thread, why do we need atomics?
Just to make GlobalStats::get

41

set() is unused?

80

"void get(uptr* S) const" would make it more readable

This revision is now accepted and ready to land.Mar 6 2019, 11:31 AM
cryptoad updated this revision to Diff 189553.Mar 6 2019, 12:15 PM
cryptoad marked 3 inline comments as done.

Removing atomics: the Primary stats live in per-thread cache,
the Secondary stats will be protected by a mutex.
Changing GlobalStats::get to take a uptr *.

lib/scudo/standalone/stats.h
32

Removed atomics.

41

I am just using it in tests.

vitalybuka added inline comments.Mar 6 2019, 12:45 PM
lib/scudo/standalone/stats.h
32

Oh, I didn't ask to remove.
GlobalStats does LocalStats::get under the locks
but writes also should be under the lock, or atomics as before

cryptoad updated this revision to Diff 189568.Mar 6 2019, 1:00 PM

After discussion, re-adding the atomics but with a comment.

morehouse accepted this revision.Mar 6 2019, 4:33 PM
morehouse added inline comments.
lib/scudo/standalone/stats.h
80

Maybe:

for (StatType Ty = 0; Ty < StatCount; Ty++)
  add(Ty, S->get(Ty));
cryptoad updated this revision to Diff 189724.Mar 7 2019, 8:34 AM
cryptoad marked an inline comment as done.

Using Matt's suggestion.

cryptoad updated this revision to Diff 189734.Mar 7 2019, 9:36 AM

Undid previous change as it broke on Fuchsia.

cryptoad marked 3 inline comments as done.Mar 7 2019, 11:04 AM
This revision was automatically updated to reflect the committed changes.