Allows users of sanitizer printf() to create wrappers around the sanitizer Printf function. Used in GWP-ASan to allow us to default-provide a function pointer to Printf(), as the signature taken by GWP-ASan arguments has an 'int' return type.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 30966 Build 30965: arc lint + arc unit
Event Timeline
compiler-rt/lib/sanitizer_common/sanitizer_common.h | ||
---|---|---|
25 | No, please don't include any system headers here. |
Is this meant as a test-only thing?
I don't think we want sanitizer Printf anywhere close to production.
IMHO, we could use printf() in gwpasan directly, with a guard against recursive malloc.
compiler-rt/lib/sanitizer_common/sanitizer_common.h | ||
---|---|---|
25 | stdarg could be fine... |
This is meant to be the default printf() implementation for GwpAsan, for if the allocator doesn't provide us with its own printf().
Don't we already do this with HWASan/etc?
I don't think this will work in a signal-safe manner. The only times that GwpAsan calls printf() is in the signal handler, and even if we temporarily disable sampled allocations, we still are exercising non-reentrant functions in a signal handler.
IMHO, we could use printf() in gwpasan directly, with a guard against recursive malloc.
I don't think this will work in a signal-safe manner. The only times that GwpAsan calls printf() is in the signal handler, and even if we temporarily disable sampled allocations, we still are exercising non-reentrant functions in a signal handler.
Right, I did not think of this.
I don't think stdarg.h is dangerous. You could also move the declaration to a separate header.
@kcc ?
- Changed sanitizer printf functions to be in their own header, rather than in sanitizer_common. This primarily is useful to avoid having system libraries in sanitizer_common.h.
@kcc: WDYT about this instead?
compiler-rt/lib/sanitizer_common/sanitizer_deadlock_detector.h | ||
---|---|---|
253–254 | Removing these comments LGTM, but it should be done in a separate patch |
compiler-rt/lib/sanitizer_common/sanitizer_printf.h | ||
---|---|---|
13 | Well, no, that's not any better. Sadly, the sanitizer run-time is not your common C++ program :( |
compiler-rt/lib/sanitizer_common/sanitizer_printf.h | ||
---|---|---|
13 | How about we move the include into a separate header, sanitizer_vprintf.h, and never use it anywhere? |
After discussing offline, we've decided to change the user code to require the printf function as void printf(const char* fmt, ...) instead of the cstdlib int printf(const char* fmt, ...), which allows us to pass the function pointer from __sanitizer::Printf() directly rather than wrapping it.
This change is no longer needed.
few more?