[sanitizer] Intercept sem_open/sem_unlink

[vitalybuka]

Summary

Without interceptor implementation may call strlen on internal
buffers causing false msan errors.

[vitalybuka]

rebase

[morehouse]

LGTM

[fmayer]

Broke the android bot https://lab.llvm.org/buildbot/#/builders/77/builds/8458

[ro]

The new test FAILs on Solaris, and, I believe, rightly so:

For one, there's a warning:

`
compiler-rt/test/sanitizer_common/TestCases/Posix/sem_open.cpp:14:38: warning: format specifies type 'int' but the argument has type 'pid_t' (aka 'long') [-Wformat]
  sprintf(name, "/sem_open_test_%d", getpid());
                                ~~   ^~~~~~~~
                                %ld

pid_t is int for 64-bit complations, but long for 32-bit ones. This can be fixed by printing as %ld and casting the getpid return value to
long to match.

Worse, however, the test execution FAILs:

Assertion failed: sem_close(s2) == 0, file /vol/llvm/src/llvm-project/local/compiler-rt/test/sanitizer_common/TestCases/Posix/sem_open.cpp, line 23

sem_close(s2) succeeds on Linux, but fails on Solaris with EINVAL. Looking closer, s1 and s2 are identical, as expected. On Solaris,
sem_close(s1) frees and deallocates the sem_t, so it has become invalid (unmapped even when looking with gdb) at the time sem_close(s2) is called.

Both Solaris sem_close(3C) and the corresponding XPG7 page state

The effect of subsequent use of the semaphore indicated by sem by this process is undefined.

so this behaviour seems perfectly valid.

[vitalybuka]

In D107615#2941342, @ro wrote:

The new test FAILs on Solaris, and, I believe, rightly so:

For one, there's a warning:

`
compiler-rt/test/sanitizer_common/TestCases/Posix/sem_open.cpp:14:38: warning: format specifies type 'int' but the argument has type 'pid_t' (aka 'long') [-Wformat]
  sprintf(name, "/sem_open_test_%d", getpid());
                                ~~   ^~~~~~~~
                                %ld

pid_t is int for 64-bit complations, but long for 32-bit ones. This can be fixed by printing as %ld and casting the getpid return value to
long to match.

Worse, however, the test execution FAILs:

Assertion failed: sem_close(s2) == 0, file /vol/llvm/src/llvm-project/local/compiler-rt/test/sanitizer_common/TestCases/Posix/sem_open.cpp, line 23

sem_close(s2) succeeds on Linux, but fails on Solaris with EINVAL. Looking closer, s1 and s2 are identical, as expected. On Solaris,
sem_close(s1) frees and deallocates the sem_t, so it has become invalid (unmapped even when looking with gdb) at the time sem_close(s2) is called.

Both Solaris sem_close(3C) and the corresponding XPG7 page state

The effect of subsequent use of the semaphore indicated by sem by this process is undefined.

so this behaviour seems perfectly valid.

There is also

If a process makes multiple successful calls to sem_open() with the same value for name, the same semaphore address is returned for each such successful call, provided that there have been no calls to sem_unlink(3C) for this semaphore.

Opening twice is not essential to the test, so I will remove it and fix type if sprintf.
bf6000dc98df82d9b493e0b5d247538f51d6b9ac

[ro]

In D107615#2942737, @vitalybuka wrote:

Opening twice is not essential to the test, so I will remove it and fix type if sprintf.
bf6000dc98df82d9b493e0b5d247538f51d6b9ac

Excellent, thanks.

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
 
INTERCEPTOR(int, sem_getvalue, __sanitizer_sem_t *s, int *sval) {
   COMMON_INTERCEPTOR_ENTER(ctx, sem_getvalue, s, sval);
   int res = REAL(sem_getvalue)(s, sval);
   if (res == 0) {
     COMMON_INTERCEPTOR_ACQUIRE(ctx, (uptr)s);
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, sval, sizeof(*sval));
   }
   return res;
 }
+
+INTERCEPTOR(__sanitizer_sem_t *, sem_open, const char *name, int oflag, ...) {
+  void *ctx;
+  va_list ap;
+  va_start(ap, oflag);
+  u32 mode = va_arg(ap, u32);
+  u32 value = va_arg(ap, u32);
+  COMMON_INTERCEPTOR_ENTER(ctx, sem_open, name, oflag, mode, value);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, name, REAL(strlen)(name) + 1);
+  __sanitizer_sem_t *s = REAL(sem_open)(name, oflag, mode, value);
+  if (s)
+    COMMON_INTERCEPTOR_INITIALIZE_RANGE(s, sizeof(*s));
+  va_end(ap);
+  return s;
+}
+
+INTERCEPTOR(int, sem_unlink, const char *name) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, sem_unlink, name);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, name, REAL(strlen)(name) + 1);
+  return REAL(sem_unlink)(name);
+}
+
 #  define INIT_SEM                            \
     COMMON_INTERCEPT_FUNCTION(sem_init);      \
     COMMON_INTERCEPT_FUNCTION(sem_destroy);   \
     COMMON_INTERCEPT_FUNCTION(sem_wait);      \
     COMMON_INTERCEPT_FUNCTION(sem_trywait);   \
     COMMON_INTERCEPT_FUNCTION(sem_timedwait); \
     COMMON_INTERCEPT_FUNCTION(sem_post);      \
-    COMMON_INTERCEPT_FUNCTION(sem_getvalue);
+    COMMON_INTERCEPT_FUNCTION(sem_getvalue);  \
+    COMMON_INTERCEPT_FUNCTION(sem_open);      \
+    COMMON_INTERCEPT_FUNCTION(sem_unlink);
 #else
 #  define INIT_SEM
 #endif  // SANITIZER_INTERCEPT_SEM
 
 #if SANITIZER_INTERCEPT_PTHREAD_SETCANCEL
 INTERCEPTOR(int, pthread_setcancelstate, int state, int *oldstate) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, pthread_setcancelstate, state, oldstate);

compiler-rt/test/sanitizer_common/TestCases/Posix/sem_open.cpp

+// RUN: %clangxx -O0 %s -o %t && %run %t
+
+#include <assert.h>
+#include <fcntl.h>
+#include <semaphore.h>
+#include <stdio.h>
+#include <unistd.h>
+
+int main() {
+  char name[1024];
+  sprintf(name, "/sem_open_test_%d", getpid());
+
+  sem_t *s1 = sem_open(name, O_CREAT, 0644, 123);
+  assert(s1 != SEM_FAILED);
+
+  sem_t *s2 = sem_open(name, O_CREAT, 0644, 123);
+  assert(s2 != SEM_FAILED);
+
+  assert(sem_close(s1) == 0);
+  assert(sem_close(s2) == 0);
+
+  assert(sem_unlink(name) == 0);
+}