* bug#33044: Invalid read access of chars of wide string in scm_seed_to_random_state @ 2018-10-15 8:44 Tom de Vries 2018-10-15 14:20 ` bug#33044: Reproduced using guile binary Tom de Vries 2018-10-15 18:59 ` bug#33044: Analysis and proposed patch Tom de Vries 0 siblings, 2 replies; 13+ messages in thread From: Tom de Vries @ 2018-10-15 8:44 UTC (permalink / raw) To: 33044 Hi, Consider min.c: ... #include <locale.h> #include "libguile.h" static void * foo (void *data) { return NULL; } int main (void) { const char *msg = setlocale (LC_CTYPE, "ja_JP.sjis"); printf ("msg: %s\n", msg); scm_with_guile (foo, NULL); return 0; } ... Compiled with guile-2.2.4: ... $ gcc min.c -I /home/vries/guile/tarball/guile-2.2.4 -lguile-2.2 -L /home/vries/guile/tarball/guile-2.2.4/libguile/.libs -Wl,-rpath=/home/vries/guile/tarball/guile-2.2.4/libguile/.libs -g ... We run into a segfault: ... $ ./a.out msg: ja_JP.sjis Segmentation fault (core dumped) ... The backtrace as reported by gdb is: ... #0 0x00007ffff7b649ba in scm_variable_ref (var=0x0) at variable.c:92 #1 0x00007ffff7b63868 in scm_throw (key=key@entry=0x7a9580, args=0x7b94c0) at throw.c:266 #2 0x00007ffff7b63e15 in scm_ithrow (key=key@entry=0x7a9580, args=<optimized out>, no_return=no_return@entry=1) at throw.c:611 #3 0x00007ffff7af51a5 in scm_error_scm (key=key@entry=0x7a9580, subr=<optimized out>, message=message@entry=0x7ba8e0, args=args@entry=0x7b9500, data=data@entry=0x4) at error.c:94 #4 0x00007ffff7af525f in scm_error (key=0x7a9580, subr=subr@entry=0x0, message=message@entry=0x7ffff7b93358 "Invalid read access of chars of wide string: ~s", args=0x7b9500, rest=rest@entry=0x4) at error.c:59 #5 0x00007ffff7af5642 in scm_misc_error (subr=subr@entry=0x0, message=message@entry=0x7ffff7b93358 "Invalid read access of chars of wide string: ~s", args=<optimized out>) at error.c:299 #6 0x00007ffff7b5aa9a in scm_i_string_chars (str=<optimized out>, str@entry=0x7ba900) at strings.c:571 #7 0x00007ffff7b3cef8 in scm_seed_to_random_state (seed=0x7ba900) at random.c:444 #8 0x00007ffff7b3ddaa in scm_init_random () at ../libguile/random.x:3 #9 0x00007ffff7b0eb41 in scm_i_init_guile (base=<optimized out>) at init.c:451 #10 0x00007ffff7b62128 in scm_i_init_thread_for_guile (base=0x7fffffffdb10, dynamic_state=0x0) at threads.c:586 #11 0x00007ffff7b62159 in with_guile (base=0x7fffffffdb10, data=0x7fffffffdb40) at threads.c:654 #12 0x00007ffff73a84a5 in GC_call_with_stack_base () from /usr/lib64/libgc.so.1 #13 0x00007ffff7b624a8 in scm_i_with_guile (dynamic_state=<optimized out>, data=<optimized out>, func=<optimized out>) at threads.c:704 #14 scm_with_guile (func=<optimized out>, data=<optimized out>) at threads.c:710 #15 0x0000000000400786 in main () at min.c:15 ... We see that the backtrace happens while handling an "Invalid read access of chars of wide string: ~s" error here: ... const char * scm_i_string_chars (SCM str) { SCM buf; size_t start; get_str_buf_start (&str, &buf, &start); if (scm_i_is_narrow_string (str)) return (const char *) STRINGBUF_CHARS (buf) + start; else scm_misc_error (NULL, "Invalid read access of chars of wide string: ~s", scm_list_1 (str)); return NULL; } ... What triggers the error is that here, we create a non-narrow string using scm_from_locale_string: ... #8 0x00007ffff7b3ddaa in scm_init_random () at ../libguile/random.x:3 3 scm_var_random_state = scm_c_define ("*random-state*", scm_seed_to_random_state (scm_from_locale_string ("URL:http://stat.fsu.edu/~geo/diehard.html")));; ... but then in scm_seed_to_random_state handle it like a narrow string by calling scm_i_string_chars: ... #define FUNC_NAME s_scm_seed_to_random_state { SCM res; if (SCM_NUMBERP (seed)) seed = scm_number_to_string (seed, SCM_UNDEFINED); SCM_VALIDATE_STRING (1, seed); res = make_rstate (scm_c_make_rstate (scm_i_string_chars (seed), scm_i_string_length (seed))); scm_remember_upto_here_1 (seed); return res; } ... Thanks, - Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Reproduced using guile binary 2018-10-15 8:44 bug#33044: Invalid read access of chars of wide string in scm_seed_to_random_state Tom de Vries @ 2018-10-15 14:20 ` Tom de Vries 2018-10-21 16:24 ` Tom de Vries 2018-10-15 18:59 ` bug#33044: Analysis and proposed patch Tom de Vries 1 sibling, 1 reply; 13+ messages in thread From: Tom de Vries @ 2018-10-15 14:20 UTC (permalink / raw) To: 33044 Hi, Using a simple scheme hello world: ... $ cat hello.scm (display "hello world") (newline) ... we're able to reproduce the problem using the guile binary: .... $ LC_CTYPE=ja_JP.sjis /home/vries/guile/2.2/install/bin/guile -s hello.scm Segmentation fault (core dumped) ... [ Note: When using 2.0, we need to set GUILE_INSTALL_LOCALE=1 in the environment, otherwise the 'LC_CTYPE=ja_JP.sjis' setting has no effect. ] Thanks, - Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Reproduced using guile binary 2018-10-15 14:20 ` bug#33044: Reproduced using guile binary Tom de Vries @ 2018-10-21 16:24 ` Tom de Vries 0 siblings, 0 replies; 13+ messages in thread From: Tom de Vries @ 2018-10-21 16:24 UTC (permalink / raw) To: 33044, Mark H Weaver [-- Attachment #1: Type: text/plain, Size: 615 bytes --] On 10/15/18 4:20 PM, Tom de Vries wrote: > Hi, > > Using a simple scheme hello world: > ... > $ cat hello.scm > (display "hello world") > (newline) > ... > we're able to reproduce the problem using the guile binary: > .... > $ LC_CTYPE=ja_JP.sjis /home/vries/guile/2.2/install/bin/guile -s hello.scm > Segmentation fault (core dumped) > ... > > [ Note: When using 2.0, we need to set GUILE_INSTALL_LOCALE=1 in the > environment, otherwise the 'LC_CTYPE=ja_JP.sjis' setting has no effect. ] > I managed to create a testcase for this, patch attached. Tested on master for x86_64, where it fails. Thanks, - Tom [-- Attachment #2: 0001-Add-standalone-test-test-ja_JP.sjis.patch --] [-- Type: text/x-patch, Size: 1212 bytes --] Add standalone test test-ja_JP.sjis Test for <https://bugs.gnu.org/33044>. * test-suite/standalone/test-ja_JP.sjis: New test. * test-suite/standalone/Makefile.am: Add test-ja_JP.sjis. --- test-suite/standalone/Makefile.am | 4 ++++ test-suite/standalone/test-ja_JP.sjis | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/test-suite/standalone/Makefile.am b/test-suite/standalone/Makefile.am index 2aba708da..c5ce4bccb 100644 --- a/test-suite/standalone/Makefile.am +++ b/test-suite/standalone/Makefile.am @@ -183,6 +183,10 @@ TESTS += test-mb-regexp check_SCRIPTS += test-use-srfi TESTS += test-use-srfi +# test-ja_JP.sjis +check_SCRIPTS += test-ja_JP.sjis +TESTS += test-ja_JP.sjis + # test-scm-c-read test_scm_c_read_SOURCES = test-scm-c-read.c test_scm_c_read_CFLAGS = ${test_cflags} diff --git a/test-suite/standalone/test-ja_JP.sjis b/test-suite/standalone/test-ja_JP.sjis new file mode 100755 index 000000000..4b7ba0d88 --- /dev/null +++ b/test-suite/standalone/test-ja_JP.sjis @@ -0,0 +1,8 @@ +#!/bin/sh +# Test whether guile can run initialization code using ja_JP.sjis locale +# (bug #33044). +unset LC_ALL +export LC_CTYPE +LC_CTYPE=ja_JP.sjis +exec guile -q -s "$0" "$@" +!# ^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#33044: Analysis and proposed patch 2018-10-15 8:44 bug#33044: Invalid read access of chars of wide string in scm_seed_to_random_state Tom de Vries 2018-10-15 14:20 ` bug#33044: Reproduced using guile binary Tom de Vries @ 2018-10-15 18:59 ` Tom de Vries 2018-10-16 1:57 ` bug#33044: Guile misbehaves in the "ja_JP.sjis" locale Mark H Weaver 1 sibling, 1 reply; 13+ messages in thread From: Tom de Vries @ 2018-10-15 18:59 UTC (permalink / raw) To: 33044 Hi, I think there are two independent problems here. ------- 1. scm_seed_to_random_state should be able to handle the case that the seed argument is a non-narrow string. 2. The *random-state* variable is documented like this: ... Note that the initial value of *random-state* is the same every time Guile starts up. Therefore, if you don’t pass a state parameter to the above procedures, and you don’t set *random-state* to (seed->random-state your-seed), where your-seed is something that isn’t the same every time, you’ll get the same sequence of “random” numbers on every run. ... However, using scm_from_locale_string to initialize *random-state* makes it possible that *random-state* differs depending on the locale used when starting Guile. So, we should use a string that's independent of the locale settings. ------- The second problem is fixed by using scm_from_latin1_string instead of scm_from_locale_string: ... diff --git a/libguile/random.c b/libguile/random.c index 4051d1f..6f014e1 100644 --- a/libguile/random.c +++ b/libguile/random.c @@ -374,7 +374,7 @@ make_rstate (scm_t_rstate *state) * Scheme level interface. */ -SCM_GLOBAL_VARIABLE_INIT (scm_var_random_state, "*random-state*", scm_seed_to_random_state (scm_from_locale_string ("URL:http://stat.fsu.edu/~geo/diehard.html"))); +SCM_GLOBAL_VARIABLE_INIT (scm_var_random_state, "*random-state*", scm_seed_to_random_state (scm_from_latin1_string ("URL:http://stat.fsu.edu/~geo/diehard.html"))); SCM_DEFINE (scm_random, "random", 1, 1, 0, (SCM n, SCM state), ... Tested on 2.0.14 and 2.2.4 tarballs. Thanks, - Tom ^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-15 18:59 ` bug#33044: Analysis and proposed patch Tom de Vries @ 2018-10-16 1:57 ` Mark H Weaver 2018-10-16 5:13 ` Mark H Weaver 2018-10-16 23:27 ` Tom de Vries 0 siblings, 2 replies; 13+ messages in thread From: Mark H Weaver @ 2018-10-16 1:57 UTC (permalink / raw) To: Tom de Vries; +Cc: 33044 retitle 33044 Guile misbehaves in the "ja_JP.sjis" locale thanks Hi Tom, Thanks for the report, analysis and patch. I agree with your analysis, and the patch looks good. However, there's also a much deeper problem here. You found and fixed one occurrence of Guile assuming that the locale encoding is ASCII- compatible. In fact, this assumption is widespread in Guile, and I would guess that it's widespread throughout the POSIX world. I admit that before I saw your message, I believed that it was legitimate to assume that the locale encoding was ASCII-compatible. Now I'm unsure, although I'll note that according to the 'localedef' utility from GNU libc, this locale is "not ISO C compliant". It printed the following message when I asked it to generate the "ja_JP.sjis" locale: [warning] character map `SHIFT_JIS' is not ASCII compatible, locale not ISO C compliant [--no-warnings=ascii] Shift_JIS is _mostly_ ASCII-compatible, except that code points 0x5C and 0x7E, which represent backslash (\) and tilde (~) in ASCII, are mapped to the Yen sign (¥) and overline (‾) in Shift_JIS. Backslash (\) and tilde (~) are multibyte characters in Shift_JIS. One common problem is that Guile often uses 'scm_from_locale_string' to create Scheme strings from ASCII-only C string literals. These should all be changed to use either 'scm_from_latin1_string' or 'scm_from_utf8_string'. I prefer the latter because modern C compilers typically use UTF-8 as the default execution character set, i.e. the character set used to encode string and character constants, regardless of the locale settings. GCC uses UTF-8 by default unless -fexec-charset=CHARSET is given at compile time. I'd prefer to promote writing code that works for arbitrary string literals, so that code needn't be adjusted if non-ASCII characters are later added. A related set of problems is that Guile often applies 'scm_from_locale_string' to char* arguments passed in from the user, or produced by third-party libraries. These issues are more difficult to address. We provide several C APIs that accept C strings without specifying what encoding is expected. If the string ultimately derives from a C string constant, we probably want UTF-8, whereas if the string came from I/O, or program arguments, then we probably want the locale encoding. For example, consider 'scm_c_eval_string'. This has been a public API function since 2002, but we did not specify the encoding of its C string argument until 2011. We chose the locale encoding in this case, which I think is reasonable, but I also expect that code exists in the wild that passes a C string literal to 'scm_c_eval_string'. Until now, problems like this have been mostly harmless, since the C string literals are typically ASCII-only. However, if we wish to support non-ASCII-compatible encodings such as Shift_JIS, we can no longer consider these problems harmless. For example, programs which pass C string literals to 'scm_c_eval_string' will fail when using the "ja_JP.sjis" locale, if any tildes or backslashes are present. Backslashes are fairly common in Scheme code. There's various other code scattered in Guile that assumes ASCII characters can searched for, and sometimes replaced with other ASCII characters. For example, several functions in load.c, including 'search_path', 'load_thunk_from_path' scan through file names in the locale encoding, scanning the bytes looking for particular ASCII codes such as '.', '/', and '\'. On MingW, 'scm_i_mirror_backslashes' in load.c converts backslashes into forward slashes byte-wise, assuming ASCII-compatibility, and this transformation is applied to file names in several places. While looking into this, I also discovered that Guile's S-expression reader, i.e. the 'read' procedure, assumes an ASCII-compatible port encoding, despite the fact that it is meant to support arbitrary encodings such as UTF-16 and UTF-32. I just filed a related bug <https://bug.gnu.org/33057> to track this probem. These are some of the problems that I'm currently aware of. I expect that this bug report will remain open for a while. To begin, I've started working on a patch to change many occurrences of 'scm_from_locale_string' to 'scm_from_utf8_string', in cases where the C string clearly originates from a C string literal. Thanks again for the detailed bug report and analysis. Regards, Mark ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-16 1:57 ` bug#33044: Guile misbehaves in the "ja_JP.sjis" locale Mark H Weaver @ 2018-10-16 5:13 ` Mark H Weaver 2018-10-16 12:52 ` John Cowan ` (2 more replies) 2018-10-16 23:27 ` Tom de Vries 1 sibling, 3 replies; 13+ messages in thread From: Mark H Weaver @ 2018-10-16 5:13 UTC (permalink / raw) To: Tom de Vries; +Cc: 33044 Mark H Weaver <mhw@netris.org> writes: > Shift_JIS is _mostly_ ASCII-compatible, except that code points 0x5C and > 0x7E, which represent backslash (\) and tilde (~) in ASCII, are mapped > to the Yen sign (¥) and overline (‾) in Shift_JIS. Backslash (\) and > tilde (~) are multibyte characters in Shift_JIS. Although I wrote above that "Backslash (\) and tilde (~) are multibyte characters in Shift_JIS", that was admittedly my assumption, based on the absence of those characters in the "First byte" map shown here: https://en.wikipedia.org/wiki/Shift_JIS#As_defined_in_JIS_X_0208:1997 However, now I'm unsure. I've spent some time attempting to find the Shift_JIS encodings for backslash and tilde, but I've not yet found an answer. I've asked Emacs 26 to write a file containing backslashes and Yen signs using the "shift_jis" encoding, and both characters seem to be mapped to the same code: 0x5C. I've also used the 'iconv' utility from GNU libc to convert backslashes and Yen signs to Shift_JIS, and it also maps these two characters to the same codes: --8<---------------cut here---------------start------------->8--- mhw@jojen ~$ echo '\\¥¥' | iconv -f UTF-8 -t SHIFT-JIS > Shift_JIS_test.txt mhw@jojen ~$ hexdump -C Shift_JIS_test.txt 00000000 5c 5c 5c 5c 0a |\\\\.| 00000005 --8<---------------cut here---------------end--------------->8--- While investigating, I found this bug for GNU libc asking to add an SJIS locale, and the developers were strongly opposed: https://bugzilla.redhat.com/show_bug.cgi?id=136290 At this point, I'm inclined to believe that Shift_JIS is not suitable as a locale encoding on POSIX systems, and that we should not try to support it in Guile. What do you think? Can you tell me how backslash and tilde are represented in Shift JIS? Regards, Mark ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-16 5:13 ` Mark H Weaver @ 2018-10-16 12:52 ` John Cowan 2018-10-16 23:38 ` Tom de Vries 2018-10-17 7:00 ` Tom de Vries 2 siblings, 0 replies; 13+ messages in thread From: John Cowan @ 2018-10-16 12:52 UTC (permalink / raw) To: Mark H Weaver; +Cc: 33044, tdevries [-- Attachment #1: Type: text/plain, Size: 2202 bytes --] At this point, I'm inclined to believe that Shift_JIS is not suitable as > a locale encoding on POSIX systems, and that we should not try to > support it in Guile. > > What do you think? > > Can you tell me how backslash and tilde are represented in Shift JIS? > They aren't: iconv is right. Japanese Windows users are used to seeing Windows pathnames that look like "C:¥foo¥bar", and when writing C, to strings like "first line¥nsecond line." So what is happening is that the character at #\x5C is *functionally* a backslash that is *displayed* as a yen sign. This is reinforced by the fact that the round-trip mapping from Shift_JIS #\x5C is U+005C BACKSLASH, whereas U+00A5 YEN SIGN is mapped only from Unicode (or other encodings) to Shift_JIS, never the other way around. This is the last survivor of the "national characters" concept of ISO 646, whereby certain 7-bit characters were interpreted differently in different countries. For Scandinavian programmers, for example, blocks in C began with æ and ended with å rather than { and } respectively, and the logical OR operator was ø. In the same way, British and Irish programmers used £ instead of # at the beginning of comments in awk and shell programs. With the arrival of Latin-{1,2,3,4} this concept was eventually abandoned, and all systems converged on ISO-646-IRV (the same as US-ASCII) *except* Japanese systems. So I recommend that you do what everyone else does and ignore the issue in JIS-based encodings, of which Shift_JIS is the only one in practical use (and it _is_ heavily used in Japan, where it is almost the only encoding for documents on desktops). Just ignoring the encoding is not an option in Japan: see the comments by Joel Rees, Norman Diamond, and Ryan Thompson at the bug you pointed to. -- John Cowan http://vrici.lojban.org/~cowan cowan@ccil.org In might the Feanorians / that swore the unforgotten oath brought war into Arvernien / with burning and with broken troth. and Elwing from her fastness dim / then cast her in the waters wide, but like a mew was swiftly borne, / uplifted o'er the roaring tide. --the Earendillinwe [-- Attachment #2: Type: text/html, Size: 2813 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-16 5:13 ` Mark H Weaver 2018-10-16 12:52 ` John Cowan @ 2018-10-16 23:38 ` Tom de Vries 2018-10-17 7:00 ` Tom de Vries 2 siblings, 0 replies; 13+ messages in thread From: Tom de Vries @ 2018-10-16 23:38 UTC (permalink / raw) To: Mark H Weaver; +Cc: 33044 On 10/16/18 7:13 AM, Mark H Weaver wrote: > While investigating, I found this bug for GNU libc asking to add an SJIS > locale, and the developers were strongly opposed: > > https://bugzilla.redhat.com/show_bug.cgi?id=136290 > Thanks for the pointer, that was interesting reading. > At this point, I'm inclined to believe that Shift_JIS is not suitable as > a locale encoding on POSIX systems, and that we should not try to > support it in Guile. > > What do you think? My interest here is limited to fixing a gdb regression, so for me not supporting it is fine. Though we should have a reasonable failure mode (so, not abort as is the case for this report). Thanks, - Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-16 5:13 ` Mark H Weaver 2018-10-16 12:52 ` John Cowan 2018-10-16 23:38 ` Tom de Vries @ 2018-10-17 7:00 ` Tom de Vries 2 siblings, 0 replies; 13+ messages in thread From: Tom de Vries @ 2018-10-17 7:00 UTC (permalink / raw) To: Mark H Weaver; +Cc: 33044 On 10/16/18 7:13 AM, Mark H Weaver wrote: > While investigating, I found this bug for GNU libc asking to add an SJIS > locale, and the developers were strongly opposed: > > https://bugzilla.redhat.com/show_bug.cgi?id=136290 FTR, that's a discussion of Fedora/RedHat developers. This OpenSuse/Suse PR ( "wrong "locale -a" output for ja_JP.SHIFT_JISX0213 and hy_AM.armscii-8" , https://bugzilla.opensuse.org/show_bug.cgi?id=162501 ) also discusses this topic and links to related glibc maintainers discussions: - http://www.sourceware.org/ml/libc-locales/2006-q3/msg00054.html - http://www.sourceware.org/ml/libc-alpha/2000-10/msg00311.html Thanks, - Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-16 1:57 ` bug#33044: Guile misbehaves in the "ja_JP.sjis" locale Mark H Weaver 2018-10-16 5:13 ` Mark H Weaver @ 2018-10-16 23:27 ` Tom de Vries 2018-10-18 1:56 ` Mark H Weaver 1 sibling, 1 reply; 13+ messages in thread From: Tom de Vries @ 2018-10-16 23:27 UTC (permalink / raw) To: Mark H Weaver; +Cc: 33044 On 10/16/18 3:57 AM, Mark H Weaver wrote: > retitle 33044 Guile misbehaves in the "ja_JP.sjis" locale > thanks > > Hi Tom, > > Thanks for the report, analysis and patch. I agree with your analysis, > and the patch looks good. > If so, can the patch be committed? I'm running into this problem in the context of gdb, which fails like this: ... $ LC_CTYPE=ja_JP.sjis gdb". Segmentation fault (core dumped) ... So, gdb (which has a dependency on libguile) aborts because of guile initialization, without gdb actually using the guile functionality, and the patch fixes this. > However, there's also a much deeper problem here. You found and fixed > one occurrence of Guile assuming that the locale encoding is ASCII- > compatible. In fact, this assumption is widespread in Guile, and I > would guess that it's widespread throughout the POSIX world. > > I admit that before I saw your message, I believed that it was > legitimate to assume that the locale encoding was ASCII-compatible. Now > I'm unsure, although I'll note that according to the 'localedef' utility > from GNU libc, this locale is "not ISO C compliant". It printed the > following message when I asked it to generate the "ja_JP.sjis" locale: > > [warning] character map `SHIFT_JIS' is not ASCII compatible, locale not ISO C compliant [--no-warnings=ascii] > > Shift_JIS is _mostly_ ASCII-compatible, except that code points 0x5C and > 0x7E, which represent backslash (\) and tilde (~) in ASCII, are mapped > to the Yen sign (¥) and overline (‾) in Shift_JIS. Backslash (\) and > tilde (~) are multibyte characters in Shift_JIS. > > One common problem is that Guile often uses 'scm_from_locale_string' to > create Scheme strings from ASCII-only C string literals. These should > all be changed to use either 'scm_from_latin1_string' or > 'scm_from_utf8_string'. I prefer the latter because modern C compilers > typically use UTF-8 as the default execution character set, i.e. the > character set used to encode string and character constants, regardless > of the locale settings. GCC uses UTF-8 by default unless > -fexec-charset=CHARSET is given at compile time. I'd prefer to promote > writing code that works for arbitrary string literals, so that code > needn't be adjusted if non-ASCII characters are later added. > > A related set of problems is that Guile often applies > 'scm_from_locale_string' to char* arguments passed in from the user, or > produced by third-party libraries. These issues are more difficult to > address. We provide several C APIs that accept C strings without > specifying what encoding is expected. If the string ultimately derives > from a C string constant, we probably want UTF-8, whereas if the string > came from I/O, or program arguments, then we probably want the locale > encoding. > > For example, consider 'scm_c_eval_string'. This has been a public API > function since 2002, but we did not specify the encoding of its C string > argument until 2011. We chose the locale encoding in this case, which I > think is reasonable, but I also expect that code exists in the wild that > passes a C string literal to 'scm_c_eval_string'. > > Until now, problems like this have been mostly harmless, since the C > string literals are typically ASCII-only. However, if we wish to > support non-ASCII-compatible encodings such as Shift_JIS, we can no > longer consider these problems harmless. For example, programs which > pass C string literals to 'scm_c_eval_string' will fail when using the > "ja_JP.sjis" locale, if any tildes or backslashes are present. > Backslashes are fairly common in Scheme code. > > There's various other code scattered in Guile that assumes ASCII > characters can searched for, and sometimes replaced with other ASCII > characters. For example, several functions in load.c, including > 'search_path', 'load_thunk_from_path' scan through file names in the > locale encoding, scanning the bytes looking for particular ASCII codes > such as '.', '/', and '\'. > > On MingW, 'scm_i_mirror_backslashes' in load.c converts backslashes into > forward slashes byte-wise, assuming ASCII-compatibility, and this > transformation is applied to file names in several places. > > While looking into this, I also discovered that Guile's S-expression > reader, i.e. the 'read' procedure, assumes an ASCII-compatible port > encoding, despite the fact that it is meant to support arbitrary > encodings such as UTF-16 and UTF-32. I just filed a related bug > <https://bug.gnu.org/33057> to track this probem. > > These are some of the problems that I'm currently aware of. I expect > that this bug report will remain open for a while. > > To begin, I've started working on a patch to change many occurrences of > 'scm_from_locale_string' to 'scm_from_utf8_string', in cases where the C > string clearly originates from a C string literal. > Thanks for the elaboration here, that's helpful for me. Thanks, - Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-16 23:27 ` Tom de Vries @ 2018-10-18 1:56 ` Mark H Weaver 2018-10-18 10:26 ` Tom de Vries 2018-10-20 2:24 ` Mark H Weaver 0 siblings, 2 replies; 13+ messages in thread From: Mark H Weaver @ 2018-10-18 1:56 UTC (permalink / raw) To: Tom de Vries; +Cc: 33044 Hi Tom, Tom de Vries <tdevries@suse.de> writes: > On 10/16/18 3:57 AM, Mark H Weaver wrote: >> Thanks for the report, analysis and patch. I agree with your analysis, >> and the patch looks good. >> > > If so, can the patch be committed? I just pushed commit c2a654b7d29f5e2f32fd1313cc80162fd0c8f992 to the stable-2.2 branch, which includes the fix from your patch (although I used 'scm_from_utf8_string' instead of 'scm_from_latin1_string'), and many other instances of the same problem. These fixes will be in the upcoming guile-2.2.5 release. Does that address the problem for you? I'll leave this bug open at least until 'seed->random-state your-seed' is fixed to support wide strings. Thanks again, Mark ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-18 1:56 ` Mark H Weaver @ 2018-10-18 10:26 ` Tom de Vries 2018-10-20 2:24 ` Mark H Weaver 1 sibling, 0 replies; 13+ messages in thread From: Tom de Vries @ 2018-10-18 10:26 UTC (permalink / raw) To: Mark H Weaver; +Cc: 33044 On 10/18/18 3:56 AM, Mark H Weaver wrote: > Hi Tom, > > Tom de Vries <tdevries@suse.de> writes: > >> On 10/16/18 3:57 AM, Mark H Weaver wrote: >>> Thanks for the report, analysis and patch. I agree with your analysis, >>> and the patch looks good. >>> >> >> If so, can the patch be committed? > > I just pushed commit c2a654b7d29f5e2f32fd1313cc80162fd0c8f992 to the > stable-2.2 branch, Thanks! > which includes the fix from your patch (although I > used 'scm_from_utf8_string' instead of 'scm_from_latin1_string'), Right, that should give the same result on that string. > and > many other instances of the same problem. These fixes will be in the > upcoming guile-2.2.5 release. > > Does that address the problem for you? > There are two pecularities I'm affected by: - I'm not able to build from git (and I haven't found instructions listing in the README what the correct auto* invocation is). So, tarballs only. - gdb does not support guile 2.2 (so I'd need a backport of this fix to stable-2.0 branch). See also "PR21104 - 7.12.1 does not compile with latest guile (2.1.6)" ( https://sourceware.org/bugzilla/show_bug.cgi?id=21104 ). As for testing, I've done the following: - applied the patch onto the 2.2 tarball, build and ran tests, and ran hello.scm reproducer - ported the patch to the 2.0 tarball, build and ran tests, and ran hello.scm reproducer - build gdb master against the the 2.0 tarball build, ran gdb guile tests, and ran the gdb reproducer. This is as far as I can take it, and all LGTM. Thanks, - Tom > I'll leave this bug open at least until 'seed->random-state your-seed' > is fixed to support wide strings. > > Thanks again, > > Mark > ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#33044: Guile misbehaves in the "ja_JP.sjis" locale 2018-10-18 1:56 ` Mark H Weaver 2018-10-18 10:26 ` Tom de Vries @ 2018-10-20 2:24 ` Mark H Weaver 1 sibling, 0 replies; 13+ messages in thread From: Mark H Weaver @ 2018-10-20 2:24 UTC (permalink / raw) To: Tom de Vries; +Cc: 33044 Mark H Weaver <mhw@netris.org> writes: > I'll leave this bug open at least until 'seed->random-state your-seed' > is fixed to support wide strings. This part is now fixed in commit fbdcf6358519c415bd2041ca09bee9b16e9d528a on the stable-2.2 branch. Mark ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-10-21 16:24 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-15 8:44 bug#33044: Invalid read access of chars of wide string in scm_seed_to_random_state Tom de Vries 2018-10-15 14:20 ` bug#33044: Reproduced using guile binary Tom de Vries 2018-10-21 16:24 ` Tom de Vries 2018-10-15 18:59 ` bug#33044: Analysis and proposed patch Tom de Vries 2018-10-16 1:57 ` bug#33044: Guile misbehaves in the "ja_JP.sjis" locale Mark H Weaver 2018-10-16 5:13 ` Mark H Weaver 2018-10-16 12:52 ` John Cowan 2018-10-16 23:38 ` Tom de Vries 2018-10-17 7:00 ` Tom de Vries 2018-10-16 23:27 ` Tom de Vries 2018-10-18 1:56 ` Mark H Weaver 2018-10-18 10:26 ` Tom de Vries 2018-10-20 2:24 ` Mark H Weaver
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).