[msan] Support %ms in scanf.

[guiand]

LGTM. Does this support the a format specifier as well, or just m?

[eugenis]

The problem with "a" is that we do not always know what it means exactly (could be a float), and it's dangerous to dereference the pointer.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc

346–349

for consistency, we use real for similar stuff in other interceptors and without temp var

[eugenis]

.

[eugenis]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc

346–349

This file uses internal_strlen everywhere. I've got rid of the temp variable.

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc

 // and http://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html
 // with a few common GNU extensions.
 //
 //===----------------------------------------------------------------------===//
 
 #include <stdarg.h>
 
 static const char *parse_number(const char *p, int *out) {
   *out = internal_atoll(p);
   while (*p >= '0' && *p <= '9')
     ++p;
   return p;
 }
 
 static const char *maybe_parse_param_index(const char *p, int *out) {
   // n$
   if (*p >= '0' && *p <= '9') {
     int number;
     const char *q = parse_number(p, &number);
     CHECK(q);
     if (*q == '$') {
       *out = number;
       p = q + 1;
     }
   }
 
   // Otherwise, do not change p. This will be re-parsed later as the field
   // width.
   return p;
 }
 
 static bool char_is_one_of(char c, const char *s) {
   return !!internal_strchr(s, c);
 }
 
 static const char *maybe_parse_length_modifier(const char *p, char ll[2]) {
   if (char_is_one_of(*p, "jztLq")) {
     ll[0] = *p;
     ++p;
   } else if (*p == 'h') {
     ll[0] = 'h';
case 'h':
       return lengthModifier[1] == 'h' ? sizeof(char) : sizeof(short);
     case 'l':
       return lengthModifier[1] == 'l' ? sizeof(long long) : sizeof(long);
     case 'q':
       return sizeof(long long);
     case 'L':
       return sizeof(long long);
     case 'j':
       return sizeof(INTMAX_T);
     case 'z':
       return sizeof(SIZE_T);
     case 't':
       return sizeof(PTRDIFF_T);
     case 0:
       return sizeof(int);
     default:
       return FSS_INVALID;
     }
   }
 
   if (format_is_float_conv(convSpecifier)) {

 };
 
 // Parse scanf format string. If a valid directive in encountered, it is
 // returned in dir. This function returns the pointer to the first
 // unprocessed character, or 0 in case of error.
 // In case of the end-of-string, a pointer to the closing \0 is returned.
 static const char *scanf_parse_next(const char *p, bool allowGnuMalloc,
                                     ScanfDirective *dir) {
   internal_memset(dir, 0, sizeof(*dir));
   dir->argIdx = -1;
 
   while (*p) {
     if (*p != '%') {
       ++p;
       continue;
     }
     dir->begin = p;
     ++p;
     // %%
     if (*p == '%') {
       ++p;
       continue;
     }
     if (*p == '\0') {
       return nullptr;
     }
     // %n$
     p = maybe_parse_param_index(p, &dir->argIdx);
     CHECK(p);
     // *
     if (*p == '*') {
       dir->suppressed = true;
       ++p;
     }
     // Field width
     if (*p >= '0' && *p <= '9') {
       p = parse_number(p, &dir->fieldWidth);
       CHECK(p);
       if (dir->fieldWidth <= 0)  // Width if at all must be non-zero
         return nullptr;
     }
     // m
     if (*p == 'm') {
       dir->allocate = true;
       ++p;
     }
static int scanf_get_value_size(ScanfDirective *dir) {
   return format_get_value_size(dir->convSpecifier, dir->lengthModifier, false);
 }
 
 // Common part of *scanf interceptors.
 // Process format string and va_list, and report all store ranges.
 // Stops when "consuming" n_inputs input items.
 static void scanf_common(void *ctx, int n_inputs, bool allowGnuMalloc,
                          const char *format, va_list aq) {
   CHECK_GT(n_inputs, 0);
   const char *p = format;
 
   COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);
 
   while (*p) {
     ScanfDirective dir;
     p = scanf_parse_next(p, allowGnuMalloc, &dir);
     if (!p)
       break;
     if (dir.convSpecifier == 0) {
       // This can only happen at the end of the format string.
       CHECK_EQ(*p, 0);
       break;
     }
     // Here the directive is valid. Do what it says.
     if (dir.argIdx != -1) {
       // Unsupported.
       break;
     }
     if (dir.suppressed)
       continue;
     int size = scanf_get_value_size(&dir);
     if (size == FSS_INVALID) {
       Report("%s: WARNING: unexpected format specifier in scanf interceptor: ",
              SanitizerToolName, "%.*s\n", dir.end - dir.begin, dir.begin);
       break;
     }
     void *argp = va_arg(aq, void *);
     if (dir.convSpecifier != 'n')
       --n_inputs;
     if (n_inputs < 0)
       break;
     if (size == FSS_STRLEN) {
       size = internal_strlen((const char *)argp) + 1;
     } else if (size == FSS_WCSLEN) {
       // FIXME: actually use wcslen() to calculate it.
       size = 0;
     }
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, argp, size);
+    // For %ms/%mc, write the allocated output buffer as well.
+    if (dir.allocate) {
+      char *buf = *(char **)argp;
+      if (buf) {
+        uptr buf_len = internal_strlen(buf) + 1;
+        COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buf_len);
+      }
+    }
   }
 }
 
 #if SANITIZER_INTERCEPT_PRINTF
 
 struct PrintfDirective {
   int fieldWidth;
   int fieldPrecision;

compiler-rt/lib/sanitizer_common/tests/sanitizer_format_interceptor_test.cpp

 static const unsigned LC = sizeof(wchar_t);
 static const unsigned D = sizeof(double);
 static const unsigned LD = sizeof(long double);
 static const unsigned F = sizeof(float);
 static const unsigned P = sizeof(char *);
 
 static void verifyFormatResults(const char *format, unsigned n,
                                 const std::vector<unsigned> &computed_sizes,
-                                va_list expected_sizes) {
-  // "+ 1" because of format string
+                                const std::vector<unsigned> &expected_sizes) {
+  // "+ 1" because of the format string
   ASSERT_EQ(n + 1,
             computed_sizes.size()) << "Unexpected number of format arguments: '"
                                    << format << "'";
   for (unsigned i = 0; i < n; ++i)
-    EXPECT_EQ(va_arg(expected_sizes, unsigned), computed_sizes[i + 1])
+    EXPECT_EQ(expected_sizes[i], computed_sizes[i + 1])
         << "Unexpect write size for argument " << i << ", format string '"
         << format << "'";
 }
 
 static const char test_buf[] = "Test string.";
 static const size_t test_buf_size = sizeof(test_buf);
 
 static const unsigned SCANF_ARGS_MAX = 16;
 
 static void testScanf3(void *ctx, int result, bool allowGnuMalloc,
                        const char *format, ...) {
   va_list ap;
   va_start(ap, format);
   scanf_common(ctx, result, allowGnuMalloc, format, ap);
   va_end(ap);
 }
 
 static void testScanf2(const char *format, int scanf_result,
                        bool allowGnuMalloc, unsigned n,
-                       va_list expected_sizes) {
-  std::vector<unsigned> scanf_sizes;
+                       va_list expected_sizes_va) {
+  std::vector<unsigned> scanf_sizes, expected_sizes;
+  for (unsigned i = 0; i < n; ++i)
+    expected_sizes.push_back(va_arg(expected_sizes_va, unsigned));
+
   // 16 args should be enough.
   testScanf3((void *)&scanf_sizes, scanf_result, allowGnuMalloc, format,
              test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
              test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
              test_buf, test_buf, test_buf, test_buf);
   verifyFormatResults(format, n, scanf_sizes, expected_sizes);
 }
 
TEST(SanitizerCommonInterceptors, Scanf) {
 
   testScanf("%*[^:]%n:%d:%1[ ]%n", 4, I, I, 2, I);
 
   testScanf("%*d%u", 1, I);
 
   testScanf("%c%d", 2, C, I);
   testScanf("%A%lf", 2, F, D);
 
-  testScanf("%ms %Lf", 2, P, LD);
   testScanf("s%Las", 1, LD);
   testScanf("%ar", 1, F);
 
   // In the cases with std::min below the format spec can be interpreted as
   // either floating-something, or (GNU extension) callee-allocated string.
   // Our conservative implementation reports one of the two possibilities with
   // the least store range.
   testScanf("%a[", 0);
TEST(SanitizerCommonInterceptors, Scanf) {
   testScanfPartial("%d%n%n%d //1\n", 1, 3, I, I, I);
   testScanfPartial("%d%n%n%d //2\n", 2, 4, I, I, I, I);
 
   testScanfPartial("%d%n%n%d %s %s", 3, 5, I, I, I, I, test_buf_size);
   testScanfPartial("%d%n%n%d %s %s", 4, 6, I, I, I, I, test_buf_size,
                    test_buf_size);
 }
 
+TEST(SanitizerCommonInterceptors, ScanfAllocate) {
+  const char *buf = "123456";
+
+  // Can not use testScanf() because this case needs a valid pointer to a string
+  // in the scanf argument.
+  {
+    std::vector<unsigned> scanf_sizes;
+    testScanf3((void *)&scanf_sizes, 2, /*allowGnuMalloc=*/false, "%ms", &buf);
+    verifyFormatResults("%ms", 2, scanf_sizes,
+                        {P, (unsigned)(strlen(buf) + 1)});
+  }
+
+  {
+    std::vector<unsigned> scanf_sizes;
+    testScanf3((void *)&scanf_sizes, 2, /*allowGnuMalloc=*/false, "%mc", &buf);
+    verifyFormatResults("%mc", 2, scanf_sizes,
+                        {P, (unsigned)(strlen(buf) + 1)});
+  }
+}
+
 static void testPrintf3(void *ctx, const char *format, ...) {
   va_list ap;
   va_start(ap, format);
   printf_common(ctx, format, ap);
   va_end(ap);
 }
 
 static void testPrintf2(const char *format, unsigned n,
-                       va_list expected_sizes) {
-  std::vector<unsigned> printf_sizes;
+                        va_list expected_sizes_va) {
+  std::vector<unsigned> printf_sizes, expected_sizes;
+  for (unsigned i = 0; i < n; ++i)
+    expected_sizes.push_back(va_arg(expected_sizes_va, unsigned));
+
   // 16 args should be enough.
   testPrintf3((void *)&printf_sizes, format,
              test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
              test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
              test_buf, test_buf, test_buf, test_buf);
   verifyFormatResults(format, n, printf_sizes, expected_sizes);
 }
 

compiler-rt/test/msan/scanf-allocate.cpp

+// RUN: %clangxx_msan -O0 %s -o %t && %run %t >%t.out 2>&1
+// FileCheck %s <%t.out
+
+#include <sanitizer/msan_interface.h>
+#include <stdio.h>
+#include <string.h>
+
+int main(int argc, char **argv) {
+  char *str;
+  sscanf("#string#", "%ms", &str);
+  printf("str = %s\n", str);
+  __msan_check_mem_is_initialized(str, strlen(str) + 1);
+  // CHECK: #string#
+}