[sanitizer_common] Define __sanitizer_FILE on NetBSD

[krytarowski]

lib/sanitizer_common/sanitizer_platform_limits_netbsd.cc

2269

Duplicate with L2231

[mgorny]

lib/sanitizer_common/sanitizer_platform_limits_netbsd.cc

2269

Fixed.

[krytarowski]

As a subtask please add missing calls to unpoison_file in interceptors (in sanitizer_common_interceptors.inc)

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5699

maybe if (fp->_bf._base && fp->_bf._size)

[mgorny]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5699

Ok. I'll even take it further, and since _size is signed, check for > 0.

[mgorny]

We've been discussing this, and I think we're doing this the wrong way. Could you help me a little understand this?

In particular, what is the purpose of unpoisoning file? Is it in order to account for stdio functions being implemented inline or as macros, and therefore user code accessing internal FILE members? Or is there some other use case for this?

If only the former, then I think there is no purpose in definining __sanitizer_FILE on NetBSD, as we support only reentrant interfaces which are all implemented as libc routine calls.

[krytarowski]

If we understand it correctly, unpoision FILE would be only for inlined routines accessing FILE buffer(s) directly. On NetBSD we enforce _REENTRANT for all sanitizers in order to support only _REENTRANT variations of calls that go through libc calls.

There is however an exception with *_unlocked functions that are still inlined (putc_unlocked(), getc_unlocked(),...) and probably for them we still want to keep SANITIZER_HAS_STRUCT_FILE=1. Such interfaces are niche but still used according to https://codesearch.debian.net/

@vitalybuka @kcc @dvyukov @eugenis

Please help to make it clearer.

[krytarowski]

lib/tsan/rtl/tsan_interceptors.cc

45–47

We might want to go for emulating _unlocked:

#define	fileno_unlocked(p)	\
    488     ((p)->_file == -1 ? -1 : (int)(unsigned short)(p)->_file)

[mgorny]

Implemented fileno_unlocked as requested.

[vitalybuka]

LGTM

[krytarowski]

In D56109#1344521, @vitalybuka wrote:

LGTM

Thanks! Any feedback regarding the raised comments in https://reviews.llvm.org/D56109#1341967 and https://reviews.llvm.org/D56109#1342059 ?

[mgorny]

Ping.

[eugenis]

In D56109#1341967, @mgorny wrote:

We've been discussing this, and I think we're doing this the wrong way. Could you help me a little understand this?

In particular, what is the purpose of unpoisoning file? Is it in order to account for stdio functions being implemented inline or as macros, and therefore user code accessing internal FILE members? Or is there some other use case for this?

If only the former, then I think there is no purpose in definining __sanitizer_FILE on NetBSD, as we support only reentrant interfaces which are all implemented as libc routine calls.

Yes, AFAIR unpoisoning of struct FILE was done specifically to support inlined and _unlocked stdio functions.

lib/sanitizer_common/sanitizer_common_interceptors.inc

 #define INIT_TSEARCH
 #endif
 
 #if SANITIZER_INTERCEPT_LIBIO_INTERNALS || SANITIZER_INTERCEPT_FOPEN || \
     SANITIZER_INTERCEPT_OPEN_MEMSTREAM
 void unpoison_file(__sanitizer_FILE *fp) {
 #if SANITIZER_HAS_STRUCT_FILE
   COMMON_INTERCEPTOR_INITIALIZE_RANGE(fp, sizeof(*fp));
+#if SANITIZER_NETBSD
+  if (fp->_bf._base && fp->_bf._size > 0)
+    COMMON_INTERCEPTOR_INITIALIZE_RANGE(fp->_bf._base,
+                                        fp->_bf._size);
+#else
   if (fp->_IO_read_base && fp->_IO_read_base < fp->_IO_read_end)
     COMMON_INTERCEPTOR_INITIALIZE_RANGE(fp->_IO_read_base,
                                         fp->_IO_read_end - fp->_IO_read_base);
+#endif
 #endif  // SANITIZER_HAS_STRUCT_FILE
 }
 #endif
 
 #if SANITIZER_INTERCEPT_LIBIO_INTERNALS
 // These guys are called when a .c source is built with -O2.
 INTERCEPTOR(int, __uflow, __sanitizer_FILE *fp) {
   void *ctx;

lib/sanitizer_common/sanitizer_platform_limits_netbsd.h

 struct __sanitizer_wordexp_t {
   uptr we_wordc;
   char **we_wordv;
   uptr we_offs;
   char *we_strings;
   uptr we_nbytes;
 };
 
-typedef char __sanitizer_FILE;
-#define SANITIZER_HAS_STRUCT_FILE 0
+struct __sanitizer_FILE {
+  unsigned char *_p;
+  int _r;
+  int _w;
+  unsigned short _flags;
+  short _file;
+  struct {
+    unsigned char *_base;
+    int _size;
+  } _bf;
+  int _lbfsize;
+  void *_cookie;
+  int (*_close)(void *ptr);
+  u64 (*_read)(void *, void *, uptr);
+  u64 (*_seek)(void *, u64, int);
+  uptr (*_write)(void *, const void *, uptr);
+  struct {
+    unsigned char *_base;
+    int _size;
+  } _ext;
+  unsigned char *_up;
+  int _ur;
+  unsigned char _ubuf[3];
+  unsigned char _nbuf[1];
+  int (*_flush)(void *ptr);
+  char _lb_unused[sizeof(uptr)];
+  int _blksize;
+  u64 _offset;
+};
+#define SANITIZER_HAS_STRUCT_FILE 1
 
 extern int shmctl_ipc_stat;
 
 // This simplifies generic code
 #define struct_shminfo_sz -1
 #define struct_shm_info_sz -1
 #define shmctl_shm_stat -1
 #define shmctl_ipc_info -1

lib/sanitizer_common/sanitizer_platform_limits_netbsd.cc

 // preprocessor macros.
 CHECK_STRUCT_SIZE_AND_OFFSET(sigaction, sa_mask);
 
 CHECK_TYPE_SIZE(wordexp_t);
 CHECK_SIZE_AND_OFFSET(wordexp_t, we_wordc);
 CHECK_SIZE_AND_OFFSET(wordexp_t, we_wordv);
 CHECK_SIZE_AND_OFFSET(wordexp_t, we_offs);
 
+COMPILER_CHECK(sizeof(__sanitizer_FILE) <= sizeof(FILE));
+CHECK_SIZE_AND_OFFSET(FILE, _p);
+CHECK_SIZE_AND_OFFSET(FILE, _r);
+CHECK_SIZE_AND_OFFSET(FILE, _w);
+CHECK_SIZE_AND_OFFSET(FILE, _flags);
+CHECK_SIZE_AND_OFFSET(FILE, _file);
+CHECK_SIZE_AND_OFFSET(FILE, _bf);
+CHECK_SIZE_AND_OFFSET(FILE, _lbfsize);
+CHECK_SIZE_AND_OFFSET(FILE, _cookie);
+CHECK_SIZE_AND_OFFSET(FILE, _close);
+CHECK_SIZE_AND_OFFSET(FILE, _read);
+CHECK_SIZE_AND_OFFSET(FILE, _seek);
+CHECK_SIZE_AND_OFFSET(FILE, _write);
+CHECK_SIZE_AND_OFFSET(FILE, _ext);
+CHECK_SIZE_AND_OFFSET(FILE, _up);
+CHECK_SIZE_AND_OFFSET(FILE, _ur);
+CHECK_SIZE_AND_OFFSET(FILE, _ubuf);
+CHECK_SIZE_AND_OFFSET(FILE, _nbuf);
+CHECK_SIZE_AND_OFFSET(FILE, _flush);
+CHECK_SIZE_AND_OFFSET(FILE, _lb_unused);
+CHECK_SIZE_AND_OFFSET(FILE, _blksize);
+CHECK_SIZE_AND_OFFSET(FILE, _offset);
+
 CHECK_TYPE_SIZE(tm);
 CHECK_SIZE_AND_OFFSET(tm, tm_sec);
 CHECK_SIZE_AND_OFFSET(tm, tm_min);
 CHECK_SIZE_AND_OFFSET(tm, tm_hour);
 CHECK_SIZE_AND_OFFSET(tm, tm_mday);
 CHECK_SIZE_AND_OFFSET(tm, tm_mon);
 CHECK_SIZE_AND_OFFSET(tm, tm_year);
 CHECK_SIZE_AND_OFFSET(tm, tm_wday);

lib/tsan/rtl/tsan_interceptors.cc

 
 #if SANITIZER_FREEBSD || SANITIZER_MAC
 #define stdout __stdoutp
 #define stderr __stderrp
 #endif
 
 #if SANITIZER_NETBSD
 #define dirfd(dirp) (*(int *)(dirp))
-#define fileno_unlocked fileno
+#define fileno_unlocked(fp) \
+  (((__sanitizer_FILE*)fp)->_file == -1 ? -1 : \
+   (int)(unsigned short)(((__sanitizer_FILE*)fp)->_file))  // NOLINT
 
-#if _LP64
-#define __sF_size 152
-#else
-#define __sF_size 88
-#endif
-
-#define stdout ((char*)&__sF + (__sF_size * 1))
-#define stderr ((char*)&__sF + (__sF_size * 2))
+#define stdout ((__sanitizer_FILE*)&__sF[1])
+#define stderr ((__sanitizer_FILE*)&__sF[2])
 
 #define nanosleep __nanosleep50
 #define vfork __vfork14
 #endif
 
 #if SANITIZER_ANDROID
 #define mallopt(a, b)
 #endif

 extern "C" int pthread_key_create(unsigned *key, void (*destructor)(void* v));
 extern "C" int pthread_setspecific(unsigned key, const void *v);
 DECLARE_REAL(int, pthread_mutexattr_gettype, void *, void *)
 DECLARE_REAL(int, fflush, __sanitizer_FILE *fp)
 DECLARE_REAL_AND_INTERCEPTOR(void *, malloc, uptr size)
 DECLARE_REAL_AND_INTERCEPTOR(void, free, void *ptr)
 extern "C" void *pthread_self();
 extern "C" void _exit(int status);
-extern "C" int fileno_unlocked(void *stream);
 #if !SANITIZER_NETBSD
+extern "C" int fileno_unlocked(void *stream);
 extern "C" int dirfd(void *dirp);
 #endif
 #if !SANITIZER_FREEBSD && !SANITIZER_ANDROID && !SANITIZER_NETBSD
 extern "C" int mallopt(int param, int value);
 #endif
 #if SANITIZER_NETBSD
 extern __sanitizer_FILE __sF[];
 #else