[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