-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[compiler-rt][memprof] adding free_sized/free_aligned_sized intercept… #154011
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
Conversation
@llvm/pr-subscribers-pgo Author: David CARLIER (devnexen) Changes…ions. Full diff: https://github.com/llvm/llvm-project/pull/154011.diff 1 Files Affected:
diff --git a/compiler-rt/lib/memprof/memprof_malloc_linux.cpp b/compiler-rt/lib/memprof/memprof_malloc_linux.cpp
index 2a028c7d0b489..68fe65475889a 100644
--- a/compiler-rt/lib/memprof/memprof_malloc_linux.cpp
+++ b/compiler-rt/lib/memprof/memprof_malloc_linux.cpp
@@ -50,6 +50,24 @@ INTERCEPTOR(void, cfree, void *ptr) {
}
#endif // SANITIZER_INTERCEPT_CFREE
+#if SANITIZER_INTERCEPT_FREE_SIZED
+INTERCEPTOR(void, free_sized, void *ptr, uptr size) {
+ if (DlsymAlloc::PointerIsMine(ptr))
+ return DlsymAlloc::Free(ptr);
+ GET_STACK_TRACE_FREE;
+ memprof_delete(ptr, size, 0, &stack, FROM_MALLOC);
+}
+#endif // SANITIZER_INTERCEPT_FREE_SIZED
+
+#if SANITIZER_INTERCEPT_FREE_ALIGNED_SIZED
+INTERCEPTOR(void, free_aligned_sized, void *ptr, uptr alignment, uptr size) {
+ if (DlsymAlloc::PointerIsMine(ptr))
+ return DlsymAlloc::Free(ptr);
+ GET_STACK_TRACE_FREE;
+ memprof_delete(ptr, size, alignment, &stack, FROM_MALLOC);
+}
+#endif // SANITIZER_INTERCEPT_FREE_ALIGNED_SIZED
+
INTERCEPTOR(void *, malloc, uptr size) {
if (DlsymAlloc::Use())
return DlsymAlloc::Allocate(size);
|
Thanks for the contribution. Is it possible to add a test which exercises these new interceptors? |
It s going to be tough, those are C23 standard however, in practice, libc in general haven't really implemented it (see here for a glibc patch in the work). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test the interceptors we don't actually need them to be implemented. Here is an example where I patched in your changes and modified an existing test to demonstrate that the interceptors worked. The tests pass on Linux ELF with static and dyanmic linked compiler runtime.
Can you add a new test for these based on this approach?
✅ With the latest revision this PR passed the C/C++ code formatter. |
5a17dea
to
456b1a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the test is passing because it intercepts free not the free_sized variants you are interested in. You can verify by adding some Printf logging to the new interceptors. For the test we can simply skip defining the body of the free_sized variants and the memprof runtime will provide the right (interceptor) variant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void free_sized(void *ptr, [[maybe_unused]] size_t) { free(ptr); } | |
void free_sized(void *ptr, size_t); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void free_aligned_sized(void *ptr, [[maybe_unused]] size_t alignment, | |
[[maybe_unused]] size_t size) { | |
free(ptr); | |
} | |
void free_aligned_sized(void *ptr, size_t alignment, size_t size); |
456b1a8
to
8804587
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: alignment
8804587
to
4658451
Compare
4658451
to
44b70b0
Compare
…ions.