[PATCH] D45646: [tsan] Zero out the shadow memory for the stack and TLS in ThreadFinish

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek created this revision.
kubamracek added reviewers: dvyukov, delcypher, george.karpenkov.
kubamracek added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

It turns out that DontNeedShadowFor() on Darwin doesn't actually zero out nor release the memory, and this can lead to a crash when the memory is reused (by a new thread). The particular problem on Darwin is because we actually use the shadow memory to store `ThreadState *` (as a fake thread-local storage), and if this contains a stale value, we'll crash.

I'll try to add a test case, but it seems it's pretty hard to trigger.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D45646

Files:
  lib/tsan/rtl/tsan_rtl_thread.cc


Index: lib/tsan/rtl/tsan_rtl_thread.cc
===================================================================
--- lib/tsan/rtl/tsan_rtl_thread.cc
+++ lib/tsan/rtl/tsan_rtl_thread.cc
@@ -276,10 +276,14 @@
 void ThreadFinish(ThreadState *thr) {
   ThreadCheckIgnore(thr);
   StatInc(thr, StatThreadFinish);
-  if (thr->stk_addr && thr->stk_size)
+  if (thr->stk_addr && thr->stk_size) {
+    MemoryResetRange(thr, /*pc=*/ 1, thr->stk_addr, thr->stk_size);
     DontNeedShadowFor(thr->stk_addr, thr->stk_size);
-  if (thr->tls_addr && thr->tls_size)
+  }
+  if (thr->tls_addr && thr->tls_size) {
+    MemoryResetRange(thr, /*pc=*/ 1, thr->tls_addr, thr->tls_size);
     DontNeedShadowFor(thr->tls_addr, thr->tls_size);
+  }
   thr->is_dead = true;
   ctx->thread_registry->FinishThread(thr->tid);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45646.142492.patch
Type: text/x-patch
Size: 794 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180413/2fed2b2f/attachment.bin>