[PATCH] More string interceptors: strstr, strcasestr, strcspn, strpbrk

[Maria Guseva]

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:279
@@ +278,3 @@
+    COMMON_INTERCEPTOR_READ_STRING_OF_LEN(ctx, s2, len2, r ? len2 + 1 :
+                                   (len2 <= len1 ? len2 : len1 + 1));
+}
----------------
dvyukov wrote:
> I am not sure such complex computations are necessary. People should not rely on such things.
> If anything, I think, the fifth argument should be 1, because if I call strstr("aaaaaaaaaaaa", "bcdef") I can "expect" that the function will read only the first symbol "b" from the second string.
Do you mean the following: (ctx, s2, len2, r ? len2 + 1 : (len2 <= len1 ? 1 : len1 + 1)); ?
In that case we may not check real read of bigger length from s2 in case there is some not null intersection of s1 and s2.
The most accurate result would be if repeat explicitly strstr computations and find that real length. But I've just added non-strict check assuming the worst case.  


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:322
@@ +321,3 @@
+    COMMON_INTERCEPTOR_READ_STRING(ctx, s2,
+        s1 && *s1 ? internal_strchr(s2, s1[r]) - s2 + 1 : 0);
+    COMMON_INTERCEPTOR_READ_STRING(ctx, s1, r + 1);
----------------
dvyukov wrote:
> I don't believe this is necessary. Make it just:
> READ_RANGE(s2, REAL(strlen)(s2) + 1);
Why not?
Indeed we don't need to read the full length of s2. User may expect it in non-strict case.

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:337
@@ +336,3 @@
+        r ? internal_strchr(s2, *r) - s2 + 1 :
+        *s1 ? REAL(strlen)(s2) + 1 : 0);
+    COMMON_INTERCEPTOR_READ_STRING(ctx, s1,
----------------
dvyukov wrote:
> I don't believe this complex code is necessary. Make it just:
> READ_RANGE(s2, REAL(strlen)(s2) + 1);
The same - please explain. My intention was to avoid strict checks when possible.

================
Comment at: lib/sanitizer_common/sanitizer_flags.inc:156
@@ -155,1 +155,3 @@
             "If set check that string arguments are properly null-terminated")
+COMMON_FLAG(bool, intercept_strstr, true,
+            "If set, uses custom wrappers for strstr and strcasestr functions "
----------------
dvyukov wrote:
> do we need all these additional flags? why?
It was discussed in initial patch for this http://reviews.llvm.org/D6056#92330: glider suggested to have individual flags. I agree, it's more flexible.

================
Comment at: lib/sanitizer_common/sanitizer_platform_interceptors.h:58
@@ +57,3 @@
+#define SANITIZER_INTERCEPT_STRSTR 1
+#define SANITIZER_INTERCEPT_STRCASESTR SI_NOT_WINDOWS
+#define SANITIZER_INTERCEPT_STRCSPN 1
----------------
dvyukov wrote:
> this is unused, delete
It's my bug, SANITIZER_INTERCEPT_STRCASESTR must be used in sanitizer_common_interceptors.inc cause indeed there is no strcasestr on Windows. I'll fix it. 

The rest is always 1 so I agree it could be removed. I wonder why there are other defines allways equal to 1 in sanitizer_platform_interceptors.h?

http://reviews.llvm.org/D9017

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/