[msan] Better open_memstream support.

Move fflush and fclose interceptors to sanitizer_common.
Use a metadata map to keep information about the external locations
that must be updated when the file is written to.

llvm-svn: 208676

compiler-rt/lib/sanitizer_common/sanitizer_addrhashmap.h

@@ -17,6 +17,7 @@
 #include "sanitizer_common.h"
 #include "sanitizer_mutex.h"
 #include "sanitizer_atomic.h"
+#include "sanitizer_allocator_internal.h"
 
 namespace __sanitizer {
 
@@ -66,9 +67,12 @@ class AddrHashMap {
 
   class Handle {
    public:
-    Handle(AddrHashMap<T, kSize> *map, uptr addr, bool remove = false);
+    Handle(AddrHashMap<T, kSize> *map, uptr addr);
+    Handle(AddrHashMap<T, kSize> *map, uptr addr, bool remove);
+    Handle(AddrHashMap<T, kSize> *map, uptr addr, bool remove, bool create);
+
     ~Handle();
-    T *operator -> ();
+    T *operator->();
     bool created() const;
     bool exists() const;
 
@@ -81,6 +85,7 @@ class AddrHashMap {
     uptr                   addidx_;
     bool                   created_;
     bool                   remove_;
+    bool                   create_;
   };
 
  private:
@@ -92,12 +97,32 @@ class AddrHashMap {
   uptr calcHash(uptr addr);
 };
 
+template<typename T, uptr kSize>
+AddrHashMap<T, kSize>::Handle::Handle(AddrHashMap<T, kSize> *map, uptr addr) {
+  map_ = map;
+  addr_ = addr;
+  remove_ = false;
+  create_ = true;
+  map_->acquire(this);
+}
+
 template<typename T, uptr kSize>
 AddrHashMap<T, kSize>::Handle::Handle(AddrHashMap<T, kSize> *map, uptr addr,
     bool remove) {
   map_ = map;
   addr_ = addr;
   remove_ = remove;
+  create_ = true;
+  map_->acquire(this);
+}
+
+template<typename T, uptr kSize>
+AddrHashMap<T, kSize>::Handle::Handle(AddrHashMap<T, kSize> *map, uptr addr,
+    bool remove, bool create) {
+  map_ = map;
+  addr_ = addr;
+  remove_ = remove;
+  create_ = create;
   map_->acquire(this);
 }
 
@@ -106,8 +131,8 @@ AddrHashMap<T, kSize>::Handle::~Handle() {
   map_->release(this);
 }
 
-template<typename T, uptr kSize>
-T *AddrHashMap<T, kSize>::Handle::operator -> () {
+template <typename T, uptr kSize>
+T *AddrHashMap<T, kSize>::Handle::operator->() {
   return &cell_->val;
 }
 
@@ -207,7 +232,7 @@ void AddrHashMap<T, kSize>::acquire(Handle *h) {
   }
 
   // The element does not exist, no need to create it if we want to remove.
-  if (h->remove_) {
+  if (h->remove_ || !h->create_) {
     b->mtx.Unlock();
     return;
   }

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

@@ -28,6 +28,8 @@
 //   COMMON_INTERCEPTOR_HANDLE_RECVMSG
 //===----------------------------------------------------------------------===//
 #include "interception/interception.h"
+#include "sanitizer_addrhashmap.h"
+#include "sanitizer_placement_new.h"
 #include "sanitizer_platform_interceptors.h"
 #include "sanitizer_tls_get_addr.h"
 
@@ -73,6 +75,52 @@
 #define COMMON_INTERCEPTOR_FILE_CLOSE(ctx, file) {}
 #endif
 
+struct FileMetadata {
+  // For open_memstream().
+  char **addr;
+  SIZE_T *size;
+};
+
+struct CommonInterceptorMetadata {
+  enum {
+    CIMT_INVALID = 0,
+    CIMT_FILE
+  } type;
+  union {
+    FileMetadata file;
+  };
+};
+
+typedef AddrHashMap<CommonInterceptorMetadata, 31051> MetadataHashMap;
+
+static MetadataHashMap *interceptor_metadata_map;
+
+static void SetInterceptorMetadata(__sanitizer_FILE *addr,
+                                   const FileMetadata &file) {
+  MetadataHashMap::Handle h(interceptor_metadata_map, (uptr)addr);
+  CHECK(h.created());
+  h->type = CommonInterceptorMetadata::CIMT_FILE;
+  h->file = file;
+}
+
+static const FileMetadata *GetInterceptorMetadata(__sanitizer_FILE *addr) {
+  MetadataHashMap::Handle h(interceptor_metadata_map, (uptr)addr,
+                            /* remove */ false,
+                            /* create */ false);
+  if (h.exists()) {
+    CHECK(!h.created());
+    CHECK(h->type == CommonInterceptorMetadata::CIMT_FILE);
+    return &h->file;
+  } else {
+    return 0;
+  }
+}
+
+static void DeleteInterceptorMetadata(void *addr) {
+  MetadataHashMap::Handle h(interceptor_metadata_map, (uptr)addr, true);
+  CHECK(h.exists());
+}
+
 #if SANITIZER_INTERCEPT_TEXTDOMAIN
 INTERCEPTOR(char*, textdomain, const char *domainname) {
   void *ctx;
@@ -624,6 +672,14 @@ INTERCEPTOR(char *, strptime, char *s, char *format, __sanitizer_tm *tm) {
 #define INIT_STRPTIME
 #endif
 
+void update_FILE(__sanitizer_FILE *fp) {
+#if SANITIZER_HAS_STRUCT_FILE
+  const FileMetadata *m = GetInterceptorMetadata(fp);
+  if (m)
+    COMMON_INTERCEPTOR_INITIALIZE_RANGE(*m->addr, *m->size);
+#endif  // SANITIZER_HAS_STRUCT_FILE
+}
+
 #if SANITIZER_INTERCEPT_SCANF || SANITIZER_INTERCEPT_PRINTF
 #include "sanitizer_common_interceptors_format.inc"
 
@@ -789,7 +845,8 @@ FORMAT_INTERCEPTOR_IMPL(__isoc99_sscanf, __isoc99_vsscanf, str, format)
 INTERCEPTOR(int, vprintf, const char *format, va_list ap)
 VPRINTF_INTERCEPTOR_IMPL(vprintf, format, ap)
 
-INTERCEPTOR(int, vfprintf, void *stream, const char *format, va_list ap)
+INTERCEPTOR(int, vfprintf, __sanitizer_FILE *stream, const char *format,
+            va_list ap)
 VPRINTF_INTERCEPTOR_IMPL(vfprintf, stream, format, ap)
 
 INTERCEPTOR(int, vsnprintf, char *str, SIZE_T size, const char *format,
@@ -806,8 +863,8 @@ VASPRINTF_INTERCEPTOR_IMPL(vasprintf, strp, format, ap)
 INTERCEPTOR(int, __isoc99_vprintf, const char *format, va_list ap)
 VPRINTF_INTERCEPTOR_IMPL(__isoc99_vprintf, format, ap)
 
-INTERCEPTOR(int, __isoc99_vfprintf, void *stream, const char *format,
-            va_list ap)
+INTERCEPTOR(int, __isoc99_vfprintf, __sanitizer_FILE *stream,
+            const char *format, va_list ap)
 VPRINTF_INTERCEPTOR_IMPL(__isoc99_vfprintf, stream, format, ap)
 
 INTERCEPTOR(int, __isoc99_vsnprintf, char *str, SIZE_T size, const char *format,
@@ -824,7 +881,7 @@ VSPRINTF_INTERCEPTOR_IMPL(__isoc99_vsprintf, str, format,
 INTERCEPTOR(int, printf, const char *format, ...)
 FORMAT_INTERCEPTOR_IMPL(printf, vprintf, format)
 
-INTERCEPTOR(int, fprintf, void *stream, const char *format, ...)
+INTERCEPTOR(int, fprintf, __sanitizer_FILE *stream, const char *format, ...)
 FORMAT_INTERCEPTOR_IMPL(fprintf, vfprintf, stream, format)
 
 INTERCEPTOR(int, sprintf, char *str, const char *format, ...) // NOLINT
@@ -840,7 +897,8 @@ FORMAT_INTERCEPTOR_IMPL(asprintf, vasprintf, strp, format)
 INTERCEPTOR(int, __isoc99_printf, const char *format, ...)
 FORMAT_INTERCEPTOR_IMPL(__isoc99_printf, __isoc99_vprintf, format)
 
-INTERCEPTOR(int, __isoc99_fprintf, void *stream, const char *format, ...)
+INTERCEPTOR(int, __isoc99_fprintf, __sanitizer_FILE *stream, const char *format,
+            ...)
 FORMAT_INTERCEPTOR_IMPL(__isoc99_fprintf, __isoc99_vfprintf, stream, format)
 
 INTERCEPTOR(int, __isoc99_sprintf, char *str, const char *format, ...)
@@ -3729,10 +3787,12 @@ INTERCEPTOR(void *, tsearch, void *key, void **rootp,
 #if SANITIZER_INTERCEPT_LIBIO_INTERNALS || SANITIZER_INTERCEPT_FOPEN || \
     SANITIZER_INTERCEPT_OPEN_MEMSTREAM
 void unpoison_file(__sanitizer_FILE *fp) {
+#if SANITIZER_HAS_STRUCT_FILE
   COMMON_INTERCEPTOR_INITIALIZE_RANGE(fp, sizeof(*fp));
   if (fp->_IO_read_base && fp->_IO_read_base < fp->_IO_read_end)
     COMMON_INTERCEPTOR_INITIALIZE_RANGE(fp->_IO_read_base,
                                         fp->_IO_read_end - fp->_IO_read_base);
+#endif  // SANITIZER_HAS_STRUCT_FILE
 }
 #endif
 
@@ -3863,6 +3923,8 @@ INTERCEPTOR(__sanitizer_FILE *, open_memstream, char **ptr, SIZE_T *sizeloc) {
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, sizeof(*ptr));
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, sizeloc, sizeof(*sizeloc));
     unpoison_file(res);
+    FileMetadata file = {ptr, sizeloc};
+    SetInterceptorMetadata(res, file);
   }
   return res;
 }
@@ -3875,6 +3937,8 @@ INTERCEPTOR(__sanitizer_FILE *, open_wmemstream, wchar_t **ptr,
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, sizeof(*ptr));
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, sizeloc, sizeof(*sizeloc));
     unpoison_file(res);
+    FileMetadata file = {(char **)ptr, sizeloc};
+    SetInterceptorMetadata(res, file);
   }
   return res;
 }
@@ -3937,7 +4001,47 @@ INTERCEPTOR(void, _obstack_newchunk, __sanitizer_obstack *obstack, int length) {
 #define INIT_OBSTACK
 #endif
 
+#if SANITIZER_INTERCEPT_FFLUSH
+INTERCEPTOR(int, fflush, __sanitizer_FILE *fp) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, fflush, fp);
+  int res = REAL(fflush)(fp);
+  // FIXME: handle fp == NULL
+  if (fp) {
+    const FileMetadata *m = GetInterceptorMetadata(fp);
+    if (m) COMMON_INTERCEPTOR_INITIALIZE_RANGE(*m->addr, *m->size);
+  }
+  return res;
+}
+#define INIT_FFLUSH COMMON_INTERCEPT_FUNCTION(fflush);
+#else
+#define INIT_FFLUSH
+#endif
+
+#if SANITIZER_INTERCEPT_FCLOSE
+INTERCEPTOR(int, fclose, __sanitizer_FILE *fp) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, fclose, fp);
+  int res = REAL(fclose)(fp);
+  if (fp) {
+    COMMON_INTERCEPTOR_FILE_CLOSE(ctx, fp);
+    const FileMetadata *m = GetInterceptorMetadata(fp);
+    if (m) {
+      COMMON_INTERCEPTOR_INITIALIZE_RANGE(*m->addr, *m->size);
+      DeleteInterceptorMetadata(fp);
+    }
+  }
+  return res;
+}
+#define INIT_FCLOSE COMMON_INTERCEPT_FUNCTION(fclose);
+#else
+#define INIT_FCLOSE
+#endif
+
 static void InitializeCommonInterceptors() {
+  static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
+  interceptor_metadata_map = new((void *)&metadata_mem) MetadataHashMap();
+
   INIT_TEXTDOMAIN;
   INIT_STRCMP;
   INIT_STRNCMP;
@@ -4072,4 +4176,6 @@ static void InitializeCommonInterceptors() {
   INIT_FOPEN;
   INIT_OPEN_MEMSTREAM;
   INIT_OBSTACK;
+  INIT_FFLUSH;
+  INIT_FCLOSE;
 }

compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

@@ -197,8 +197,10 @@
 #define SANITIZER_INTERCEPT_XDR SI_LINUX_NOT_ANDROID
 #define SANITIZER_INTERCEPT_TSEARCH SI_LINUX_NOT_ANDROID || SI_MAC
 #define SANITIZER_INTERCEPT_LIBIO_INTERNALS SI_LINUX_NOT_ANDROID
-#define SANITIZER_INTERCEPT_FOPEN SI_LINUX_NOT_ANDROID
+#define SANITIZER_INTERCEPT_FOPEN SI_NOT_WINDOWS
 #define SANITIZER_INTERCEPT_OPEN_MEMSTREAM SI_LINUX_NOT_ANDROID
 #define SANITIZER_INTERCEPT_OBSTACK SI_LINUX_NOT_ANDROID
+#define SANITIZER_INTERCEPT_FFLUSH SI_NOT_WINDOWS
+#define SANITIZER_INTERCEPT_FCLOSE SI_NOT_WINDOWS
 
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h

@@ -661,6 +661,10 @@ namespace __sanitizer {
     __sanitizer_FILE *_chain;
     int _fileno;
   };
+# define SANITIZER_HAS_STRUCT_FILE 1
+#else
+  typedef void __sanitizer_FILE;
+# define SANITIZER_HAS_STRUCT_FILE 0
 #endif
 
 #if SANITIZER_LINUX && !SANITIZER_ANDROID && \

compiler-rt/lib/tsan/rtl/tsan_interceptors.cc

@@ -53,6 +53,7 @@ extern "C" int pthread_sigmask(int how, const __sanitizer_sigset_t *set,
                                __sanitizer_sigset_t *oldset);
 // REAL(sigfillset) defined in common interceptors.
 DECLARE_REAL(int, sigfillset, __sanitizer_sigset_t *set)
+DECLARE_REAL(int, fflush, __sanitizer_FILE *fp)
 extern "C" void *pthread_self();
 extern "C" void _exit(int status);
 extern "C" int *__errno_location();
@@ -62,7 +63,7 @@ extern "C" void *__libc_calloc(uptr size, uptr n);
 extern "C" void *__libc_realloc(void *ptr, uptr size);
 extern "C" void __libc_free(void *ptr);
 extern "C" int mallopt(int param, int value);
-extern void *stdout, *stderr;
+extern __sanitizer_FILE *stdout, *stderr;
 const int PTHREAD_MUTEX_RECURSIVE = 1;
 const int PTHREAD_MUTEX_RECURSIVE_NP = 1;
 const int EINVAL = 22;
@@ -1621,19 +1622,6 @@ TSAN_INTERCEPTOR(void*, tmpfile64, int fake) {
   return res;
 }
 
-TSAN_INTERCEPTOR(int, fclose, void *stream) {
-  // libc file streams can call user-supplied functions, see fopencookie.
-  {
-    SCOPED_TSAN_INTERCEPTOR(fclose, stream);
-    if (stream) {
-      int fd = fileno_unlocked(stream);
-      if (fd >= 0)
-        FdClose(thr, pc, fd);
-    }
-  }
-  return REAL(fclose)(stream);
-}
-
 TSAN_INTERCEPTOR(uptr, fread, void *ptr, uptr size, uptr nmemb, void *f) {
   // libc file streams can call user-supplied functions, see fopencookie.
   {
@@ -1652,14 +1640,6 @@ TSAN_INTERCEPTOR(uptr, fwrite, const void *p, uptr size, uptr nmemb, void *f) {
   return REAL(fwrite)(p, size, nmemb, f);
 }
 
-TSAN_INTERCEPTOR(int, fflush, void *stream) {
-  // libc file streams can call user-supplied functions, see fopencookie.
-  {
-    SCOPED_TSAN_INTERCEPTOR(fflush, stream);
-  }
-  return REAL(fflush)(stream);
-}
-
 TSAN_INTERCEPTOR(void, abort, int fake) {
   SCOPED_TSAN_INTERCEPTOR(abort, fake);
   REAL(fflush)(0);
@@ -2367,10 +2347,8 @@ void InitializeInterceptors() {
   TSAN_INTERCEPT(unlink);
   TSAN_INTERCEPT(tmpfile);
   TSAN_INTERCEPT(tmpfile64);
-  TSAN_INTERCEPT(fclose);
   TSAN_INTERCEPT(fread);
   TSAN_INTERCEPT(fwrite);
-  TSAN_INTERCEPT(fflush);
   TSAN_INTERCEPT(abort);
   TSAN_INTERCEPT(puts);
   TSAN_INTERCEPT(rmdir);

compiler-rt/test/msan/open_memstream.cc

@@ -1,18 +0,0 @@
-// RUN: %clangxx_msan -m64 -O0 -g -xc++ %s -o %t && %run %t
-// RUN: %clangxx_msan -m64 -O3 -g -xc++ %s -o %t && %run %t
-
-#include <stdio.h>
-#include <stdlib.h>
-
-int main(void) {
-  char *buf;
-  size_t buf_len = 42;
-  FILE *fp = open_memstream(&buf, &buf_len);
-  fprintf(fp, "hello");
-  fflush(fp);
-  printf("buf_len = %zu\n", buf_len);
-  for (int j = 0; j < buf_len; j++) {
-    printf("buf[%d] = %c\n", j, buf[j]);
-  }
-  return 0;
-}

compiler-rt/test/sanitizer_common/TestCases/open_memstream.cc

@@ -0,0 +1,57 @@
+// RUN: %clangxx -m64 -O0 -g -xc++ %s -o %t && %run %t
+// RUN: %clangxx -m64 -O3 -g -xc++ %s -o %t && %run %t
+
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#ifndef __has_feature
+#define __has_feature(x) 0
+#endif
+
+#if __has_feature(memory_sanitizer)
+#include <sanitizer/msan_interface.h>
+static void check_mem_is_good(void *p, size_t s) {
+  __msan_check_mem_is_initialized(p, s);
+}
+#elif __has_feature(address_sanitizer)
+#include <sanitizer/asan_interface.h>
+static void check_mem_is_good(void *p, size_t s) {
+  assert(__asan_region_is_poisoned(p, s) == 0);
+}
+#else
+static void check_mem_is_good(void *p, size_t s) {}
+#endif
+
+static void run(void) {
+  char *buf;
+  size_t buf_len;
+  fprintf(stderr, " &buf %p, &buf_len %p\n", &buf, &buf_len);
+  FILE *fp = open_memstream(&buf, &buf_len);
+  fprintf(fp, "hello");
+  fflush(fp);
+  check_mem_is_good(&buf, sizeof(buf));
+  check_mem_is_good(&buf_len, sizeof(buf_len));
+  check_mem_is_good(buf, buf_len);
+
+  char *p = new char[1024];
+  memset(p, 'a', 1023);
+  p[1023] = 0;
+  for (int i = 0; i < 100; ++i)
+    fprintf(fp, "%s", p);
+  delete[] p;
+  fflush(fp);
+  fprintf(stderr, " %p addr %p, len %zu\n", &buf, buf, buf_len);
+  check_mem_is_good(&buf, sizeof(buf));
+  check_mem_is_good(&buf_len, sizeof(buf_len));
+  check_mem_is_good(buf, buf_len);
+  fclose(fp);
+  free(buf);
+}
+
+int main(void) {
+  for (int i = 0; i < 100; ++i)
+    run();
+  return 0;
+}