[LSan] Introduce a callback mechanism to allow adding data reachable from ThreadContexts to the frontier.

[delcypher]

Summary

This mechanism is intended to provide a way to treat the arg pointer
of a created (but not yet started) thread as reachable. In future
patches this will be implemented in ForEachRegisteredThreadContextCb.

A separate implementation of ForEachRegisteredThreadContextCb exists
for ASan and LSan runtimes because they need to be implemented
differently in future patches.

rdar://problem/63537240

[vitalybuka]

compiler-rt/lib/lsan/lsan_common.cpp

549

to avoid frontier access from asan
could you please just:

InternalMmapVector<uptr> ptrs;
GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
      ForEachRegisteredThreadContextCb, &ptrs);

for (: ptr) {
  // process ptr here.
}

another option is nested callback but it seems unnecessary

559

I would prefer if we handle this in ProcessThreads(suspended_threads, frontier) for suspended_threads
But I guess we don't know if just created threads are in suspended_threads.

Still this is very Thread related
so maybe call GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
from the ProcessThreads() with commend that GetThreadRegistryLocked may contains threads

compiler-rt/lib/lsan/lsan_common.h

52–53

forward declaration instead of the new include?

146

Maybe ForEachRegisteredThreadContextCb -> GetAdditionalThreadContextPtrs to show what is expected from this CB

[delcypher]

compiler-rt/lib/lsan/lsan_common.cpp

549

Good idea. This means that ASan and LSan can share some logic here and it avoids the problems I ran into trying to use flags()->log_pointers from asan_allocator.cpp

559

I would prefer if we handle this in ProcessThreads(suspended_threads, frontier) for suspended_threads

But I guess we don't know if just created threads are in suspended_threads.

I don't think we can use suspended_threads. I glanced at the origin of the suspend_threads list and on Linux that comes from reading /proc/*/task (see ThreadLister::ThreadLister). That won't necessarily contain be the threads (not started) we are interested in. The types are also wrong. The list of suspended threads look like tid_t but we actually need the ThreadContext objects which would require a O(N) look up into the thread registry anyway. It's simpler to just walk the thread registry.

I could move the code walking the thread registry into ProcessThreads. However, that function is already pretty big. Can I just change ProcessThreads() to call ProcessThreadRegistry() at the end to make the code easier to read?

compiler-rt/lib/lsan/lsan_common.h

52–53

I'll try it out. I can't actually remember why this include was necessary. It might be that's actually needed for the subsequent patch.

146

Good idea. I'll do rename.

[delcypher]

Address first round of feedback.

[vitalybuka]

Thanks, LGTM.

compiler-rt/lib/asan/asan_allocator.cpp

if (!m ||
       !m->AddrIsInside(addr)) {
     return kIgnoreObjectInvalid;
   }
   if (m->lsan_tag == kIgnored)
     return kIgnoreObjectAlreadyIgnored;
   m->lsan_tag = __lsan::kIgnored;
   return kIgnoreObjectSuccess;
 }
+
+void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
+  // This function can be used to treat memory reachable from `tctx` as live.
+  // This is useful for threads that have been created but not yet started.
+
+  // This is currently a no-op because the ASan `pthread_create()` interceptor
+  // blocks until the child thread starts which keeps the thread's `arg` pointer
+  // live.
+}
+
 }  // namespace __lsan
 
 // ---------------------- Interface ---------------- {{{1
 using namespace __asan;
 
 // ASan allocator doesn't reserve extra bytes, so normally we would
 // just return "size". We don't want to expose our redzone sizes, etc here.
 uptr __sanitizer_get_estimated_allocated_size(uptr size) {

compiler-rt/lib/lsan/lsan_allocator.cpp

if (m->allocated && (uptr)p < (uptr)chunk + m->requested_size) {
     if (m->tag == kIgnored)
       return kIgnoreObjectAlreadyIgnored;
     m->tag = kIgnored;
     return kIgnoreObjectSuccess;
   } else {
     return kIgnoreObjectInvalid;
   }
 }
+
+void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
+  // This function can be used to treat memory reachable from `tctx` as live.
+  // This is useful for threads that have been created but not yet started.
+
+  // This is currently a no-op because the LSan `pthread_create()` interceptor
+  // blocks until the child thread starts which keeps the thread's `arg` pointer
+  // live.
+}
+
 } // namespace __lsan
 
 using namespace __lsan;
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 uptr __sanitizer_get_current_allocated_bytes() {
   uptr stats[AllocatorStatCount];

compiler-rt/lib/lsan/lsan_common.h

 #elif SANITIZER_NETBSD || SANITIZER_FUCHSIA
 #define CAN_SANITIZE_LEAKS 1
 #else
 #define CAN_SANITIZE_LEAKS 0
 #endif
 
 namespace __sanitizer {
 class FlagParser;
 class ThreadRegistry;
+class ThreadContextBase;
 struct DTLS;
 }
 
 namespace __lsan {
 
 // Chunk tags.
 enum ChunkTag {
   kDirectlyLeaked = 0,  // default
struct CheckForLeaksParam {
   LeakReport leak_report;
   bool success = false;
 };
 
 InternalMmapVector<RootRegion> const *GetRootRegions();
 void ScanRootRegion(Frontier *frontier, RootRegion const &region,
                     uptr region_begin, uptr region_end, bool is_readable);
 void ForEachExtraStackRangeCb(uptr begin, uptr end, void* arg);
+void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs);
 // Run stoptheworld while holding any platform-specific locks, as well as the
 // allocator and thread registry locks.
 void LockStuffAndStopTheWorld(StopTheWorldCallback callback,
                               CheckForLeaksParam* argument);
 
 void ScanRangeForPointers(uptr begin, uptr end,
                           Frontier *frontier,
                           const char *region_type, ChunkTag tag);

compiler-rt/lib/lsan/lsan_common.cpp

 #else
 
 #if SANITIZER_ANDROID
 // FIXME: Move this out into *libcdep.cpp
 extern "C" SANITIZER_WEAK_ATTRIBUTE void __libc_iterate_dynamic_tls(
     pid_t, void (*cb)(void *, void *, uptr, void *), void *);
 #endif
 
+static void ProcessThreadRegistry(Frontier *frontier) {
+  InternalMmapVector<uptr> ptrs;
+  GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
+      GetAdditionalThreadContextPtrs, &ptrs);
+
+  for (uptr i = 0; i < ptrs.size(); ++i) {
+    void *ptr = reinterpret_cast<void *>(ptrs[i]);
+    uptr chunk = PointsIntoChunk(ptr);
+    if (!chunk)
+      continue;
+    LsanMetadata m(chunk);
+    if (!m.allocated())
+      continue;
+
+    // Mark as reachable and add to frontier.
+    LOG_POINTERS("Treating pointer %p from ThreadContext as reachable\n", ptr);
+    m.set_tag(kReachable);
+    frontier->push_back(chunk);
+  }
+}
+
 // Scans thread data (stacks and TLS) for heap pointers.
 static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
                            Frontier *frontier) {
   InternalMmapVector<uptr> registers;
   for (uptr i = 0; i < suspended_threads.ThreadCount(); i++) {
     tid_t os_id = static_cast<tid_t>(suspended_threads.GetThreadID(i));
     LOG_THREADS("Processing thread %d.\n", os_id);
     uptr stack_begin, stack_end, tls_begin, tls_end, cache_begin, cache_end;
#else
       } else {
         // We are handling a thread with DTLS under destruction. Log about
         // this and continue.
         LOG_THREADS("Thread %d has DTLS under destruction.\n", os_id);
       }
 #endif
     }
   }
+
+  // Add pointers reachable from ThreadContexts
+  ProcessThreadRegistry(frontier);
 }
 
 #endif  // SANITIZER_FUCHSIA
 
 void ScanRootRegion(Frontier *frontier, const RootRegion &root_region,
                     uptr region_begin, uptr region_end, bool is_readable) {
   uptr intersection_begin = Max(root_region.begin, region_begin);
   uptr intersection_end = Min(region_end, root_region.begin + root_region.size);
void ProcessPC(Frontier *frontier) {
   arg.stack_depot_reverse_map = &stack_depot_reverse_map;
   arg.skip_linker_allocations =
       flags()->use_tls && flags()->use_ld_allocations && GetLinker() != nullptr;
   ForEachChunk(MarkInvalidPCCb, &arg);
 }
 
 // Sets the appropriate tag on each chunk.
 static void ClassifyAllChunks(SuspendedThreadsList const &suspended_threads,
                               Frontier *frontier) {
   const InternalMmapVector<u32> &suppressed_stacks =
       GetSuppressionContext()->GetSortedSuppressedStacks();
   if (!suppressed_stacks.empty()) {
     ForEachChunk(IgnoredSuppressedCb,
                  const_cast<InternalMmapVector<u32> *>(&suppressed_stacks));
   }
   ForEachChunk(CollectIgnoredCb, frontier);
   ProcessGlobalRegions(frontier);
   ProcessThreads(suspended_threads, frontier);
   ProcessRootRegions(frontier);
   FloodFillTag(frontier, kReachable);
 
   CHECK_EQ(0, frontier->size());
   ProcessPC(frontier);
 
   // The check here is relatively expensive, so we do this in a separate flood
   // fill. That way we can skip the check for chunks that are reachable
   // otherwise.