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

[scudo] Add parameters for ring buffer and stack depot sizes #74539

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented Dec 6, 2023

These will be used in follow-up CLs, committing this separately because it needs a matching change in AOSP. This way we can avoid complicated multi-repo rollbacks if something is wrong with the follow up CLs.

These will be used in follow-up CLs, committing this separately
because it needs a matching change in AOSP. This way we can avoid
complicated multi-repo rollbacks if something is wrong with the follow
up CLs.
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 6, 2023

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

Author: Florian Mayer (fmayer)

Changes

These will be used in follow-up CLs, committing this separately because it needs a matching change in AOSP. This way we can avoid complicated multi-repo rollbacks if something is wrong with the follow up CLs.


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

2 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/include/scudo/interface.h (+2-1)
  • (modified) compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp (+7-6)
diff --git a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
index 260f1a7bd84bb..a664b9825f209 100644
--- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
+++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
@@ -72,7 +72,8 @@ typedef void (*iterate_callback)(uintptr_t base, size_t size, void *arg);
 // pointer.
 void __scudo_get_error_info(struct scudo_error_info *error_info,
                             uintptr_t fault_addr, const char *stack_depot,
-                            const char *region_info, const char *ring_buffer,
+                            size_t stack_depot_size, const char *region_info,
+                            const char *ring_buffer, size_t ring_buffer_size,
                             const char *memory, const char *memory_tags,
                             uintptr_t memory_addr, size_t memory_size);
 
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp b/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
index 4fed44779b902..f203615ab3602 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c_bionic.cpp
@@ -38,12 +38,13 @@ static scudo::Allocator<scudo::Config, SCUDO_PREFIX(malloc_postinit)>
 // TODO(kostyak): support both allocators.
 INTERFACE void __scudo_print_stats(void) { Allocator.printStats(); }
 
-INTERFACE void
-__scudo_get_error_info(struct scudo_error_info *error_info,
-                       uintptr_t fault_addr, const char *stack_depot,
-                       const char *region_info, const char *ring_buffer,
-                       const char *memory, const char *memory_tags,
-                       uintptr_t memory_addr, size_t memory_size) {
+INTERFACE void __scudo_get_error_info(
+    struct scudo_error_info *error_info, uintptr_t fault_addr,
+    const char *stack_depot, size_t stack_depot_size, const char *region_info,
+    const char *ring_buffer, size_t ring_buffer_size, const char *memory,
+    const char *memory_tags, uintptr_t memory_addr, size_t memory_size) {
+  (void)(stack_depot_size);
+  (void)(ring_buffer_size);
   Allocator.getErrorInfo(error_info, fault_addr, stack_depot, region_info,
                          ring_buffer, memory, memory_tags, memory_addr,
                          memory_size);

Copy link
Contributor

@cferris1000 cferris1000 left a comment

Choose a reason for hiding this comment

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

This seems fine, but given the absurd number of parameters to these functions, there should be a follow-up at some point that converts this into a structure. That way we wouldn't need to change the interface in the future.

@fmayer fmayer merged commit e68c265 into llvm:main Dec 6, 2023
7 checks passed
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.

None yet

3 participants