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/Transforms/Utils/SanitizerStats.cpp | ||
---|---|---|
54 | Is that a correct error handling for the unknown format? Should we warn the user? May be even fail hard? | |
tools/sanstats/sanstats.cpp | ||
42 | This expression is hard to read. For example, I had to think thrice, if it increments Begin or the value that Begin points to. Please, split it into a few lines, like: while (Begin < End && Pos != Size) { Result |= uint64_t(uint8_t(*Begin)) << (Pos * 8); Begin++; Pos++; } Also, if that's a hot function, I would recommend to consider a faster implementation that will read the required number of bytes at once, and then swap the bytes with one of the standard helper functions. If this function is rarely called, feel free to leave it as is (modulo my comment above) | |
48 | isn't a reimplementation of strnlen? | |
66 | At this point Begin could be greater than End. May be check in the previous line? | |
127 | I highly suggest to check Begin < End and optionally check that Begin == End after the loop. Otherwise, it's not hard to imaging that a tiny mistake somewhere would just Begin slightly past End and then we'll have to read after the end of the buffer until SEGFAULT stops this heroic attempt to overflow uintptr. |
- Address review comments
lib/Transforms/Utils/SanitizerStats.cpp | ||
---|---|---|
54 | It would probably be appropriate to call report_fatal_error here. | |
tools/sanstats/sanstats.cpp | ||
48 | Yes, but strnlen is not part of the C standard, and I think it's clearer to be explicit. | |
66 |
We defend against this in ReadLE.
I considered this, but it's harder to distinguish between short reads and EOF this way. | |
127 | I added an assertion that should catch this. |
- Switch to an alternative in-memory representation that avoids the need for linker magic
Is that a correct error handling for the unknown format? Should we warn the user? May be even fail hard?