[PATCH] Sanitize printf functions

Yury Gribov tetra2005 at gmail.com
Thu Jan 9 22:10:01 PST 2014



================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:360
@@ +359,3 @@
+
+#define skip_scalar_arg(aq, convSpec, size)           \
+  do {                                                \
----------------
Yury Gribov wrote:
> Evgeniy Stepanov wrote:
> > Does it really need to be a macro?
> I was under impression that pointers to va_list are not portable C but http://gcc.gnu.org/ml/gcc-help/1999-q3n/msg00283.html suggests that they're sane. I'll give it a try tomorrow.
Ok, even though standard kind of allows this, GCC has a long and famous history of pointers to va_list not working (see e.g. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557). Looks like passing va_list by reference is unportable.

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:98
@@ +97,3 @@
+      if (output)
+        dir->printErrno = true;
+      else
----------------
Yury Gribov wrote:
> Evgeniy Stepanov wrote:
> > This does not look right. %m in printf is a complete conversion specifier, it should end parsing of the current directive. And it seems like "m" could go into convSpecifier, and printErrno could be removed.
> > 
> > %m in printf is a complete conversion specifier, it should end parsing of the current directive
> 
> I agree that we are currently parsing a wider format language than really necessary. But same is true for %n in original scanf implementation and we didn't bother handling it.
> And it seems like "m" could go into convSpecifier, and printErrno could be removed.

Agreed to this one, will fix.


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



More information about the llvm-commits mailing list