-
Notifications
You must be signed in to change notification settings - Fork 558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cygwin: setlocale(LC_ALL, "ja_JP.utf8") panic: strxfrm() gets absurd #13768
Comments
From @rurbanSubject: cygwin: setlocale(LC_ALL, "ja_JP.utf8") panic: strxfrm() gets absurd This is a bug report for perl from rurban@x-ray.at, cygwin 5.18.2 $ ./perl -MPOSIX -e'setlocale(LC_ALL, "ja_JP.utf8")' all other locales before ja_JP.utf8 in lib/locale.t do work fine. Flags: This perlbug was built using Perl 5.14.2 - Thu Jul 12 13:58:56 CDT 2012 Site configuration information for perl 5.14.2: Configured by rurban at Mon Sep 17 12:58:11 CDT 2012. Summary of my perl5 (revision 5 version 14 subversion 2) configuration: Locally applied patches: @INC for perl 5.14.2: Environment for perl 5.14.2: |
From @khwilliamsonOn 04/25/2014 01:33 PM, rurban@cpanel.net (via RT) wrote:
It works on my Linux; I notice that there is no -I parameter in your |
The RT System itself - Status changed from 'new' to 'open' |
From @rurbanOn 04/25/2014 03:02 PM, karl williamson via RT wrote:
yes, sorry. This fails only on cygwin, only tested on 32bit. I needed to this patch to pass the locale tests: Inline Patch--- perl-5.18.2/lib/locale.t~ 2014-01-06 16:46:45.000000000 -0600
+++ perl-5.18.2/lib/locale.t 2014-04-25 14:28:31.455279000 -0500
@@ -528,7 +528,9 @@
sub trylocale {
my $locale = shift;
return if grep { $locale eq $_ } @Locale;
- return unless setlocale(LC_ALL, $locale);
+ # warn "# setlocale(LC_ALL, $locale)\n";
+ return if ($^O eq 'cygwin' and $locale =~
-- |
From @khwilliamsonOn 04/25/2014 02:10 PM, Reini Urban wrote:
I had a look at the code. It's from these two lines in locale.c: That '50' looks like a guess. You could try bumping it up until |
From @jhiOn Wednesday-201406-04, 20:30, Karl Williamson wrote:
Yes, a guess, by yours truly, from the time when mammoths and I've got a better idea than futzing with the futz factor. Just a minute. |
From @jhiOn Wednesday-201406-04, 21:22, Jarkko Hietaniemi wrote:
So try the attached patch. Instead of baking in a futz factor (which NOTE 1: if the expansion factor of 50 really is not enough, this means NOTE 2: now that I think of it, if the strxfrm() count-only feature |
From @jhiInline Patchdiff --git a/locale.c b/locale.c
index 8d49fac..a2e8374 100644
--- a/locale.c
+++ b/locale.c
@@ -313,18 +313,16 @@ Perl_new_collate(pTHX_ const char *newcoll)
|| strEQ(newcoll, "POSIX"));
{
- /* 2: at most so many chars ('a', 'b'). */
- /* 50: surely no system expands a char more. */
-#define XFRMBUFSIZE (2 * 50)
- char xbuf[XFRMBUFSIZE];
- const Size_t fa = strxfrm(xbuf, "a", XFRMBUFSIZE);
- const Size_t fb = strxfrm(xbuf, "ab", XFRMBUFSIZE);
- const SSize_t mult = fb - fa;
- if (mult < 1 && !(fa == 0 && fb == 0))
- Perl_croak(aTHX_ "panic: strxfrm() gets absurd - a => %"UVuf", ab => %"UVuf,
- (UV) fa, (UV) fb);
- PL_collxfrm_base = (fa > (Size_t)mult) ? (fa - mult) : 0;
- PL_collxfrm_mult = mult;
+ const char a[] = "A";
+ const char b[] = "ABCDEFGHIJ abcdefghij 0123456789";
+ const Size_t fa = strxfrm(NULL, a, 0);
+ const Size_t fb = strxfrm(NULL, b, 0);
+ if (fa == 0 || fb == 0 || fa >= fb) {
+ Perl_croak(aTHX_ "panic: strxfrm() gets absurd: "
+ "%"UVuf" cf %"UVuf, (UV)fa, (UV)fb);
+ }
+ PL_collxfrm_mult = (double)fb / sizeof(b) + 1;
+ PL_collxfrm_base = fa > PL_collxfrm_mult ? fa : PL_collxfrm_mult;
}
}
|
From @tonycozOn Fri Apr 25 12:33:27 2014, rurban@cpanel.net wrote:
I don't see this panic with current cygwin: tony@saturn ~/dev/perl/git/perl tony@saturn ~/dev/perl/git/perl Tested with blead and 5.18.2 (with that minimal fix to cygwin/cygwin.c). I do however see a bug in cygwin's strxfrm() implementation, here's the code from src/winsup/cygwin/nlsfuncs.cc: extern "C" size_t if (!collate_lcid) when the key string received doesn't fit in the buffer, LCMapStringW() returns 0 and sets GetLastError() to ERROR_INSUFFICIENT_BUFFER, at which point strxfrm() returns the size of the output buffer - *not* the size of the sort key as required by ANSI C. We also have a couple of bugs in POSIX::strxfrm(), but that's not used by the code that produces the panic above. Tony |
From @tonycozOn Wed Jun 04 23:58:26 2014, tonyc wrote:
...
Reported to cygwin at https://cygwin.com/ml/cygwin/2016-04/msg00232.html Tony |
From @tonycozOn Mon Apr 11 22:10:38 2016, tonyc wrote:
Fixed in nightlies: https://cygwin.com/ml/cygwin/2016-04/msg00247.html https://cygwin.com/ml/cygwin/2016-04/msg00279.html Again, this doesn't seem to be the cause of the problem Reini originally reported (which I haven't reprotduced.) Tony |
From @khwilliamsonOn Wed Jun 04 23:58:26 2014, tonyc wrote:
I'm pretty sure I understand how the panic happens with this cygwin bug, and have a work-around smoking for 5.25. It seems to me though, that if a platform's strxfrm() is more badly broken that we shouldn't panic, but just not do locale-based collation for that locale, reverting to code point order, as we do for non-locale collation. Avoiding a panic is generally considered a good thing. My question is, should we raise a warning instead? |
From @khwilliamsonFixed by 79f120c |
@khwilliamson - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#121734 (status was 'resolved')
Searchable as RT121734$
The text was updated successfully, but these errors were encountered: