Bug 211743 - [PATCH] newlocale() and/or uselocale() not working properly
Summary: [PATCH] newlocale() and/or uselocale() not working properly
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.3-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Andrey A. Chernov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-10 23:52 UTC by Karl Williamson
Modified: 2018-09-22 10:00 UTC (History)
4 users (show)

See Also:
ache: mfc-stable11+
ache: mfc-stable10+


Attachments
Reproduce newlocale/uselocale problem (391 bytes, text/x-csrc)
2016-08-10 23:52 UTC, Karl Williamson
no flags Details
Make catopen() understand threaded locale (997 bytes, patch)
2016-08-23 17:04 UTC, Andrey A. Chernov
no flags Details | Diff
Make catopen() understand threaded locale (1.20 KB, patch)
2016-08-23 17:45 UTC, Andrey A. Chernov
no flags Details | Diff
10.3 Screen shots of the output of localetest.c (33.95 KB, image/png)
2018-02-13 03:07 UTC, Karl Williamson
no flags Details
11.0 output of localetest.c (30.24 KB, image/png)
2018-02-13 03:13 UTC, Karl Williamson
no flags Details
localetest.c output when not using newlocale/uselocale (26.07 KB, image/png)
2018-02-13 03:15 UTC, Karl Williamson
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Karl Williamson 2016-08-10 23:52:43 UTC
Created attachment 173530 [details]
Reproduce newlocale/uselocale problem

The attached .c file calls newlocale()  to create a locale_t object for all locale categories using the C locale.  setlocale() is then called to switch the LC_MESSAGES locale to one that we've determined reproduces the problem.  Then uselocale() is called on the C locale object.  This should switch all categories, including LC_MESSAGES, to the C locale.  That should cause strerror() to print its message in ASCII.  Instead it prints using the setlocale() locale.

Not shown in this demo program, no errors are raised.

This code works properly on Linux and Darwin.

I'm extending Perl5 to use the newer locale operations which promise thread safety.  Our current work around is to just pretend that these operations don't exist in freebsd, and use the old setlocale() operation with mutexes.
Comment 1 James E Keenan 2016-08-11 13:28:30 UTC
Being tracked by Perl 5 Porters at:
https://rt.perl.org/Ticket/Display.html?id=128867
Comment 2 Andrey A. Chernov freebsd_committer freebsd_triage 2016-08-23 16:26:27 UTC
Either your example or our strerror() implementation is incorrect, I can't say for sure due to quick looking. strerror() calls catopen() which overrides your uselocale() with global context using
lang = setlocale(LC_MESSAGES, NULL);
So, it seems fixing perl for that is incorrect too.
Comment 3 Andrey A. Chernov freebsd_committer freebsd_triage 2016-08-23 17:04:29 UTC
Created attachment 173973 [details]
Make catopen() understand threaded locale

Quick fix minimally tested. Needs review.
Comment 4 Andrey A. Chernov freebsd_committer freebsd_triage 2016-08-23 17:45:46 UTC
Created attachment 173974 [details]
Make catopen() understand threaded locale
Comment 5 James E Keenan 2016-08-23 19:08:54 UTC
Should some tests be added to the FreeBSD test suite to confirm the validity of the patch?

Thank you very much.
Jim Keenan
Comment 6 Andrey A. Chernov freebsd_committer freebsd_triage 2016-08-23 20:51:53 UTC
(In reply to James E Keenan from comment #5)
The patch fix this test case, but experimenting more in this area for some time I found another bug, now really in the newlocale() (and querylocale()). Fix just
committed:
https://svnweb.freebsd.org/base?view=revision&revision=304703
I'll take this bug and inform you when the things will be ready.
Comment 7 Andrey A. Chernov freebsd_committer freebsd_triage 2016-08-24 18:31:21 UTC
Additional commit to make test example finally working:
https://svnweb.freebsd.org/base?view=revision&revision=304755
Comment 8 commit-hook freebsd_committer freebsd_triage 2016-08-26 21:20:12 UTC
A commit references this bug:

Author: ache
Date: Fri Aug 26 21:19:24 UTC 2016
New revision: 304862
URL: https://svnweb.freebsd.org/changeset/base/304862

Log:
  MFC r304703, r304755

  1) _locale.h
  LC_*_MASK bit shifting order was partially broken from the initial commit
  time at year 2012. Only LC_COLLATE_MASK and LC_CTYPE_MASK are in the
  right order.

  The order here should match XLC_* from "xlocale_private.h" which, in turn,
  match LC_* publicly visible order from <locale.h> which determines how
  locale components are stored in the structure.
  LC_*_MASK -> XLC_* translation done as "ffs(mask) - 1" in the querylocale()
  and equivalent shift loop in the newlocale(), so mapped to some wrong
  components (excluding two mentioned above).

  Formally the fix is ABI breakage, but old code using those masks
  never works properly in any case.
  Only newlocale() and querylocale() are affected.

  2) msgcat.c
  Use current locale (f.e. set by thread). It was global locale always
  previously.

  PR:     211743

Changes:
_U  stable/10/
  stable/10/include/xlocale/_locale.h
  stable/10/lib/libc/nls/msgcat.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2016-08-26 21:24:15 UTC
A commit references this bug:

Author: ache
Date: Fri Aug 26 21:23:38 UTC 2016
New revision: 304863
URL: https://svnweb.freebsd.org/changeset/base/304863

Log:
  MFC r304703, r304755

  1) _locale.h
  LC_*_MASK bit shifting order was partially broken from the initial commit
  time at year 2012. Only LC_COLLATE_MASK and LC_CTYPE_MASK are in the
  right order.

  The order here should match XLC_* from "xlocale_private.h" which, in turn,
  match LC_* publicly visible order from <locale.h> which determines how
  locale components are stored in the structure.
  LC_*_MASK -> XLC_* translation done as "ffs(mask) - 1" in the querylocale()
  and equivalent shift loop in the newlocale(), so mapped to some wrong
  components (excluding two mentioned above).

  Formally the fix is ABI breakage, but old code using those masks
  never works properly in any case.
  Only newlocale() and querylocale() are affected.

  2) msgcat.c
  Use current locale (f.e. set by thread). It was global locale always
  previously.

  PR:     211743

Changes:
_U  stable/11/
  stable/11/include/xlocale/_locale.h
  stable/11/lib/libc/nls/msgcat.c
Comment 10 Andrey A. Chernov freebsd_committer freebsd_triage 2016-08-26 21:38:19 UTC
Things fixed at least in the scope of this test example provided.
Based on __FreeBSD_version define the fix is in:
1200004 and up
1100502 >= version < 1200000
1003507 >= version < 1100000
Comment 11 Karl Williamson 2018-02-13 03:07:39 UTC
Created attachment 190563 [details]
10.3 Screen shots of the output of localetest.c
Comment 12 Karl Williamson 2018-02-13 03:08:51 UTC
This does not appear to be fixed except in releases around 1200056
Comment 13 Karl Williamson 2018-02-13 03:13:20 UTC
Created attachment 190564 [details]
11.0 output of localetest.c
Comment 14 Karl Williamson 2018-02-13 03:15:31 UTC
Created attachment 190565 [details]
localetest.c output when not using newlocale/uselocale

These screen shots are courtesy James E Keenan
Comment 15 Yuri Pankov 2018-07-13 22:10:31 UTC
(In reply to Karl Williamson from comment #11)
The versions which have the fix are provided in comment #10:
10.x: 1003507, which comes *after* releng/10.3 was branched, try 10.4
11.x: 1100502, which comes *after* releng/11.0 was branched, try 11.1
Comment 16 Kubilay Kocak freebsd_committer freebsd_triage 2018-09-22 10:00:16 UTC
Resolved (committed/merged) per comment 10 and comment 15