[PATCH] D58723: [scudo][standalone] Add bytemap classes

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: lib/scudo/standalone/bytemap.h:22
+    Map = reinterpret_cast<u8 *>(map(nullptr, Size, "scudo:bytemap"));
+  }
+  void init() { initLinkerInitialized(); }
----------------
morehouse wrote:
> cryptoad wrote:
> > morehouse wrote:
> > > Having two init functions is a little confusing.  Would it make more sense to have a constexpr constructor and a single init() function?
> > With this I followed the way it was done in sanitizer_common with an init function that would skip the zero initialization if not needed (most our structures are either mapped or static). Would you have an example of what you are suggesting, I am not sure I fully understand how to organize it?
> Essentially, make the zero-initialization explicit in the `constexpr` constructor, and then do all dynamic initialization in `init`.
> 
> ```
> class FlatByteMap {
>   constexpr FlatByteMap() : Map(nullptr) {}
>   void init() {
>     Map = reinterpret_cast<u8 *>(map(nullptr, Size, "scudo:bytemap"));
>   }
> };
> 
> class TwoLevelByteMap {
>   constexpr TwoLevelByteMap() : Level1Map(nullptr), Mutex{} {}
>   void init() {
>     Level1Map = reinterpret_cast<atomic_uptr *>(
>         map(nullptr, sizeof(atomic_uptr) * Level1Size, "scudo:bytemap"));
>   }
> };
> ```
> 
> The `constexpr` constructor will initialize any statics / globals at compile time while still working at runtime for non-static instantiations.  Then the call to `init` completes any dynamic initialization required.
The linker-init function will work fine, but since we're reimplementing much of sanitizer_common for Scudo now, maybe it's a good opportunity to modernize a bit.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D58723