[LSAN][Fuchsia] Added ForEachExtraThreadStackRange to support Fuchsia code.

[vitalybuka]

compiler-rt/lib/lsan/lsan_common.h

112–116

why do we both?

[kstoimenov]

compiler-rt/lib/lsan/lsan_common.h

112–116

Because code in lsan_common_fuchsia.cpp should not depend on LSAN specific function like GetLsanThreadRegistryLocked. That is the reason why we were getting unresolved external for ASAN builds.

[kstoimenov]

Removed callback.

[vitalybuka]

compiler-rt/lib/asan/asan_thread.cpp

536

void* to actual type

compiler-rt/lib/lsan/lsan_thread.cpp

77–78

remove ForEachExtraStackRange and use GetExtraThreadStackRanges instead

77–78

GetExtraThreadStackRanges(os_id, vector<>...)

[vitalybuka]

compiler-rt/lib/asan/asan_thread.cpp

509–510

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

154

P1, ingore other comment if this one works

I guess the only problem that we have empty SuspendedThreadsListFuchsia

instead of stuff on line 149
maybe introduce lsan::GetAllThreads() and use it
for params->callback(lsan::GetAllThreads(), params->argument)

[kstoimenov]

Removed callbacks.

[kstoimenov]

I've removed callbacks from the interface. PTAL.

[vitalybuka]

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

154

P1, ingore other comment if this one works

I guess the only problem that we have empty SuspendedThreadsListFuchsia

instead of stuff on line 149
maybe introduce lsan::GetAllThreads() and use it
for params->callback(lsan::GetAllThreads(), params->argument)

what about this? seems easy and will make fuchsia loop consistent with the rest

[kstoimenov]

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

154

Could you please elaborate or provide sample code?

[kstoimenov]

Added ScanExtraStackRanges.

[kstoimenov]

I think this should work. I added ScanExtraStackRanges helper which is used both by lsan_common.cpp and lsan_common_fuchsia.cpp.

[vitalybuka]

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

148–154

[vitalybuka]

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

148–154

I see, this does not work :(
Please upload your favorite snapshot.

[kstoimenov]

Fixed fuchsia.

[kstoimenov]

Fixed compilation on Fuchsia.

[kstoimenov]

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

148–154

This shapshot compiles on Fuchsia.

[vitalybuka]

remove GetAllThreads

compiler-rt/lib/asan/asan_thread.cpp

bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
   // ASan doesn't keep allocator caches in TLS, so these are unused.
   *cache_begin = 0;
   *cache_end = 0;
   *dtls = t->dtls();
   return true;
 }
 
 void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches) {}
 
-void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
-                            void *arg) {
+void GetThreadExtraStackRangesLocked(tid_t os_id,
+                                     InternalMmapVector<Range> *ranges) {
   __asan::AsanThread *t = __asan::GetAsanThreadByOsIDLocked(os_id);
   if (!t)
     return;
   __asan::FakeStack *fake_stack = t->get_fake_stack();
   if (!fake_stack)
     return;
-  fake_stack->ForEachFakeFrame(callback, arg);
+
+  fake_stack->ForEachFakeFrame(
+      [](uptr begin, uptr end, void *arg) {
+        reinterpret_cast<InternalMmapVector<Range> *>(arg)->push_back(
+            {begin : begin, end : end});
+      },
+      ranges);
+}
+
+void GetThreadExtraStackRangesLocked(InternalMmapVector<Range> *ranges) {
+  GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
+      [](ThreadContextBase *tctx, void *arg) {
+        GetThreadExtraStackRangesLocked(
+            tctx->os_id, reinterpret_cast<InternalMmapVector<Range> *>(arg));
+      },
+      ranges);
 }
 
 void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {
   GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
       [](ThreadContextBase *tctx, void *ptrs) {
         // Look for the arg pointer of threads that have been created or are
         // running. This is necessary to prevent false positive leaks due to the
         // AsanThread holding the only live reference to a heap object.  This
         // can happen because the `pthread_create()` interceptor doesn't wait
         // for the child thread to start before returning and thus loosing the
         // the only live reference to the heap object on the stack.

compiler-rt/lib/hwasan/hwasan_thread.cpp

bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
   *cache_begin = 0;
   *cache_end = 0;
   *dtls = t->dtls();
   return true;
 }
 
 void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches) {}
 
-void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
-                            void *arg) {}
+void GetThreadExtraStackRangesLocked(tid_t os_id,
+                                     InternalMmapVector<Range> *ranges) {}
+void GetThreadExtraStackRangesLocked(InternalMmapVector<Range> *ranges) {}
 
 void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {}
 void GetRunningThreadsLocked(InternalMmapVector<tid_t> *threads) {}
 
 }  // namespace __lsan

compiler-rt/lib/lsan/lsan_common.h

 };
 
 enum IgnoreObjectResult {
   kIgnoreObjectSuccess,
   kIgnoreObjectAlreadyIgnored,
   kIgnoreObjectInvalid
 };
 
+struct Range {
+  uptr begin;
+  uptr end;
+};
+
 //// --------------------------------------------------------------------------
 //// Poisoning prototypes.
 //// --------------------------------------------------------------------------
 
 // Returns true if [addr, addr + sizeof(void *)) is poisoned.
 bool WordIsPoisoned(uptr addr);
 
 //// --------------------------------------------------------------------------

 // gettid() from the main thread. Our solution is to call this function before
 // leak checking and also before every call to pthread_create() (to handle cases
 // where leak checking is initiated from a non-main thread).
 void EnsureMainThreadIDIsCorrect();
 
 bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
                            uptr *cache_end, DTLS **dtls);
 void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches);
-void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
-                            void *arg);
+void GetThreadExtraStackRangesLocked(InternalMmapVector<Range> *ranges);
+void GetThreadExtraStackRangesLocked(tid_t os_id,
+                                     InternalMmapVector<Range> *ranges);
 void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs);
 void GetRunningThreadsLocked(InternalMmapVector<tid_t> *threads);
 
 //// --------------------------------------------------------------------------
 //// Allocator prototypes.
 //// --------------------------------------------------------------------------
 
 // Wrappers for allocator's ForceLock()/ForceUnlock().
 void LockAllocator();
struct CheckForLeaksParam {
   tid_t caller_tid;
   uptr caller_sp;
   bool success = false;
 };
 
 InternalMmapVectorNoCtor<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);
 // 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);
 void ScanGlobalRange(uptr begin, uptr end, Frontier *frontier);
+void ScanExtraStackRanges(tid_t os_id, Frontier *frontier);
 
 // Functions called from the parent tool.
 const char *MaybeCallLsanDefaultOptions();
 void InitCommonLsan();
 void DoLeakCheck();
 void DoRecoverableLeakCheckVoid();
 void DisableCounterUnderflow();
 bool DisabledInThisThread();

compiler-rt/lib/lsan/lsan_common.cpp

if (begin < allocator_begin)
                            kReachable);
     if (allocator_end < end)
       ScanRangeForPointers(allocator_end, end, frontier, "GLOBAL", kReachable);
   } else {
     ScanRangeForPointers(begin, end, frontier, "GLOBAL", kReachable);
   }
 }
 
-void ForEachExtraStackRangeCb(uptr begin, uptr end, void *arg) {
-  Frontier *frontier = reinterpret_cast<Frontier *>(arg);
-  ScanRangeForPointers(begin, end, frontier, "FAKE STACK", kReachable);
+void ScanExtraStackRanges(const InternalMmapVector<Range> &ranges,
+                          Frontier *frontier) {
+  for (uptr i = 0; i < ranges.size(); i++) {
+    ScanRangeForPointers(ranges[i].begin, ranges[i].end, frontier, "FAKE STACK",
+                         kReachable);
+  }
 }
 
 #  if SANITIZER_FUCHSIA
 
 // Fuchsia handles all threads together with its own callback.
 static void ProcessThreads(SuspendedThreadsList const &, Frontier *, tid_t,
                            uptr) {}
 
static void ProcessThreadRegistry(Frontier *frontier) {
   }
 }
 
 // Scans thread data (stacks and TLS) for heap pointers.
 static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
                            Frontier *frontier, tid_t caller_tid,
                            uptr caller_sp) {
   InternalMmapVector<uptr> registers;
+  InternalMmapVector<Range> extra_ranges;
   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 %llu.\n", os_id);
     uptr stack_begin, stack_end, tls_begin, tls_end, cache_begin, cache_end;
     DTLS *dtls;
     bool thread_found =
         GetThreadRangesLocked(os_id, &stack_begin, &stack_end, &tls_begin,
                               &tls_end, &cache_begin, &cache_end, &dtls);
if (flags()->use_stacks) {
         LOG_THREADS("Skipped %d guard page(s) to obtain stack %p-%p.\n",
                     skipped, (void *)stack_begin, (void *)stack_end);
       } else {
         // Shrink the stack range to ignore out-of-scope values.
         stack_begin = sp;
       }
       ScanRangeForPointers(stack_begin, stack_end, frontier, "STACK",
                            kReachable);
-      ForEachExtraStackRange(os_id, ForEachExtraStackRangeCb, frontier);
+      extra_ranges.clear();
+      GetThreadExtraStackRangesLocked(os_id, &extra_ranges);
+      ScanExtraStackRanges(extra_ranges, frontier);
     }
 
     if (flags()->use_tls) {
       if (tls_begin) {
         LOG_THREADS("TLS at %p-%p.\n", (void *)tls_begin, (void *)tls_end);
         // If the tls and cache ranges don't overlap, scan full tls range,
         // otherwise, only scan the non-overlapping portions
         if (cache_begin == cache_end || tls_end < cache_begin ||

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

__sanitizer_memory_snapshot(
       flags()->use_stacks ? stacks : nullptr,
       flags()->use_registers ? registers : nullptr,
       flags()->use_tls ? tls : nullptr,
       [](zx_status_t, void *data) {
         auto params = static_cast<const Params *>(data);
 
         // We don't use the thread registry at all for enumerating the threads
         // and their stacks, registers, and TLS regions.  So use it separately
-        // just for the allocator cache, and to call ForEachExtraStackRange,
+        // just for the allocator cache, and to call ScanExtraStackRanges,
         // which ASan needs.
         if (flags()->use_stacks) {
-          GetLsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
-              [](ThreadContextBase *tctx, void *arg) {
-                ForEachExtraStackRange(tctx->os_id, ForEachExtraStackRangeCb,
-                                       arg);
-              },
-              &params->argument->frontier);
+          InternalMmapVector<Range> ranges;
+          GetThreadExtraStackRangesLocked(&ranges);
+          ScanExtraStackRanges(ranges, &params->argument->frontier);
         }
-
         params->callback(SuspendedThreadsListFuchsia(), params->argument);
       },
       &params);
 }
 
 }  // namespace __lsan
 
 // This is declared (in extern "C") by <zircon/sanitizer.h>.
 // _Exit calls this directly to intercept and change the status value.
 int __sanitizer_process_exit_hook(int status) {
   return __lsan::ExitHook(status);
 }
 
 #endif

compiler-rt/lib/lsan/lsan_thread.cpp

 }
 
 void EnsureMainThreadIDIsCorrect() {
   if (GetCurrentThread() == kMainTid)
     CurrentThreadContext()->os_id = GetTid();
 }
 
 ///// Interface to the common LSan module. /////
 
-void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
-                            void *arg) {}
+void GetThreadExtraStackRangesLocked(tid_t os_id,
+                                     InternalMmapVector<Range> *ranges) {}
+void GetThreadExtraStackRangesLocked(InternalMmapVector<Range> *ranges) {}
 
 void LockThreadRegistry() { thread_registry->Lock(); }
 
 void UnlockThreadRegistry() { thread_registry->Unlock(); }
 
 ThreadRegistry *GetLsanThreadRegistryLocked() {
   thread_registry->CheckLocked();
   return thread_registry;