[PATCH] D36385: [asan] Refactor thread creation bookkeeping

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/asan/asan_thread.cc:290
+                       const_cast<uptr *>(&stack_size),
+                       &tls_begin_, &tls_size);
   stack_top_ = stack_bottom_ + stack_size;
----------------
What was wrong with the wrapping here?


================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.cc:255
+// Normally this is called when the thread is about to exit.
+// If aborted is true, then this thread was never really started.
+// We just did CreateThread for a prospective new thread before
----------------
What does 'aborted' refer to?


================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.cc:272
+    CHECK_EQ(tctx->status, ThreadStatusCreated);
+    tctx->detached = false;
+    dead = true;
----------------
The only reason you set detached to false here is to force SetFinished() to set status to ThreadStatusFinished, right? If so, wouldn't it be more appropriate to modify SetFinished to do this:

  // Comment why checking status is important.
  if (!detached || status == ThreadStatusCreated)
    status = ThreadStatusFinished;

then you won't need dead and detached = false.


https://reviews.llvm.org/D36385