[PATCH] Sanitize printf functions

Yury Gribov tetra2005 at gmail.com
Fri Jan 17 03:28:48 PST 2014



================
Comment at: lib/asan/lit_tests/TestCases/printf-2.c:20
@@ +19,3 @@
+  // CHECK-ON: heap-use-after-free
+  // CHECK-ON-NOT: 0 12 1.239 34
+  // CHECK-OFF: 0 12 1.239 
----------------
Alexey Samsonov wrote:
> Yury Gribov wrote:
> > Alexey Samsonov wrote:
> > > Could you match a file/line in the stack trace in error report? E.g. see TestCases/null_deref.cc
> > Well, ok although I'd consider this an overkill.
> We generally don't want many internal frames from ASan runtime to appear in the error report. So it's nice to verify that error report looks somewhat like this:
> #0 __interceptor_printf ....
> #1 main printf-2.c:16
Got it.

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:25
@@ -33,1 +24,3 @@
 
+static const char *maybe_parse_signed(const char *p, int *out) {
+  if (*p == '-')
----------------
Alexey Samsonov wrote:
> Yury Gribov wrote:
> > Alexey Samsonov wrote:
> > > Quick check: is it OK you will consume the first "-" in "--" string?
> > %-- is invalid syntax anyway so I think it's ok.
> Alright, I agree that %-- is just invalid scanf/printf syntax and we may not necessarily parse it correctly, but now we move this to function "maybe_parse_signed", and its name doesn't tell much about this hidden assumptions ("-" consumed from "--", "0" results in NULL returned), so one could use it in a wrong way somewhere else. IMO It's better to specify this assumptions explicitly here.
I agree that whole parse_number/parse_signed thing is a bit messy. Let me see if I can prettify it with next commit.


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



More information about the llvm-commits mailing list