[PATCH] Sanitize printf functions

Yury Gribov tetra2005 at gmail.com
Tue Jan 21 03:43:30 PST 2014



================
Comment at: lib/sanitizer_common/tests/sanitizer_scanf_interceptor_test.cc:37
@@ -26,3 +36,3 @@
 
-static const char scanf_buf[] = "Test string.";
-static size_t scanf_buf_size = sizeof(scanf_buf);
+static const unsigned I = sizeof(int);          // NOLINT
+static const unsigned L = sizeof(long);         // NOLINT
----------------
Alexander Potapenko wrote:
> Can you please add a comment above describing why NOLINT is necessary here?
I copy-pasted this from TEST(SanitizerCommonInterceptors, Scanf). Seems to work with NOLINT removed so I'll update the patch.

================
Comment at: lib/sanitizer_common/tests/sanitizer_scanf_interceptor_test.cc:43
@@ +42,3 @@
+static const unsigned D = sizeof(double);       // NOLINT
+static const unsigned LD = sizeof(long double); // NOLINT
+static const unsigned F = sizeof(float);        // NOLINT
----------------
Alexander Potapenko wrote:
> At least two spaces between the code and the comment, please (you'll need to reformat the rest of declarations)
Likewise.

================
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
+
----------------
Kostya Serebryany wrote:
> Alexey Samsonov wrote:
> > 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?
> This was my suggestion, I think this *is* what we want. 
Yeah, I guess Kostya wants to explicitly test that printf checks are disabled by default for now.

================
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;
----------------
Alexey Samsonov wrote:
> q is always non-NULL
+1

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


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



More information about the llvm-commits mailing list