(In reply to Filip Matzner from comment #0)
> Consider the following code:
> 
> #include <climits>
> #include <boost/log/trivial.hpp>
> int main()
> {
>     BOOST_LOG_TRIVIAL(info)
>       << std::to_string(std::numeric_limits<double>::quiet_NaN());

There's no crash if BOOST_LOG_TRIVIAL(info) is replaced with std::cout, which suggests there's nothing wrong with the std::to_string code or the std::string constructor.

It crashes with either the old or new std:string code. Either way, it crashes when calling __builtin_mcmcpy


> }
> 
> 
> When compiled with the following command:
> g++ -std=c++17 -DBOOST_ALL_DYN_LINK -pthread -lboost_log -O3 test.cpp
> 
> It crashes with SEGFAULT in std::string constructor:
> 
> #0  0x0000555555556221 in std::char_traits<char>::copy (__n=3,
> __s2=0x7fffffffd4e0 "nan", __s1=<optimized out>) at
> /usr/include/c++/7/bits/char_traits.h:350
> 
> Note that the SEGFAULT does not happen with -O2, but only -O3.

It crashes with -O2 -fipa-cp-clone

Compiled with GCC 6 it doesn't crash. Compiled with GCC 7.1+ it crashes.
Preprocessed with GCC 6 and compiled with GCC 7+ it crashes.

Created attachment 44319 [details]
Preprocessed testcase

This is preprocessed with boost 1.64.0 and needs libboost_log.so from that version to link. Maybe this is enough to compare the optimized dumps with and without -fipa-cp-clone.

I also see that replacing the call to __builtin_memcpy with memcpy (in char_traits::copy immediately after the __builtin_printf call) doesn't crash.

Thank you for digging into this. I actually hit the issue in a large code base that crashed even without printing anything to boost log. But when cutting off parts of the code, the error happened nondeterministically, so it was a bit difficult to build a minimal working example. The posted example was the most reasonable working example I could come up with.

As a workaround in the large code base, I replaced std::to_string calls with boost::lexical_cast.

Started with:

SVN revision: 242674
Author: msebor
Enable -fprintf-return-value by default.  Tested on powerpc64le and x86.

gcc/c-family/ChangeLog:

	* c.opt (-fprintf-return-value): Enable by default.

gcc/ChangeLog:

	* doc/invoke.texi (-fprintf-return-value): Document that option
	is enabled by default.

Let me isolate that more.

Created attachment 44334 [details]
almost reduced test-case

$ g++ segfault.ii -pthread -lboost_log -O3 -g -fprintf-return-value && valgrind ./a.out

==29544== Invalid write of size 8
==29544==    at 0x400FC7: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > af::__to_xstring<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, char>(int (*)(char*, unsigned long, char const*, __va_list_tag*), long, char const*, ...) [clone .constprop.9] (in /tmp/a.out)
==29544==    by 0x400D78: to_string (<stdin>:100)
==29544==    by 0x400D78: main (<stdin>:345)
==29544==  Address 0x1fff001000 is not stack'd, malloc'd or (recently) free'd
==29544== 
==29544== 
==29544== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==29544==  Access not within mapped region at address 0x1FFF001000
==29544==    at 0x400FC7: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > af::__to_xstring<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, char>(int (*)(char*, unsigned long, char const*, __va_list_tag*), long, char const*, ...) [clone .constprop.9] (in /tmp/a.out)
==29544==    by 0x400D78: to_string (<stdin>:100)
==29544==    by 0x400D78: main (<stdin>:345)

I'm suspecting IPA CP clone somehow smashing varargs?

Can you please Martin take a look?

The IPA (and first tree) dumps look all normal.  But even when I patch IPA-CP to create a clone but not to modify it in any way, I still get the segfault.  I'll look where we start diverging next.

After a more careful look: The testcase from comment #5 calls
__builtin_alloca(1) and then tries to vnsprintf into that memory, so I
decided I'd go back to the original testcase.

It indeed does segfaults when IPA-CP takes place, but it seems there
is already undefined behavior in what IPA-CP sees as its input,
looking into release_ssa tree dump:

__gnu_cxx::__to_xstring<std::__cxx11::basic_string<char>, char> (int (*<T4030>) (char *, size_t, const char *, struct  *) __convf, size_t __n, const char * __fmt)
{
  struct forward_iterator_tag D.128814;
  struct  __args[1];
  char * __s;
  sizetype _1;
  char * _2;
  const int _11;
  char[16] * _15;

  <bb 2> [local count: 1073741825]:
  __s_6 = __builtin_alloca (__n_4(D));
  __builtin_va_start (&__args, 0);
  _11 = __convf_8(D) (__s_6, __n_4(D), __fmt_9(D), &__args);
  __builtin_va_end (&__args);
  _1 = (sizetype) _11;
  _2 = __s_6 + _1;
  _15 = &_13(D)->D.22460._M_local_buf;
  MEM[(struct _Alloc_hider *)_13(D)]._M_p = _15;
  std::__cxx11::basic_string<char>::_M_construct<char*> (_13(D), __s_6, _2, D.128814);
  __args ={v} {CLOBBER};
  return _13(D);
}

The two lines:

  _15 = &_13(D)->D.22460._M_local_buf;
  MEM[(struct _Alloc_hider *)_13(D)]._M_p = _15;

clearly take some value out of thin air and then store into it into a
random place and (in the subsequent call), the address of that random
place is passed to some constructor...  after inlining, the
default-defs proliferate even some more.

As early as the ssa dump we have, in the same function,

  <bb 3> :
  __len_13 = _12;
  __builtin_va_end (&__args);
  std::allocator<char>::allocator (&D.122645);
  _1 = (sizetype) __len_13;
  _2 = __s_7 + _1;
  std::__cxx11::basic_string<char>::basic_string<char*> (_16(D), __s_7, _2, &D.122645);

i.e. construction of basic_string<char> at an undefined address.

And in the previous dump (fixup_cfg1), we have

  <bb 3> :
  __len = D.127713;
  __builtin_va_end (&__args);
  std::allocator<char>::allocator (&D.122645);
  _1 = (sizetype) __len;
  _2 = __s + _1;
  std::__cxx11::basic_string<char>::basic_string<char*> (<retval>, __s, _2, &D.122645);

So I'd say that it is either gimplification or SSA creation that does
not handle this situation correctly.

Thanks Martin for nice analysis. Richi can you take a look at that?

So after gimplification I see

      __s = __builtin_alloca (__n);
      __builtin_va_start (&__args, 0);
      __len = __convf (__s, __n, __fmt, &__args);
      __builtin_va_end (&__args);
      std::allocator<char>::allocator (&D.109301);
      try
        {
          try
            {
              _1 = (sizetype) __len;
              _2 = __s + _1;
              std::__cxx11::basic_string<char>::basic_string<char*> (<retval>, __s, _2, &D.109301);
              return <retval>;
            }
          finally
            {
              std::allocator<char>::~allocator (&D.109301);
            }

which after SSA rewrite looks like

  std::__cxx11::basic_string<char>::basic_string<char*> (_16(D), __s_7, _2, &D.109301);

So we're rewriting <retval> into SSA which is expected, <retval>
is DECL_BY_REFERENCE.  So that is a red herring.

I suppose the new printf code simply doesn't handle default-defs that
come in as DECL_BY_REFERENCE RESULT_DECL.

Note what the pass causes is simply treating

_3 = vsnprintf (__s_2, 328, "%f", &__args);

as not returning zero and thus eliding a == 0 check.  For some reason
it says _3 is [8, 322] though but obviously 'NaN' prints as 3 characters
only...

So, martin?  Reduced testcase:

volatile double x;
int main()
{
  char *s = __builtin_alloca (220);
  x = __builtin_nan("nan");
  if (__builtin_snprintf (s, 220, "%f", x) != 3)
    __builtin_abort ();
  return 0;
}

The sprintf pass doesn't consider NaNs and infinities.  The output for the -fdump-tree-printf-return-value option shows:

;; Function main (main, funcdef_no=0, decl_uid=1898, cgraph_uid=1, symbol_order=1) (executed once)

__builtin_snprintf: objsize = 220, fmtstr = "%f"
  Directive 1 at offset 0: "%f"
    Result: 8, 8, 317, 322 (8, 8, 317, 322)
  Directive 2 at offset 2: "", length = 1
  Setting potentially out-of-bounds return value range [8, 322].

The range should be [3, 322].  Let me handle it.

Patch: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00165.html

Author: msebor
Date: Wed Jul  4 02:19:35 2018
New Revision: 262368

URL: https://gcc.gnu.org/viewcvs?rev=262368&root=gcc&view=rev
Log:
PR tree-optimization/86274 - SEGFAULT when logging std::to_string(NAN)

gcc/ChangeLog:

	PR tree-optimization/86274
	* gimple-ssa-sprintf.c (fmtresult::type_max_digits): Verify
	precondition.
	(format_floating): Correct handling of infinities and NaNs.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86274
	* gcc.dg/tree-ssa/builtin-sprintf-9.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-10.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-15.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf.c: Same.
	* gcc.dg/tree-ssa/pr83198.c: Same.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/builtin-sprintf.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-9.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-ssa-sprintf.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-10.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-15.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr83198.c

Fixed on trunk via r262368.

Author: msebor
Date: Wed Jul  4 18:58:51 2018
New Revision: 262419

URL: https://gcc.gnu.org/viewcvs?rev=262419&root=gcc&view=rev
Log:
gcc/testsuite/ChangeLog:

	PR tree-optimization/86274
	* gcc.dg/tree-ssa/builtin-sprintf-9.c: Fix typo.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-9.c

Thank you everyone. Your efficiency is breathtaking.

Author: msebor
Date: Sat Jul 14 21:32:10 2018
New Revision: 262661

URL: https://gcc.gnu.org/viewcvs?rev=262661&root=gcc&view=rev
Log:
PR tree-optimization/86274 - SEGFAULT when logging std::to_string(NAN)

gcc/ChangeLog:

	PR tree-optimization/86274
	* gimple-ssa-sprintf.c (fmtresult::type_max_digits): Verify
	precondition.
	(format_floating): Correct handling of infinities and NaNs.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86274
	* gcc.dg/tree-ssa/builtin-sprintf-9.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-10.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-15.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: Same.
	* gcc.dg/tree-ssa/builtin-sprintf.c: Same.
	* gcc.dg/tree-ssa/pr83198.c: Same.


Added:
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/torture/builtin-sprintf.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-9.c
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/gimple-ssa-sprintf.c
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-10.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-15.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-7.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/tree-ssa/pr83198.c

Backported to GCC 8 in r262661.

Fixed in GCC 8.2.

Same issue. But also seg faults with o4 and gcc 7.5

https://godbolt.org/z/G7G7nx1Tf

#include <cmath>
#include <iostream>
#include <limits>
#include <string>

void test() {
    std::cout << std::to_string(0.f) << '\n'; // make it "0" and it wont crash
}

int main() {
    std::cout << std::to_string(INFINITY) << '\n';
}