[memprof] Simplify initialized flags.

[snehasish]

Summary

As discussed in D145428, the memprof_init_is_running check can be moved
to the end of the initialization routine to avoid intercepting
allocations during initialization. Also, the memprof_init_done flag can
be removed and replaced with memprof_inited. Finally, memprof_inited can
also be moved to the end of the method.

Tested on the existing check-memprof tests; memprof profile collection
succeeded on a large internal workload.

[snehasish]

compiler-rt/lib/memprof/memprof_rtl.cpp

198

@tejohnson

I stepped through the code for ThreadStart and identified potential allocation calls. I found that:

• The thread itself uses MmapOrDie [1]
• The thread registry uses placement new [2]
• The allocator context uses placement new [3]
• The sanitizer thread registry push_back calls sanitizer internal Realloc [4]

So perhaps this comment is stale? Either way, I think even if there is an allocation call, intercepting it isn't worthwhile. Do you have any concerns based on this comment?

[1] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/memprof/memprof_thread.cpp#L79
[2] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/memprof/memprof_thread.cpp#L60
[3] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/memprof/memprof_thread.cpp#L48
[4] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp#L147

[davidxl]

looks good to me (and less confusing ;) ).

[snehasish]

clang-format, rebase.

[tejohnson]

lgtm, although you may want to try to test I suggest below if we want to be more sure that my comment is stale.

compiler-rt/lib/memprof/memprof_rtl.cpp

198

I can't recall exactly what motivated this, but I don't think it was wanting to intercept those allocation calls. I'm guessing that at one point the code was structured so that making an allocation without things being inited properly caused a crash or other issue (I don't think there should be anything special about malloc vs new in that regard - we try to intercept both). If you want to be extra sure, you could just add a temporary call to malloc in the code below and run a simple instrumented test to make sure nothing breaks.

[snehasish]

compiler-rt/lib/memprof/memprof_rtl.cpp

198

Done, added a call to malloc in the MemprofInitInternal and stepped through it and it looks good. Also Symbolizer::LateInitialize() allocates memory (intercepted via DlsymAlloc) so there was already an allocation after moving the inited flag.