14220.cc fails due stack growth:

-bash-4.3$ ./14220.exe

Pid 28500 received a SIGSEGV for stack growth failure.
Possible causes: insufficient memory or swap space,
or stack size exceeded maxssiz. 
Segmentation fault

-bash-4.3$ ulimit -s 
16384

Increasing requires kernel rebuild...

Still present in gcc 9.

Could this be due to using alloca? The tests should not require a huge stack, so either alloca isn't usable or maybe there's a bug causing an infinite recursion.

On 2018-09-03 10:57 AM, redi at gcc dot gnu.org wrote:
> Could this be due to using alloca? The tests should not require a huge stack,
> so either alloca isn't usable or maybe there's a bug causing an infinite
> recursion.
(gdb) r
Starting program: 
/mnt/gnu/gcc/objdir-test/hppa64-hp-hpux11.11/libstdc++-v3/testsuite/14220.exe
warning: Private mapping of shared library text was not specified
by the executable; setting a breakpoint in a shared library which
is not privately mapped will not work.  See the HP-UX 11i v3 chatr
manpage for methods to privately map shared library text.

Pid 2143 received a SIGSEGV for stack growth failure.
Possible causes: insufficient memory or swap space,
or stack size exceeded maxssiz.

Program received signal SIGSEGV, Segmentation fault.
0xc000000000270de8 in memmove () from /lib/pa20_64/libc.2
(gdb) bt
#0  0xc000000000270de8 in memmove () from /lib/pa20_64/libc.2
#1  0xc0000000005bba38 in std::ctype<char>::do_widen (
     __to=0x800003fffdff0dd0 <crc_table+120> "1.", '0' <repeats 42 times>,
     __hi=0x800003fffdff0d9f <crc_table+71> "",
     __lo=0x800003fffdff0da0 <crc_table+72> "1.", '0' <repeats 42 times>,
     this=<optimized out>)
     at 
/mnt/gnu/gcc/objdir/hppa64-hp-hpux11.11/libstdc++-v3/include/bits/locale_facets.h:1107
#2  std::ctype<char>::widen (
     __to=0x800003fffdff0dd0 <crc_table+120> "1.", '0' <repeats 42 times>,
     __hi=0x800003fffdff0d9f <crc_table+71> "",
     __lo=0x800003fffdff0da0 <crc_table+72> "1.", '0' <repeats 42 times>,
     this=<optimized out>)
     at 
/mnt/gnu/gcc/objdir/hppa64-hp-hpux11.11/libstdc++-v3/include/bits/locale_facets.h:908
#3  std::num_put<char, std::ostreambuf_iterator<char, 
std::char_traits<char> > >::_M_insert_float<double> 
(this=0x800003fffdfe57a0, __s=..., __io=...,
     __fill=43 '+', __mod=<optimized out>, __v=1)
     at 
/mnt/gnu/gcc/objdir/hppa64-hp-hpux11.11/libstdc++-v3/include/bits/locale_facets.tcc:1048
#4  0xc0000000005bbe68 in std::num_put<char, 
std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put 
(this=<optimized out>,
     __s=<error reading variable: Cannot access memory at address 0x10>,
---Type <return> to continue, or q <return> to quit---
     __io=..., __fill=<optimized out>, __v=<optimized out>)
     at 
/mnt/gnu/gcc/objdir/hppa64-hp-hpux11.11/libstdc++-v3/include/bits/locale_facets.tcc:1157
#5  0x4000000000003794 in std::num_put<char, 
std::ostreambuf_iterator<char, std::char_traits<char> > >::put (__v=1, 
__fill=43 '+', __io=..., __s=...,
     this=0x800003fffdfe57a0)
     at 
/mnt/gnu/gcc/gcc/libstdc++-v3/testsuite/22_locale/num_put/put/char/14220.cc:38
#6  test01 ()
     at 
/mnt/gnu/gcc/gcc/libstdc++-v3/testsuite/22_locale/num_put/put/char/14220.cc:38
#7  0x4000000000003a74 in main ()
     at 
/mnt/gnu/gcc/gcc/libstdc++-v3/testsuite/22_locale/num_put/put/char/14220.cc:45

On the failing call to memmove, we have the following arguments:

(gdb) p/x $r26
$1 = 0x800003fffdff0dd0
(gdb) p/x $r25
$2 = 0x800003fffdff0da0
(gdb) p/x $r24
$3 = 0xffffffffffffffff

(gdb) frame 1
#1  0x800003fffdfa0a38 in std::ctype<char>::do_widen (
     __to=0x800003fffdff0dd0 <crc_table+120> "\200",
     __hi=0x800003fffdff0d9f <crc_table+71> "",
     __lo=0x800003fffdff0da0 <crc_table+72> "1.", '0' <repeats 42 times>,
     this=<optimized out>)
     at 
/mnt/gnu/gcc/objdir/hppa64-hp-hpux11.11/libstdc++-v3/include/bits/locale_facets.h:1107
1107            __builtin_memcpy(__to, __lo, __hi - __lo);
(gdb) p __hi
$4 = 0x800003fffdff0d9f <crc_table+71> ""
(gdb) p __lo
$5 = 0x800003fffdff0da0 <crc_table+72> "1.", '0' <repeats 42 times>
(gdb) p/x __hi - __lo
$6 = 0xffffffffffffffff

On 2018-09-03 12:03 PM, dave.anglin at bell dot net wrote:
> (gdb) p/x __hi - __lo
> $6 = 0xffffffffffffffff
(gdb) frame 3
#3  std::num_put<char, std::ostreambuf_iterator<char, 
std::char_traits<char> > >::_M_insert_float<double> 
(this=0x800003fffdde37a0, __s=..., __io=...,
     __fill=43 '+', __mod=<optimized out>, __v=1)
     at 
/mnt/gnu/gcc/objdir/hppa64-hp-hpux11.11/libstdc++-v3/include/bits/locale_facets.tcc:1048
1048            __ctype.widen(__cs, __cs + __len, __ws);
(gdb) p __len
$9 = -1

And the previous line an alloca call using __len:

	_CharT* __ws = static_cast<_CharT*>(__builtin_alloca(sizeof(_CharT)
							     * __len));
	__ctype.widen(__cs, __cs + __len, __ws);


The code assumes __convert_from_v always returns a valid length, but it seems to be failing and returning -1.

I assume the implementation in libstdc++-v3/config/locale/generic/c_locale.h is used for HPUX, so one of these is returning an error, which we then use as __len:

#if _GLIBCXX_USE_C99_STDIO
    const int __ret = __builtin_vsnprintf(__out, __size, __fmt, __args);
#else
    const int __ret = __builtin_vsprintf(__out, __fmt, __args);
#endif

Could you see what this prints on the target please?

#include <cerrno>

int
getlen(char* __out, int __size __attribute__((unused)), const char* __fmt, ...)
{
  __builtin_va_list __args;
  __builtin_va_start(__args, __fmt);

#if _GLIBCXX_USE_C99_STDIO
  __builtin_puts("using vsnprintf");
  const int __ret = __builtin_vsnprintf(__out, __size, __fmt, __args);
#else
  __builtin_puts("using vsprintf");
  const int __ret = __builtin_vsprintf(__out, __fmt, __args);
#endif
  return __ret;
}

int main()
{
  char out[64];
  errno = 0;
  int len = getlen(out, 0, "%.*g", 6, 1.99);
  __builtin_printf("returned %d, errno = %d\n", len, errno);
}

On 2018-09-04 6:38 AM, redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>
> --- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> Could you see what this prints on the target please?
# ./test
using vsnprintf
returned 4, errno = 0

>
> #include <cerrno>
>
> int
> getlen(char* __out, int __size __attribute__((unused)), const char* __fmt, ...)
> {
>    __builtin_va_list __args;
>    __builtin_va_start(__args, __fmt);
>
> #if _GLIBCXX_USE_C99_STDIO
>    __builtin_puts("using vsnprintf");
>    const int __ret = __builtin_vsnprintf(__out, __size, __fmt, __args);
> #else
>    __builtin_puts("using vsprintf");
>    const int __ret = __builtin_vsprintf(__out, __fmt, __args);
> #endif
>    return __ret;
> }
>
> int main()
> {
>    char out[64];
>    errno = 0;
>    int len = getlen(out, 0, "%.*g", 6, 1.99);
>    __builtin_printf("returned %d, errno = %d\n", len, errno);
> }
>

On 2018-09-04 4:51 AM, redi at gcc dot gnu.org wrote:
> The code assumes __convert_from_v always returns a valid length, but it seems
> to be failing and returning -1.
vsnprintf/snprintf can return a negative value if an error occurs.

On 2018-09-04 7:58 AM, dave.anglin at bell dot net wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>
> --- Comment #10 from dave.anglin at bell dot net ---
> On 2018-09-04 4:51 AM, redi at gcc dot gnu.org wrote:
>> The code assumes __convert_from_v always returns a valid length, but it seems
>> to be failing and returning -1.
> vsnprintf/snprintf can return a negative value if an error occurs.
I think we are hitting one of these problems:

JAGaf80770: vsnprintf and snprintf return doesn't conform to C99
JAGaf47646: with small buffer vsnprintf always returns -1

Probably, we should use libiberty version unless we can work around the 
buffer size issue.

(In reply to dave.anglin from comment #11)
> JAGaf47646: with small buffer vsnprintf always returns -1

Aha, that is probably it. We pass 0 as the size, which is supposed to make vsnprintf tell you how many bytes it would have written (as that's how we find out the required length).

It's curious that it printed "returned 4, errno = 0" when you tested it though. Maybe GCC optimized the call and didn't use the OS function.

Does it return -1 if you use -fno-builtin-vsnprintf ?

On 2018-09-04 9:48 AM, redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>
> --- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> (In reply to dave.anglin from comment #11)
>> JAGaf47646: with small buffer vsnprintf always returns -1
> Aha, that is probably it. We pass 0 as the size, which is supposed to make
> vsnprintf tell you how many bytes it would have written (as that's how we find
> out the required length).
>
> It's curious that it printed "returned 4, errno = 0" when you tested it though.
> Maybe GCC optimized the call and didn't use the OS function.
>
> Does it return -1 if you use -fno-builtin-vsnprintf ?
No.  I checked .s file and GCC isn't optimizing the call.  It returns -1 
when passed a nonzero size
that is too small.

Passing 4 causes it to return -1:
using vsnprintf
returned -1, errno = 0

Passing 5 is okay:
using vsnprintf
returned 4, errno = 0

Maybe it's returning an incorrect length when passed 0 in some locales?

Dave

On 2018-09-04 10:16 AM, dave.anglin at bell dot net wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>
> --- Comment #13 from dave.anglin at bell dot net ---
> On 2018-09-04 9:48 AM, redi at gcc dot gnu.org wrote:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>>
>> --- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
>> (In reply to dave.anglin from comment #11)
>>> JAGaf47646: with small buffer vsnprintf always returns -1
>> Aha, that is probably it. We pass 0 as the size, which is supposed to make
>> vsnprintf tell you how many bytes it would have written (as that's how we find
>> out the required length).
>>
>> It's curious that it printed "returned 4, errno = 0" when you tested it though.
>> Maybe GCC optimized the call and didn't use the OS function.
>>
>> Does it return -1 if you use -fno-builtin-vsnprintf ?
> No.  I checked .s file and GCC isn't optimizing the call.  It returns -1
> when passed a nonzero size
> that is too small.
>
> Passing 4 causes it to return -1:
> using vsnprintf
> returned -1, errno = 0
>
> Passing 5 is okay:
> using vsnprintf
> returned 4, errno = 0
>
> Maybe it's returning an incorrect length when passed 0 in some locales?
No, it uses the buffer when passed a a size of 0:
https://stackoverflow.com/questions/619497/heap-corruption-in-hp-ux

On 2018-09-04 4:51 AM, redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>
> --- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> And the previous line an alloca call using __len:
>
>          _CharT* __ws = static_cast<_CharT*>(__builtin_alloca(sizeof(_CharT)
>                                                               * __len));
>          __ctype.widen(__cs, __cs + __len, __ws);
>
>
> The code assumes __convert_from_v always returns a valid length, but it seems
> to be failing and returning -1.
It appears to me we are using the first hunk of the #if:

#if _GLIBCXX_USE_C99_STDIO
         // Precision is always used except for hexfloat format.
         const bool __use_prec =
           (__io.flags() & ios_base::floatfield) != ios_base::floatfield;

         // First try a buffer perhaps big enough (most probably sufficient
         // for non-ios_base::fixed outputs)
         int __cs_size = __max_digits * 3;
         char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));
         if (__use_prec)
           __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
                                         __fbuf, __prec, __v);
         else
           __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
                                         __fbuf, __v);

         // If the buffer was not large enough, try again with the 
correct size.
         if (__len >= __cs_size)
           {
             __cs_size = __len + 1;
             __cs = static_cast<char*>(__builtin_alloca(__cs_size));
             if (__use_prec)
               __len = std::__convert_from_v(_S_get_c_locale(), __cs, 
__cs_size,
                                             __fbuf, __prec, __v);
             else
               __len = std::__convert_from_v(_S_get_c_locale(), __cs, 
__cs_size,
                                             __fbuf, __v);
           }
#else
         // Consider the possibility of long ios_base::fixed outputs
         const bool __fixed = __io.flags() & ios_base::fixed;
         const int __max_exp =
__gnu_cxx::__numeric_traits<_ValueT>::__max_exponent10;

         // The size of the output string is computed as follows.
         // ios_base::fixed outputs may need up to __max_exp + 1 chars
         // for the integer part + __prec chars for the fractional part
         // + 3 chars for sign, decimal point, '\0'. On the other hand,
         // for non-fixed outputs __max_digits * 2 + __prec chars are
         // largely sufficient.
         const int __cs_size = __fixed ? __max_exp + __prec + 4
                                       : __max_digits * 2 + __prec;
         char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));
         __len = std::__convert_from_v(_S_get_c_locale(), __cs, 0, __fbuf,
                                       __prec, __v);
#endif

In the first hunk, __cs_size is not zero but __max_digits * 3.  If the 
buffer is too small,
__len will be -1.  We need to check for -1 in second if.  Is 
incrementing __cs_size by
1 enough?

Yes I've just realised that passing 0 was a red herring, because we take the 
#if _GLIBCXX_USE_C99_STDIO branch. Sorry.

I don't think __cs_size + 1 will be enough in general. There's no reason to think it's only 1 byte too small.

Maybe we need a kluge like:

--- a/libstdc++-v3/include/bits/locale_facets.tcc
+++ b/libstdc++-v3/include/bits/locale_facets.tcc
@@ -1008,6 +1008,20 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
          __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
                                        __fbuf, __v);
 
+#ifdef __hpux
+       while (__len == -1 && __cs_size < 1024)
+         {
+           __cs_size *= 2;
+           char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));
+           if (__use_prec)
+             __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
+                 __fbuf, __prec, __v);
+           else
+             __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
+                 __fbuf, __v);
+         }
+#endif
+
        // If the buffer was not large enough, try again with the correct size.
        if (__len >= __cs_size)
          {

Created attachment 44659 [details]
locale_facets.tcc.d

On 2018-09-04 11:20 AM, redi at gcc dot gnu.org wrote:
> I don't think __cs_size + 1 will be enough in general. There's no reason to
> think it's only 1 byte too small.
>
> Maybe we need a kluge like:
Maybe.  I read in manpage that old glibc versions also return -1 when 
the output is truncated.

I was about to try the attached change.  Was hoping that a slightly 
larger __cs_size would work.

I don't think we care about glibc < 2.0.6 though.

__max_digits * 4 is not enough for:

  std::cout << std::fixed << std::numeric_limits<long double>::max();

That needs 4940 bytes, but we don't want to unconditionally alloca that amount.

Maybe:

--- a/libstdc++-v3/include/bits/locale_facets.tcc
+++ b/libstdc++-v3/include/bits/locale_facets.tcc
@@ -1008,6 +1008,11 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
          __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
                                        __fbuf, __v);
 
+#ifdef __hpux
+       if (__len == -1)
+         __len = __cs_size = 5000;
+#endif
+
        // If the buffer was not large enough, try again with the correct size.
        if (__len >= __cs_size)
          {
@@ -1020,6 +1025,11 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
              __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
                                            __fbuf, __v);
          }
+
+#ifdef __hpux
+       if (__len == -1)
+         return __s;
+#endif
 #else
        // Consider the possibility of long ios_base::fixed outputs
        const bool __fixed = __io.flags() & ios_base::fixed;

On 2018-09-04 12:35 PM, redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>
> --- Comment #18 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> I don't think we care about glibc < 2.0.6 though.
>
> __max_digits * 4 is not enough for:
>
>    std::cout << std::fixed << std::numeric_limits<long double>::max();
>
> That needs 4940 bytes, but we don't want to unconditionally alloca that amount.
>
> Maybe:
>
> --- a/libstdc++-v3/include/bits/locale_facets.tcc
> +++ b/libstdc++-v3/include/bits/locale_facets.tcc
> @@ -1008,6 +1008,11 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
>            __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
>                                          __fbuf, __v);
>
> +#ifdef __hpux
> +       if (__len == -1)
> +         __len = __cs_size = 5000;
> +#endif
> +
>          // If the buffer was not large enough, try again with the correct size.
>          if (__len >= __cs_size)
>            {
> @@ -1020,6 +1025,11 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
>                __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
>                                              __fbuf, __v);
>            }
> +
> +#ifdef __hpux
> +       if (__len == -1)
> +         return __s;
> +#endif
>   #else
>          // Consider the possibility of long ios_base::fixed outputs
>          const bool __fixed = __io.flags() & ios_base::fixed;
>
Testing.

(In reply to Jonathan Wakely from comment #18)
> +#ifdef __hpux
> +       if (__len == -1)
> +         return __s;
> +#endif

N.B. I really don't want to just abort here, nor assume that __len is valid, and then continue onwards with undefined behaviour. But I don't think this function has any way to report an error. Maybe it should throw an exception instead.

It also occurs to me that in some cases we could reuse the original alloca result for the final output string instead of doing yet another alloca, e.g. if we alloca(5000) and find __len is less than (5000 / sizeof(wchar_t)) then we could reuse it. Or maybe beyond a certain size we should switch to the heap.

This should never fail:

--- a/libstdc++-v3/include/bits/locale_facets.tcc
+++ b/libstdc++-v3/include/bits/locale_facets.tcc
@@ -1000,7 +1000,11 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
        // First try a buffer perhaps big enough (most probably sufficient
        // for non-ios_base::fixed outputs)
        int __cs_size = __max_digits * 3;
-       char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));
+       char* __cs;
+
+__retry_convert_from_v:
+
+       __cs = static_cast<char*>(__builtin_alloca(__cs_size));
        if (__use_prec)
          __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
                                        __fbuf, __prec, __v);
@@ -1020,6 +1024,15 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
              __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
                                            __fbuf, __v);
          }
+       else if (__builtin_expect(__len == -1, false))
+         {
+           const bool __fixed = __io.flags() & ios_base::fixed;
+           const int __max_exp =
+             __gnu_cxx::__numeric_traits<_ValueT>::__max_exponent10;
+           __cs_size = __fixed ? __max_exp + __prec + 4
+                               : __max_digits * 2 + __prec;
+           goto __retry_convert_from_v;
+         }
 #else
        // Consider the possibility of long ios_base::fixed outputs
        const bool __fixed = __io.flags() & ios_base::fixed;



Another option is to simply define _GLIBCXX_BROKEN_VSNPRINTF in config/os/hpux/os_defines.h and then force the use of vsprintf, which is always called with a large enough buffer:

diff --git a/libstdc++-v3/config/locale/generic/c_locale.h b/libstdc++-v3/config/locale/generic/c_locale.h
index 0d208166063..3045931c840 100644
--- a/libstdc++-v3/config/locale/generic/c_locale.h
+++ b/libstdc++-v3/config/locale/generic/c_locale.h
@@ -70,7 +70,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     __builtin_va_list __args;
     __builtin_va_start(__args, __fmt);
 
-#if _GLIBCXX_USE_C99_STDIO
+#if _GLIBCXX_USE_C99_STDIO && !_GLIBCXX_HAVE_BROKEN_VSNPRINTF
     const int __ret = __builtin_vsnprintf(__out, __size, __fmt, __args);
 #else
     const int __ret = __builtin_vsprintf(__out, __fmt, __args);
diff --git a/libstdc++-v3/config/os/hpux/os_defines.h b/libstdc++-v3/config/os/hpux/os_defines.h
index 1003477fe35..5a9c4faf75d 100644
--- a/libstdc++-v3/config/os/hpux/os_defines.h
+++ b/libstdc++-v3/config/os/hpux/os_defines.h
@@ -109,4 +109,9 @@ typedef long int __padding_type;
 #if defined (__hppa__)
 #define _GLIBCXX_HAVE_BROKEN_STRTOLD 1
 #endif
+
+// The vnsprintf function returns -1 when the buffer is too small.
+// See PR libstdc++/68737.
+#define _GLIBCXX_HAVE_BROKEN_VSNPRINTF 1
+
 #endif
diff --git a/libstdc++-v3/include/bits/locale_facets.tcc b/libstdc++-v3/include/bits/locale_facets.tcc
index 39da5766075..d5fa91e97d6 100644
--- a/libstdc++-v3/include/bits/locale_facets.tcc
+++ b/libstdc++-v3/include/bits/locale_facets.tcc
@@ -992,7 +992,7 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
        char __fbuf[16];
        __num_base::_S_format_float(__io, __fbuf, __mod);
 
-#if _GLIBCXX_USE_C99_STDIO
+#if _GLIBCXX_USE_C99_STDIO && !_GLIBCXX_HAVE_BROKEN_VSNPRINTF
        // Precision is always used except for hexfloat format.
        const bool __use_prec =
          (__io.flags() & ios_base::floatfield) != ios_base::floatfield;

I prefer the first approach although it might loop if there is an 
error.  Probably, it needs
to check if the current __cs_size is less than the new __cs_size before 
doing a retry.
Another option would be to just duplicate the code instead of branching 
back.

It's my understanding that the vsnprintf bug is fixed in 11.31 and some 
HP-UX patch sets.
We win with the first approach when the initial buffer is large enough.

It's possible for vsnprintf on linux to return a negative value if an 
output error occurs.
So, there's some justification to check the return on linux.

On 2018-09-05 8:07 AM, redi at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68737
>
> --- Comment #21 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> This should never fail:
>
> --- a/libstdc++-v3/include/bits/locale_facets.tcc
> +++ b/libstdc++-v3/include/bits/locale_facets.tcc
> @@ -1000,7 +1000,11 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
>          // First try a buffer perhaps big enough (most probably sufficient
>          // for non-ios_base::fixed outputs)
>          int __cs_size = __max_digits * 3;
> -       char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));
> +       char* __cs;
> +
> +__retry_convert_from_v:
> +
> +       __cs = static_cast<char*>(__builtin_alloca(__cs_size));
>          if (__use_prec)
>            __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
>                                          __fbuf, __prec, __v);
> @@ -1020,6 +1024,15 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
>                __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
>                                              __fbuf, __v);
>            }
> +       else if (__builtin_expect(__len == -1, false))
> +         {
> +           const bool __fixed = __io.flags() & ios_base::fixed;
> +           const int __max_exp =
> +             __gnu_cxx::__numeric_traits<_ValueT>::__max_exponent10;
> +           __cs_size = __fixed ? __max_exp + __prec + 4
> +                               : __max_digits * 2 + __prec;
> +           goto __retry_convert_from_v;
> +         }
>   #else
>          // Consider the possibility of long ios_base::fixed outputs
>          const bool __fixed = __io.flags() & ios_base::fixed;
>
>
>
> Another option is to simply define _GLIBCXX_BROKEN_VSNPRINTF in
> config/os/hpux/os_defines.h and then force the use of vsprintf, which is always
> called with a large enough buffer:
>
> diff --git a/libstdc++-v3/config/locale/generic/c_locale.h
> b/libstdc++-v3/config/locale/generic/c_locale.h
> index 0d208166063..3045931c840 100644
> --- a/libstdc++-v3/config/locale/generic/c_locale.h
> +++ b/libstdc++-v3/config/locale/generic/c_locale.h
> @@ -70,7 +70,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       __builtin_va_list __args;
>       __builtin_va_start(__args, __fmt);
>
> -#if _GLIBCXX_USE_C99_STDIO
> +#if _GLIBCXX_USE_C99_STDIO && !_GLIBCXX_HAVE_BROKEN_VSNPRINTF
>       const int __ret = __builtin_vsnprintf(__out, __size, __fmt, __args);
>   #else
>       const int __ret = __builtin_vsprintf(__out, __fmt, __args);
> diff --git a/libstdc++-v3/config/os/hpux/os_defines.h
> b/libstdc++-v3/config/os/hpux/os_defines.h
> index 1003477fe35..5a9c4faf75d 100644
> --- a/libstdc++-v3/config/os/hpux/os_defines.h
> +++ b/libstdc++-v3/config/os/hpux/os_defines.h
> @@ -109,4 +109,9 @@ typedef long int __padding_type;
>   #if defined (__hppa__)
>   #define _GLIBCXX_HAVE_BROKEN_STRTOLD 1
>   #endif
> +
> +// The vnsprintf function returns -1 when the buffer is too small.
> +// See PR libstdc++/68737.
> +#define _GLIBCXX_HAVE_BROKEN_VSNPRINTF 1
> +
>   #endif
> diff --git a/libstdc++-v3/include/bits/locale_facets.tcc
> b/libstdc++-v3/include/bits/locale_facets.tcc
> index 39da5766075..d5fa91e97d6 100644
> --- a/libstdc++-v3/include/bits/locale_facets.tcc
> +++ b/libstdc++-v3/include/bits/locale_facets.tcc
> @@ -992,7 +992,7 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
>          char __fbuf[16];
>          __num_base::_S_format_float(__io, __fbuf, __mod);
>
> -#if _GLIBCXX_USE_C99_STDIO
> +#if _GLIBCXX_USE_C99_STDIO && !_GLIBCXX_HAVE_BROKEN_VSNPRINTF
>          // Precision is always used except for hexfloat format.
>          const bool __use_prec =
>            (__io.flags() & ios_base::floatfield) != ios_base::floatfield;
>

I prefer just forcing the use of vsprintf with a larger buffer, it avoids adding more complexity to the code.

(In reply to dave.anglin from comment #22)
> I prefer the first approach although it might loop if there is an 
> error.  Probably, it needs
> to check if the current __cs_size is less than the new __cs_size before 
> doing a retry.

Yes, it needs to avoid looping forever.

> Another option would be to just duplicate the code instead of branching 
> back.

Yes, but it's already duplicated and I didn't want to add another one.

> It's my understanding that the vsnprintf bug is fixed in 11.31 and some 
> HP-UX patch sets.
> We win with the first approach when the initial buffer is large enough.
> 
> It's possible for vsnprintf on linux to return a negative value if an 
> output error occurs.
> So, there's some justification to check the return on linux.

I'm not sure if an output error is ever possible when writing to a buffer (rather than to a file) when we control the format specification.

I'll return to this another time.

On 2018-09-05 11:53 AM, redi at gcc dot gnu.org wrote:
> I prefer just forcing the use of vsprintf with a larger buffer, it avoids
> adding more complexity to the code.
Will test in next build.

On 2018-09-05 8:07 a.m., redi at gcc dot gnu.org wrote:
> Another option is to simply define _GLIBCXX_BROKEN_VSNPRINTF in
> config/os/hpux/os_defines.h and then force the use of vsprintf, which is always
> called with a large enough buffer:
>
> diff --git a/libstdc++-v3/config/locale/generic/c_locale.h
> b/libstdc++-v3/config/locale/generic/c_locale.h
> index 0d208166063..3045931c840 100644
> --- a/libstdc++-v3/config/locale/generic/c_locale.h
> +++ b/libstdc++-v3/config/locale/generic/c_locale.h
> @@ -70,7 +70,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      __builtin_va_list __args;
>      __builtin_va_start(__args, __fmt);
>
> -#if _GLIBCXX_USE_C99_STDIO
> +#if _GLIBCXX_USE_C99_STDIO && !_GLIBCXX_HAVE_BROKEN_VSNPRINTF
>      const int __ret = __builtin_vsnprintf(__out, __size, __fmt, __args);
>  #else
>      const int __ret = __builtin_vsprintf(__out, __fmt, __args);
> diff --git a/libstdc++-v3/config/os/hpux/os_defines.h
> b/libstdc++-v3/config/os/hpux/os_defines.h
> index 1003477fe35..5a9c4faf75d 100644
> --- a/libstdc++-v3/config/os/hpux/os_defines.h
> +++ b/libstdc++-v3/config/os/hpux/os_defines.h
> @@ -109,4 +109,9 @@ typedef long int __padding_type;
>  #if defined (__hppa__)
>  #define _GLIBCXX_HAVE_BROKEN_STRTOLD 1
>  #endif
> +
> +// The vnsprintf function returns -1 when the buffer is too small.
> +// See PR libstdc++/68737.
> +#define _GLIBCXX_HAVE_BROKEN_VSNPRINTF 1
> +
>  #endif
> diff --git a/libstdc++-v3/include/bits/locale_facets.tcc
> b/libstdc++-v3/include/bits/locale_facets.tcc
> index 39da5766075..d5fa91e97d6 100644
> --- a/libstdc++-v3/include/bits/locale_facets.tcc
> +++ b/libstdc++-v3/include/bits/locale_facets.tcc
> @@ -992,7 +992,7 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
>         char __fbuf[16];
>         __num_base::_S_format_float(__io, __fbuf, __mod);
>
> -#if _GLIBCXX_USE_C99_STDIO
> +#if _GLIBCXX_USE_C99_STDIO && !_GLIBCXX_HAVE_BROKEN_VSNPRINTF
>         // Precision is always used except for hexfloat format.
>         const bool __use_prec =
>           (__io.flags() & ios_base::floatfield) != ios_base::floatfield;
The above fix is okay for now.

Dave

Author: redi
Date: Mon Jan 28 23:58:57 2019
New Revision: 268350

URL: https://gcc.gnu.org/viewcvs?rev=268350&root=gcc&view=rev
Log:
PR libstdc++/68737 Do not use vsnprintf on HPUX

It doesn't conform to the spec, so use vsprintf with a large buffer
instead.

	PR libstdc++/68737
	* config/locale/generic/c_locale.h (__convert_from_v)
	[_GLIBCXX_USE_C99_STDIO]: Also check _GLIBCXX_HAVE_BROKEN_VSNPRINTF.
	* config/os/hpux/os_defines.h: Define _GLIBCXX_HAVE_BROKEN_VSNPRINTF.
	* include/bits/locale_facets.tcc (num_put::_M_insert_float)
	[_GLIBCXX_USE_C99_STDIO]: Also check _GLIBCXX_HAVE_BROKEN_VSNPRINTF.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/config/locale/generic/c_locale.h
    trunk/libstdc++-v3/config/os/hpux/os_defines.h
    trunk/libstdc++-v3/include/bits/locale_facets.tcc

Done - do you want to keep this open?

On 2019-01-29 4:53 a.m., redi at gcc dot gnu.org wrote:
> Done - do you want to keep this open?
Could the change be backported?  I will test in coming days.

Dave, did you test this for the gcc-8 branch? Do you still want it backported to that branch?

It's fixed in 9.1.0 and later.

I'll test when I get a chance.

On 2020-07-13 7:20 a.m., redi at gcc dot gnu.org wrote:
> Dave, did you test this for the gcc-8 branch? Do you still want it backported
> to that branch?
Yes please.  The patch applies cleanly on the gcc-8 branch.  Then, we can close this PR.

Thanks,
Dave

The releases/gcc-8 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:c54d4e218f18c66ce1ad9d7e7762357fd7edacb7

commit r8-10367-gc54d4e218f18c66ce1ad9d7e7762357fd7edacb7
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jan 28 23:58:57 2019 +0000

    PR libstdc++/68737 Do not use vsnprintf on HPUX
    
    It doesn't conform to the spec, so use vsprintf with a large buffer
    instead.
    
            PR libstdc++/68737
            * config/locale/generic/c_locale.h (__convert_from_v)
            [_GLIBCXX_USE_C99_STDIO]: Also check _GLIBCXX_HAVE_BROKEN_VSNPRINTF.
            * config/os/hpux/os_defines.h: Define _GLIBCXX_HAVE_BROKEN_VSNPRINTF.
            * include/bits/locale_facets.tcc (num_put::_M_insert_float)
            [_GLIBCXX_USE_C99_STDIO]: Also check _GLIBCXX_HAVE_BROKEN_VSNPRINTF.
    
    (cherry picked from commit c98f255154798847bdd1fc6ce33266c1a1ddc13a)

Also fixed for 8.5.0 now.