Page MenuHomePhabricator

Correct ctype(3) functions with NLS on NetBSD
ClosedPublic

Authored by krytarowski on Jan 12 2018, 4:39 PM.

Details

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>

Diff Detail

Event Timeline

krytarowski created this revision.Jan 12 2018, 4:39 PM
krytarowski edited the summary of this revision. (Show Details)Jan 12 2018, 4:40 PM
krytarowski edited the summary of this revision. (Show Details)

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

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 added inline comments.Jan 20 2018, 2:55 PM
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 added inline comments.Feb 14 2018, 5:53 PM
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 added inline comments.Feb 15 2018, 10:58 AM
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 added inline comments.Feb 15 2018, 11:04 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
3210

I will prepare the needed patch.

  • add test
  • split netbsd specific code to a new function
krytarowski marked 5 inline comments as done.Feb 19 2018, 1:16 PM
vitalybuka accepted this revision.Feb 23 2018, 4:46 PM
vitalybuka added inline comments.
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.

This revision is now accepted and ready to land.Feb 23 2018, 4:46 PM

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 added a comment.EditedFeb 23 2018, 4:58 PM

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.

This revision was automatically updated to reflect the committed changes.