[PATCH] Sanitize printf functions

Alexey Samsonov samsonov at google.com
Tue Jan 21 03:27:32 PST 2014


  Only a few minor nits (I've also committed part of this patch already). I think it's fine for submission (once you're re-upload the diff addressing the comments).


================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:98
@@ -97,2 +97,3 @@
 
-static int AppendString(char **buff, const char *buff_end, const char *s) {
+static int AppendString(char **buff, const char *buff_end, int precision,
+                        const char *s) {
----------------
I've submitted this part in r199724 (it's not directly related to printf interceptors).

================
Comment at: lib/asan/lit_tests/TestCases/printf-5.c:4
@@ +3,3 @@
+// RUN: ASAN_OPTIONS=check_printf=0 %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+// RUN: %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+
----------------
Once again, what's the point of testing the default configuration? In this way you'll have to update the tests on a flag flip, is this really what we want? Can we remove it, or at least have a single test for default configuration?

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:48
@@ +47,3 @@
+    const char *q = parse_number(p, &number);
+    if (q && *q == '$') {
+      *out = number;
----------------
q is always non-NULL

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:451
@@ +450,3 @@
+
+#define skip_scalar_arg(aq, convSpecifier, size)                   \
+  do {                                                             \
----------------
Please either make it a function (I see, va_list might be a problem), or name it as SKIP_SCALAR_ARG


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



More information about the llvm-commits mailing list