[sanitizer] Add fgets, fputs and puts into sanitizer_common

Summary:
Add fgets, fputs and puts to sanitizer_common. This adds ASAN coverage
for these functions, extends MSAN support from fgets to fputs/puts and
extends TSAN support from puts to fputs.

Fixes: https://github.com/google/sanitizers/issues/952

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D46545

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@334450 91177308-0d34-0410-b5e6-96231b3b80d8

10 files changed, 210 insertions, 29 deletions

diff --git a/lib/esan/esan_interceptors.cpp b/lib/esan/esan_interceptors.cpp
index da0bf4df5..0c596f1cf 100644
--- a/lib/esan/esan_interceptors.cpp
+++ b/lib/esan/esan_interceptors.cpp
@@ -316,13 +316,6 @@ INTERCEPTOR(int, unlink, char *path) {
   return REAL(unlink)(path);
 }
 
-INTERCEPTOR(int, puts, const char *s) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, puts, s);
-  COMMON_INTERCEPTOR_READ_RANGE(ctx, s, internal_strlen(s));
-  return REAL(puts)(s);
-}
-
 INTERCEPTOR(int, rmdir, char *path) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, rmdir, path);
@@ -493,9 +486,6 @@ void initializeInterceptors() {
   INTERCEPT_FUNCTION(creat);
   ESAN_MAYBE_INTERCEPT_CREAT64;
   INTERCEPT_FUNCTION(unlink);
-  INTERCEPT_FUNCTION(fread);
-  INTERCEPT_FUNCTION(fwrite);
-  INTERCEPT_FUNCTION(puts);
   INTERCEPT_FUNCTION(rmdir);
 
   ESAN_MAYBE_INTERCEPT_SIGNAL;
diff --git a/lib/msan/msan_interceptors.cc b/lib/msan/msan_interceptors.cc
index 33fc63492..48d038131 100644
--- a/lib/msan/msan_interceptors.cc
+++ b/lib/msan/msan_interceptors.cc
@@ -748,15 +748,6 @@ INTERCEPTOR(int, socketpair, int domain, int type, int protocol, int sv[2]) {
   return res;
 }
 
-INTERCEPTOR(char *, fgets, char *s, int size, void *stream) {
-  ENSURE_MSAN_INITED();
-  InterceptorScope interceptor_scope;
-  char *res = REAL(fgets)(s, size, stream);
-  if (res)
-    __msan_unpoison(s, REAL(strlen)(s) + 1);
-  return res;
-}
-
 #if !SANITIZER_FREEBSD && !SANITIZER_NETBSD
 INTERCEPTOR(char *, fgets_unlocked, char *s, int size, void *stream) {
   ENSURE_MSAN_INITED();
@@ -1609,7 +1600,6 @@ void InitializeInterceptors() {
   INTERCEPT_FUNCTION(pipe);
   INTERCEPT_FUNCTION(pipe2);
   INTERCEPT_FUNCTION(socketpair);
-  INTERCEPT_FUNCTION(fgets);
   MSAN_MAYBE_INTERCEPT_FGETS_UNLOCKED;
   INTERCEPT_FUNCTION(getrlimit);
   MSAN_MAYBE_INTERCEPT_GETRLIMIT64;
diff --git a/lib/sanitizer_common/sanitizer_common_interceptors.inc b/lib/sanitizer_common/sanitizer_common_interceptors.inc
index cd035ec58..474365ec2 100644
--- a/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -1189,6 +1189,50 @@ INTERCEPTOR(SSIZE_T, pwritev64, int fd, __sanitizer_iovec *iov, int iovcnt,
 #define INIT_PWRITEV64
 #endif
 
+#if SANITIZER_INTERCEPT_FGETS
+INTERCEPTOR(char *, fgets, char *s, SIZE_T size, void *file) {
+  // libc file streams can call user-supplied functions, see fopencookie.
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, fgets, s, size, file);
+  // FIXME: under ASan the call below may write to freed memory and corrupt
+  // its metadata. See
+  // https://github.com/google/sanitizers/issues/321.
+  char *res = REAL(fgets)(s, size, file);
+  if (res)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, s, REAL(strlen)(s) + 1);
+  return res;
+}
+#define INIT_FGETS COMMON_INTERCEPT_FUNCTION(fgets)
+#else
+#define INIT_FGETS
+#endif
+
+#if SANITIZER_INTERCEPT_FPUTS
+INTERCEPTOR(int, fputs, char *s, void *file) {
+  // libc file streams can call user-supplied functions, see fopencookie.
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, fputs, s, file);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, s, REAL(strlen)(s) + 1);
+  return REAL(fputs)(s, file);
+}
+#define INIT_FPUTS COMMON_INTERCEPT_FUNCTION(fputs)
+#else
+#define INIT_FPUTS
+#endif
+
+#if SANITIZER_INTERCEPT_PUTS
+INTERCEPTOR(int, puts, char *s) {
+  // libc file streams can call user-supplied functions, see fopencookie.
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, puts, s);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, s, REAL(strlen)(s) + 1);
+  return REAL(puts)(s);
+}
+#define INIT_PUTS COMMON_INTERCEPT_FUNCTION(puts)
+#else
+#define INIT_PUTS
+#endif
+
 #if SANITIZER_INTERCEPT_PRCTL
 INTERCEPTOR(int, prctl, int option, unsigned long arg2,
             unsigned long arg3,                        // NOLINT
@@ -7236,6 +7280,9 @@ static void InitializeCommonInterceptors() {
   INIT_WRITEV;
   INIT_PWRITEV;
   INIT_PWRITEV64;
+  INIT_FGETS;
+  INIT_FPUTS;
+  INIT_PUTS;
   INIT_PRCTL;
   INIT_LOCALTIME_AND_FRIENDS;
   INIT_STRPTIME;
diff --git a/lib/sanitizer_common/sanitizer_platform_interceptors.h b/lib/sanitizer_common/sanitizer_platform_interceptors.h
index d89a009f5..89c0f9d4f 100644
--- a/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ b/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -164,6 +164,9 @@
 
 #define SANITIZER_INTERCEPT_FREAD SI_POSIX
 #define SANITIZER_INTERCEPT_FWRITE SI_POSIX
+#define SANITIZER_INTERCEPT_FGETS SI_POSIX
+#define SANITIZER_INTERCEPT_FPUTS SI_POSIX
+#define SANITIZER_INTERCEPT_PUTS SI_POSIX
 
 #define SANITIZER_INTERCEPT_PREAD64 SI_LINUX_NOT_ANDROID || SI_SOLARIS32
 #define SANITIZER_INTERCEPT_PWRITE64 SI_LINUX_NOT_ANDROID || SI_SOLARIS32
diff --git a/lib/tsan/rtl/tsan_interceptors.cc b/lib/tsan/rtl/tsan_interceptors.cc
index 580097430..e2074ed6b 100644
--- a/lib/tsan/rtl/tsan_interceptors.cc
+++ b/lib/tsan/rtl/tsan_interceptors.cc
@@ -1736,12 +1736,6 @@ TSAN_INTERCEPTOR(void, abort, int fake) {
   REAL(abort)(fake);
 }
 
-TSAN_INTERCEPTOR(int, puts, const char *s) {
-  SCOPED_TSAN_INTERCEPTOR(puts, s);
-  MemoryAccessRange(thr, pc, (uptr)s, internal_strlen(s), false);
-  return REAL(puts)(s);
-}
-
 TSAN_INTERCEPTOR(int, rmdir, char *path) {
   SCOPED_TSAN_INTERCEPTOR(rmdir, path);
   Release(thr, pc, Dir2addr(path));
@@ -2706,10 +2700,7 @@ void InitializeInterceptors() {
   TSAN_INTERCEPT(unlink);
   TSAN_INTERCEPT(tmpfile);
   TSAN_MAYBE_INTERCEPT_TMPFILE64;
-  TSAN_INTERCEPT(fread);
-  TSAN_INTERCEPT(fwrite);
   TSAN_INTERCEPT(abort);
-  TSAN_INTERCEPT(puts);
   TSAN_INTERCEPT(rmdir);
   TSAN_INTERCEPT(closedir);
 
diff --git a/test/asan/TestCases/Posix/fgets_fputs.cc b/test/asan/TestCases/Posix/fgets_fputs.cc
new file mode 100644
index 000000000..6f0696393
--- /dev/null
+++ b/test/asan/TestCases/Posix/fgets_fputs.cc
@@ -0,0 +1,46 @@
+// RUN: %clangxx_asan -g %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-FGETS
+// RUN: not %run %t 2 2>&1 | FileCheck %s --check-prefix=CHECK-FPUTS
+// RUN: not %run %t 3 3 2>&1 | FileCheck %s --check-prefix=CHECK-PUTS
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int test_fgets() {
+  FILE *fp = fopen("/etc/passwd", "r");
+  char buf[2];
+  fgets(buf, sizeof(buf) + 1, fp); // BOOM
+  fclose(fp);
+  return 0;
+}
+
+int test_fputs() {
+  FILE *fp = fopen("/dev/null", "w");
+  char buf[1] = {'x'}; // Note: not nul-terminated
+  fputs(buf, fp);      // BOOM
+  return fclose(fp);
+}
+
+void test_puts() {
+  char *p = strdup("x");
+  free(p);
+  puts(p); // BOOM
+}
+
+int main(int argc, char *argv[]) {
+  if (argc == 1)
+    test_fgets();
+  else if (argc == 2)
+    test_fputs();
+  else
+    test_puts();
+  return 0;
+}
+
+// CHECK-FGETS: {{.*ERROR: AddressSanitizer: stack-buffer-overflow}}
+// CHECK-FGETS: #{{.*}} in {{(wrap_|__interceptor_)?}}fgets
+// CHECK-FPUTS: {{.*ERROR: AddressSanitizer: stack-buffer-overflow}}
+// CHECK-FPUTS: #{{.*}} in {{(wrap_|__interceptor_)?}}fputs
+// CHECK-PUTS: {{.*ERROR: AddressSanitizer: heap-use-after-free}}
+// CHECK-PUTS: #{{.*}} in {{(wrap_|__interceptor_)?}}puts
diff --git a/test/msan/fgets_fputs.cc b/test/msan/fgets_fputs.cc
new file mode 100644
index 000000000..1e9694398
--- /dev/null
+++ b/test/msan/fgets_fputs.cc
@@ -0,0 +1,47 @@
+// RUN: %clangxx_msan -g %s -o %t
+// RUN: %run %t
+// RUN: not %run %t 2 2>&1 | FileCheck %s --check-prefix=CHECK-FPUTS
+// RUN: not %run %t 3 3 2>&1 | FileCheck %s --check-prefix=CHECK-PUTS
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int test_fgets() {
+  FILE *fp = fopen("/dev/zero", "r");
+  char c;
+
+  if (!fgets(&c, 1, fp))
+    return 1;
+
+  if (c == '1') // No error
+    return 2;
+
+  fclose(fp);
+  return 0;
+}
+
+int test_fputs() {
+  FILE *fp = fopen("/dev/null", "w");
+  char buf[2];
+  fputs(buf, fp); // BOOM
+  return fclose(fp);
+}
+
+void test_puts() {
+  char buf[2];
+  puts(buf); // BOOM
+}
+
+int main(int argc, char *argv[]) {
+  if (argc == 1)
+    test_fgets();
+  else if (argc == 2)
+    test_fputs();
+  else
+    test_puts();
+  return 0;
+}
+
+// CHECK-FPUTS: Uninitialized bytes in __interceptor_fputs at offset 0 inside
+// CHECK-PUTS: Uninitialized bytes in __interceptor_puts at offset 0 inside
diff --git a/test/sanitizer_common/TestCases/Posix/fgets.cc b/test/sanitizer_common/TestCases/Posix/fgets.cc
new file mode 100644
index 000000000..4eda4c1fc
--- /dev/null
+++ b/test/sanitizer_common/TestCases/Posix/fgets.cc
@@ -0,0 +1,20 @@
+// RUN: %clangxx -g %s -o %t && %run %t
+
+#include <stdio.h>
+
+int main(void) {
+  FILE *fp;
+  char buf[2];
+  char *s;
+
+  fp = fopen("/etc/passwd", "r");
+  if (!fp)
+    return 1;
+
+  s = fgets(buf, sizeof(buf), fp);
+  if (!s)
+    return 2;
+
+  fclose(fp);
+  return 0;
+}
diff --git a/test/sanitizer_common/TestCases/Posix/fputs_puts.cc b/test/sanitizer_common/TestCases/Posix/fputs_puts.cc
new file mode 100644
index 000000000..8e8f7d384
--- /dev/null
+++ b/test/sanitizer_common/TestCases/Posix/fputs_puts.cc
@@ -0,0 +1,18 @@
+// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s
+// CHECK: {{^foobar$}}
+
+#include <stdio.h>
+
+int main(void) {
+  int r;
+
+  r = fputs("foo", stdout);
+  if (r < 0)
+    return 1;
+
+  r = puts("bar");
+  if (r < 0)
+    return 1;
+
+  return 0;
+}
diff --git a/test/tsan/race_on_fputs.cc b/test/tsan/race_on_fputs.cc
new file mode 100644
index 000000000..53042e3d3
--- /dev/null
+++ b/test/tsan/race_on_fputs.cc
@@ -0,0 +1,29 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
+#include "test.h"
+
+char s[] = "abracadabra";
+
+void *Thread0(void *p) {
+  fputs(s, stdout);
+  barrier_wait(&barrier);
+  return 0;
+}
+
+void *Thread1(void *p) {
+  barrier_wait(&barrier);
+  s[3] = 'z';
+  return 0;
+}
+
+int main() {
+  barrier_init(&barrier, 2);
+  pthread_t th[2];
+  pthread_create(&th[0], 0, Thread0, 0);
+  pthread_create(&th[1], 0, Thread1, 0);
+  pthread_join(th[0], 0);
+  pthread_join(th[1], 0);
+  fprintf(stderr, "DONE");
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK: DONE