[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

525

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

540

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

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

525

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

540

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

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

uptr PointsIntoChunk(void *p) {
   uptr chunk = m->Beg();
   if (m->AddrIsInside(addr))
     return chunk;
   if (IsSpecialCaseOfOperatorNew0(chunk, m->UsedSize(), addr))
     return chunk;
   return 0;
 }
 
+void ForEachRegisteredThreadContextCb(ThreadContextBase *tctx, void *arg) {
+  // 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.
+}
+
 uptr GetUserBegin(uptr chunk) {
   __asan::AsanChunk *m = __asan::instance.GetAsanChunkByAddrFastLocked(chunk);
   return m ? m->Beg() : 0;
 }
 
 LsanMetadata::LsanMetadata(uptr chunk) {
   metadata_ = chunk ? reinterpret_cast<void *>(chunk - __asan::kChunkHeaderSize)
                     : nullptr;

compiler-rt/lib/lsan/lsan_common.h

 #define LSAN_COMMON_H
 
 #include "sanitizer_common/sanitizer_allocator.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_platform.h"
 #include "sanitizer_common/sanitizer_stoptheworld.h"
 #include "sanitizer_common/sanitizer_symbolizer.h"
+#include "sanitizer_common/sanitizer_thread_registry.h"
 
 // LeakSanitizer relies on some Glibc's internals (e.g. TLS machinery) on Linux.
 // Also, LSan doesn't like 32 bit architectures
 // because of "small" (4 bytes) pointer size that leads to high false negative
 // ratio on large leaks. But we still want to have it for some 32 bit arches
 // (e.g. x86), see https://github.com/google/sanitizers/issues/403.
 // To enable LeakSanitizer on a new architecture, one needs to implement the
 // internal_clone function as well as (probably) adjust the TLS machinery for

 #elif SANITIZER_NETBSD || SANITIZER_FUCHSIA
 #define CAN_SANITIZE_LEAKS 1
 #else
 #define CAN_SANITIZE_LEAKS 0
 #endif
 
 namespace __sanitizer {
 class FlagParser;
 class ThreadRegistry;
 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 ForEachRegisteredThreadContextCb(ThreadContextBase *tctx, void *arg);
 // 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

void ProcessPC(Frontier *frontier) {
   InvalidPCParam arg;
   arg.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);
 }
 
+void ProcessThreadRegistry(Frontier *frontier) {
+  GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
+      ForEachRegisteredThreadContextCb, frontier);
+}
+
 // 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);
+  ProcessThreadRegistry(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

compiler-rt/lib/lsan/lsan_thread.cpp

 
 void UnlockThreadRegistry() { thread_registry->Unlock(); }
 
 ThreadRegistry *GetThreadRegistryLocked() {
   thread_registry->CheckLocked();
   return thread_registry;
 }
 
+void ForEachRegisteredThreadContextCb(ThreadContextBase *tctx, void *arg) {
+  // 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