[sanitizer] Move stat/__xstat to the common interceptors

[zhaoqin]

Summary

Adds stat/__xstat to the common interceptors.

Removes the now-duplicate stat/__xstat interceptor from msan/tsan/esan.
This adds stat/__xstat to asan, which previously did not intercept it.

[aizatsky]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5520

The direction of this CL is good. I worry that stat/xstat behavior is not really clear.

Other interceptors (e.g. SANITIZER_INTERCEPT_STRCHR) are completely enabled/disabled by a guard. In this case it is not fully enabled/disabled, but an implementation is switched.

I suggest having two guards: one that turns it on/off, the other one chooses the implementation. WDYT?

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5520

These are actually independent interceptors, use
SANITIZER_INTERCEPT_STAT
and
SANITIZER_INTERCEPT___XSTAT

lib/sanitizer_common/sanitizer_platform_interceptors.h

40

This is not used anywhere.
Just use !SI_FREEBSD instead, it is shorter.

[zhaoqin]

PTAL

[aizatsky]

lib/sanitizer_common/sanitizer_platform_interceptors.h

300

It looks like both stat & xstat will be intercepted on mac & android.

Should this be !SANITIZER_INTERCEPT_STAT?

[zhaoqin]

lib/sanitizer_common/sanitizer_platform_interceptors.h

300

If so, I am not sure if we need SANITIZER_INTERCEPT___XSTAT.
I thought SANITIZER_INTERCEPT___XSTAT is !SANITIZER_INTERCEPT_STAT, but Evgeniy suggested to add SANITIZER_INTERCEPT___XSTAT separately.
Evgeniy could you please comment on this?

[zhaoqin]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5527

should I add flags check here too?
I feel that the write cannot be skipped for correctness, e.g., msan mark buffer as defined.

[eugenis]

It looks like SANITIZER_INTERCEPT___XSTAT is !SANITIZER_INTERCEPT_STAT.

This is in line with other common interceptors. Looking at sanitizer_platform_interceptors.h, one can easily figure out which functions are intercepted on any platform.

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5527

Write is necessary for msan.
Ideally, if intercept_stat==0, MSan should do WRITE_RANGE, and other sanitizers should do nothing.
Maybe these flags should be handled differently. MSan already has logic for disabling checks in nested interceptors, see MSanInterceptorContext::in_interceptor_scope. Setting that to true disables all checks, but keeps writes (i.e. msan_unpoison calls) enabled. If something like that is implemented in other sanitizers, then intercept_stat could just set this (or similar) flag in COMMON_INTERCEPTOR_ENTER.

For now just keep the WRITE_RANGE unconditionally.

[eugenis]

LGTM

[zhaoqin]

I do not have the commit right yet.
Please help me commit the CL if you think it is ready to commit, .
Thanks!

[aizatsky]

Qin,

It probably broke windows bot:

http://lab.llvm.org:8011/builders/sanitizer-windows/builds/21425/steps/build%20compiler-rt/logs/stdio

Please acknowledge and I'll rollback.

[aizatsky]

Reverted in http://reviews.llvm.org/rL268448

[zhaoqin]

Fixed build error on Windows

lib/esan/esan_interceptors.cpp

   return REAL(strncpy)(dst, src, n);
 }
 
-#if SANITIZER_FREEBSD || SANITIZER_MAC || SANITIZER_ANDROID
-INTERCEPTOR(int, stat, const char *path, void *buf) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, stat, path, buf);
-  COMMON_INTERCEPTOR_READ_STRING(ctx, path, 0);
-  return REAL(stat)(path, buf);
-#define ESAN_INTERCEPT_STAT INTERCEPT_FUNCTION(stat)
-#else
-INTERCEPTOR(int, __xstat, int version, const char *path, void *buf) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, __xstat, version, path, buf);
-  COMMON_INTERCEPTOR_READ_STRING(ctx, path, 0);
-  return REAL(__xstat)(version, path, buf);
-}
-#define ESAN_INTERCEPT_STAT INTERCEPT_FUNCTION(__xstat)
-#endif
-
 #if SANITIZER_LINUX && !SANITIZER_ANDROID && !SANITIZER_FREEBSD
 INTERCEPTOR(int, __xstat64, int version, const char *path, void *buf) {
   void *ctx;

   INTERCEPT_FUNCTION(strcpy); // NOLINT
   INTERCEPT_FUNCTION(strncpy);
 
-  ESAN_INTERCEPT_STAT;
   ESAN_MAYBE_INTERCEPT_STAT64;
   ESAN_MAYBE_INTERCEPT___XSTAT64;
   ESAN_INTERCEPT_LSTAT;

lib/msan/msan_interceptors.cc

 #define MSAN_MAYBE_INTERCEPT___FXSTATAT64
 #endif
 
-#if SANITIZER_FREEBSD
-INTERCEPTOR(int, stat, char *path, void *buf) {
-  ENSURE_MSAN_INITED();
-  int res = REAL(stat)(path, buf);
-  if (!res)
-    __msan_unpoison(buf, __sanitizer::struct_stat_sz);
-  return res;
-}
-# define MSAN_INTERCEPT_STAT INTERCEPT_FUNCTION(stat)
-#else
-INTERCEPTOR(int, __xstat, int magic, char *path, void *buf) {
-  ENSURE_MSAN_INITED();
-  int res = REAL(__xstat)(magic, path, buf);
-  if (!res)
-    __msan_unpoison(buf, __sanitizer::struct_stat_sz);
-  return res;
-}
-# define MSAN_INTERCEPT_STAT INTERCEPT_FUNCTION(__xstat)
-#endif
-
 #if !SANITIZER_FREEBSD
 INTERCEPTOR(int, __xstat64, int magic, char *path, void *buf) {
   ENSURE_MSAN_INITED();

   INTERCEPT_FUNCTION(fcvt);
   MSAN_MAYBE_INTERCEPT___FXSTAT;
   MSAN_INTERCEPT_FSTATAT;
-  MSAN_INTERCEPT_STAT;
   MSAN_MAYBE_INTERCEPT___LXSTAT;
   MSAN_MAYBE_INTERCEPT___FXSTAT64;
   MSAN_MAYBE_INTERCEPT___FXSTATAT64;

lib/sanitizer_common/sanitizer_common_interceptors.inc

 #define INIT_RECV_RECVFROM
 #endif
 
+#if SANITIZER_INTERCEPT_STAT
+INTERCEPTOR(int, stat, const char *path, void *buf) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, stat, path, buf);
+  if (common_flags()->intercept_stat)
+    COMMON_INTERCEPTOR_READ_STRING(ctx, path, 0);
+  int res = REAL(stat)(path, buf);
+  if (!res)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, __sanitizer::struct_stat_sz);
+  return res;
+}
+#define INIT_STAT COMMON_INTERCEPT_FUNCTION(stat)
+#else
+#define INIT_STAT
+#endif
+
+#if SANITIZER_INTERCEPT___XSTAT
+INTERCEPTOR(int, __xstat, int version, const char *path, void *buf) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, __xstat, version, path, buf);
+  if (common_flags()->intercept_stat)
+    COMMON_INTERCEPTOR_READ_STRING(ctx, path, 0);
+  int res = REAL(__xstat)(version, path, buf);
+  if (!res)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, __sanitizer::struct_stat_sz);
+  return res;
+}
+#define INIT___XSTAT COMMON_INTERCEPT_FUNCTION(__xstat)
+#else
+#define INIT___XSTAT
+#endif
+
+// FIXME: add other *stat interceptor
+
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map = new((void *)&metadata_mem) MetadataHashMap();

   INIT_CTERMID;
   INIT_CTERMID_R;
   INIT_RECV_RECVFROM;
+  INIT_STAT;
+  INIT___XSTAT;
+  // FIXME: add other *stat interceptors.
 }

lib/sanitizer_common/sanitizer_flags.inc

 COMMON_FLAG(bool, intercept_intrin, true,
             "If set, uses custom wrappers for memset/memcpy/memmove "
             "intrinsics to find more errors.")
+COMMON_FLAG(bool, intercept_stat, true,
+            "If set, uses custom wrappers for *stat functions "
+            "to find more errors.")
 COMMON_FLAG(bool, decorate_proc_maps, false, "If set, decorate sanitizer "
                                              "mappings in /proc/self/maps with "
                                              "user-readable names")

lib/sanitizer_common/sanitizer_platform_interceptors.h

 # define SI_LINUX_NOT_ANDROID 0
 #endif
 
+#if SANITIZER_ANDROID
+# define SI_ANDROID 1
+#else
+# define SI_ANDROID 0
+#endif
+
 #if SANITIZER_FREEBSD
 # define SI_FREEBSD 1
 #else

 #define SANITIZER_INTERCEPTOR_HOOKS SI_LINUX
 #define SANITIZER_INTERCEPT_RECV_RECVFROM SI_NOT_WINDOWS
 
+#define SANITIZER_INTERCEPT_STAT (SI_FREEBSD || SI_MAC || SI_ANDROID)
+#define SANITIZER_INTERCEPT___XSTAT !SANITIZER_INTERCEPT_STAT && SI_NOT_WINDOWS
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

lib/tsan/rtl/tsan_interceptors.cc

 }
 
 #if SANITIZER_LINUX && !SANITIZER_ANDROID
-TSAN_INTERCEPTOR(int, __xstat, int version, const char *path, void *buf) {
-  SCOPED_TSAN_INTERCEPTOR(__xstat, version, path, buf);
-  READ_STRING(thr, pc, path, 0);
-  return REAL(__xstat)(version, path, buf);
-}
-#define TSAN_MAYBE_INTERCEPT___XSTAT TSAN_INTERCEPT(__xstat)
-#else
-#define TSAN_MAYBE_INTERCEPT___XSTAT
-#endif
-
-TSAN_INTERCEPTOR(int, stat, const char *path, void *buf) {
-#if SANITIZER_FREEBSD || SANITIZER_MAC || SANITIZER_ANDROID
-  SCOPED_TSAN_INTERCEPTOR(stat, path, buf);
-  READ_STRING(thr, pc, path, 0);
-  return REAL(stat)(path, buf);
-#else
-  SCOPED_TSAN_INTERCEPTOR(__xstat, 0, path, buf);
-  READ_STRING(thr, pc, path, 0);
-  return REAL(__xstat)(0, path, buf);
-#endif
-}
-
-#if SANITIZER_LINUX && !SANITIZER_ANDROID
 TSAN_INTERCEPTOR(int, __xstat64, int version, const char *path, void *buf) {
   SCOPED_TSAN_INTERCEPTOR(__xstat64, version, path, buf);
   READ_STRING(thr, pc, path, 0);

 
   TSAN_INTERCEPT(pthread_once);
 
-  TSAN_INTERCEPT(stat);
-  TSAN_MAYBE_INTERCEPT___XSTAT;
   TSAN_MAYBE_INTERCEPT_STAT64;
   TSAN_MAYBE_INTERCEPT___XSTAT64;
   TSAN_INTERCEPT(lstat);