[sanitizer] Add strlen to the common interceptors

[bruening]

Summary

Adds strlen to the common interceptors, under a new common flag
intercept_strlen. This provides better sharing of interception code among
sanitizers and cleans up the inconsistent type declarations of the
previously duplicated interceptors.

Removes the now-duplicate strlen interceptor from asan, msan, and tsan.
The entry check semantics are normalized now for msan and asan, whose
private strlen interceptors contained multiple layers of checks that
included impossible-to-reach code. The new semantics are identical to the
old: bypass interception if in the middle of init or if both on Mac and not
initialized; else, call the init routine and proceed.

Preserves asan's prior replace_str flag semantics by disabling
intercept_strlen when replace_str is disabled.

[samsonov]

Looks mostly good, thank you for working on this.

lib/asan/asan_flags.cc

162

Hm... I think I would prefer to break the old (non-default) use case rather than have this ugly workaround... As I understand it, we will gradually phase out ASAN_OPTIONS=replace_str flag anyway, so maybe we can get away with a warning instead:

if (!f->replace_str && common_flags()->intercept_strlen) {
  Report("WARNING: strlen interceptor is enabled even though replace_str=0. Use intercept_strlen=0 to disable it.");
}

lib/asan/asan_interceptors.cc

151

Probably you can omit this comment: I believe there are more cases when libc function may be called while __asan_init is still running. E.g. it may be required for FreeBSD: see https://llvm.org/svn/llvm-project/compiler-rt/trunk@222885

lib/sanitizer_common/sanitizer_common_interceptors.inc

205

I don't think you need it here: looks like this will be handled by COMMON_INTERCEPTOR_ENTER macro.

211

I think it should be just COMMON_INTERCEPTOR_READ_RANGE - COMMON_INTERCEPTOR_READ_STRING is only used for the special-handling of cases where we are accessing only some prefix of a null-terminated string.

[bruening]

Removes the common flags override and makes other changes as suggested by
the review.

lib/asan/asan_flags.cc

162

Sure, I needed feedback on this one: did not want to break backward compatibility without a project owner signing off.

lib/sanitizer_common/sanitizer_common_interceptors.inc

205

Some common interceptors have their own init check which is what this is based on, but most don't and it does seem to be superfluous. Perhaps I will send send a separate CL removing the existing checks.

[samsonov]

LGTM, thank you!

[bruening]

As before, could you commit the patch for me? Thank you.

[bruening]

lib/sanitizer_common/sanitizer_common_interceptors.inc

205

Note that this is not quite the same as the COMMON_INTERCEPTOR_ENTER, because REAL(strlen) is not initialized until InitializeCommonInterceptors() is called (on Linux at least). Thus any use of strlen prior to that point needs to use internal_strlen(), not REAL(strlen). Given that nothing broke since this was committed, perhaps we can assume that nothing needs strlen that early.

lib/asan/asan_flags.cc

#endif
   }
   if (f->quarantine_size >= 0)
     f->quarantine_size_mb = f->quarantine_size >> 20;
   if (f->quarantine_size_mb < 0) {
     const int kDefaultQuarantineSizeMb =
         (ASAN_LOW_MEMORY) ? 1UL << 6 : 1UL << 8;
     f->quarantine_size_mb = kDefaultQuarantineSizeMb;
   }
+  // Preserve the old meanings of flags from prior to moving interceptors
+  // from asan into the common set of interceptors.
+  // TODO(bruening): we'd need the parser to record whether it encountered an
+  // explicit replace_strlen token to support enabling only it while disabling
+  // replace_str.
+  if (!f->replace_str) {
+    CommonFlags cf;
+    cf.CopyFrom(*common_flags());
+    cf.intercept_strlen = false;
+    OverrideCommonFlags(cf);
+  }
 }
 
 }  // namespace __asan
 
 #if !SANITIZER_SUPPORTS_WEAK_HOOKS
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 const char* __asan_default_options() { return ""; }
 }  // extern "C"
 #endif

lib/asan/asan_interceptors.cc

#define ASAN_INTERCEPTOR_ENTER(ctx, func)                                      \
   ctx = (void *)&_ctx;                                                         \
   (void) ctx;                                                                  \
 
 #define COMMON_INTERCEPT_FUNCTION(name) ASAN_INTERCEPT_FUNC(name)
 #define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
   ASAN_WRITE_RANGE(ctx, ptr, size)
 #define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
   ASAN_READ_RANGE(ctx, ptr, size)
+// We check asan_init_is_running to handle situations that include:
+// + strlen is called from malloc_default_purgeable_zone()
+//   in __asan::ReplaceSystemAlloc() on Mac.
 #define COMMON_INTERCEPTOR_ENTER(ctx, func, ...)                               \
   ASAN_INTERCEPTOR_ENTER(ctx, func);                                           \
   do {                                                                         \
     if (asan_init_is_running)                                                  \
       return REAL(func)(__VA_ARGS__);                                          \
     if (SANITIZER_MAC && UNLIKELY(!asan_inited))                               \
       return REAL(func)(__VA_ARGS__);                                          \
     ENSURE_ASAN_INITED();                                                      \
INTERCEPTOR(char*, strdup, const char *s) {
   }
   GET_STACK_TRACE_MALLOC;
   void *new_mem = asan_malloc(length + 1, &stack);
   REAL(memcpy)(new_mem, s, length + 1);
   return reinterpret_cast<char*>(new_mem);
 }
 #endif
 
-INTERCEPTOR(SIZE_T, strlen, const char *s) {
-  void *ctx;
-  ASAN_INTERCEPTOR_ENTER(ctx, strlen);
-  if (UNLIKELY(!asan_inited)) return internal_strlen(s);
-  // strlen is called from malloc_default_purgeable_zone()
-  // in __asan::ReplaceSystemAlloc() on Mac.
-  if (asan_init_is_running) {
-    return REAL(strlen)(s);
-  }
-  ENSURE_ASAN_INITED();
-  SIZE_T length = REAL(strlen)(s);
-  if (flags()->replace_str) {
-    ASAN_READ_RANGE(ctx, s, length + 1);
-  }
-  return length;
-}
-
 INTERCEPTOR(SIZE_T, wcslen, const wchar_t *s) {
   void *ctx;
   ASAN_INTERCEPTOR_ENTER(ctx, wcslen);
   SIZE_T length = REAL(wcslen)(s);
   if (!asan_init_is_running) {
     ENSURE_ASAN_INITED();
     ASAN_READ_RANGE(ctx, s, (length + 1) * sizeof(wchar_t));
   }
void InitializeAsanInterceptors() {
   if (PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE) {
     ASAN_INTERCEPT_FUNC(memcpy);
   }
 
   // Intercept str* functions.
   ASAN_INTERCEPT_FUNC(strcat);  // NOLINT
   ASAN_INTERCEPT_FUNC(strchr);
   ASAN_INTERCEPT_FUNC(strcpy);  // NOLINT
-  ASAN_INTERCEPT_FUNC(strlen);
   ASAN_INTERCEPT_FUNC(wcslen);
   ASAN_INTERCEPT_FUNC(strncat);
   ASAN_INTERCEPT_FUNC(strncpy);
 #if ASAN_INTERCEPT_STRDUP
   ASAN_INTERCEPT_FUNC(strdup);
 #endif
 #if ASAN_INTERCEPT_STRNLEN
   ASAN_INTERCEPT_FUNC(strnlen);

lib/msan/msan_interceptors.cc

 
 using namespace __msan;
 
 using __sanitizer::memory_order;
 using __sanitizer::atomic_load;
 using __sanitizer::atomic_store;
 using __sanitizer::atomic_uintptr_t;
 
+DECLARE_REAL(SIZE_T, strlen, const char *s)
+
 #if SANITIZER_FREEBSD
 #define __errno_location __error
 #endif
 
 // True if this is a nested interceptor.
 static THREADLOCAL int in_interceptor_scope;
 
 extern "C" int *__errno_location(void);

 INTERCEPTOR(void, malloc_stats, void) {
   // FIXME: implement, but don't call REAL(malloc_stats)!
 }
 #define MSAN_MAYBE_INTERCEPT_MALLOC_STATS INTERCEPT_FUNCTION(malloc_stats)
 #else
 #define MSAN_MAYBE_INTERCEPT_MALLOC_STATS
 #endif
 
-INTERCEPTOR(SIZE_T, strlen, const char *s) {
-  if (msan_init_is_running)
-    return REAL(strlen)(s);
-  ENSURE_MSAN_INITED();
-  SIZE_T res = REAL(strlen)(s);
-  CHECK_UNPOISONED(s, res + 1);
-  return res;
-}
-
 INTERCEPTOR(SIZE_T, strnlen, const char *s, SIZE_T n) {
   ENSURE_MSAN_INITED();
   SIZE_T res = REAL(strnlen)(s, n);
   SIZE_T scan_size = (res == n) ? res : res + 1;
   CHECK_UNPOISONED(s, scan_size);
   return res;
 }
 
void InitializeInterceptors() {
   INTERCEPT_FUNCTION(wmemmove);
   INTERCEPT_FUNCTION(strcpy);  // NOLINT
   INTERCEPT_FUNCTION(stpcpy);  // NOLINT
   INTERCEPT_FUNCTION(strdup);
   MSAN_MAYBE_INTERCEPT___STRDUP;
   INTERCEPT_FUNCTION(strndup);
   MSAN_MAYBE_INTERCEPT___STRNDUP;
   INTERCEPT_FUNCTION(strncpy);  // NOLINT
-  INTERCEPT_FUNCTION(strlen);
   INTERCEPT_FUNCTION(strnlen);
   INTERCEPT_FUNCTION(gcvt);
   INTERCEPT_FUNCTION(strcat);  // NOLINT
   INTERCEPT_FUNCTION(strncat);  // NOLINT
   INTERCEPT_STRTO(strtod);
   INTERCEPT_STRTO(strtof);
   INTERCEPT_STRTO(strtold);
   INTERCEPT_STRTO(strtol);

lib/sanitizer_common/sanitizer_common_interceptors.inc

 //===-- sanitizer_common_interceptors.inc -----------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Common function interceptors for tools like AddressSanitizer,
 // ThreadSanitizer, MemorySanitizer, etc.
 //
 // This file should be included into the tool's interceptor file,
-// which has to define it's own macros:
+// which has to define its own macros:
 //   COMMON_INTERCEPTOR_ENTER
 //   COMMON_INTERCEPTOR_ENTER_NOIGNORE
 //   COMMON_INTERCEPTOR_READ_RANGE
 //   COMMON_INTERCEPTOR_WRITE_RANGE
 //   COMMON_INTERCEPTOR_INITIALIZE_RANGE
 //   COMMON_INTERCEPTOR_DIR_ACQUIRE
 //   COMMON_INTERCEPTOR_FD_ACQUIRE
 //   COMMON_INTERCEPTOR_FD_RELEASE

 }
 
 UNUSED static void DeleteInterceptorMetadata(void *addr) {
   MetadataHashMap::Handle h(interceptor_metadata_map, (uptr)addr, true);
   CHECK(h.exists());
 }
 #endif  // SI_NOT_WINDOWS
 
+#if SANITIZER_INTERCEPT_STRLEN
+INTERCEPTOR(SIZE_T, strlen, const char *s) {
+  if (COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED)
+    return internal_strlen(s);
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, strlen, s);
+  SIZE_T result = REAL(strlen)(s);
+  if (common_flags()->intercept_strlen)
+    COMMON_INTERCEPTOR_READ_STRING(ctx, s, result + 1);
+  return result;
+}
+#define INIT_STRLEN COMMON_INTERCEPT_FUNCTION(strlen)
+#else
+#define INIT_STRLEN
+#endif
+
 #if SANITIZER_INTERCEPT_TEXTDOMAIN
 INTERCEPTOR(char*, textdomain, const char *domainname) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, textdomain, domainname);
   COMMON_INTERCEPTOR_READ_STRING(ctx, domainname, 0);
   char *domain = REAL(textdomain)(domainname);
   if (domain) {
     COMMON_INTERCEPTOR_INITIALIZE_RANGE(domain, REAL(strlen)(domain) + 1);

 #define INIT_RECV_RECVFROM
 #endif
 
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map = new((void *)&metadata_mem) MetadataHashMap();
 
   INIT_TEXTDOMAIN;
+  INIT_STRLEN;
   INIT_STRCMP;
   INIT_STRNCMP;
   INIT_STRCASECMP;
   INIT_STRNCASECMP;
   INIT_STRSTR;
   INIT_STRCASESTR;
   INIT_STRSPN;
   INIT_STRPBRK;

lib/sanitizer_common/sanitizer_flags.inc

COMMON_FLAG(bool, intercept_strstr, true,
             "If set, uses custom wrappers for strstr and strcasestr functions "
             "to find more errors.")
 COMMON_FLAG(bool, intercept_strspn, true,
             "If set, uses custom wrappers for strspn and strcspn function "
             "to find more errors.")
 COMMON_FLAG(bool, intercept_strpbrk, true,
             "If set, uses custom wrappers for strpbrk function "
             "to find more errors.")
+COMMON_FLAG(bool, intercept_strlen, true,
+            "If set, uses custom wrappers for strlen function "
+            "to find more errors.")
 COMMON_FLAG(bool, intercept_memcmp, true,
             "If set, uses custom wrappers for memcmp function "
             "to find more errors.")
 COMMON_FLAG(bool, strict_memcmp, true,
           "If true, assume that memcmp(p1, p2, n) always reads n bytes before "
           "comparing p1 and p2.")
 COMMON_FLAG(bool, decorate_proc_maps, false, "If set, decorate sanitizer "
                                              "mappings in /proc/self/maps with "

lib/sanitizer_common/sanitizer_platform_interceptors.h

 #endif
 
 #if SANITIZER_IOS
 # define SI_IOS 1
 #else
 # define SI_IOS 0
 #endif
 
+#define SANITIZER_INTERCEPT_STRLEN 1
 #define SANITIZER_INTERCEPT_STRCMP 1
 #define SANITIZER_INTERCEPT_STRSTR 1
 #define SANITIZER_INTERCEPT_STRCASESTR SI_NOT_WINDOWS
 #define SANITIZER_INTERCEPT_STRSPN 1
 #define SANITIZER_INTERCEPT_STRPBRK 1
 #define SANITIZER_INTERCEPT_TEXTDOMAIN SI_LINUX_NOT_ANDROID
 #define SANITIZER_INTERCEPT_STRCASECMP SI_NOT_WINDOWS
 #define SANITIZER_INTERCEPT_MEMCMP 1

lib/tsan/rtl/tsan_interceptors.cc

 }
 
 TSAN_INTERCEPTOR(uptr, malloc_usable_size, void *p) {
   SCOPED_INTERCEPTOR_RAW(malloc_usable_size, p);
   return user_alloc_usable_size(p);
 }
 #endif
 
-TSAN_INTERCEPTOR(uptr, strlen, const char *s) {
-  SCOPED_TSAN_INTERCEPTOR(strlen, s);
-  uptr len = internal_strlen(s);
-  MemoryAccessRange(thr, pc, (uptr)s, len + 1, false);
-  return len;
-}
-
 TSAN_INTERCEPTOR(void*, memset, void *dst, int v, uptr size) {
   // On FreeBSD we get here from libthr internals on thread initialization.
   if (!COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED) {
     SCOPED_TSAN_INTERCEPTOR(memset, dst, v, size);
     MemoryAccessRange(thr, pc, (uptr)dst, size, true);
   }
   return internal_memset(dst, v, size);
 }
#endif
   TSAN_INTERCEPT(mmap);
   TSAN_MAYBE_INTERCEPT_MMAP64;
   TSAN_INTERCEPT(munmap);
   TSAN_MAYBE_INTERCEPT_MEMALIGN;
   TSAN_INTERCEPT(valloc);
   TSAN_MAYBE_INTERCEPT_PVALLOC;
   TSAN_INTERCEPT(posix_memalign);
 
-  TSAN_INTERCEPT(strlen);
   TSAN_INTERCEPT(memset);
   TSAN_INTERCEPT(memcpy);
   TSAN_INTERCEPT(memmove);
   TSAN_INTERCEPT(strchr);
   TSAN_INTERCEPT(strchrnul);
   TSAN_INTERCEPT(strrchr);
   TSAN_INTERCEPT(strcpy);  // NOLINT
   TSAN_INTERCEPT(strncpy);