[PATCH] Sanitize printf functions

Alexander Potapenko glider at google.com
Tue Dec 31 02:45:17 PST 2013


  Looks mostly good.


================
Comment at: asan/lit_tests/TestCases/printf-4.c:6
@@ +5,3 @@
+#include <string.h>
+#include <stdlib.h>
+int main() {
----------------
Please sort the includes alphabetically (here and in other tests)

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:11
@@ -11,2 +10,3 @@
+// Scanf/printf implementation for use in *Sanitizer interceptors.
 // Follows http://pubs.opengroup.org/onlinepubs/9699919799/functions/fscanf.html
 // with a few common GNU extensions.
----------------
How about this line? Is there a printf specification we're following?

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:323
@@ +322,3 @@
+
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);
+
----------------
Please swap this line with the above declaration and remove the empty line below.

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:337
@@ +336,3 @@
+    if (dir.argIdx != -1) {
+      // Unsupported.
+      break;
----------------
How does this comment relate to the above one?

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:350
@@ +349,3 @@
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, argp, size);
+      // FIXME: detect overlaps with format string?
+      continue;
----------------
Good point, probably worth doing.


http://llvm-reviews.chandlerc.com/D2480



More information about the llvm-commits mailing list