[PATCH] [sanitizer] Improve scanf interceptor

Evgeniy Stepanov eugenis at google.com
Fri Feb 8 01:39:33 PST 2013



================
Comment at: sanitizer_common_interceptors_scanf.inc:143
@@ +142,3 @@
+    c == 'f' || c == 'F' ||
+    c == 'g' || c == 'G';
+}
----------------
Alexey Samsonov wrote:
> my man doesn't know about "A", "F", "G". Is there online page with specs you used? Probably you can give a link in the code.
I've added the link to the file comment.

================
Comment at: sanitizer_common_interceptors_scanf.inc:80
@@ +79,3 @@
+    // m
+    if (*p == 'm') {
+      dir->allocate = true;
----------------
Alexey Samsonov wrote:
> Hm? My "man scanf" tells it should be 'a'
In POSIX it is "m". "a" is ambiguous between and GNU and POSIX, therefore it's banned with certain characters (see bottom of scanf_parse_next).

================
Comment at: sanitizer_common_interceptors_scanf.inc:48
@@ -111,1 +47,3 @@
+    }
+    // A number. Either %n$ syntax, or the field width.
     if (*p >= '0' && *p <= '9') {
----------------
Alexey Samsonov wrote:
> I think you try to distinguish %n$ case (try to parse a number, check if it's followed by $), and if not, fall back to the general case, parsing stuff in this order:
> 1) allocation suppression
> 2) "a" character
> 3) field width
> 4) type modifier
> 5) conv specifier
Fine, lets do it like this. It is slower (the number is parsed twice in the common case), but slightly shorter.


================
Comment at: sanitizer_common_interceptors_scanf.inc:70
@@ +69,3 @@
+      }
+      // Field width.
+      if (*p >= '0' && *p <= '9') {
----------------
Alexey Samsonov wrote:
> Can you hoist parsing field width to a function?
I hoisted parsing a number to a function, not sure what else can be done here.

================
Comment at: sanitizer_common_interceptors_scanf.inc:85
@@ +84,3 @@
+    // Length modifier.
+    if (*p == 'j' || *p == 'z' || *p == 't' || *p == 'L' || *p == 'q') {
+      dir->lengthModifier[0] = *p;
----------------
Alexey Samsonov wrote:
> Can you use smth like internal_strchr(*p, "jztLq") here and below?
It would be slow.

================
Comment at: sanitizer_common_interceptors_scanf.inc:172
@@ +171,3 @@
+  if (scanf_is_integer_conv(dir->convSpecifier)) {
+    switch (dir->lengthModifier[0]) {
+    case 'h':
----------------
Alexey Samsonov wrote:
> Why did you drop arrays of pairs { ('j', sizeof(INTMAX_T) , ... }?
This data does not tabulate well. We would have a special case for "hh" and "ll". And, anyway, it is used in one place only.

================
Comment at: sanitizer_common_interceptors_scanf.inc:251
@@ +250,3 @@
+
+  printf("START: %s\n", format);
+
----------------
Alexey Samsonov wrote:
> Remove this
done


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



More information about the llvm-commits mailing list