Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove LLDB introspection entrypoints from the shim #68450

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

rsundahl
Copy link
Contributor

@rsundahl rsundahl commented Oct 6, 2023

Rather than forwarding the LLDB entrypoints into asan_abi stable ABI, leave them unimplemented for now. Doing so allows the shim and asan_abi to be solely constrained to the entrypoints expected by the instrumentation. We will take up a model for other stable ABIs (tsan, ubsan, common, etc.) at a later date after choosing an inter-sanitizer stable abi pattern after discussion and RFC. This commit modifies https://reviews.llvm.org/D159545 by simply eliminating them from the shim entirely.

rdar://115974403

Rather than forwarding the LLDB entrypoints into asan_abi stable
ABI, leave them unimplemented for now. Doing so allows the shim
and asan_abi to be solely constrained to the entrypoints expected
by the instrumentation. We will take up a model for other stable
ABIs (tsan, ubsan, common, etc.) at a later date after choosing
an inter-sanitizer stable abi pattern after discussion and RFC.
This commit modifies https://reviews.llvm.org/D159545 by simply
eliminating them from the shim entirely.

rdar://115974403
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 6, 2023

@llvm/pr-subscribers-compiler-rt-sanitizer

Changes

Rather than forwarding the LLDB entrypoints into asan_abi stable ABI, leave them unimplemented for now. Doing so allows the shim and asan_abi to be solely constrained to the entrypoints expected by the instrumentation. We will take up a model for other stable ABIs (tsan, ubsan, common, etc.) at a later date after choosing an inter-sanitizer stable abi pattern after discussion and RFC. This commit modifies https://reviews.llvm.org/D159545 by simply eliminating them from the shim entirely.

rdar://115974403


Full diff: https://github.com/llvm/llvm-project/pull/68450.diff

4 Files Affected:

  • (modified) compiler-rt/lib/asan_abi/asan_abi.cpp (-18)
  • (modified) compiler-rt/lib/asan_abi/asan_abi.h (-16)
  • (modified) compiler-rt/lib/asan_abi/asan_abi_shim.cpp (-33)
  • (modified) compiler-rt/lib/asan_abi/asan_abi_tbd.txt (+12)
diff --git a/compiler-rt/lib/asan_abi/asan_abi.cpp b/compiler-rt/lib/asan_abi/asan_abi.cpp
index 74efcb158bd1306..cf8663024eb736c 100644
--- a/compiler-rt/lib/asan_abi/asan_abi.cpp
+++ b/compiler-rt/lib/asan_abi/asan_abi.cpp
@@ -86,22 +86,4 @@ void *__asan_abi_stack_malloc_always_n(size_t scale, size_t size) {
 
 // Functions concerning fake stack free
 void __asan_abi_stack_free_n(int scale, void *p, size_t n) {}
-
-// Functions concerning introspection (including lldb support)
-void *__asan_abi_get_alloc_stack(void *addr, void **trace, size_t size,
-                                 int *thread_id) {
-  return NULL;
-}
-void __asan_abi_report_error(void *pc, void *bp, void *sp, void *addr,
-                             bool is_write, size_t access_size, int exp) {}
-void __asan_abi_set_error_report_callback(void (*callback)(const char *)) {}
-void __asan_abi_describe_address(void *addr) {}
-bool __asan_abi_report_present(void) { return false; }
-void *__asan_abi_get_report_pc(void) { return NULL; }
-void *__asan_abi_get_report_bp(void) { return NULL; }
-void *__asan_abi_get_report_sp(void) { return NULL; }
-void *__asan_abi_get_report_address(void) { return NULL; }
-int __asan_abi_get_report_access_type(void) { return 0; }
-size_t __asan_abi_get_report_access_size(void) { return 0; }
-const char *__asan_abi_get_report_description(void) { return NULL; }
 }
diff --git a/compiler-rt/lib/asan_abi/asan_abi.h b/compiler-rt/lib/asan_abi/asan_abi.h
index 546af3822e50b27..8702bcd133919a1 100644
--- a/compiler-rt/lib/asan_abi/asan_abi.h
+++ b/compiler-rt/lib/asan_abi/asan_abi.h
@@ -87,21 +87,5 @@ void *__asan_abi_stack_malloc_always_n(size_t scale, size_t size);
 // Functions concerning fake stack free
 void __asan_abi_stack_free_n(int scale, void *p, size_t n);
 
-// Functions concerning introspection (including lldb support)
-void *__asan_abi_get_alloc_stack(void *addr, void **trace, size_t size,
-                                 int *thread_id);
-void __asan_abi_report_error(void *pc, void *bp, void *sp, void *addr,
-                             bool is_write, size_t access_size, int exp);
-void __asan_abi_set_error_report_callback(void (*callback)(const char *));
-void __asan_abi_describe_address(void *addr);
-bool __asan_abi_report_present(void);
-void *__asan_abi_get_report_pc(void);
-void *__asan_abi_get_report_bp(void);
-void *__asan_abi_get_report_sp(void);
-void *__asan_abi_get_report_address(void);
-int __asan_abi_get_report_access_type(void);
-size_t __asan_abi_get_report_access_size(void);
-const char *__asan_abi_get_report_description(void);
-
 __END_DECLS
 #endif // ASAN_ABI_H
diff --git a/compiler-rt/lib/asan_abi/asan_abi_shim.cpp b/compiler-rt/lib/asan_abi/asan_abi_shim.cpp
index f039bf7c7f6d7f8..bf3cba3178efbdf 100644
--- a/compiler-rt/lib/asan_abi/asan_abi_shim.cpp
+++ b/compiler-rt/lib/asan_abi/asan_abi_shim.cpp
@@ -478,37 +478,4 @@ void __asan_stack_free_9(uptr ptr, uptr size) {
 void __asan_stack_free_10(uptr ptr, uptr size) {
   __asan_abi_stack_free_n(10, (void *)ptr, size);
 }
-
-// Functions concerning introspection (including lldb support)
-uptr __asan_get_alloc_stack(uptr addr, uptr *trace, uptr size, u32 *thread_id) {
-  return (uptr)__asan_abi_get_alloc_stack((void *)addr, (void **)trace,
-                                          (size_t)size, (int *)thread_id);
-}
-void __asan_report_error(uptr pc, uptr bp, uptr sp, uptr addr, int is_write,
-                         uptr access_size, u32 exp) {
-  __asan_abi_report_error((void *)pc, (void *)bp, (void *)sp, (void *)addr,
-                          (bool)is_write, (size_t)access_size, (int)exp);
-}
-void __asan_set_error_report_callback(void (*callback)(const char *)) {
-  __asan_abi_set_error_report_callback(callback);
-}
-void __asan_describe_address(uptr addr) {
-  __asan_abi_describe_address((void *)addr);
-}
-int __asan_report_present(void) { return __asan_abi_report_present(); }
-uptr __asan_get_report_pc(void) { return (uptr)__asan_abi_get_report_pc(); }
-uptr __asan_get_report_bp(void) { return (uptr)__asan_abi_get_report_bp(); }
-uptr __asan_get_report_sp(void) { return (uptr)__asan_abi_get_report_sp(); }
-uptr __asan_get_report_address(void) {
-  return (uptr)__asan_abi_get_report_address();
-}
-int __asan_get_report_access_type(void) {
-  return __asan_abi_get_report_access_type();
-}
-uptr __asan_get_report_access_size(void) {
-  return (uptr)__asan_abi_get_report_access_size();
-}
-const char *__asan_get_report_description(void) {
-  return __asan_abi_get_report_description();
-}
 }
diff --git a/compiler-rt/lib/asan_abi/asan_abi_tbd.txt b/compiler-rt/lib/asan_abi/asan_abi_tbd.txt
index 2022c0b94283e66..a712093d7b21351 100644
--- a/compiler-rt/lib/asan_abi/asan_abi_tbd.txt
+++ b/compiler-rt/lib/asan_abi/asan_abi_tbd.txt
@@ -8,3 +8,15 @@ __asan_on_error
 __asan_print_accumulated_stats
 __asan_set_death_callback
 __asan_update_allocation_context
+__asan_describe_address
+__asan_get_alloc_stack
+__asan_get_report_access_size
+__asan_get_report_access_type
+__asan_get_report_address
+__asan_get_report_bp
+__asan_get_report_description
+__asan_get_report_pc
+__asan_get_report_sp
+__asan_report_error
+__asan_report_present
+__asan_set_error_report_callback

__asan_get_report_description
__asan_get_report_pc
__asan_get_report_sp
__asan_report_error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we call __asan_report_error from the instrumented program. @rsundahl Can you please verify this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a "text-based stub library (.tbd)" file or just a "to be defined", list of symbols we need to figure out what to do with?
If it's the former, what/where is this consumed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list of above is the list of entrypoints that are required by the llvm instrumentation to be provided by the runtime. The test looking for the symbols is: AddressSanitizerABI-arm64-darwin :: TestCases/Darwin/llvm_interface_symbols.cpp. It is likely true that __asan_report_error may not be used by us @usama54321, but it seems to be used by Linux and Windows platforms but it looks like the test is making an unqualified requirements of all platforms that the runtime includes the symbols that it does.

@rsundahl rsundahl merged commit 07d2e90 into llvm:main Oct 9, 2023
5 checks passed
rsundahl added a commit to swiftlang/llvm-project that referenced this pull request Oct 16, 2023
Rather than forwarding the LLDB entrypoints into asan_abi stable ABI,
leave them unimplemented for now. Doing so allows the shim and asan_abi
to be solely constrained to the entrypoints expected by the
instrumentation. We will take up a model for other stable ABIs (tsan,
ubsan, common, etc.) at a later date after choosing an inter-sanitizer
stable abi pattern after discussion and RFC. This commit modifies
https://reviews.llvm.org/D159545 by simply eliminating them from the
shim entirely.

rdar://115974403
(cherry picked from commit 07d2e90)
rsundahl added a commit to swiftlang/llvm-project that referenced this pull request Oct 18, 2023
Remove LLDB introspection entrypoints from the shim (llvm#68450)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants