Handle NetBSD symbol renaming in msan_interceptors.cc

[krytarowski]

Summary

NetBSD renames symbols for historical and compat reasons.

Add required symbol renames in sanitizer_common_interceptors.inc:

• gettimeofday -> __gettimeofday50
• getrusage -> __getrusage50
• shmctl -> __shmctl50

Additionally handle sigaction symbol mangling.
Rename the function symbol in the file to SIGACTION_SYMNAME and define
it as __sigaction14 for NetBSD and sigaction for !NetBSD. We cannot use
simple renaming with the proprocessor, as there are valid fields named
sigaction and they must be left intact.

Sponsored by <The NetBSD Foundation>

[vitalybuka]

lib/msan/msan_interceptors.cc

53

Why not the same for __libc_thr_keycreate?

[krytarowski]

lib/msan/msan_interceptors.cc

53

This one we are not intercepting here, but calling with REAL(). We need to use a 3rd party symbol name in order to not mangle too much, as there are struct fields named 'sigaction'.

[vitalybuka]

lib/msan/msan_interceptors.cc

53

could you please use all capitals in macro names for consistency?

maybe better place for this macros is
sanitizer_platform_limits_netbsd.h and sanitizer_platform_limits_posix.h

[krytarowski]

lib/msan/msan_interceptors.cc

53

could you please use all capitals in macro names for consistency?

Do you mean SIGACTION_SYMNAME?

[vitalybuka]

lib/msan/msan_interceptors.cc

53

yes

[krytarowski]

• rename sigaction_symname to SIGACTION_SYMNAME
• put SIGACTION_SYMNAME in sanitizer_common/sanitizer_platform_limits_posix.h and ..netbsd.h

[vitalybuka]

lib/msan/msan_interceptors.cc

38

I expected you remove these too. However looks like we do the same in sanitizer_common_interceptors.inc

shmctl50 already defined there. Could you just delete "#define shmctl shmctl50" and "DECLARE_REAL(int, shmctl, int shmid, int cmd, void *buf)"
and move "INTERCEPTOR(void *, shmat" to just before "INTERCEPTOR(int, dl_iterate_phdr", after sanitizer_common_interceptors.inc ?

lib/sanitizer_common/sanitizer_platform_limits_netbsd.h

579

So we don't remove gettimeofday ang getrusage, maybe we could return this back. It's up to you.

[krytarowski]

lib/msan/msan_interceptors.cc

38

It's not clear to me. shmctl is used in the shmat interceptor.

[vitalybuka]

lib/msan/msan_interceptors.cc

38

Correct. You just need to move shmat interceptor after "#include sanitizer_common_interceptors.inc"
sanitizer_common_interceptors.inc already declares READ(shmctl)

[krytarowski]

Apply changes from review.

[krytarowski]

I think it's done now.

[vitalybuka]

lgtm

lib/msan/msan_interceptors.cc

 
 #include "interception/interception.h"
 #include "msan.h"
 #include "msan_chained_origin_depot.h"
 #include "msan_origin.h"
 #include "msan_thread.h"
 #include "msan_poisoning.h"
 #include "sanitizer_common/sanitizer_platform_limits_posix.h"
+#include "sanitizer_common/sanitizer_platform_limits_netbsd.h"
 #include "sanitizer_common/sanitizer_allocator.h"
 #include "sanitizer_common/sanitizer_allocator_interface.h"
 #include "sanitizer_common/sanitizer_allocator_internal.h"
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_errno.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 
+#if SANITIZER_NETBSD
+#define gettimeofday __gettimeofday50
+#define getrusage __getrusage50
+#endif
+
 #include <stdarg.h>
 // ACHTUNG! No other system header includes in this file.
 // Ideally, we should get rid of stdarg.h as well.
 
 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)
 DECLARE_REAL(SIZE_T, strnlen, const char *s, SIZE_T maxlen)
 DECLARE_REAL(void *, memcpy, void *dest, const void *src, uptr n)
 DECLARE_REAL(void *, memset, void *dest, int c, uptr n)
 
 // True if this is a nested interceptor.
 static THREADLOCAL int in_interceptor_scope;
 
 struct InterceptorScope {
INTERCEPTOR(int, __cxa_atexit, void (*func)(void *), void *arg,
   ENSURE_MSAN_INITED();
   MSanAtExitRecord *r =
       (MSanAtExitRecord *)InternalAlloc(sizeof(MSanAtExitRecord));
   r->func = func;
   r->arg = arg;
   return REAL(__cxa_atexit)(MSanAtExitWrapper, r, dso_handle);
 }
 
-DECLARE_REAL(int, shmctl, int shmid, int cmd, void *buf)
-
-INTERCEPTOR(void *, shmat, int shmid, const void *shmaddr, int shmflg) {
-  ENSURE_MSAN_INITED();
-  void *p = REAL(shmat)(shmid, shmaddr, shmflg);
-  if (p != (void *)-1) {
-    __sanitizer_shmid_ds ds;
-    int res = REAL(shmctl)(shmid, shmctl_ipc_stat, &ds);
-    if (!res) {
-      __msan_unpoison(p, ds.shm_segsz);
-    }
-  }
-  return p;
-}
-
 static void BeforeFork() {
   StackDepotLockAll();
   ChainedOriginDepotLockAll();
 }
 
 static void AfterFork() {
   ChainedOriginDepotUnlockAll();
   StackDepotUnlockAll();
if (act) {
       uptr new_cb = (pnew_act->sa_flags & __sanitizer::sa_siginfo)
                         ? (uptr)SignalAction
                         : (uptr)SignalHandler;
       if (cb != __sanitizer::sig_ign && cb != __sanitizer::sig_dfl) {
         atomic_store(&sigactions[signo], cb, memory_order_relaxed);
         pnew_act->sigaction = (decltype(pnew_act->sigaction))new_cb;
       }
     }
-    res = REAL(sigaction)(signo, pnew_act, oldact);
+    res = REAL(SIGACTION_SYMNAME)(signo, pnew_act, oldact);
     if (res == 0 && oldact) {
       uptr cb = (uptr)oldact->sigaction;
       if (cb == (uptr)SignalAction || cb == (uptr)SignalHandler) {
         oldact->sigaction = (decltype(oldact->sigaction))old_cb;
       }
     }
   } else {
-    res = REAL(sigaction)(signo, act, oldact);
+    res = REAL(SIGACTION_SYMNAME)(signo, act, oldact);
   }
 
   if (res == 0 && oldact) {
     __msan_unpoison(oldact, sizeof(__sanitizer_sigaction));
   }
   return res;
 }
 
if (info) {
     if (info->dlpi_name)
       __msan_unpoison(info->dlpi_name, REAL(strlen)(info->dlpi_name) + 1);
   }
   dl_iterate_phdr_data *cbdata = (dl_iterate_phdr_data *)data;
   UnpoisonParam(3);
   return cbdata->callback(info, size, cbdata->data);
 }
 
+INTERCEPTOR(void *, shmat, int shmid, const void *shmaddr, int shmflg) {
+  ENSURE_MSAN_INITED();
+  void *p = REAL(shmat)(shmid, shmaddr, shmflg);
+  if (p != (void *)-1) {
+    __sanitizer_shmid_ds ds;
+    int res = REAL(shmctl)(shmid, shmctl_ipc_stat, &ds);
+    if (!res) {
+      __msan_unpoison(p, ds.shm_segsz);
+    }
+  }
+  return p;
+}
+
 INTERCEPTOR(int, dl_iterate_phdr, dl_iterate_phdr_cb callback, void *data) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, dl_iterate_phdr, callback, data);
   dl_iterate_phdr_data cbdata;
   cbdata.callback = callback;
   cbdata.data = data;
   int res = REAL(dl_iterate_phdr)(msan_dl_iterate_phdr_cb, (void *)&cbdata);
   return res;

lib/sanitizer_common/sanitizer_platform_limits_netbsd.h

 // For sigaction, which is a function and struct at the same time,
 // and thus requires explicit "struct" in sizeof() expression.
 #define CHECK_STRUCT_SIZE_AND_OFFSET(CLASS, MEMBER)                      \
   COMPILER_CHECK(sizeof(((struct __sanitizer_##CLASS *)NULL)->MEMBER) == \
                  sizeof(((struct CLASS *)NULL)->MEMBER));                \
   COMPILER_CHECK(offsetof(struct __sanitizer_##CLASS, MEMBER) ==         \
                  offsetof(struct CLASS, MEMBER))
 
+#define SIGACTION_SYMNAME __sigaction14
+
 #endif  // SANITIZER_NETBSD
 
 #endif

lib/sanitizer_common/sanitizer_platform_limits_posix.h

 // For sigaction, which is a function and struct at the same time,
 // and thus requires explicit "struct" in sizeof() expression.
 #define CHECK_STRUCT_SIZE_AND_OFFSET(CLASS, MEMBER)                       \
   COMPILER_CHECK(sizeof(((struct __sanitizer_##CLASS *) NULL)->MEMBER) == \
                  sizeof(((struct CLASS *) NULL)->MEMBER));                \
   COMPILER_CHECK(offsetof(struct __sanitizer_##CLASS, MEMBER) ==          \
                  offsetof(struct CLASS, MEMBER))
 
+#define SIGACTION_SYMNAME sigaction
+
 #endif  // SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_MAC
 
 #endif