[Bug libstdc++/30416] New: SIGSEGV in valarray::cshift(n) on empty array

[Bug libstdc++/30416] New: SIGSEGV in valarray::cshift(n) on empty array

[sebor at roguewave dot com]

AFAIK, the program below should have well-defined behavior but it abends with
gcc 4.1.0 on Solaris 9.

$ cat t.cpp && g++ -g t.cpp -static && gdb ./a.out
#include <valarray>

int main ()
{
    const std::valarray<int> a;

    a.cshift (1);
} 
GNU gdb 6.5
Copyright (C) 2006 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "sparc-sun-solaris2.9"...
(gdb) run
Starting program: a.out 

Program received signal SIGSEGV, Segmentation fault.
0x00024db8 in memcpy ()
(gdb) where
#0  0x00024db8 in memcpy ()
#1  0x00010368 in std::_Array_copy_ctor<int, true>::_S_do_it (__b=0x0, 
    __e=0x4, __o=0xffbffb94)
    at
/usr/local/gcc-4.1.0/lib/gcc/sparc-sun-solaris2.9/4.1.0/../../../../include/c++/4.1.0/bits/valarray_array.h:165
#2  0x00010398 in std::__valarray_copy_construct<int> (__b=0x0, __e=0x4, 
    __o=0xffbffb94)
    at
/usr/local/gcc-4.1.0/lib/gcc/sparc-sun-solaris2.9/4.1.0/../../../../include/c++/4.1.0/bits/valarray_array.h:174
#3  0x0001057c in std::valarray<int>::cshift (this=0xffbffc20, __n=1)
    at
/usr/local/gcc-4.1.0/lib/gcc/sparc-sun-solaris2.9/4.1.0/../../../../include/c++/4.1.0/valarray:821
#4  0x00010288 in main () at /nfs/b20/sebor/tmp/t.cpp:8


-- 
           Summary: SIGSEGV in valarray::cshift(n) on empty array
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: sebor at roguewave dot com


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2007-01-09 22:02 -------
Confirmed.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
      Known to fail|                            |3.3.6
   Last reconfirmed|0000-00-00 00:00:00         |2007-01-09 22:02:21
               date|                            |


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[gdr at integrable-solutions dot net]

------- Comment #2 from gdr at integrable-solutions dot net  2007-01-09 22:05 -------
Subject: Re:   New: SIGSEGV in valarray::cshift(n) on empty array

"sebor at roguewave dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| AFAIK, the program below should have well-defined behavior but it abends with
| gcc 4.1.0 on Solaris 9.

Why do you think it should have well-dfeined behaviour?

-- Gaby


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[chris at bubblescope dot net]

------- Comment #3 from chris at bubblescope dot net  2007-01-09 22:21 -------
The standard refers to "(l+n)%size()", so if size()=0, that seems to be
undefined. On the other hand, it seems fairly obvious what should happen in
this case (ie nothing).

On an unrelated note, isn't there a another bug in the standard here, as it
seems to be assuming that (-1)%n = (n-1) for positive n, which isn't required
by the standard?


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[sebor at roguewave dot com]

------- Comment #4 from sebor at roguewave dot com  2007-01-09 22:34 -------
(In reply to comment #3)
> The standard refers to "(l+n)%size()", so if size()=0, that seems to be
> undefined. On the other hand, it seems fairly obvious what should happen in
> this case (ie nothing).

The requirement is to "return an object of length size() each of whose
elements..." If there are no elements the "..." doesn't/cannot apply. Note
that an analogous expression is used in rotate_copy which I assume we all
agree is well defined for zero-length ranges.


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[pcarlini at suse dot de]

------- Comment #5 from pcarlini at suse dot de  2007-01-09 23:17 -------
Well, IMHO, avoiding this SIGSEGV is so easy, I would change anyway both shift
and cshift (i.e., wrap everything in a check that size() > 0), and be done with
it, if nobody strongly disagree... While we are at it, quickly looking at the
code I don't think the current code deals correctly with { n : |n| > size() },
both shift and cshift, a much more general issue, if I read correctly the
standard, and also rather straightforward to solve.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pcarlini at suse dot de


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[gdr at integrable-solutions dot net]

------- Comment #6 from gdr at integrable-solutions dot net  2007-01-10 03:32 -------
Subject: Re:  SIGSEGV in valarray::cshift(n) on empty array

"chris at bubblescope dot net" <gcc-bugzilla@gcc.gnu.org> writes:

| The standard refers to "(l+n)%size()", so if size()=0, that seems to be
| undefined. On the other hand, it seems fairly obvious what should happen in
| this case (ie nothing).

On the other other hand [semi ;-)], even without resorting to
undefinedness of "% size()", it takes a special rule to define the
cyclic rotation of nothing.  I don't see that special rule, which is
why I asked why it was obvious that the result it well-defined.

| On an unrelated note, isn't there a another bug in the standard here, as it
| seems to be assuming that (-1)%n = (n-1) for positive n, which isn't required
| by the standard?

the "c" in "cshift" stands for "cyclic", and yes the standard should have
described the behviour in terms of cyclic permutation instead of
showing an implementation.

-- Gaby


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[gdr at integrable-solutions dot net]

------- Comment #7 from gdr at integrable-solutions dot net  2007-01-10 03:33 -------
Subject: Re:  SIGSEGV in valarray::cshift(n) on empty array

"sebor at roguewave dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| (In reply to comment #3)
| > The standard refers to "(l+n)%size()", so if size()=0, that seems to be
| > undefined. On the other hand, it seems fairly obvious what should happen in
| > this case (ie nothing).
| 
| The requirement is to "return an object of length size() each of whose
| elements..." If there are no elements the "..." doesn't/cannot apply. 

I disagree.  The specification is not that of unconditional return.
And if that is what is intended, then that is not what the standard wrote.

-- Gaby


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[gdr at integrable-solutions dot net]

------- Comment #8 from gdr at integrable-solutions dot net  2007-01-10 03:37 -------
Subject: Re:  SIGSEGV in valarray::cshift(n) on empty array

"pcarlini at suse dot de" <gcc-bugzilla@gcc.gnu.org> writes:

| Well, IMHO, avoiding this SIGSEGV is so easy, I would change anyway both
shift
| and cshift (i.e., wrap everything in a check that size() > 0), and be done
with
| it, if nobody strongly disagree... 

Paolo -- there is what the standard says, and what we can or should implement.
I believe there is a genuine bug in the standard here, no matter how
easy it is to avoid the segmentation fault.  This specific corner case is
most likely run into in testsuites; we better have the standard be
clear and let the testsuite implement what the standard says.

-- Gaby


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[sebor at roguewave dot com]

------- Comment #9 from sebor at roguewave dot com  2007-01-10 05:55 -------
Even if the spec did permit undefined behavior it would make sense to implement
something reasonable if it's easy (as Paolo suggests it might be) and if the
cost
is acceptable. But just to put your mind at ease I'll write up an issue and
make
it clear that cshift really is required to simply return *this for empty
arrays.


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[pcarlini at suse dot de]

------- Comment #10 from pcarlini at suse dot de  2007-01-10 18:56 -------
(In reply to comment #8)
> Paolo -- there is what the standard says, and what we can or should implement.
> I believe there is a genuine bug in the standard here, no matter how
> easy it is to avoid the segmentation fault.  This specific corner case is
> most likely run into in testsuites; we better have the standard be
> clear and let the testsuite implement what the standard says.

Gaby, right now I'm *much* more interested in the general issue which I stated
at the end of my previous message: for v3, the corner case pointed out by
Martin seems just a special case of the much more general issue with size != 0
and { n : |n| > size }. AFAICS, we are not doing the right thing in all such
cases too, that is we are segfaulting where we should not. Do you agree?


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gdr at integrable-solutions
                   |                            |dot net


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[pcarlini at suse dot de]

------- Comment #11 from pcarlini at suse dot de  2007-01-10 19:18 -------
Forgot: assuming we imagine the standard clarified per your proposal on LWG (or
read as such for all practical matters), I understand that not segfaulting when
size == 0 would be a conforming behavior. Then, in v3, we could change the code
to *always* avoid segfaulting, that is, for any n and any size, in a consistent
way. I vounteer to implement that behavior.


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[gdr at integrable-solutions dot net]

------- Comment #12 from gdr at integrable-solutions dot net  2007-01-10 23:27 -------
Subject: Re:  SIGSEGV in valarray::cshift(n) on empty array

"pcarlini at suse dot de" <gcc-bugzilla@gcc.gnu.org> writes:

| Forgot: assuming we imagine the standard clarified per your proposal
| on LWG (or read as such for all practical matters), I understand
| that not segfaulting when size == 0 would be a conforming
| behavior.

Yes, it is interesting to note that a specific request to make the
standard much clearer on the semantics is curiously construed as an
argument for arguing that the standard says the beahviour is undefined.
No wonder.

-- Gaby


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[pcarlini at suse dot de]

------- Comment #13 from pcarlini at suse dot de  2007-01-10 23:47 -------
Ok, thanks a lot. I'm going to work on those changes for v3.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pcarlini at suse dot de
                   |dot org                     |
             Status|NEW                         |ASSIGNED


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[paolo at gcc dot gnu dot org]

------- Comment #14 from paolo at gcc dot gnu dot org  2007-01-12 11:09 -------
Subject: Bug 30416

Author: paolo
Date: Fri Jan 12 11:09:26 2007
New Revision: 120720

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=120720
Log:
2007-01-12  Paolo Carlini  <pcarlini@suse.de>

        PR libstdc++/30416
        * include/std/valarray (valarray<>::shift, valarray<>::cshift):
        Do not segfault when |n| > size.
        * testsuite/26_numerics/valarray/30416.cc: New.

Added:
    trunk/libstdc++-v3/testsuite/26_numerics/valarray/30416.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/valarray


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[pcarlini at suse dot de]

-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.2.0


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[gdr at integrable-solutions dot net]

------- Comment #15 from gdr at integrable-solutions dot net  2007-01-12 13:06 -------
Subject: Re:  SIGSEGV in valarray::cshift(n) on empty array

"paolo at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Subject: Bug 30416
| 
| Author: paolo
| Date: Fri Jan 12 11:09:26 2007
| New Revision: 120720
| 
| URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=120720
| Log:
| 2007-01-12  Paolo Carlini  <pcarlini@suse.de>

Paolo --

  Your patch had issues I just commented on.

I expected you'd have waited for me to review it.  :-(
I was travelling back to CS all day yesterday.

-- Gaby


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[paolo at gcc dot gnu dot org]

------- Comment #16 from paolo at gcc dot gnu dot org  2007-01-12 13:35 -------
Subject: Bug 30416

Author: paolo
Date: Fri Jan 12 13:34:47 2007
New Revision: 120722

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=120722
Log:
2007-01-12  Paolo Carlini  <pcarlini@suse.de>

        PR libstdc++/30416 (continued)
        * include/std/valarray (valarray<>::shift, valarray<>::cshift):
        Allways return the same variable, thus facilitating NRVO.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/valarray


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[paolo at gcc dot gnu dot org]

------- Comment #17 from paolo at gcc dot gnu dot org  2007-01-12 14:56 -------
Subject: Bug 30416

Author: paolo
Date: Fri Jan 12 14:56:29 2007
New Revision: 120725

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=120725
Log:
2007-01-12  Paolo Carlini  <pcarlini@suse.de>

        PR libstdc++/30416 (continued)
        * include/std/std_valarray.h (valarray<>::shift, valarray<>::cshift):
        Allways return the same variable, thus facilitating NRVO.

2007-01-12  Paolo Carlini  <pcarlini@suse.de>

        PR libstdc++/30416
        * include/std/std_valarray.h (valarray<>::shift, valarray<>::cshift):
        Do not segfault when |n| > size.
        * testsuite/26_numerics/valarray/30416.cc: New.

Added:
   
branches/gcc-4_2-branch/libstdc++-v3/testsuite/26_numerics/valarray/30416.cc
Modified:
    branches/gcc-4_2-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_2-branch/libstdc++-v3/include/std/std_valarray.h


-- 


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

[Bug libstdc++/30416] SIGSEGV in valarray::cshift(n) on empty array

[pcarlini at suse dot de]

------- Comment #18 from pcarlini at suse dot de  2007-01-12 14:57 -------
Fixed for 4.2.0.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


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