Correct ctype(3) functions with NLS on NetBSD

[krytarowski]

Summary

The setlocale(3) function reloads the ctype(3) arrays from
external files. This happens behind the scenes in the internals
of libc (citrus library, runes functions etc).

ctype(3) functions like isspace(3) can be provided with two
variations on NetBSD: inlined or via a global symbol in libc:

#if defined(_NETBSD_SOURCE) && !defined(_CTYPE_NOINLINE) && \
    !defined(__cplusplus)
#include <sys/ctype_inline.h>
#else
#include <sys/ctype_bits.h>
#endif

The in-lined versions are de-facto array lookup operations.

#define isspace(c)      ((int)((_ctype_tab_ + 1)[(c)] & _CTYPE_S))

After setting setlocale(3) the ctype(3) arrays (_ctype_tab_,
_toupper_tab_, _tolower_tab_) are reload behind the scenes
and they are required to be marked as initialized.

Set them initialized inside the common setlocale(3) interceptor.

The arrays are of size of 257 elements: 0..255 + 1 (EOF).

This corrects errors on NetBSD/amd64 in applications
prebuilt with MSan.

Sponsored by <The NetBSD Foundation>

[vitalybuka]

Please update compiler-rt/test/msan/setlocale.cc

lib/sanitizer_common/sanitizer_common_interceptors.inc

3210

should this be done only for category == LC_CTYPE

[krytarowski]

In D42020#982283, @vitalybuka wrote:

Please update compiler-rt/test/msan/setlocale.cc

It works as it is.

Do you mean to add a specific test for LC_CTYPE?

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3210

It's possible to add if() for LC_CTYPE || LC_ALL, but I would need to define it in sanitizer_platform_limits_netbsd.*.

In real life, I'm not sure that this is really needed, overhead is negligible and hardly ever we will execute this code needlessly.

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3210

Can please create SetLocale() (or better name). Define it in sanitizer_netbsd.cpp and define empty one for other platforms?

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3210

And it would be nice to have a test which fails on NetBsd without this change.

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3210

I will prepare the needed patch.

[krytarowski]

• add test
• split netbsd specific code to a new function

[krytarowski]

ping?

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3195

How about new file:
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_netbsd.inc

and moving all netbsd pieces there?
It's OK to do as a followup patch.

[krytarowski]

I can move the patches there. How about D43543, D43541, D43539? Can I commit as it is and move them in one patch to a new file.

[krytarowski]

Hmm on the other hand.. these interceptors will be useful for NetBSD forks/descendants like FreeBSD (OpenBSD?) and perhaps other BSD descendants like SunOS (Solaris). I just don't enable them without prior-testing.

lib/sanitizer_common/sanitizer_common_interceptors.inc

 #define sigpending __sigpending14
 #define sigprocmask __sigprocmask14
 #define sigtimedwait __sigtimedwait50
 #define stat __stat50
 #define time __time50
 #define times __times13
 #define wait3 __wait350
 #define wait4 __wait450
+extern const unsigned short *_ctype_tab_;
+extern const short *_toupper_tab_;
+extern const short *_tolower_tab_;
 #endif
 
 // Platform-specific options.
 #if SANITIZER_MAC
 namespace __sanitizer {
 bool PlatformHasDifferentMemcpyAndMemmove();
 }
 #define PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE \

 }
 
 #define INIT_PTRACE COMMON_INTERCEPT_FUNCTION(ptrace);
 #else
 #define INIT_PTRACE
 #endif
 
 #if SANITIZER_INTERCEPT_SETLOCALE
+static void unpoison_ctype_arrays(void *ctx) {
+#if SANITIZER_NETBSD
+  // These arrays contain 256 regular elements in unsigned char range + 1 EOF
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, _ctype_tab_, 257 * sizeof(short));
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, _toupper_tab_, 257 * sizeof(short));
+  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, _tolower_tab_, 257 * sizeof(short));
+#endif
+}
+
 INTERCEPTOR(char *, setlocale, int category, char *locale) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, setlocale, category, locale);
   if (locale)
     COMMON_INTERCEPTOR_READ_RANGE(ctx, locale, REAL(strlen)(locale) + 1);
   char *res = REAL(setlocale)(category, locale);
-  if (res) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, res, REAL(strlen)(res) + 1);
+  if (res) {
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, res, REAL(strlen)(res) + 1);
+    unpoison_ctype_arrays(ctx);
+  }
   return res;
 }
 
 #define INIT_SETLOCALE COMMON_INTERCEPT_FUNCTION(setlocale);
 #else
 #define INIT_SETLOCALE
 #endif
 

test/sanitizer_common/TestCases/ctype.c

+// RUN: %clang %s -o %t && %run %t 2>&1 | FileCheck %s
+
+#include <ctype.h>
+#include <limits.h>
+#include <locale.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+void check_ctype(void) {
+  unsigned char c;
+  volatile size_t i = 0; /* a dummy variable to prevent optimizing code out */
+
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isalpha(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isascii(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isblank(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!iscntrl(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isdigit(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isgraph(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!islower(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isprint(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!ispunct(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isspace(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isupper(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isxdigit(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!isalnum(c);
+
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!tolower(c);
+  for (c = 0; c < UCHAR_MAX; c++)
+    i += !!toupper(c);
+
+  i += !!isalpha(EOF);
+  i += !!isascii(EOF);
+  i += !!isblank(EOF);
+  i += !!iscntrl(EOF);
+  i += !!isdigit(EOF);
+  i += !!isgraph(EOF);
+  i += !!islower(EOF);
+  i += !!isprint(EOF);
+  i += !!ispunct(EOF);
+  i += !!isspace(EOF);
+  i += !!isupper(EOF);
+  i += !!isxdigit(EOF);
+  i += !!isalnum(EOF);
+
+  i += !!tolower(EOF);
+  i += !!toupper(EOF);
+
+  if (i)
+    return;
+  else
+    return;
+}
+
+int main(int argc, char **argv) {
+  check_ctype();
+
+  setlocale(LC_ALL, "");
+
+  check_ctype();
+
+  setlocale(LC_ALL, "en_US.UTF-8");
+
+  check_ctype();
+
+  setlocale(LC_CTYPE, "pl_PL.UTF-8");
+
+  check_ctype();
+
+  printf("OK\n");
+
+  // CHECK: OK
+
+  return 0;
+}