[msan] Rewrite strto* interceptors and add a few more.

[earthdok]

Summary

Express the strto* interceptors though macros. This removes a lot of
duplicate code and fixes a couple of copypasto bugs (where "res" was declared of
a different type than the actual return type). Also, add a few more interceptors
for strto*_l.

[earthdok]

I started similarly rewriting the strto* tests in tests/msan_test.cc, but it turns out that the tests in this file are not run as part of check-msan. What's up with that?

Also, who uses the __-prefixed versions? Should we add them for the newly added functions as well?

[eugenis]

libstdc++ depends on these:

__strtof_l
__strtod_l
strtoul
strtold_l

Please add all of them for completeness.
Also, could you move this to sanitizer_common_interceptors.inc, while you are at it?

[earthdok]

Also, could you move this to sanitizer_common_interceptors.inc, while you are at it?

Maybe in a separate CL. But what would that achieve? These interceptors wouldn't have any effect in ASan (and there's already a conflicting implementation of strol).

[eugenis]

The fact that there is an strtol interceptor in asan means there is _something_ to achieve, right? :)
Common interceptors would do READ on the input string, and let us catch asan bugs and tsan data races on that memory.
ASan interceptor has interesting look ahead logic that should also be merged to the common interceptor.

OK to do it in a separate commit later.
LGTM.

[earthdok]

rewrite tests

lib/msan/msan_interceptors.cc

INTERCEPTOR(char *, strncat, char *dest, const char *src, SIZE_T n) {  // NOLINT
   SIZE_T dest_size = REAL(strlen)(dest);
   SIZE_T copy_size = REAL(strnlen)(src, n);
   char *res = REAL(strncat)(dest, src, n);  // NOLINT
   __msan_copy_poison(dest + dest_size, src, copy_size);
   __msan_unpoison(dest + dest_size + copy_size, 1); // \0
   return res;
 }
 
-INTERCEPTOR(long, strtol, const char *nptr, char **endptr,  // NOLINT
-            int base) {
-  ENSURE_MSAN_INITED();
-  long res = REAL(strtol)(nptr, endptr, base);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(long long, strtoll, const char *nptr, char **endptr,  // NOLINT
-            int base) {
-  ENSURE_MSAN_INITED();
-  long res = REAL(strtoll)(nptr, endptr, base);  //NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(unsigned long, strtoul, const char *nptr, char **endptr,  // NOLINT
-            int base) {
-  ENSURE_MSAN_INITED();
-  unsigned long res = REAL(strtoul)(nptr, endptr, base);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(unsigned long long, strtoull, const char *nptr,  // NOLINT
-            char **endptr, int base) {
-  ENSURE_MSAN_INITED();
-  unsigned long res = REAL(strtoull)(nptr, endptr, base);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(double, strtod, const char *nptr, char **endptr) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  double res = REAL(strtod)(nptr, endptr);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(double, strtod_l, const char *nptr, char **endptr,
-            void *loc) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  double res = REAL(strtod_l)(nptr, endptr, loc);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(double, __strtod_l, const char *nptr, char **endptr,
-            void *loc) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  double res = REAL(__strtod_l)(nptr, endptr, loc);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(float, strtof, const char *nptr, char **endptr) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  float res = REAL(strtof)(nptr, endptr);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(float, strtof_l, const char *nptr, char **endptr,
-            void *loc) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  float res = REAL(strtof_l)(nptr, endptr, loc);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(float, __strtof_l, const char *nptr, char **endptr,
-            void *loc) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  float res = REAL(__strtof_l)(nptr, endptr, loc);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(long double, strtold, const char *nptr, char **endptr) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  long double res = REAL(strtold)(nptr, endptr);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
-}
-
-INTERCEPTOR(long double, strtold_l, const char *nptr, char **endptr,
-            void *loc) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  long double res = REAL(strtold_l)(nptr, endptr, loc);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
+// Hack: always pass nptr and endptr as part of __VA_ARGS_ to avoid having to
+// deal with empty __VA_ARGS__ in the case of INTERCEPTOR_STRTO.
+#define INTERCEPTOR_STRTO_BODY(ret_type, func, ...) \
+  ENSURE_MSAN_INITED();                             \
+  ret_type res = REAL(func)(__VA_ARGS__);           \
+  if (!__msan_has_dynamic_component()) {            \
+    __msan_unpoison(endptr, sizeof(*endptr));       \
+  }                                                 \
   return res;
-}
 
-INTERCEPTOR(long double, __strtold_l, const char *nptr, char **endptr,
-            void *loc) {  // NOLINT
-  ENSURE_MSAN_INITED();
-  long double res = REAL(__strtold_l)(nptr, endptr, loc);  // NOLINT
-  if (!__msan_has_dynamic_component()) {
-    __msan_unpoison(endptr, sizeof(*endptr));
-  }
-  return res;
+#define INTERCEPTOR_STRTO(ret_type, func)                        \
+  INTERCEPTOR(ret_type, func, const char *nptr, char **endptr) { \
+    INTERCEPTOR_STRTO_BODY(ret_type, func, nptr, endptr);        \
+  }
+
+#define INTERCEPTOR_STRTO_BASE(ret_type, func)                             \
+  INTERCEPTOR(ret_type, func, const char *nptr, char **endptr, int base) { \
+    INTERCEPTOR_STRTO_BODY(ret_type, func, nptr, endptr, base);            \
   }
+
+#define INTERCEPTOR_STRTO_LOC(ret_type, func)                               \
+  INTERCEPTOR(ret_type, func, const char *nptr, char **endptr, void *loc) { \
+    INTERCEPTOR_STRTO_BODY(ret_type, func, nptr, endptr, loc);              \
+  }
+
+#define INTERCEPTOR_STRTO_BASE_LOC(ret_type, func)                       \
+  INTERCEPTOR(ret_type, func, const char *nptr, char **endptr, int base, \
+              void *loc) {                                               \
+    INTERCEPTOR_STRTO_BODY(ret_type, func, nptr, endptr, base, loc);     \
+  }
+
+INTERCEPTOR_STRTO(double, strtod)                           // NOLINT
+INTERCEPTOR_STRTO(float, strtof)                            // NOLINT
+INTERCEPTOR_STRTO(long double, strtold)                     // NOLINT
+INTERCEPTOR_STRTO_BASE(long, strtol)                        // NOLINT
+INTERCEPTOR_STRTO_BASE(long long, strtoll)                  // NOLINT
+INTERCEPTOR_STRTO_BASE(unsigned long, strtoul)              // NOLINT
+INTERCEPTOR_STRTO_BASE(unsigned long long, strtoull)        // NOLINT
+INTERCEPTOR_STRTO_LOC(double, strtod_l)                     // NOLINT
+INTERCEPTOR_STRTO_LOC(double, __strtod_l)                   // NOLINT
+INTERCEPTOR_STRTO_LOC(float, strtof_l)                      // NOLINT
+INTERCEPTOR_STRTO_LOC(float, __strtof_l)                    // NOLINT
+INTERCEPTOR_STRTO_LOC(long double, strtold_l)               // NOLINT
+INTERCEPTOR_STRTO_LOC(long double, __strtold_l)             // NOLINT
+INTERCEPTOR_STRTO_BASE_LOC(long, strtol_l)                  // NOLINT
+INTERCEPTOR_STRTO_BASE_LOC(long long, strtoll_l)            // NOLINT
+INTERCEPTOR_STRTO_BASE_LOC(unsigned long, strtoul_l)        // NOLINT
+INTERCEPTOR_STRTO_BASE_LOC(unsigned long long, strtoull_l)  // NOLINT
 
 INTERCEPTOR(int, vasprintf, char **strp, const char *format, va_list ap) {
   ENSURE_MSAN_INITED();
   int res = REAL(vasprintf)(strp, format, ap);
   if (res >= 0 && !__msan_has_dynamic_component()) {
     __msan_unpoison(strp, sizeof(*strp));
     __msan_unpoison(*strp, res + 1);
   }
void InitializeInterceptors() {
   INTERCEPT_FUNCTION(strtod_l);
   INTERCEPT_FUNCTION(__strtod_l);
   INTERCEPT_FUNCTION(strtof);
   INTERCEPT_FUNCTION(strtof_l);
   INTERCEPT_FUNCTION(__strtof_l);
   INTERCEPT_FUNCTION(strtold);
   INTERCEPT_FUNCTION(strtold_l);
   INTERCEPT_FUNCTION(__strtold_l);
+  INTERCEPT_FUNCTION(strtol_l);
+  INTERCEPT_FUNCTION(strtoll_l);
+  INTERCEPT_FUNCTION(strtoul_l);
+  INTERCEPT_FUNCTION(strtoull_l);
   INTERCEPT_FUNCTION(vasprintf);
   INTERCEPT_FUNCTION(asprintf);
   INTERCEPT_FUNCTION(vsprintf);
   INTERCEPT_FUNCTION(vsnprintf);
   INTERCEPT_FUNCTION(vswprintf);
   INTERCEPT_FUNCTION(sprintf);  // NOLINT
   INTERCEPT_FUNCTION(snprintf);
   INTERCEPT_FUNCTION(swprintf);

lib/msan/tests/msan_test.cc

TEST(MemorySanitizer, strncat_overflow) {  // NOLINT
   strncat(a, b, 2);
   EXPECT_NOT_POISONED(a[3]);
   EXPECT_POISONED(a[4]);
   EXPECT_NOT_POISONED(a[5]);
   EXPECT_POISONED(a[6]);
   EXPECT_POISONED(a[7]);
 }
 
-TEST(MemorySanitizer, strtol) {
-  char *e;
-  ASSERT_EQ(1, strtol("1", &e, 10));
-  EXPECT_NOT_POISONED((S8) e);
-}
-
-TEST(MemorySanitizer, strtoll) {
-  char *e;
-  ASSERT_EQ(1, strtoll("1", &e, 10));
-  EXPECT_NOT_POISONED((S8) e);
-}
-
-TEST(MemorySanitizer, strtoul) {
-  char *e;
-  ASSERT_EQ(1, strtoul("1", &e, 10));
-  EXPECT_NOT_POISONED((S8) e);
-}
-
-TEST(MemorySanitizer, strtoull) {
-  char *e;
-  ASSERT_EQ(1, strtoull("1", &e, 10));
-  EXPECT_NOT_POISONED((S8) e);
-}
+#define TEST_STRTO_INT(func_name)         \
+  TEST(MemorySanitizer, func_name) {      \
+    char *e;                              \
+    EXPECT_EQ(1, func_name("1", &e, 10)); \
+    EXPECT_NOT_POISONED((S8)e);           \
+  }
+
+#define TEST_STRTO_FLOAT(func_name)     \
+  TEST(MemorySanitizer, func_name) {    \
+    char *e;                            \
+    EXPECT_NE(0, func_name("1.5", &e)); \
+    EXPECT_NOT_POISONED((S8)e);         \
+  }
+
+#define TEST_STRTO_FLOAT_LOC(func_name)                          \
+  TEST(MemorySanitizer, func_name) {                             \
+    locale_t loc = newlocale(LC_NUMERIC_MASK, "C", (locale_t)0); \
+    char *e;                                                     \
+    EXPECT_NE(0, func_name("1.5", &e, loc));                     \
+    EXPECT_NOT_POISONED((S8)e);                                  \
+    freelocale(loc);                                             \
+  }
+
+#define TEST_STRTO_INT_LOC(func_name)                            \
+  TEST(MemorySanitizer, func_name) {                             \
+    locale_t loc = newlocale(LC_NUMERIC_MASK, "C", (locale_t)0); \
+    char *e;                                                     \
+    ASSERT_EQ(1, func_name("1", &e, 10, loc));                   \
+    EXPECT_NOT_POISONED((S8)e);                                  \
+    freelocale(loc);                                             \
+  }
+
+TEST_STRTO_INT(strtol)
+TEST_STRTO_INT(strtoll)
+TEST_STRTO_INT(strtoul)
+TEST_STRTO_INT(strtoull)
+
+TEST_STRTO_FLOAT(strtof)
+TEST_STRTO_FLOAT(strtod)
+TEST_STRTO_FLOAT(strtold)
+
+TEST_STRTO_FLOAT_LOC(strtof_l)
+TEST_STRTO_FLOAT_LOC(strtod_l)
+TEST_STRTO_FLOAT_LOC(strtold_l)
+
+TEST_STRTO_INT_LOC(strtol_l)
+TEST_STRTO_INT_LOC(strtoll_l)
+TEST_STRTO_INT_LOC(strtoul_l)
+TEST_STRTO_INT_LOC(strtoull_l)
 
 // https://code.google.com/p/memory-sanitizer/issues/detail?id=36
 TEST(MemorySanitizer, DISABLED_strtoimax) {
   char *e;
   ASSERT_EQ(1, strtoimax("1", &e, 10));
   EXPECT_NOT_POISONED((S8) e);
 }
 
 // https://code.google.com/p/memory-sanitizer/issues/detail?id=36
 TEST(MemorySanitizer, DISABLED_strtoumax) {
   char *e;
   ASSERT_EQ(1, strtoumax("1", &e, 10));
   EXPECT_NOT_POISONED((S8) e);
 }
 
-TEST(MemorySanitizer, strtod) {
-  char *e;
-  ASSERT_NE(0, strtod("1.5", &e));
-  EXPECT_NOT_POISONED((S8) e);
-}
-
 #ifdef __GLIBC__
+extern "C" float __strtof_l(const char *nptr, char **endptr, locale_t loc);
+TEST_STRTO_FLOAT_LOC(__strtof_l)
 extern "C" double __strtod_l(const char *nptr, char **endptr, locale_t loc);
-TEST(MemorySanitizer, __strtod_l) {
-  locale_t loc = newlocale(LC_NUMERIC_MASK, "C", (locale_t)0);
-  char *e;
-  ASSERT_NE(0, __strtod_l("1.5", &e, loc));
-  EXPECT_NOT_POISONED((S8) e);
-  freelocale(loc);
-}
+TEST_STRTO_FLOAT_LOC(__strtod_l)
+extern "C" long double __strtold_l(const char *nptr, char **endptr,
+                                   locale_t loc);
+TEST_STRTO_FLOAT_LOC(__strtold_l)
 #endif  // __GLIBC__
 
-TEST(MemorySanitizer, strtof) {
-  char *e;
-  ASSERT_NE(0, strtof("1.5", &e));
-  EXPECT_NOT_POISONED((S8) e);
-}
-
-TEST(MemorySanitizer, strtold) {
-  char *e;
-  ASSERT_NE(0, strtold("1.5", &e));
-  EXPECT_NOT_POISONED((S8) e);
-}
-
 TEST(MemorySanitizer, modf) {
   double x, y;
   x = modf(2.1, &y);
   EXPECT_NOT_POISONED(y);
 }
 
 TEST(MemorySanitizer, modff) {
   float x, y;