[dfsan] Fix a bug in strcasecmp() and strncasecmp(): Compare the lowercase versions of the characters when choosing a return value.

[skerner]

Summary

Resolves this bug:

https://bugs.llvm.org/show_bug.cgi?id=38369

[morehouse]

compiler-rt/test/dfsan/custom.cpp

217

IIUC, this test would still pass with the buggy code. I think the case we want is:

s1 = "ABCZ";
s2 = "abcy";
...
assert(rv > 0);

Since 'y' - 'Z' > 0 but 'y' - 'z' < 0.

264

Same as above.

[skerner]

Address review comments.

compiler-rt/test/dfsan/custom.cpp

217

Since 'y' - 'Z' > 0 but 'y' - 'z' < 0.

You are correct. It has been a few decades since I looked at an ASCII-code chart, and I was under the impression that lowercase chars had smaller values than uppercase.

Fixed.

[morehouse]

Looks like the diff lost the actual fix. Please add it back and I'll merge.

[skerner]

Address review comments.

[skerner]

Address review comments.

[skerner]

Diff issues...

[skerner]

All changes in one commit.

[skerner]

Looks like the diff lost the actual fix. Please add it back and I'll merge.

Done. PTAL.

[morehouse]

LGTM

compiler-rt/lib/dfsan/dfsan_custom.cpp

SANITIZER_INTERFACE_ATTRIBUTE int __dfsw_strcmp(const char *s1, const char *s2,
   }
   return 0;
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE int
 __dfsw_strcasecmp(const char *s1, const char *s2, dfsan_label s1_label,
                   dfsan_label s2_label, dfsan_label *ret_label) {
   for (size_t i = 0;; ++i) {
-    if (tolower(s1[i]) != tolower(s2[i]) || s1[i] == 0 || s2[i] == 0) {
+    char s1_lower = tolower(s1[i]);
+    char s2_lower = tolower(s2[i]);
+
+    if (s1_lower != s2_lower || s1[i] == 0 || s2[i] == 0) {
       if (flags().strict_data_dependencies) {
         *ret_label = 0;
       } else {
         *ret_label = dfsan_union(dfsan_read_label(s1, i + 1),
                                  dfsan_read_label(s2, i + 1));
       }
-      return s1[i] - s2[i];
+      return s1_lower - s2_lower;
     }
   }
   return 0;
 }
 
 DECLARE_WEAK_INTERCEPTOR_HOOK(dfsan_weak_hook_strncmp, uptr caller_pc,
                               const char *s1, const char *s2, size_t n,
                               dfsan_label s1_label, dfsan_label s2_label,
__dfsw_strncasecmp(const char *s1, const char *s2, size_t n,
                    dfsan_label s1_label, dfsan_label s2_label,
                    dfsan_label n_label, dfsan_label *ret_label) {
   if (n == 0) {
     *ret_label = 0;
     return 0;
   }
 
   for (size_t i = 0;; ++i) {
-    if (tolower(s1[i]) != tolower(s2[i]) || s1[i] == 0 || s2[i] == 0 ||
-        i == n - 1) {
+    char s1_lower = tolower(s1[i]);
+    char s2_lower = tolower(s2[i]);
+
+    if (s1_lower != s2_lower || s1[i] == 0 || s2[i] == 0 || i == n - 1) {
       if (flags().strict_data_dependencies) {
         *ret_label = 0;
       } else {
         *ret_label = dfsan_union(dfsan_read_label(s1, i + 1),
                                  dfsan_read_label(s2, i + 1));
       }
-      return s1[i] - s2[i];
+      return s1_lower - s2_lower;
     }
   }
   return 0;
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE void *__dfsw_calloc(size_t nmemb, size_t size,
                                                   dfsan_label nmemb_label,
                                                   dfsan_label size_label,

compiler-rt/test/dfsan/custom.cpp

#endif
 
   rv = strcasecmp(str1, str3);
   assert(rv == 0);
 #ifdef STRICT_DATA_DEPENDENCIES
   ASSERT_ZERO_LABEL(rv);
 #else
   ASSERT_LABEL(rv, dfsan_union(i_label, j_label));
 #endif
+
+  char s1[] = "ABCz";
+  char s2[] = "abcY";
+  dfsan_set_label(i_label, &s1[3], 1);
+  dfsan_set_label(j_label, &s2[3], 1);
+
+  rv = strcasecmp(s1, s2);
+  assert(rv > 0); // 'z' > 'Y'
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_ZERO_LABEL(rv);
+#else
+  ASSERT_LABEL(rv, dfsan_union(i_label, j_label));
+#endif
 }
 
 void test_strncasecmp() {
   char str1[] = "Str1", str2[] = "str2";
   dfsan_set_label(i_label, &str1[3], 1);
   dfsan_set_label(j_label, &str2[3], 1);
 
   int rv = strncasecmp(str1, str2, sizeof(str1));
   assert(rv < 0);
 #ifdef STRICT_DATA_DEPENDENCIES
   ASSERT_ZERO_LABEL(rv);
 #else
   ASSERT_LABEL(rv, dfsan_union(i_label, j_label));
 #endif
 
   rv = strncasecmp(str1, str2, 3);
   assert(rv == 0);
   ASSERT_ZERO_LABEL(rv);
+
+  char s1[] = "ABCz";
+  char s2[] = "abcY";
+  dfsan_set_label(i_label, &s1[3], 1);
+  dfsan_set_label(j_label, &s2[3], 1);
+
+  rv = strncasecmp(s1, s2, 0);
+  assert(rv == 0); // Compare zero chars.
+  ASSERT_ZERO_LABEL(rv);
+
+  rv = strncasecmp(s1, s2, 1);
+  assert(rv == 0); // 'a' == 'A'
+  ASSERT_ZERO_LABEL(rv);
+
+  rv = strncasecmp(s1, s2, 2);
+  assert(rv == 0); // 'b' == 'B'
+  ASSERT_ZERO_LABEL(rv);
+
+  rv = strncasecmp(s1, s2, 3);
+  assert(rv == 0); // 'c' == 'C'
+  ASSERT_ZERO_LABEL(rv);
+
+  rv = strncasecmp(s1, s2, 4);
+  assert(rv > 0); // 'z' > 'Y'
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_ZERO_LABEL(rv);
+#else
+  ASSERT_LABEL(rv, dfsan_union(i_label, j_label));
+#endif
 }
 
 void test_strchr() {
   char str1[] = "str1";
   dfsan_set_label(i_label, &str1[3], 1);
 
   char *crv = strchr(str1, 'r');
   assert(crv == &str1[2]);