[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