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
Event Timeline
lib/sanitizer_common/sanitizer_flags.cc | ||
---|---|---|
51 | Are we restricted to use C++ strings here? The function is super scary right now. | |
99 | 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 | ||
40 | s/unsigned/size_t/ maybe? | |
47 | Probably, it makes sense to check that start != NULL to fail cleanly if WriteModuleReport is called on an unregistered module. |
lib/stats/stats.cc | ||
---|---|---|
50 | This lazy file opening just complicates things. Maybe move to a separate function and call it from WriteFullReport ? | |
lib/stats/stats_client.cc | ||
13 | Please mention that it is intended to be linked into each individual module, and that it can not use the sanitizer_common infrastructure. | |
95 | Why dlsym and not just declare an external function and call it directly? | |
test/cfi/stats.cpp | ||
2 | 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 | ||
---|---|---|
41 | It doesn't matter, this will never be more than 8. | |
48 | Added a CHECK. | |
lib/stats/stats_client.cc | ||
81 | This won't work on Windows where DSOs can't directly call into the main executable. | |
test/cfi/stats.cpp | ||
3 | 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 | I don't think we can use std::string here. | |
99 | 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 | 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 | 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 | You're right. Thank you for the reminder. |
lib/stats/stats.cc | ||
---|---|---|
87 | There could be a data race if this is called from concurrent dlopen-ed module constructors. |
Are we restricted to use C++ strings here? The function is super scary right now.