Re: crash in newlocale()

Re: crash in newlocale()

[Takashi Yano]

On Thu, 11 Aug 2022 11:29:28 +1000
Tony Cook wrote:
> Hello everyone,
> 
> While tracking down a crash in development versions of perl the boostrap
> miniperl executable was crashing early in the build process:
> 
> $ gdb --args ./miniperl -e0
> GNU gdb (GDB) (Cygwin 11.2-1) 11.2
> Copyright (C) 2022 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "x86_64-pc-cygwin".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <https://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
>     <http://www.gnu.org/software/gdb/documentation/>.
> 
> For help, type "help".
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from ./miniperl...
> (gdb) b locale.c:1154
> Breakpoint 1 at 0x10077db13: file locale.c, line 1154.
> (gdb) r
> Starting program: /home/tony/dev/perl/git/perl/miniperl -e0
> [New Thread 13624.0x1c7c]
> [New Thread 13624.0x1978]
> [New Thread 13624.0x2958]
> [New Thread 13624.0x3374]
> 
> Thread 1 "miniperl" hit Breakpoint 1, S_emulate_setlocale_i (my_perl=0x800049910, index=0, new_locale=0x800066628 "en_US.UTF-8", recalc_LC_ALL=-1, line=4026) at locale.c:1154
> 1154            new_obj = newlocale(mask, new_locale, basis_obj);
> (gdb) bt
> #0  S_emulate_setlocale_i (my_perl=0x800049910, index=0,
>     new_locale=0x800066628 "en_US.UTF-8", recalc_LC_ALL=-1, line=4026)
>     at locale.c:1154
> #1  0x0000000100783849 in Perl_init_i18nl10n (my_perl=0x800049910, printwarn=1)
>     at locale.c:4026
> #2  0x0000000100443c80 in perl_construct (my_perl=0x800049910)
>     at /home/tony/dev/perl/git/perl/perl.c:447
> #3  0x00000001007b7483 in main (argc=2, argv=0xffffcc30, env=0x8000281a0)
>     at miniperlmain.c:108
> (gdb) p mask
> $1 = 4
> (gdb) p new_locale
> $2 = 0x800066628 "en_US.UTF-8"
> (gdb) p basis_obj
> $3 = (locale_t) 0x1802b3060 <__C_locale>
> (gdb) n
> 
> Thread 1 "miniperl" received signal SIGSEGV, Segmentation fault.
> 0x000000080004a310 in ?? ()
> (gdb) bt
> #0  0x000000080004a310 in ?? ()
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> (gdb)
> 
> While I get a SEGV from miniperl, a simple reproducer returns a
> SIGTRAP:
> 
> tony@enceladus ~/dev/perl/git
> $ cat newlocale-test.c
> #include <locale.h>
> #include <stdio.h>
> 
> int main() {
>   locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> 
>   locale_t st2 = newlocale(LC_CTYPE_MASK, "en_US.UTF-8", st);
>   printf("Done\n");
>   return 0;
> }
> tony@enceladus ~/dev/perl/git
> $ gcc -onewlocale-test -g newlocale-test.c
> 
> tony@enceladus ~/dev/perl/git
> $ gdb ./newlocale-test.exe
> GNU gdb (GDB) (Cygwin 11.2-1) 11.2
> Copyright (C) 2022 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "x86_64-pc-cygwin".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <https://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
>     <http://www.gnu.org/software/gdb/documentation/>.
> 
> For help, type "help".
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from ./newlocale-test.exe...
> (gdb) r
> Starting program: /home/tony/dev/perl/git/newlocale-test.exe
> [New Thread 9176.0x3a8c]
> [New Thread 9176.0x2014]
> [New Thread 9176.0x2bc4]
> [Thread 9176.0x2014 exited with code 3221225477]
> [Thread 9176.0x3a8c exited with code 3221225477]
> [Thread 9176.0x2bc4 exited with code 3221225477]
> 
> Program terminated with signal SIGTRAP, Trace/breakpoint trap.
> The program no longer exists.
> (gdb) b main
> Breakpoint 1 at 0x10040108d: file newlocale-test.c, line 5.
> (gdb) r
> Starting program: /home/tony/dev/perl/git/newlocale-test.exe
> [New Thread 13668.0x2a7c]
> [New Thread 13668.0x15a0]
> [New Thread 13668.0x2158]
> 
> Thread 1 "newlocale-test" hit Breakpoint 1, main () at newlocale-test.c:5
> 5         locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> (gdb) n
> 7         locale_t st2 = newlocale(LC_CTYPE_MASK, "en_US.UTF-8", st);
> (gdb)
> [Thread 13668.0x2158 exited with code 3221225477]
> [Thread 13668.0x35d0 exited with code 3221225477]
> [Thread 13668.0x2a7c exited with code 3221225477]
> 
> Program terminated with signal SIGTRAP, Trace/breakpoint trap.
> The program no longer exists.
> (gdb)

I looked into this problem and found the access violation
occurs at:
newlib/libc/locale/newlocale.c
@@ 188,7 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
	  {
	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL; <-- Here!!!
	  }
#endif /* __HAVE_LOCALE_INFO__ */
      _freelocale_r (p, base);

This is because
locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
returns
  extern const struct __locale_t __C_locale;
  return (struct __locale_t *) &__C_locale;
, which is in the const area that cannot be modified.

This seems to be a newlib bug.

I also found following patch solves the issue.

diff --git a/newlib/libc/locale/newlocale.c b/newlib/libc/locale/newlocale.c
index 0789d5fd9..1974665ec 100644
--- a/newlib/libc/locale/newlocale.c
+++ b/newlib/libc/locale/newlocale.c
@@ -108,10 +108,12 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
     }
   /* If the new locale is supposed to be all default locale, just return
      a pointer to the default locale. */
+#if 0
   if ((!base && category_mask == 0)
       || (category_mask == LC_VALID_MASK
 	  && (!strcmp (locale, "C") || !strcmp (locale, "POSIX"))))
     return __get_C_locale ();
+#endif
   /* Start with setting all values to the default locale values. */
   tmp_locale = *__get_C_locale ();
   /* Fill out new category strings. */

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: crash in newlocale()

[Ken Brown]

On 8/11/2022 8:18 AM, Takashi Yano wrote:
> On Thu, 11 Aug 2022 11:29:28 +1000
> Tony Cook wrote:
>> $ cat newlocale-test.c
>> #include <locale.h>
>> #include <stdio.h>
>>
>> int main() {
>>    locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
>>
>>    locale_t st2 = newlocale(LC_CTYPE_MASK, "en_US.UTF-8", st);
>>    printf("Done\n");
>>    return 0;
>> }
>> Program terminated with signal SIGTRAP, Trace/breakpoint trap.
>> The program no longer exists.
>> (gdb)
> 
> I looked into this problem and found the access violation
> occurs at:
> newlib/libc/locale/newlocale.c
> @@ 188,7 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> 	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
> 	  {
> 	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
> 	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL; <-- Here!!!
> 	  }
> #endif /* __HAVE_LOCALE_INFO__ */
>        _freelocale_r (p, base);
> 
> This is because
> locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> returns
>    extern const struct __locale_t __C_locale;
>    return (struct __locale_t *) &__C_locale;
> , which is in the const area that cannot be modified.
> 
> This seems to be a newlib bug.
> 
> I also found following patch solves the issue.
> 
> diff --git a/newlib/libc/locale/newlocale.c b/newlib/libc/locale/newlocale.c
> index 0789d5fd9..1974665ec 100644
> --- a/newlib/libc/locale/newlocale.c
> +++ b/newlib/libc/locale/newlocale.c
> @@ -108,10 +108,12 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
>       }
>     /* If the new locale is supposed to be all default locale, just return
>        a pointer to the default locale. */
> +#if 0
>     if ((!base && category_mask == 0)
>         || (category_mask == LC_VALID_MASK
>   	  && (!strcmp (locale, "C") || !strcmp (locale, "POSIX"))))
>       return __get_C_locale ();
> +#endif
>     /* Start with setting all values to the default locale values. */
>     tmp_locale = *__get_C_locale ();
>     /* Fill out new category strings. */

Good work tracking this down!

I was about to suggest that instead of just disabling this optimization, you 
call _duplocale_r to return a duplicate of __get_C_locale ().  But then I saw 
the following in the definition of _duplocale_r in duplocale.c:

   /* The "C" locale is used statically, never copied. */
   else if (locobj == __get_C_locale ())
     return __get_C_locale ();

This looks like another bug.  Shouldn't duplocale always return a modifiable object?

Ken

Re: crash in newlocale()

[Corinna Vinschen]

On Aug 11 21:18, Takashi Yano wrote:
> On Thu, 11 Aug 2022 11:29:28 +1000
> Tony Cook wrote:
> > Hello everyone,
> > 
> > While tracking down a crash in development versions of perl the boostrap
> > miniperl executable was crashing early in the build process:
> > [...]
> 
> I looked into this problem and found the access violation
> occurs at:
> newlib/libc/locale/newlocale.c
> @@ 188,7 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> 	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
> 	  {
> 	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
> 	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL; <-- Here!!!
> 	  }
> #endif /* __HAVE_LOCALE_INFO__ */
>       _freelocale_r (p, base);
> 
> This is because
> locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> returns
>   extern const struct __locale_t __C_locale;
>   return (struct __locale_t *) &__C_locale;
> , which is in the const area that cannot be modified.
> 
> This seems to be a newlib bug.
> 
> I also found following patch solves the issue.
> 
> diff --git a/newlib/libc/locale/newlocale.c b/newlib/libc/locale/newlocale.c
> index 0789d5fd9..1974665ec 100644
> --- a/newlib/libc/locale/newlocale.c
> +++ b/newlib/libc/locale/newlocale.c
> @@ -108,10 +108,12 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
>      }
>    /* If the new locale is supposed to be all default locale, just return
>       a pointer to the default locale. */
> +#if 0
>    if ((!base && category_mask == 0)
>        || (category_mask == LC_VALID_MASK
>  	  && (!strcmp (locale, "C") || !strcmp (locale, "POSIX"))))
>      return __get_C_locale ();
> +#endif
>    /* Start with setting all values to the default locale values. */
>    tmp_locale = *__get_C_locale ();
>    /* Fill out new category strings. */

I think the right thing to do is to avoid freeing this data, if the base
locale is the C locale.  I sent a patch to cygwin-patches for review.
Can you please take a look?


Thanks,
Corinna

Re: crash in newlocale()

[Corinna Vinschen]

On Aug 11 19:35, Corinna Vinschen wrote:
> On Aug 11 21:18, Takashi Yano wrote:
> > On Thu, 11 Aug 2022 11:29:28 +1000
> > Tony Cook wrote:
> > > Hello everyone,
> > > 
> > > While tracking down a crash in development versions of perl the boostrap
> > > miniperl executable was crashing early in the build process:
> > > [...]
> > 
> > I looked into this problem and found the access violation
> > occurs at:
> > newlib/libc/locale/newlocale.c
> > @@ 188,7 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> > 	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
> > 	  {
> > 	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
> > 	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL; <-- Here!!!
> > 	  }
> > #endif /* __HAVE_LOCALE_INFO__ */
> >       _freelocale_r (p, base);
> > 
> > This is because
> > locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> > returns
> >   extern const struct __locale_t __C_locale;
> >   return (struct __locale_t *) &__C_locale;
> > , which is in the const area that cannot be modified.
> > 
> > This seems to be a newlib bug.
> > 
> > I also found following patch solves the issue.
> > 
> > diff --git a/newlib/libc/locale/newlocale.c b/newlib/libc/locale/newlocale.c
> > index 0789d5fd9..1974665ec 100644
> > --- a/newlib/libc/locale/newlocale.c
> > +++ b/newlib/libc/locale/newlocale.c
> > @@ -108,10 +108,12 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> >      }
> >    /* If the new locale is supposed to be all default locale, just return
> >       a pointer to the default locale. */
> > +#if 0
> >    if ((!base && category_mask == 0)
> >        || (category_mask == LC_VALID_MASK
> >  	  && (!strcmp (locale, "C") || !strcmp (locale, "POSIX"))))
> >      return __get_C_locale ();
> > +#endif
> >    /* Start with setting all values to the default locale values. */
> >    tmp_locale = *__get_C_locale ();
> >    /* Fill out new category strings. */
> 
> I think the right thing to do is to avoid freeing this data, if the base
> locale is the C locale.  I sent a patch to cygwin-patches for review.
> Can you please take a look?

Resent to the newlib list where it belongs, d'oh.


Corinna

Re: crash in newlocale()

[Takashi Yano]

On Thu, 11 Aug 2022 19:35:39 +0200
Corinna Vinschen wrote:
> On Aug 11 21:18, Takashi Yano wrote:
> > On Thu, 11 Aug 2022 11:29:28 +1000
> > Tony Cook wrote:
> > > Hello everyone,
> > > 
> > > While tracking down a crash in development versions of perl the boostrap
> > > miniperl executable was crashing early in the build process:
> > > [...]
> > 
> > I looked into this problem and found the access violation
> > occurs at:
> > newlib/libc/locale/newlocale.c
> > @@ 188,7 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> > 	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
> > 	  {
> > 	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
> > 	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL; <-- Here!!!
> > 	  }
> > #endif /* __HAVE_LOCALE_INFO__ */
> >       _freelocale_r (p, base);
> > 
> > This is because
> > locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> > returns
> >   extern const struct __locale_t __C_locale;
> >   return (struct __locale_t *) &__C_locale;
> > , which is in the const area that cannot be modified.
> > 
> > This seems to be a newlib bug.
> > 
> > I also found following patch solves the issue.
> > 
> > diff --git a/newlib/libc/locale/newlocale.c b/newlib/libc/locale/newlocale.c
> > index 0789d5fd9..1974665ec 100644
> > --- a/newlib/libc/locale/newlocale.c
> > +++ b/newlib/libc/locale/newlocale.c
> > @@ -108,10 +108,12 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> >      }
> >    /* If the new locale is supposed to be all default locale, just return
> >       a pointer to the default locale. */
> > +#if 0
> >    if ((!base && category_mask == 0)
> >        || (category_mask == LC_VALID_MASK
> >  	  && (!strcmp (locale, "C") || !strcmp (locale, "POSIX"))))
> >      return __get_C_locale ();
> > +#endif
> >    /* Start with setting all values to the default locale values. */
> >    tmp_locale = *__get_C_locale ();
> >    /* Fill out new category strings. */
> 
> I think the right thing to do is to avoid freeing this data, if the base
> locale is the C locale.  I sent a patch to cygwin-patches for review.
> Can you please take a look?

LGTM. Thanks!

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: crash in newlocale()

[Takashi Yano]

On Thu, 11 Aug 2022 12:29:31 -0400
Ken Brown wrote:
> On 8/11/2022 8:18 AM, Takashi Yano wrote:
> > On Thu, 11 Aug 2022 11:29:28 +1000
> > Tony Cook wrote:
> >> $ cat newlocale-test.c
> >> #include <locale.h>
> >> #include <stdio.h>
> >>
> >> int main() {
> >>    locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> >>
> >>    locale_t st2 = newlocale(LC_CTYPE_MASK, "en_US.UTF-8", st);
> >>    printf("Done\n");
> >>    return 0;
> >> }
> >> Program terminated with signal SIGTRAP, Trace/breakpoint trap.
> >> The program no longer exists.
> >> (gdb)
> > 
> > I looked into this problem and found the access violation
> > occurs at:
> > newlib/libc/locale/newlocale.c
> > @@ 188,7 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> > 	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
> > 	  {
> > 	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
> > 	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL; <-- Here!!!
> > 	  }
> > #endif /* __HAVE_LOCALE_INFO__ */
> >        _freelocale_r (p, base);
> > 
> > This is because
> > locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> > returns
> >    extern const struct __locale_t __C_locale;
> >    return (struct __locale_t *) &__C_locale;
> > , which is in the const area that cannot be modified.
> > 
> > This seems to be a newlib bug.
> > 
> > I also found following patch solves the issue.
> > 
> > diff --git a/newlib/libc/locale/newlocale.c b/newlib/libc/locale/newlocale.c
> > index 0789d5fd9..1974665ec 100644
> > --- a/newlib/libc/locale/newlocale.c
> > +++ b/newlib/libc/locale/newlocale.c
> > @@ -108,10 +108,12 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> >       }
> >     /* If the new locale is supposed to be all default locale, just return
> >        a pointer to the default locale. */
> > +#if 0
> >     if ((!base && category_mask == 0)
> >         || (category_mask == LC_VALID_MASK
> >   	  && (!strcmp (locale, "C") || !strcmp (locale, "POSIX"))))
> >       return __get_C_locale ();
> > +#endif
> >     /* Start with setting all values to the default locale values. */
> >     tmp_locale = *__get_C_locale ();
> >     /* Fill out new category strings. */
> 
> Good work tracking this down!
> 
> I was about to suggest that instead of just disabling this optimization, you 
> call _duplocale_r to return a duplicate of __get_C_locale ().  But then I saw 
> the following in the definition of _duplocale_r in duplocale.c:
> 
>    /* The "C" locale is used statically, never copied. */
>    else if (locobj == __get_C_locale ())
>      return __get_C_locale ();
> 
> This looks like another bug.  Shouldn't duplocale always return a modifiable object?

Thanks!
Corinna's patch avoids the problem also for this case.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: crash in newlocale()

[Corinna Vinschen]

On Aug 12 08:38, Takashi Yano wrote:
> On Thu, 11 Aug 2022 19:35:39 +0200
> Corinna Vinschen wrote:
> > On Aug 11 21:18, Takashi Yano wrote:
> > > On Thu, 11 Aug 2022 11:29:28 +1000
> > > Tony Cook wrote:
> > > > Hello everyone,
> > > > 
> > > > While tracking down a crash in development versions of perl the boostrap
> > > > miniperl executable was crashing early in the build process:
> > > > [...]
> > > 
> > > I looked into this problem and found the access violation
> > > occurs at:
> > > newlib/libc/locale/newlocale.c
> > > @@ 188,7 @@ _newlocale_r (struct _reent *p, int category_mask, const char *locale,
> > > 	if (tmp_locale.lc_cat[i].buf == (const void *) -1)
> > > 	  {
> > > 	    tmp_locale.lc_cat[i].buf = base->lc_cat[i].buf;
> > > 	    base->lc_cat[i].ptr = base->lc_cat[i].buf = NULL; <-- Here!!!
> > > 	  }
> > > #endif /* __HAVE_LOCALE_INFO__ */
> > >       _freelocale_r (p, base);
> > > 
> > > This is because
> > > locale_t st = newlocale(LC_ALL_MASK, "C", (locale_t)0);
> > > returns
> > >   extern const struct __locale_t __C_locale;
> > >   return (struct __locale_t *) &__C_locale;
> > > , which is in the const area that cannot be modified.
> > > [...]
> > I think the right thing to do is to avoid freeing this data, if the base
> > locale is the C locale.  I sent a patch to cygwin-patches for review.
> > Can you please take a look?
> 
> LGTM. Thanks!

Thanks for checking!  I pushed the patch with addition release msg
to the 3.3 branch as well.


Corinna