This is part of a new statistics gathering feature for the sanitizers.
See clang/docs/SanitizerStats.rst for further info and docs.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/sanitizer_common/sanitizer_flags.cc | ||
---|---|---|
51 ↗ | (On Diff #44831) | Are we restricted to use C++ strings here? The function is super scary right now. |
98 ↗ | (On Diff #44831) | I don't think that the flag value is limited by kMaxPathLength. Can you please choose a better limit? Or somehow check that the value fits the buffer? |
lib/stats/stats.cc | ||
39 ↗ | (On Diff #44831) | s/unsigned/size_t/ maybe? |
46 ↗ | (On Diff #44831) | Probably, it makes sense to check that start != NULL to fail cleanly if WriteModuleReport is called on an unregistered module. |
lib/stats/stats.cc | ||
---|---|---|
49 ↗ | (On Diff #44831) | This lazy file opening just complicates things. Maybe move to a separate function and call it from WriteFullReport ? |
lib/stats/stats_client.cc | ||
12 ↗ | (On Diff #44831) | Please mention that it is intended to be linked into each individual module, and that it can not use the sanitizer_common infrastructure. |
94 ↗ | (On Diff #44831) | Why dlsym and not just declare an external function and call it directly? |
test/cfi/stats.cpp | ||
1 ↗ | (On Diff #44831) | Does this work with the cross-dso mode? |
- Switch to an alternative in-memory representation that avoids the need for linker magic
- Address review comments
lib/stats/stats.cc | ||
---|---|---|
40 ↗ | (On Diff #44937) | It doesn't matter, this will never be more than 8. |
47 ↗ | (On Diff #44937) | Added a CHECK. |
lib/stats/stats_client.cc | ||
80 ↗ | (On Diff #44937) | This won't work on Windows where DSOs can't directly call into the main executable. |
test/cfi/stats.cpp | ||
2 ↗ | (On Diff #44937) | Almost; this initially gave duplicate symbol errors. To avoid a combinatorial explosion in the number of static sanitizer libraries I decided to start improving how we link them. First step: link stats without -whole-archive, and use -u to pull in the stats runtime. Also added an integration test here. |
- Add CHECK to SubstituteForFlagValue
lib/sanitizer_common/sanitizer_flags.cc | ||
---|---|---|
51 ↗ | (On Diff #44942) | I don't think we can use std::string here. |
98 ↗ | (On Diff #44942) | kMaxPathLength is 4096, which is probably enough (we use it in a bunch of other places elsewhere). I don't think this code matters enough to worry about using a variable-size buffer. I've added a CHECK to the above function. |
lib/sanitizer_common/sanitizer_flags.cc | ||
---|---|---|
51 ↗ | (On Diff #44943) | why? not that I insist, I just probably don't understand the constraints and interested to learn about that. |
lib/sanitizer_common/sanitizer_flags.cc | ||
---|---|---|
51 ↗ | (On Diff #44943) | If the C++ standard library is compiled with sanitizer instrumentation, we would have a circular dependency between sanitizer_common and the stdlib. |
lib/sanitizer_common/sanitizer_flags.cc | ||
---|---|---|
51 ↗ | (On Diff #44943) | You're right. Thank you for the reminder. |
lib/stats/stats.cc | ||
---|---|---|
87 ↗ | (On Diff #44943) | There could be a data race if this is called from concurrent dlopen-ed module constructors. |