[PATCH] D16174: Introduce sanstats tool and llvm::CreateSanitizerStatReport function.

[Ivan Krasin via llvm-commits]

krasin1 added a subscriber: krasin1.

================
Comment at: lib/Transforms/Utils/SanitizerStats.cpp:54
@@ +53,3 @@
+  case Triple::UnknownObjectFormat:
+    break;
+  }
----------------
Is that a correct error handling for the unknown format? Should we warn the user? May be even fail hard?

================
Comment at: tools/sanstats/sanstats.cpp:42
@@ +41,3 @@
+  while (Begin < End && Pos != Size)
+    Result |= uint64_t(uint8_t(*Begin++)) << (Pos++ * 8);
+  return Result;
----------------
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)

================
Comment at: tools/sanstats/sanstats.cpp:48
@@ +47,3 @@
+  const char *FilenameBegin = Begin;
+  while (Begin != End && *Begin)
+    ++Begin;
----------------
isn't a reimplementation of strnlen?

================
Comment at: tools/sanstats/sanstats.cpp:66
@@ +65,3 @@
+    Begin += SizeofPtr;
+    uint64_t Data = ReadLE(SizeofPtr, Begin, End);
+    Begin += SizeofPtr;
----------------
At this point Begin could be greater than End. May be check in the previous line?
Alternatively, make ReadLE to accept a pointer to Begin and let it to update it.

================
Comment at: tools/sanstats/sanstats.cpp:127
@@ +126,3 @@
+  char SizeofPtr = *Begin++;
+  while (Begin != End) {
+    Begin = ReadModule(SizeofPtr, Begin, End);
----------------
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.


http://reviews.llvm.org/D16174