tsan: introduce New/Alloc/Free helpers

We frequenty allocate sizeof(T) memory and call T ctor on that memory
(C++ new keyword effectively). Currently it's quite verbose and
usually takes 2 lines of code.
Add New<T>() helper that does it much more concisely.

Rename internal_free to Free that also sets the pointer to nullptr.
Shorter and safer.

Rename internal_alloc to Alloc, just shorter.

Reviewed By: vitalybuka, melver

Differential Revision: https://reviews.llvm.org/D107085

compiler-rt/lib/tsan/go/tsan_go.cpp

@@ -27,11 +27,9 @@ bool IsExpectedReport(uptr addr, uptr size) {
   return false;
 }
 
-void *internal_alloc(uptr sz) { return InternalAlloc(sz); }
+void *Alloc(uptr sz) { return InternalAlloc(sz); }
 
-void internal_free(void *p) {
-  InternalFree(p);
-}
+void FreeImpl(void *p) { InternalFree(p); }
 
 // Callback into Go.
 static void (*go_runtime_cb)(uptr cmd, void *ctx);
@@ -101,14 +99,16 @@ ReportLocation *SymbolizeData(uptr addr) {
     MBlock *b = ctx->metamap.GetBlock(cbctx.start);
     if (!b)
       return 0;
-    ReportLocation *loc = ReportLocation::New(ReportLocationHeap);
+    auto loc = New<ReportLocation>();
+    loc->type = ReportLocationHeap;
     loc->heap_chunk_start = cbctx.start;
     loc->heap_chunk_size = b->siz;
     loc->tid = b->tid;
     loc->stack = SymbolizeStackId(b->stk);
     return loc;
   } else {
-    ReportLocation *loc = ReportLocation::New(ReportLocationGlobal);
+    auto loc = New<ReportLocation>();
+    loc->type = ReportLocationGlobal;
     loc->global.name = internal_strdup(cbctx.name ? cbctx.name : "??");
     loc->global.file = internal_strdup(cbctx.file ? cbctx.file : "??");
     loc->global.line = cbctx.line;
@@ -140,7 +140,7 @@ Processor *ThreadState::proc() {
 extern "C" {
 
 static ThreadState *AllocGoroutine() {
-  ThreadState *thr = (ThreadState *)internal_alloc(sizeof(ThreadState));
+  auto thr = (ThreadState *)Alloc(sizeof(ThreadState));
   internal_memset(thr, 0, sizeof(*thr));
   return thr;
 }
@@ -226,7 +226,7 @@ void __tsan_go_start(ThreadState *parent, ThreadState **pthr, void *pc) {
 
 void __tsan_go_end(ThreadState *thr) {
   ThreadFinish(thr);
-  internal_free(thr);
+  Free(thr);
 }
 
 void __tsan_proc_create(Processor **pproc) {

compiler-rt/lib/tsan/rtl/tsan_external.cpp

@@ -92,7 +92,7 @@ void __tsan_external_register_header(void *tag, const char *header) {
   header = internal_strdup(header);
   char *old_header =
       (char *)atomic_exchange(header_ptr, (uptr)header, memory_order_seq_cst);
-  if (old_header) internal_free(old_header);
+  Free(old_header);
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

@@ -385,14 +385,14 @@ static void at_exit_wrapper() {
 
   Acquire(cur_thread(), (uptr)0, (uptr)ctx);
   ((void(*)())ctx->f)();
-  InternalFree(ctx);
+  Free(ctx);
 }
 
 static void cxa_at_exit_wrapper(void *arg) {
   Acquire(cur_thread(), 0, (uptr)arg);
   AtExitCtx *ctx = (AtExitCtx*)arg;
   ((void(*)(void *arg))ctx->f)(ctx->arg);
-  InternalFree(ctx);
+  Free(ctx);
 }
 
 static int setup_at_exit_wrapper(ThreadState *thr, uptr pc, void(*f)(),
@@ -418,7 +418,7 @@ TSAN_INTERCEPTOR(int, __cxa_atexit, void (*f)(void *a), void *arg, void *dso) {
 
 static int setup_at_exit_wrapper(ThreadState *thr, uptr pc, void(*f)(),
       void *arg, void *dso) {
-  AtExitCtx *ctx = (AtExitCtx*)InternalAlloc(sizeof(AtExitCtx));
+  auto *ctx = New<AtExitCtx>();
   ctx->f = f;
   ctx->arg = arg;
   Release(thr, pc, (uptr)ctx);
@@ -455,14 +455,14 @@ static void on_exit_wrapper(int status, void *arg) {
   Acquire(thr, pc, (uptr)arg);
   AtExitCtx *ctx = (AtExitCtx*)arg;
   ((void(*)(int status, void *arg))ctx->f)(status, ctx->arg);
-  InternalFree(ctx);
+  Free(ctx);
 }
 
 TSAN_INTERCEPTOR(int, on_exit, void(*f)(int, void*), void *arg) {
   if (in_symbolizer())
     return 0;
   SCOPED_TSAN_INTERCEPTOR(on_exit, f, arg);
-  AtExitCtx *ctx = (AtExitCtx*)InternalAlloc(sizeof(AtExitCtx));
+  auto *ctx = New<AtExitCtx>();
   ctx->f = (void(*)())f;
   ctx->arg = arg;
   Release(thr, pc, (uptr)ctx);

compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp

@@ -89,7 +89,7 @@ static void AddExpectRace(ExpectRace *list,
       return;
     }
   }
-  race = (ExpectRace *)internal_alloc(sizeof(ExpectRace));
+  race = New<ExpectRace>();
   race->addr = addr;
   race->size = size;
   race->file = f;
@@ -305,7 +305,7 @@ void INTERFACE_ATTRIBUTE AnnotateFlushExpectedRaces(char *f, int l) {
     }
     race->prev->next = race->next;
     race->next->prev = race->prev;
-    internal_free(race);
+    Free(race);
   }
 }
 

compiler-rt/lib/tsan/rtl/tsan_mman.cpp

@@ -336,7 +336,7 @@ void invoke_free_hook(void *ptr) {
   RunFreeHooks(ptr);
 }
 
-void *internal_alloc(uptr sz) {
+void *Alloc(uptr sz) {
   ThreadState *thr = cur_thread();
   if (thr->nomalloc) {
     thr->nomalloc = 0;  // CHECK calls internal_malloc().
@@ -345,7 +345,7 @@ void *internal_alloc(uptr sz) {
   return InternalAlloc(sz, &thr->proc()->internal_alloc_cache);
 }
 
-void internal_free(void *p) {
+void FreeImpl(void *p) {
   ThreadState *thr = cur_thread();
   if (thr->nomalloc) {
     thr->nomalloc = 0;  // CHECK calls internal_malloc().

compiler-rt/lib/tsan/rtl/tsan_mman.h

@@ -48,13 +48,28 @@ void invoke_malloc_hook(void *ptr, uptr size);
 void invoke_free_hook(void *ptr);
 
 // For internal data structures.
-void *internal_alloc(uptr sz);
-void internal_free(void *p);
+void *Alloc(uptr sz);
+void FreeImpl(void *p);
 
 template <typename T>
-void DestroyAndFree(T *p) {
+T *New() {
+  return new (Alloc(sizeof(T))) T();
+}
+
+template <typename T>
+void Free(T *&p) {
+  if (p == nullptr)
+    return;
+  FreeImpl(p);
+  p = nullptr;
+}
+
+template <typename T>
+void DestroyAndFree(T *&p) {
+  if (p == nullptr)
+    return;
   p->~T();
-  internal_free(p);
+  Free(p);
 }
 
 }  // namespace __tsan

compiler-rt/lib/tsan/rtl/tsan_report.cpp

@@ -19,22 +19,6 @@
 
 namespace __tsan {
 
-ReportStack::ReportStack() : frames(nullptr), suppressable(false) {}
-
-ReportStack *ReportStack::New() {
-  void *mem = internal_alloc(sizeof(ReportStack));
-  return new(mem) ReportStack();
-}
-
-ReportLocation::ReportLocation(ReportLocationType type)
-    : type(type), global(), heap_chunk_start(0), heap_chunk_size(0), tid(0),
-      fd(0), suppressable(false), stack(nullptr) {}
-
-ReportLocation *ReportLocation::New(ReportLocationType type) {
-  void *mem = internal_alloc(sizeof(ReportLocation));
-  return new(mem) ReportLocation(type);
-}
-
 class Decorator: public __sanitizer::SanitizerCommonDecorator {
  public:
   Decorator() : SanitizerCommonDecorator() { }

compiler-rt/lib/tsan/rtl/tsan_report.h

@@ -38,12 +38,8 @@ enum ReportType {
 };
 
 struct ReportStack {
-  SymbolizedStack *frames;
-  bool suppressable;
-  static ReportStack *New();
-
- private:
-  ReportStack();
+  SymbolizedStack *frames = nullptr;
+  bool suppressable = false;
 };
 
 struct ReportMopMutex {
@@ -73,19 +69,15 @@ enum ReportLocationType {
 };
 
 struct ReportLocation {
-  ReportLocationType type;
-  DataInfo global;
-  uptr heap_chunk_start;
-  uptr heap_chunk_size;
-  uptr external_tag;
-  int tid;
-  int fd;
-  bool suppressable;
-  ReportStack *stack;
-
-  static ReportLocation *New(ReportLocationType type);
- private:
-  explicit ReportLocation(ReportLocationType type);
+  ReportLocationType type = ReportLocationGlobal;
+  DataInfo global = {};
+  uptr heap_chunk_start = 0;
+  uptr heap_chunk_size = 0;
+  uptr external_tag = 0;
+  int tid = kInvalidTid;
+  int fd = 0;
+  bool suppressable = false;
+  ReportStack *stack = nullptr;
 };
 
 struct ReportThread {

compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

@@ -101,8 +101,7 @@ static ThreadContextBase *CreateThreadContext(u32 tid) {
       CHECK("unable to mprotect" && 0);
     }
   }
-  void *mem = internal_alloc(sizeof(ThreadContext));
-  return new(mem) ThreadContext(tid);
+  return new (Alloc(sizeof(ThreadContext))) ThreadContext(tid);
 }
 
 #if !SANITIZER_GO
@@ -577,9 +576,9 @@ NOINLINE
 void GrowShadowStack(ThreadState *thr) {
   const int sz = thr->shadow_stack_end - thr->shadow_stack;
   const int newsz = 2 * sz;
-  uptr *newstack = (uptr *)internal_alloc(newsz * sizeof(uptr));
+  auto newstack = (uptr *)Alloc(newsz * sizeof(uptr));
   internal_memcpy(newstack, thr->shadow_stack, sz * sizeof(uptr));
-  internal_free(thr->shadow_stack);
+  Free(thr->shadow_stack);
   thr->shadow_stack = newstack;
   thr->shadow_stack_pos = newstack + sz;
   thr->shadow_stack_end = newstack + newsz;

compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp

@@ -122,7 +122,7 @@ static ReportStack *SymbolizeStack(StackTrace trace) {
   }
   StackStripMain(top);
 
-  ReportStack *stack = ReportStack::New();
+  auto *stack = New<ReportStack>();
   stack->frames = top;
   return stack;
 }
@@ -157,8 +157,7 @@ bool ShouldReport(ThreadState *thr, ReportType typ) {
 
 ScopedReportBase::ScopedReportBase(ReportType typ, uptr tag) {
   ctx->thread_registry.CheckLocked();
-  void *mem = internal_alloc(sizeof(ReportDesc));
-  rep_ = new(mem) ReportDesc;
+  rep_ = New<ReportDesc>();
   rep_->typ = typ;
   rep_->tag = tag;
   ctx->report_mtx.Lock();
@@ -167,7 +166,6 @@ ScopedReportBase::ScopedReportBase(ReportType typ, uptr tag) {
 ScopedReportBase::~ScopedReportBase() {
   ctx->report_mtx.Unlock();
   DestroyAndFree(rep_);
-  rep_ = nullptr;
 }
 
 void ScopedReportBase::AddStack(StackTrace stack, bool suppressable) {
@@ -178,8 +176,7 @@ void ScopedReportBase::AddStack(StackTrace stack, bool suppressable) {
 
 void ScopedReportBase::AddMemoryAccess(uptr addr, uptr external_tag, Shadow s,
                                        StackTrace stack, const MutexSet *mset) {
-  void *mem = internal_alloc(sizeof(ReportMop));
-  ReportMop *mop = new(mem) ReportMop;
+  auto *mop = New<ReportMop>();
   rep_->mops.PushBack(mop);
   mop->tid = s.tid();
   mop->addr = addr + s.addr0();
@@ -207,8 +204,7 @@ void ScopedReportBase::AddThread(const ThreadContext *tctx, bool suppressable) {
     if ((u32)rep_->threads[i]->id == tctx->tid)
       return;
   }
-  void *mem = internal_alloc(sizeof(ReportThread));
-  ReportThread *rt = new(mem) ReportThread;
+  auto *rt = New<ReportThread>();
   rep_->threads.PushBack(rt);
   rt->id = tctx->tid;
   rt->os_id = tctx->os_id;
@@ -278,8 +274,7 @@ void ScopedReportBase::AddMutex(const SyncVar *s) {
     if (rep_->mutexes[i]->id == s->uid)
       return;
   }
-  void *mem = internal_alloc(sizeof(ReportMutex));
-  ReportMutex *rm = new(mem) ReportMutex;
+  auto *rm = New<ReportMutex>();
   rep_->mutexes.PushBack(rm);
   rm->id = s->uid;
   rm->addr = s->addr;
@@ -311,8 +306,7 @@ void ScopedReportBase::AddDeadMutex(u64 id) {
     if (rep_->mutexes[i]->id == id)
       return;
   }
-  void *mem = internal_alloc(sizeof(ReportMutex));
-  ReportMutex *rm = new(mem) ReportMutex;
+  auto *rm = New<ReportMutex>();
   rep_->mutexes.PushBack(rm);
   rm->id = id;
   rm->addr = 0;
@@ -328,7 +322,8 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
   int creat_tid = kInvalidTid;
   u32 creat_stack = 0;
   if (FdLocation(addr, &fd, &creat_tid, &creat_stack)) {
-    ReportLocation *loc = ReportLocation::New(ReportLocationFD);
+    auto *loc = New<ReportLocation>();
+    loc->type = ReportLocationFD;
     loc->fd = fd;
     loc->tid = creat_tid;
     loc->stack = SymbolizeStackId(creat_stack);
@@ -350,7 +345,8 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
     b = JavaHeapBlock(addr, &block_begin);
   if (b != 0) {
     ThreadContext *tctx = FindThreadByTidLocked(b->tid);
-    ReportLocation *loc = ReportLocation::New(ReportLocationHeap);
+    auto *loc = New<ReportLocation>();
+    loc->type = ReportLocationHeap;
     loc->heap_chunk_start = (uptr)allocator()->GetBlockBegin((void *)addr);
     loc->heap_chunk_size = b->siz;
     loc->external_tag = b->tag;
@@ -363,8 +359,8 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) {
   }
   bool is_stack = false;
   if (ThreadContext *tctx = IsThreadStackOrTls(addr, &is_stack)) {
-    ReportLocation *loc =
-        ReportLocation::New(is_stack ? ReportLocationStack : ReportLocationTLS);
+    auto *loc = New<ReportLocation>();
+    loc->type = is_stack ? ReportLocationStack : ReportLocationTLS;
     loc->tid = tctx->tid;
     rep_->locs.PushBack(loc);
     AddThread(tctx);
@@ -743,8 +739,7 @@ void PrintCurrentStack(ThreadState *thr, uptr pc) {
 ALWAYS_INLINE USED void PrintCurrentStackSlow(uptr pc) {
 #if !SANITIZER_GO
   uptr bp = GET_CURRENT_FRAME();
-  BufferedStackTrace *ptrace =
-      new (internal_alloc(sizeof(BufferedStackTrace))) BufferedStackTrace();
+  auto *ptrace = New<BufferedStackTrace>();
   ptrace->Unwind(pc, bp, nullptr, false);
 
   for (uptr i = 0; i < ptrace->size / 2; i++) {

compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp

@@ -99,7 +99,7 @@ void ThreadContext::OnStarted(void *arg) {
 #else
   // Setup dynamic shadow stack.
   const int kInitStackSize = 8;
-  thr->shadow_stack = (uptr *)internal_alloc(kInitStackSize * sizeof(uptr));
+  thr->shadow_stack = (uptr *)Alloc(kInitStackSize * sizeof(uptr));
   thr->shadow_stack_pos = thr->shadow_stack;
   thr->shadow_stack_end = thr->shadow_stack + kInitStackSize;
 #endif
@@ -122,8 +122,7 @@ void ThreadContext::OnStarted(void *arg) {
 
 void ThreadContext::OnFinished() {
 #if SANITIZER_GO
-  internal_free(thr->shadow_stack);
-  thr->shadow_stack = nullptr;
+  Free(thr->shadow_stack);
   thr->shadow_stack_pos = nullptr;
   thr->shadow_stack_end = nullptr;
 #endif
@@ -419,7 +418,7 @@ void FiberSwitchImpl(ThreadState *from, ThreadState *to) {
 }
 
 ThreadState *FiberCreate(ThreadState *thr, uptr pc, unsigned flags) {
-  void *mem = internal_alloc(sizeof(ThreadState));
+  void *mem = Alloc(sizeof(ThreadState));
   ThreadState *fiber = static_cast<ThreadState *>(mem);
   internal_memset(fiber, 0, sizeof(*fiber));
   int tid = ThreadCreate(thr, pc, 0, true);
@@ -433,7 +432,7 @@ void FiberDestroy(ThreadState *thr, uptr pc, ThreadState *fiber) {
   FiberSwitchImpl(thr, fiber);
   ThreadFinish(fiber);
   FiberSwitchImpl(fiber, thr);
-  internal_free(fiber);
+  Free(fiber);
 }
 
 void FiberSwitch(ThreadState *thr, uptr pc,

compiler-rt/lib/tsan/rtl/tsan_stack_trace.cpp

@@ -23,13 +23,10 @@ VarSizeStackTrace::~VarSizeStackTrace() {
 }
 
 void VarSizeStackTrace::ResizeBuffer(uptr new_size) {
-  if (trace_buffer) {
-    internal_free(trace_buffer);
-  }
-  trace_buffer =
-      (new_size > 0)
-          ? (uptr *)internal_alloc(new_size * sizeof(trace_buffer[0]))
-          : nullptr;
+  Free(trace_buffer);
+  trace_buffer = (new_size > 0)
+                     ? (uptr *)Alloc(new_size * sizeof(trace_buffer[0]))
+                     : nullptr;
   trace = trace_buffer;
   size = new_size;
 }

compiler-rt/lib/tsan/rtl/tsan_symbolize.cpp

@@ -110,7 +110,8 @@ ReportLocation *SymbolizeData(uptr addr) {
   DataInfo info;
   if (!Symbolizer::GetOrInit()->SymbolizeData(addr, &info))
     return 0;
-  ReportLocation *ent = ReportLocation::New(ReportLocationGlobal);
+  auto *ent = New<ReportLocation>();
+  ent->type = ReportLocationGlobal;
   internal_memcpy(&ent->global, &info, sizeof(info));
   return ent;
 }

compiler-rt/lib/tsan/tests/unit/tsan_mman_test.cpp

@@ -18,9 +18,9 @@
 namespace __tsan {
 
 TEST(Mman, Internal) {
-  char *p = (char *)internal_alloc(10);
+  char *p = (char *)Alloc(10);
   EXPECT_NE(p, (char*)0);
-  char *p2 = (char *)internal_alloc(20);
+  char *p2 = (char *)Alloc(20);
   EXPECT_NE(p2, (char*)0);
   EXPECT_NE(p2, p);
   for (int i = 0; i < 10; i++) {
@@ -29,8 +29,8 @@ TEST(Mman, Internal) {
   for (int i = 0; i < 20; i++) {
     ((char*)p2)[i] = 42;
   }
-  internal_free(p);
-  internal_free(p2);
+  Free(p);
+  Free(p2);
 }
 
 TEST(Mman, User) {