[sanitizers] __sanitizer_get_module_and_offset_for_pc interface function

[aizatsky]

Summary

The function computes full module name and coverts pc into offset.

[aizatsky]

clang-format

[kcc]

lib/sanitizer_common/sanitizer_common_libcdep.cc

174 ↗

(On Diff #78427)

Hmmmm? Leftover from earlier experiments?

183 ↗

(On Diff #78427)

does this need to be null-terminated?

test/sanitizer_common/TestCases/get_module_and_offset_for_pc.cc

25 ↗

(On Diff #78427)

please add a loop which will call something like TestCallerPc w/o logging in a long loop,
to ensure that we are not insanely slow.
Ensure that this test really runs slow on the current implementation (with the leftover ListOfModules)

I would also add an extra DSO to the test.
(e.g. like in test/asan/TestCases/Linux/stack-trace-dlclose.cc)

[aizatsky]

review

[aizatsky]

ptal

lib/sanitizer_common/sanitizer_common_libcdep.cc

183 ↗

(On Diff #78427)

why not? null-terminated strings are easier to work with.

[kcc]

lib/sanitizer_common/sanitizer_common_libcdep.cc

183 ↗

(On Diff #78427)

probably I badly phrased the question.
Does this code guarantee that the module_name will be null terminated?

lib/tsan/go/buildgo.sh

28 ↗

(On Diff #80121)

Ar the changes in this file related to this patch?

[aizatsky]

lib/sanitizer_common/sanitizer_common_libcdep.cc

183 ↗

(On Diff #78427)

yes. that's how it is stored in LoadedModule.

lib/tsan/go/buildgo.sh

28 ↗

(On Diff #80121)

yes. sanitizer_common_libcdep.cc now depends on symbolizer.

[kcc]

lib/sanitizer_common/sanitizer_common_libcdep.cc

183 ↗

(On Diff #78427)

But what if module_name_len is less than the length of the module string stored in LoadedModule?

[aizatsky]

forcing 0-termination

[aizatsky]

ptal

lib/sanitizer_common/sanitizer_common_libcdep.cc

183 ↗

(On Diff #78427)

Got it. Sorry, a bit slow.

[kcc]

I am still very much confused on why we have to change build scripts, including things that touch Go and FreeBSD. Something is wrong here (or I can't get it).
Also, __sanitizer_symbolize_pc is defined in sanitizer_stacktrace_libcdep.cc
Wouldn't it be better to implement the new API there as well?

[aizatsky]

In D26820#612483, @kcc wrote:

I am still very much confused on why we have to change build scripts, including things that touch Go and FreeBSD. Something is wrong here (or I can't get it).

This is what happens without this change on check-all: https://gist.github.com/mikea/53f4dc2c43bab95e97e442be44d5cd72

Also, __sanitizer_symbolize_pc is defined in sanitizer_stacktrace_libcdep.cc
Wouldn't it be better to implement the new API there as well?

I find _stacktrace_libcdep.cc to be unexpected location for symbolization implementation, isn't it?

[kcc]

Also, __sanitizer_symbolize_pc is defined in sanitizer_stacktrace_libcdep.cc
Wouldn't it be better to implement the new API there as well?

I find _stacktrace_libcdep.cc to be unexpected location for symbolization implementation, isn't it?

stacktrace_libcdep deals with symbolizing stack traces, so an API about symbolizing makes sense there.
And I bet if you move your function there you won't have to change any build files.

[aizatsky]

In D26820#612518, @kcc wrote:

Also, __sanitizer_symbolize_pc is defined in sanitizer_stacktrace_libcdep.cc
Wouldn't it be better to implement the new API there as well?

I find _stacktrace_libcdep.cc to be unexpected location for symbolization implementation, isn't it?

stacktrace_libcdep deals with symbolizing stack traces, so an API about symbolizing makes sense there.
And I bet if you move your function there you won't have to change any build files.

Moved. It is accidental that it works: go needs a subset of runtime but doesn't actually specify the surface of that runtime. You will get into go runtime by merely putting your code into sanitizer_common_libcdep.cc.

[aizatsky]

moved code

[kcc]

LGTM with one nit.

lib/sanitizer_common/sanitizer_stacktrace_libcdep.cc

138 ↗

(On Diff #80166)

replace with uptr pc, remove NOLINT, remove the cast below.
Just like in __sanitizer_symbolize_pc.

[aizatsky]

using uptr

lib/sanitizer_common/sanitizer_stacktrace_libcdep.cc

138 ↗

(On Diff #80166)

Done except NOLINT.
You need it otherwise clang-tidy complains:

/usr/local/google/home/aizatsky/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cc:136:35: warning: invalid case style for function 'sanitizer_get_module_and_offset_for_pc' [readability-identifier-naming]
SANITIZER_INTERFACE_ATTRIBUTE int sanitizer_get_module_and_offset_for_pc(

^
SanitizerGetModuleAndOffsetForPc

[kcc]

Maybe don't use clang-tidy or tune it to accept this naming?
Anyway, up to you.

compiler-rt/trunk/include/sanitizer/common_interface_defs.h

#endif
   // Note that the fake stack mechanism is disabled during fiber switch, so if a
   // signal callback runs during the switch, it will not benefit from the stack
   // use-after-return detection.
   void __sanitizer_start_switch_fiber(void **fake_stack_save,
                                       const void *bottom, size_t size);
   void __sanitizer_finish_switch_fiber(void *fake_stack_save,
                                        const void **bottom_old,
                                        size_t *size_old);
+
+  // 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/trunk/lib/asan/asan_win_dll_thunk.cc

 INTERFACE_FUNCTION(__sanitizer_unaligned_load64)
 INTERFACE_FUNCTION(__sanitizer_unaligned_store16)
 INTERFACE_FUNCTION(__sanitizer_unaligned_store32)
 INTERFACE_FUNCTION(__sanitizer_unaligned_store64)
 INTERFACE_FUNCTION(__sanitizer_verify_contiguous_container)
 INTERFACE_FUNCTION(__sanitizer_install_malloc_and_free_hooks)
 INTERFACE_FUNCTION(__sanitizer_start_switch_fiber)
 INTERFACE_FUNCTION(__sanitizer_finish_switch_fiber)
+INTERFACE_FUNCTION(__sanitizer_get_module_and_offset_for_pc)
 
 // TODO(timurrrr): Add more interface functions on the as-needed basis.
 
 // ----------------- Memory allocation functions ---------------------
 WRAP_V_W(free)
 WRAP_V_W(_free_base)
 WRAP_V_WW(_free_dbg)
 

compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cc

 #else
     UNREACHABLE("slow unwind requested but not available");
 #endif
   } else {
     FastUnwindStack(pc, bp, stack_top, stack_bottom, max_depth);
   }
 }
 
+static int GetModuleAndOffsetForPc(uptr pc, char *module_name,
+                                   uptr module_name_len, uptr *pc_offset) {
+  const char *found_module_name = nullptr;
+  bool ok = Symbolizer::GetOrInit()->GetModuleNameAndOffsetForPC(
+      pc, &found_module_name, pc_offset);
+
+  if (!ok) return false;
+
+  if (module_name && module_name_len) {
+    internal_strncpy(module_name, found_module_name, module_name_len);
+    module_name[module_name_len - 1] = '\x00';
+  }
+  return true;
+}
+
 }  // namespace __sanitizer
 using namespace __sanitizer;
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 void __sanitizer_symbolize_pc(uptr pc, const char *fmt, char *out_buf,
                               uptr out_buf_size) {
   if (!out_buf_size) return;
void __sanitizer_symbolize_global(uptr data_addr, const char *fmt,
   out_buf[0] = 0;
   DataInfo DI;
   if (!Symbolizer::GetOrInit()->SymbolizeData(data_addr, &DI)) return;
   InternalScopedString data_desc(GetPageSizeCached());
   RenderData(&data_desc, fmt, &DI, common_flags()->strip_path_prefix);
   internal_strncpy(out_buf, data_desc.data(), out_buf_size);
   out_buf[out_buf_size - 1] = 0;
 }
+
+SANITIZER_INTERFACE_ATTRIBUTE
+int __sanitizer_get_module_and_offset_for_pc( // NOLINT
+    uptr pc, char *module_name, uptr module_name_len, uptr *pc_offset) {
+  return __sanitizer::GetModuleAndOffsetForPc(pc, module_name, module_name_len,
+                                              pc_offset);
+}
 }  // extern "C"

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

 
 // Attempts to demangle the name via __cxa_demangle from __cxxabiv1.
 const char *DemangleCXXABI(const char *name) {
   // FIXME: __cxa_demangle aggressively insists on allocating memory.
   // There's not much we can do about that, short of providing our
   // own demangler (libc++abi's implementation could be adapted so that
   // it does not allocate). For now, we just call it anyway, and we leak
   // the returned value.
-  if (__cxxabiv1::__cxa_demangle)
+  if (&__cxxabiv1::__cxa_demangle)
     if (const char *demangled_name =
           __cxxabiv1::__cxa_demangle(name, 0, 0, 0))
       return demangled_name;
 
   return name;
 }
 
 // As of now, there are no headers for the Swift runtime. Once they are

compiler-rt/trunk/test/sanitizer_common/TestCases/get_module_and_offset_for_pc.cc

+// RUN: %clangxx -DSHARED %s -shared -o %T/get_module_and_offset_for_pc.so -fPIC
+// RUN: %clangxx -DSO_DIR=\"%T\" -O0 %s -ldl -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+// UNSUPPORTED: i386-darwin
+//
+// Tests __sanitizer_get_module_and_offset_for_pc.
+
+#include <assert.h>
+#include <dlfcn.h>
+#include <sanitizer/common_interface_defs.h>
+#include <stdio.h>
+
+#ifdef SHARED
+extern "C" {
+int foo() { return 1; }
+}
+#else
+
+void Test(void *pc, const char *name) {
+  char module_name[1024];
+  void *offset;
+  int ok = __sanitizer_get_module_and_offset_for_pc(
+      pc, module_name, sizeof(module_name), &offset);
+  if (!ok) {
+    printf("NOT FOUND %s: %p\n", name, pc);
+  } else {
+    printf("FOUND %s: %s %p\n", name, module_name, offset);
+  }
+}
+
+void TestCallerPc() { Test(__builtin_return_address(0), "callerpc"); }
+
+void TestDlsym() {
+  void *handle = dlopen(SO_DIR "/get_module_and_offset_for_pc.so", RTLD_LAZY);
+  assert(handle);
+  void *foo = dlsym(handle, "foo");
+  assert(foo);
+  Test(foo, "foo");
+  dlclose(handle);
+}
+
+// Call __sanitizer_get_module_and_offset_for_pc lots of times
+// to make sure it is not too slow.
+void TestLoop() {
+  void *pc = __builtin_return_address(0);
+  char module_name[1024];
+  void *offset;
+  for (int i = 0; i < 1000000; ++i) {
+    __sanitizer_get_module_and_offset_for_pc(pc, module_name,
+                                             sizeof(module_name), &offset);
+  }
+}
+
+int main() {
+  Test(0, "null");
+  TestCallerPc();
+  TestDlsym();
+  TestLoop();
+}
+#endif
+// CHECK: NOT FOUND null: (nil)
+// CHECK-NEXT: FOUND callerpc: {{.*}}/get_module_and_offset_for_pc.cc.tmp {{.*}}
+// CHECK-NEXT: FOUND foo: {{.*}}/get_module_and_offset_for_pc.so {{.*}}