[asan] Provide interface to iterate over all Fake stack regions

[johanengelen]

Summary

This adds an interface for a program to iterate over all Fake stack regions.
In particular, this enables a garbage collector to correctly observe object references on the Fake stack (the normal stack would be scanned, but the Fake stack cannot be scanned without this added interface).

Two functions are added. One to scan only the provided thread ID's Fake stack, and one to scan all Fake stacks for all threads known to ASan.

[johanengelen]

Included a testcase now.

[kcc]

How is this going to work when one thread calls __sanitizer_for_each_extra_stack_range with another thread's ID,
while that other thread creates and discards frames, or while that other thread is being destroyed?

Also, we already have an API used by some implementations of GC.
__asan_get_current_fake_stack & co
Is it not sufficient? Why?

Finally, please explain why you need this. I am reluctant to add functionality that I don't understand the need for.

thanks!

[johanengelen]

In D69537#1799478, @kcc wrote:

How is this going to work when one thread calls __sanitizer_for_each_extra_stack_range with another thread's ID,
while that other thread creates and discards frames, or while that other thread is being destroyed?

What is assumed is that the other threads have been stopped, indeed that should be added to its documentation. It'd be great if I can acquire a lock such that it is guaranteed that other threads do not modify the fakestack data structures (but that may be hard because IIRC the fake frame code for very small frames is 'inlined' by codegen and won't wait on a lock?). I can remove the os_tid parameter from that function, so it only works on the current thread's fakestack. The other function can then still be used to iterate over _all_ fake stacks of the process (we have a stop-the-world GC).

Also, we already have an API used by some implementations of GC.
__asan_get_current_fake_stack & co
Is it not sufficient? Why?

Can you point to code/text on how other GC's use that interface to scan for pointers over the whole fakestack?
I don't see how the current API gives me access to all the memory locations that are inside fakestacks. __asan_get_current_fake_stack returns a pointer to a FakeStack object, but the definition of that object is not public so I cannot call FakeStack::ForEachFakeFrame. I can use __asan_addr_is_in_fake_stack to get the begin and end of one frame inside the current fakestack, so I could call it many times with an address inside each fakestack frame, but where do I get these addresses from?

Finally, please explain why you need this. I am reluctant to add functionality that I don't understand the need for.

What I need is to be able to scan over all fakestack frames to scan for pointers. It's the same need that LSan has. What I did in this PR is make a public API with similar functionality that LSan implements internally (LSan can do that because it is in the same library as ASan). I need to be able to scan for such pointers from a different thread, i.e. the GC thread should be able to access the fake frame of other threads.

What's ugly in this PR that __sanitizer_for_each_extra_stack_range is a copy (with public interface types) of ForEachExtraStackRange, so that should be fixed.
__sanitizer_for_each_extra_stack_range_all_threads is meant to provide an interface to ThreadRegistry::RunCallbackForEachThreadLocked (and FakeStack::ForEachFakeFrame).

Thanks for your help.
I've been studying the sanitizer code for many hours now trying to find this functionality because I couldn't believe it is not publically available. There must be someone else with the exact same need as me. But judging from how LSan does it, this is the way to do it and it is indeed not publically available yet.

[kcc]

In D69537#1799790, @johanengelen wrote:

In D69537#1799478, @kcc wrote:

How is this going to work when one thread calls __sanitizer_for_each_extra_stack_range with another thread's ID,
while that other thread creates and discards frames, or while that other thread is being destroyed?

What is assumed is that the other threads have been stopped,

That's a very important assumption :)

indeed that should be added to its documentation. It'd be great if I can acquire a lock such that it is guaranteed that other threads do not modify the fakestack data structures

I don't think you can do this easily.

(but that may be hard because IIRC the fake frame code for very small frames is 'inlined' by codegen and won't wait on a lock?). I can remove the os_tid parameter from that function, so it only works on the current thread's fakestack.

Yes, you can. But then, what will be the difference between the proposed interface and the existing one?

The other function can then still be used to iterate over _all_ fake stacks of the process (we have a stop-the-world GC).

Also, we already have an API used by some implementations of GC.
__asan_get_current_fake_stack & co
Is it not sufficient? Why?

Can you point to code/text on how other GC's use that interface to scan for pointers over the whole fakestack?

https://cs.chromium.org/search/?q=__asan_get_current_fake_stack&sq=package:chromium&type=cs

I don't see how the current API gives me access to all the memory locations that are inside fakestacks. __asan_get_current_fake_stack returns a pointer to a FakeStack object, but the definition of that object is not public so I cannot call FakeStack::ForEachFakeFrame. I can use __asan_addr_is_in_fake_stack to get the begin and end of one frame inside the current fakestack, so I could call it many times with an address inside each fakestack frame, but where do I get these addresses from?

Finally, please explain why you need this. I am reluctant to add functionality that I don't understand the need for.

What I need is to be able to scan over all fakestack frames to scan for pointers. It's the same need that LSan has. What I did in this PR is make a public API with similar functionality that LSan implements internally (LSan can do that because it is in the same library as ASan). I need to be able to scan for such pointers from a different thread, i.e. the GC thread should be able to access the fake frame of other threads.

What's ugly in this PR that __sanitizer_for_each_extra_stack_range is a copy (with public interface types) of ForEachExtraStackRange, so that should be fixed.
__sanitizer_for_each_extra_stack_range_all_threads is meant to provide an interface to ThreadRegistry::RunCallbackForEachThreadLocked (and FakeStack::ForEachFakeFrame).

Thanks for your help.
I've been studying the sanitizer code for many hours now trying to find this functionality because I couldn't believe it is not publically available. There must be someone else with the exact same need as me. But judging from how LSan does it, this is the way to do it and it is indeed not publically available yet.

[johanengelen]

I'm closing this.
Implementation using the existing API: https://github.com/ldc-developers/ldc/pull/3888 (see the druntime submodule changes).

compiler-rt/include/sanitizer/common_interface_defs.h

 ///
 /// \param fake_stack_save Fake stack save location.
 /// \param bottom_old [out] Bottom address of old stack.
 /// \param size_old [out] Size of old stack in bytes.
 void __sanitizer_finish_switch_fiber(void *fake_stack_save,
                                      const void **bottom_old,
                                      size_t *size_old);
 
+/// Calls the user-provided <c>callback</c> for each Fake stack region for the
+/// specified thread ID.
+///
+/// \param os_tid Thread ID. Only the Fake stack of this thread ID is considered.
+/// \param callback User-provided callback. For each stack region,
+///                 <c>callback</c> is called with <c>begin</c> and
+///                 <c>end</c> marking the stack span and <c>arg</c>
+///                 equal to the <c>arg</c> parameter value passed.
+/// \param arg This value is passed as last parameter to <c>callback</c>.
+void __sanitizer_for_each_extra_stack_range(
+    uint64_t os_tid, void (*callback)(size_t begin, size_t end, void *arg),
+    void *arg);
+
+/// Calls the user-provided <c>callback</c> for each Fake stack region for
+/// threads that have a Fake stack.
+///
+/// \param callback User-provided callback. For each stack region,
+///                 <c>callback</c> is called with <c>begin</c> and
+///                 <c>end</c> marking the stack span and <c>arg</c>
+///                 equal to the user-provided <c>arg</c> value.
+/// \param arg This value is passed as last parameter to <c>callback</c>.
+void __sanitizer_for_each_extra_stack_range_all_threads(
+    void (*callback)(size_t begin, size_t end, void *arg), void *arg);
+
 // Get full module name and calculate pc offset within it.
 // Returns 1 if pc belongs to some module, 0 if module was not found.
 int __sanitizer_get_module_and_offset_for_pc(void *pc, char *module_path,
                                              size_t module_path_len,
                                              void **pc_offset);
 
 #ifdef __cplusplus
 }  // extern "C"
 #endif
 
 #endif  // SANITIZER_COMMON_INTERFACE_DEFS_H

compiler-rt/lib/asan/asan_interface.inc

 INTERFACE_FUNCTION(__asan_unpoison_intra_object_redzone)
 INTERFACE_FUNCTION(__asan_unpoison_memory_region)
 INTERFACE_FUNCTION(__asan_unpoison_stack_memory)
 INTERFACE_FUNCTION(__asan_unregister_globals)
 INTERFACE_FUNCTION(__asan_unregister_elf_globals)
 INTERFACE_FUNCTION(__asan_unregister_image_globals)
 INTERFACE_FUNCTION(__asan_version_mismatch_check_v8)
 INTERFACE_FUNCTION(__sanitizer_finish_switch_fiber)
+INTERFACE_FUNCTION(__sanitizer_for_each_extra_stack_range)
+INTERFACE_FUNCTION(__sanitizer_for_each_extra_stack_range_all_threads)
 INTERFACE_FUNCTION(__sanitizer_print_stack_trace)
 INTERFACE_FUNCTION(__sanitizer_ptr_cmp)
 INTERFACE_FUNCTION(__sanitizer_ptr_sub)
 INTERFACE_FUNCTION(__sanitizer_start_switch_fiber)
 INTERFACE_FUNCTION(__sanitizer_unaligned_load16)
 INTERFACE_FUNCTION(__sanitizer_unaligned_load32)
 INTERFACE_FUNCTION(__sanitizer_unaligned_load64)
 INTERFACE_FUNCTION(__sanitizer_unaligned_store16)
 INTERFACE_FUNCTION(__sanitizer_unaligned_store32)
 INTERFACE_FUNCTION(__sanitizer_unaligned_store64)
 INTERFACE_FUNCTION(__asan_update_allocation_context)
 INTERFACE_WEAK_FUNCTION(__asan_default_options)
 INTERFACE_WEAK_FUNCTION(__asan_default_suppressions)
 INTERFACE_WEAK_FUNCTION(__asan_on_error)

compiler-rt/lib/asan/asan_thread.cpp

void __sanitizer_finish_switch_fiber(void* fakestack,
   if (!t) {
     VReport(1, "__asan_finish_switch_fiber called from unknown thread\n");
     return;
   }
   t->FinishSwitchFiber((FakeStack*)fakestack,
                        (uptr*)bottom_old,
                        (uptr*)size_old);
 }
+
+SANITIZER_INTERFACE_ATTRIBUTE
+void __sanitizer_for_each_extra_stack_range(
+    u64 os_id, void (*callback)(uptr begin, uptr end, void *arg), void *arg) {
+  AsanThread *t = GetAsanThreadByOsIDLocked(os_id);
+  if (t && t->has_fake_stack())
+    t->fake_stack()->ForEachFakeFrame(callback, arg);
+}
+
+struct RichRangeIteratorCallback {
+  RangeIteratorCallback callback;
+  void *arg;
+};
+
+static void
+CallRichRangeCallback(ThreadContextBase *tctx_base, void *arg) {
+  auto *cb = reinterpret_cast<RichRangeIteratorCallback *>(arg);
+  auto *tctx = static_cast<AsanThreadContext *>(tctx_base);
+  AsanThread *t = tctx->thread;
+  if (t && t->has_fake_stack())
+    t->fake_stack()->ForEachFakeFrame(cb->callback, cb->arg);
+}
+
+SANITIZER_INTERFACE_ATTRIBUTE
+void __sanitizer_for_each_extra_stack_range_all_threads(
+    void (*callback)(uptr begin, uptr end, void *arg), void *arg) {
+  if (!__asan_option_detect_stack_use_after_return)
+    return;
+
+  RichRangeIteratorCallback cb = {callback, arg};
+  {
+    ThreadRegistryLock l(&asanThreadRegistry());
+    asanThreadRegistry().RunCallbackForEachThreadLocked(CallRichRangeCallback,
+                                                        &cb);
+  }
 }
+}  // extern "C"

compiler-rt/test/asan/TestCases/iterate-fakestacks.cpp

+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %env_asan_opts=detect_stack_use_after_return=1 %run %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+
+#include <sanitizer/common_interface_defs.h>
+
+// The FakeStack region also contains the redzones on both sides of the local variable(s).
+// This means that reading from the region will read from the redzones too, which would
+// trigger ASan stack-buffer-overread errors. Disabling instrumentation of this function
+// bypasses the poison check / solves the errors. The purpose of this testcase is not to test
+// whether ASan triggers on bad reads, but instead to test whether fakestack iteration
+// callback is called with the correct address range that includes local variables.
+__attribute__((no_sanitize("address")))
+void callback(size_t begin, size_t end, void *arg) {
+  printf("%s:", arg);
+  for (auto p = begin; p < end; p++) {
+    char *c = (char*)p;
+    if (*c >= 32) // Filter out ASCII control codes
+        printf("%c", *c);
+  }
+  printf("\n");
+}
+
+__attribute__((noinline)) void frame3() {
+  char localvars[6] = "Three";
+  const char *arg = "from frame three";
+  printf("FRAME3\n");
+  __sanitizer_for_each_extra_stack_range_all_threads(&callback, (void*)arg);
+}
+
+__attribute__((noinline)) void frame2() {
+  char localvars[50] = "A$@N";
+  frame3();
+}
+
+int main(int argc, char **argv) {
+  char localvars[8] = "L1V|\\/|";
+  frame2();
+
+  const char *arg = "from main";
+  printf("MAIN\n");
+  __sanitizer_for_each_extra_stack_range_all_threads(&callback, (void*)arg);
+  printf("DONE\n");
+  return 0;
+}
+
+// CHECK: FRAME3
+// There is no guarantee on the iteration order of the frames
+// CHECK-DAG: from frame three:{{.*}}Three
+// CHECK-DAG: from frame three:{{.*}}A$@N
+// CHECK-DAG: from frame three:{{.*}}L1V|\/|
+// CHECK: MAIN
+// CHECK-NEXT: from main:{{.*}}L1V|\/|
+// CHECK-NEXT: DONE