[Bug libstdc++/57997] New: Segmentation fault after returning valarray expression from an auto function

[Bug libstdc++/57997] New: Segmentation fault after returning valarray expression from an auto function

[roystgnr at ices dot utexas.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

            Bug ID: 57997
           Summary: Segmentation fault after returning valarray expression
                    from an auto function
           Product: gcc
           Version: 4.8.1
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: roystgnr at ices dot utexas.edu

The following code:

////////

#include <valarray>

template<typename T>
auto f1(const T& x, const T& y)
-> decltype(x*y/2)
{ return x*y/2; }

int main()
{
  std::valarray<float> a(4, 1);
  std::valarray<float> b(4, 1);
  std::valarray<float> c(4, 1);

  c = a*b/2;
  c = f1(a,b);

  return 0;
} 

////////

compiled with g++ 4.8.1 (-std=c++11), dies at runtime with a segmentation
fault, stack trace:

(gdb) where
#0  0x0000000000400e60 in std::__multiplies::operator()<float>
(this=0x7fffffffd95f, 
    __x=@0x0: <error reading variable>, __y=@0x603030: 4)
    at /usr/include/c++/4.8/bits/valarray_before.h:204
#1  0x0000000000400ddc in std::_BinBase<std::__multiplies,
std::valarray<float>, std::valarray<float> >::operator[] (this=0x7fffffffda50,
__i=0)
    at /usr/include/c++/4.8/bits/valarray_before.h:524
#2  0x0000000000400d4d in std::_BinBase2<std::__divides,
std::_BinClos<std::__multiplies, std::_ValArray, std::_ValArray, float, float>
>::operator[] (this=0x7fffffffdac0, __i=0)
    at /usr/include/c++/4.8/bits/valarray_before.h:545
#3  0x0000000000400d0b in std::_Expr<std::_BinClos<std::__divides, std::_Expr,
std::_Constant, std::_BinClos<std::__multiplies, std::_ValArray,
std::_ValArray, float, float>, float>, float>::operator[] (this=0x7fffffffdac0,
__i=0)
    at /usr/include/c++/4.8/bits/valarray_after.h:216
#4  0x0000000000400c40 in std::__valarray_copy<float,
std::_BinClos<std::__divides, std::_Expr, std::_Constant,
std::_BinClos<std::__multiplies, std::_ValArray, std::_ValArray, float, float>,
float> > (__e=..., __n=1, __a=...)
    at /usr/include/c++/4.8/bits/valarray_array.tcc:146
#5  0x0000000000400a75 in
std::valarray<float>::operator=<std::_BinClos<std::__divides, std::_Expr,
std::_Constant, std::_BinClos<std::__multiplies, std::_ValArray,
std::_ValArray, float, float>, float> > (this=0x7fffffffdaa0, __e=...)
    at /usr/include/c++/4.8/valarray:823
#6  0x000000000040083c in main () at test.C:16


Examination in gdb of the expression template returned by f1 shows member
values which seem to be correct immediately after the return but which become
corrupted during the operator= evaluation.  I can't be sure, but it seems as if
sufficiently complicated expression templates (removing "/2" from the above
code doesn't trigger the problem) may be saving references to stack-allocated
(i.e. temporary, later overwritten) subexpressions.

My colleague reports being able to reproduce the problem (albeit on our
original code, not the above distilled testcase) with gcc trunk r201275.

The problem seems to lie with the valarray headers; using clang++ 3.3 with
libstdc++ results in the same failure, whereas the clang++/libc++ combination
passes our tests.

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gdr at gcc dot gnu.org
           Severity|major                       |normal

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Gaby, can you help me with this?

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[gdr at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

--- Comment #2 from Gabriel Dos Reis <gdr at gcc dot gnu.org> ---
(In reply to Paolo Carlini from comment #1)
> Gaby, can you help me with this?

I think this is typical confusion about what valarray expressions are.

f1() has some complicated return type that has essentially reference semantics
(as  permitted by the standard) and it returns a "morally" reference to the
locally constructed temporary.  The simplest fix is in the user code: do not
assume valarray expressions in general have valarray types.

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[gdr at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

--- Comment #3 from Gabriel Dos Reis <gdr at gcc dot gnu.org> ---
Also, there might be some interactions with move semantics; I don't know.

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[roystgnr at ices dot utexas.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

--- Comment #4 from Roy Stogner <roystgnr at ices dot utexas.edu> ---
I oversimplified the code here to try and make the failure as easy to
reproduce as possible.  The term here isn't con-fusion, it's "kernel
fusion", which the original user code is trying to do with libstdc++
valarray expressions (and VexCL expressions, and Eigen expressions,
etc; those cases work); this is essentially the opposite of assuming
that expression templates match their input types.  To keep equations
in their own functions (for maintainability) while still doing
parallel evaluation in a single loop (for efficiency), those functions
need to return the expression templates which embody their equations.

However, this does assume that expression template move constructors
(or copy constructors if move constructors aren't defined) don't do a
shallow copy of the representations of subexpression objects, and I
can't see anything in C++2011 or C++2014 standards drafts *mandating*
anything about those constructors.  If my amateur standards-lawyering
is correct, it would be fair for you to mark this as "wontfix" or
whatever bugzilla term signifies "go_bug_standards_committee_instead".

On the third hand, there's nothing in the current standard mandating
that the implementation *not* be robust when doing kernel fusion with
user functions, either.  Storing subexpressions by value in _BinBase
(specializing to store by reference when the subexpression is just a
valarray) might be a sufficient fix.

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> ---
In my opinion it would be great if you could concretely propose a patch. I'm
sure that both Gaby and the other library maintainers, like me, would be fast
at reviewing it. Of course, if large it would require a Copyright assignment,
but maybe not.

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[roystgnr at ices dot utexas.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

--- Comment #6 from Roy Stogner <roystgnr at ices dot utexas.edu> ---
Copyright assignment shouldn't be a problem.  The one serious non-technical
problem is going to be finding time to work on a patch.

The only technical issue I've discovered so far is that making this robust with
no efficiency penalty seems to be impossible without operator overloads that
take rvalue references.  libstdc++ policy is that C++11 features are fine to
use if wrapped in
#if __cplusplus >= 201103L
and C++03 compatibility is maintained?

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

--- Comment #7 from Paolo Carlini <paolo.carlini at oracle dot com> ---
I think so, yes. Your help is welcome anyway, worst case, we'll apply the
changes for the next release series instead of 4.9. In a few hours I will send
you privately the questionnaire to request the official Copyright Assignment
forms to FSF.

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'm pretty sure our implementation conforms to the standard, expression
templates don't mix well with auto/decltype, but libc++ does seem to make code
like this work somehow so it is a reasonable enhancement request.

[Bug libstdc++/57997] Segmentation fault after returning valarray expression from an auto function

[glisse at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57997

--- Comment #9 from Marc Glisse <glisse at gcc dot gnu.org> ---
I'd rather work on improving the warnings so we can tell the user how bad his
code is, but in case, we had a similar request in GMP, a code that was inspired
by libstdc++ valarray:

https://gmplib.org/list-archives/gmp-bugs/2014-January/003315.html

and the discussion may contain hints relevant to the valarray case.