[Sanitizer] Add interceptor for ptsname

[labath]

Summary

Works the same way as the ttyname interceptor.

[labath]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

4904

I'm not sure I understand the difference between COMMON_INTERCEPTOR_INITIALIZE_RANGE and COMMON_INTERCEPTOR_WRITE_RANGE. For example, I would have expected tsan to warn about concurrent usages of ptsname from different threads (and changing this to WRITE_RANGE does achieve that), but maybe there is a reason why these functions use the INITIALIZE macro (?)

[labath]

Abandoning in favor of D88547 (though I'd still be interested to know the answer to my inline question).

[MaskRay]

In D88559#2303129, @labath wrote:

Abandoning in favor of D88547 (though I'd still be interested to know the answer to my inline question).

My understanding:

COMMON_INTERCEPTOR_INITIALIZE_RANGE is only defined for msan.

• asan re-defines COMMON_INTERCEPTOR_WRITE_RANGE (the buffer is specified by the user - the addressability needs to be checked) but not COMMON_INTERCEPTOR_INITIALIZE_RANGE (static storage is known to be *addressable* so there is no need to check addressability).
• tsan re-defines COMMON_INTERCEPTOR_WRITE_RANGE (the buffer is specified by the user - the potential data race needs to be checked) but not COMMON_INTERCEPTOR_INITIALIZE_RANGE (data race will not be checked - potential false negatives).

[eugenis]

Consider a function that returns a pointer to a static buffer with the contents that are somehow initialized internally with proper synchronization. If the interceptor uses _WRITE, TSan will understand that as a data race because it can not see the internal synchronization. That why _INITIALIZE macro is used by MSan (the memory has somehow been initialized) but not by TSan (there may not have been a store there *right now*).

Another example is strerror which sometimes returns a pointer to static data (always initialized, does not require any sanitizer handling) and sometimes to a freshly allocated thread-local buffer.

[labath]

In D88559#2304679, @eugenis wrote:

Consider a function that returns a pointer to a static buffer with the contents that are somehow initialized internally with proper synchronization. If the interceptor uses _WRITE, TSan will understand that as a data race because it can not see the internal synchronization. That why _INITIALIZE macro is used by MSan (the memory has somehow been initialized) but not by TSan (there may not have been a store there *right now*).

Another example is strerror which sometimes returns a pointer to static data (always initialized, does not require any sanitizer handling) and sometimes to a freshly allocated thread-local buffer.

Interesting. Thanks for the explanation. So, we're preferring a false negative (not reporting a race in case the function doesn't do anything fancy -- a thing which is pretty hard to do in the ptsname case) over a false positive (reporting a race even if it does). Kind of makes sense. Not sure its the right trade-off for this function, but I don't want to revisit past design decisions.

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

enum {
     CIMT_FILE
   } type;
   union {
     FileMetadata file;
   };
 };
 
 #if SI_POSIX
 typedef AddrHashMap<CommonInterceptorMetadata, 31051> MetadataHashMap;
 
 static MetadataHashMap *interceptor_metadata_map;
 
 UNUSED 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;
 }
 
 UNUSED static const FileMetadata *GetInterceptorMetadata(
     __sanitizer_FILE *addr) {

 
 UNUSED static void DeleteInterceptorMetadata(void *addr) {
   MetadataHashMap::Handle h(interceptor_metadata_map, (uptr)addr, true);
   CHECK(h.exists());
 }
 #endif  // SI_POSIX
 
 #if SANITIZER_INTERCEPT_STRLEN
 INTERCEPTOR(SIZE_T, strlen, const char *s) {
   // Sometimes strlen is called prior to InitializeCommonInterceptors,
   // in which case the REAL(strlen) typically used in
   // COMMON_INTERCEPTOR_ENTER will fail.  We use internal_strlen here
   // to handle that.
   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_RANGE(ctx, s, result + 1);
   return result;
 }
 #define INIT_STRLEN COMMON_INTERCEPT_FUNCTION(strlen)
 #else
 #define INIT_STRLEN
 #endif
 
 #if SANITIZER_INTERCEPT_STRNLEN
 INTERCEPTOR(SIZE_T, strnlen, const char *s, SIZE_T maxlen) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, strnlen, s, maxlen);
   SIZE_T length = REAL(strnlen)(s, maxlen);
   if (common_flags()->intercept_strlen)
     COMMON_INTERCEPTOR_READ_RANGE(ctx, s, Min(length + 1, maxlen));
   return length;
 }
 #define INIT_STRNLEN COMMON_INTERCEPT_FUNCTION(strnlen)
 #else
 #define INIT_STRNLEN
 #endif
 
 #if SANITIZER_INTERCEPT_STRNDUP
 INTERCEPTOR(char*, strndup, const char *s, uptr size) {
   void *ctx;
   COMMON_INTERCEPTOR_STRNDUP_IMPL(ctx, s, size);
 }
 #define INIT_STRNDUP COMMON_INTERCEPT_FUNCTION(strndup)
 #else
 #define INIT_STRNDUP
 #endif // SANITIZER_INTERCEPT_STRNDUP
 
if (res == 0)
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, name, REAL(strlen)(name) + 1);
   return res;
 }
 #define INIT_TTYNAME_R COMMON_INTERCEPT_FUNCTION(ttyname_r);
 #else
 #define INIT_TTYNAME_R
 #endif
 
+#if SANITIZER_INTERCEPT_PTSNAME
+INTERCEPTOR(char *, ptsname, int fd) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, ptsname, fd);
+  char *res = REAL(ptsname)(fd);
+  if (res != nullptr)
+    COMMON_INTERCEPTOR_INITIALIZE_RANGE(res, REAL(strlen)(res) + 1);
+  return res;
+}
+#define INIT_PTSNAME COMMON_INTERCEPT_FUNCTION(ptsname);
+#else
+#define INIT_PTSNAME
+#endif
+
 #if SANITIZER_INTERCEPT_TEMPNAM
 INTERCEPTOR(char *, tempnam, char *dir, char *pfx) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, tempnam, dir, pfx);
   if (dir) COMMON_INTERCEPTOR_READ_RANGE(ctx, dir, REAL(strlen)(dir) + 1);
   if (pfx) COMMON_INTERCEPTOR_READ_RANGE(ctx, pfx, REAL(strlen)(pfx) + 1);
   char *res = REAL(tempnam)(dir, pfx);
   if (res) COMMON_INTERCEPTOR_INITIALIZE_RANGE(res, REAL(strlen)(res) + 1);
#endif
   INIT_PTHREAD_RWLOCKATTR_GETKIND_NP;
   INIT_PTHREAD_CONDATTR_GETPSHARED;
   INIT_PTHREAD_CONDATTR_GETCLOCK;
   INIT_PTHREAD_BARRIERATTR_GETPSHARED;
   INIT_TMPNAM;
   INIT_TMPNAM_R;
   INIT_TTYNAME;
   INIT_TTYNAME_R;
+  INIT_PTSNAME;
   INIT_TEMPNAM;
   INIT_PTHREAD_SETNAME_NP;
   INIT_PTHREAD_GETNAME_NP;
   INIT_SINCOS;
   INIT_REMQUO;
   INIT_REMQUOL;
   INIT_LGAMMA;
   INIT_LGAMMAL;

compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

#define SANITIZER_INTERCEPT_PTHREAD_CONDATTR_GETCLOCK \
   (SI_LINUX_NOT_ANDROID || SI_SOLARIS)
 #define SANITIZER_INTERCEPT_PTHREAD_BARRIERATTR_GETPSHARED \
   (SI_LINUX_NOT_ANDROID && !SI_NETBSD && !SI_OPENBSD)
 #define SANITIZER_INTERCEPT_THR_EXIT SI_FREEBSD
 #define SANITIZER_INTERCEPT_TMPNAM SI_POSIX
 #define SANITIZER_INTERCEPT_TMPNAM_R SI_LINUX_NOT_ANDROID || SI_SOLARIS
 #define SANITIZER_INTERCEPT_TTYNAME SI_POSIX
 #define SANITIZER_INTERCEPT_TTYNAME_R SI_POSIX
+#define SANITIZER_INTERCEPT_PTSNAME SI_POSIX
 #define SANITIZER_INTERCEPT_TEMPNAM SI_POSIX
 #define SANITIZER_INTERCEPT_SINCOS SI_LINUX || SI_SOLARIS
 #define SANITIZER_INTERCEPT_REMQUO SI_POSIX
 #define SANITIZER_INTERCEPT_REMQUOL (SI_POSIX && !SI_NETBSD)
 #define SANITIZER_INTERCEPT_LGAMMA SI_POSIX
 #define SANITIZER_INTERCEPT_LGAMMAL (SI_POSIX && !SI_NETBSD)
 #define SANITIZER_INTERCEPT_LGAMMA_R (SI_FREEBSD || SI_LINUX || SI_SOLARIS)
 #define SANITIZER_INTERCEPT_LGAMMAL_R SI_LINUX_NOT_ANDROID || SI_SOLARIS

compiler-rt/test/msan/Linux/forkpty.cpp

 // RUN: %clangxx_msan -O0 -g %s -lutil -o %t && %run %t
 
 #include <assert.h>
+#include <cstring>
 #include <pty.h>
+#include <stdlib.h>
 #include <unistd.h>
-#include <cstring>
 
 #include <sanitizer/msan_interface.h>
 
 int
 main (int argc, char** argv)
 {
   int parent, worker;
   openpty(&parent, &worker, NULL, NULL, NULL);
   assert(__msan_test_shadow(&parent, sizeof(parent)) == -1);
   assert(__msan_test_shadow(&worker, sizeof(worker)) == -1);
 
   char name[255];
   ttyname_r(parent, name, sizeof(name));
   assert(__msan_test_shadow(name, strlen(name) + 1) == -1);
 
   char *name_p = ttyname(parent);
   assert(__msan_test_shadow(name_p, strlen(name_p) + 1) == -1);
 
+  char *ptsname_p = ptsname(parent);
+  assert(ptsname_p != nullptr);
+  assert(__msan_test_shadow(ptsname_p, strlen(ptsname_p) + 1) == -1);
+
   int parent2;
   forkpty(&parent2, NULL, NULL, NULL);
   assert(__msan_test_shadow(&parent2, sizeof(parent2)) == -1);
 }