Bug 42182 - memory errors using valarrays
Summary: memory errors using valarrays
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-26 11:31 UTC by Christian Bruel
Modified: 2009-11-26 18:00 UTC (History)
1 user (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:


Attachments
reduced case (143 bytes, text/x-c++src)
2009-11-26 11:32 UTC, Christian Bruel
Details
computed assignment test case (145 bytes, text/x-c++src)
2009-11-26 11:35 UTC, Christian Bruel
Details
controversal testcase (252 bytes, text/x-c++src)
2009-11-26 11:38 UTC, Christian Bruel
Details
Proposed patch (612 bytes, patch)
2009-11-26 11:40 UTC, Christian Bruel
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Bruel 2009-11-26 11:31:14 UTC
The valarray mask_array implementation creates out of bound memory accesses when 
the number of the True bits is smaller that the size of array, although this
is perfect legal.

This produce valgrin error at best, sometime segfaults, or infinite loops.

To reproduce, compile the attached test:

  g++  -O0 va0.cxx 

gnx2439$ valgrind ./a.out==1547== Memcheck, a memory error detector
==1547== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==1547== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==1547== Command: ./a.out
==1547== 
==1547== Invalid read of size 1
==1547==    at 0x80489D1: void std::__valarray_copy<int>(std::_Array<int>, unsigned int, std::_Array<int>, std::_Array<bool>) (in /home/bruel/tmp/a.out)
==1547==    by 0x804889A: std::mask_array<int>::operator=(std::valarray<int> const&) const (in /home/bruel/tmp/a.out)
==1547==    by 0x80485C9: main (in /home/bruel/tmp/a.out)
==1547==  Address 0x4129061 is 0 bytes after a block of size 1 alloc'd
==1547==    at 0x40061CC: operator new(unsigned int) (vg_replace_malloc.c:214)
==1547==    by 0x8048644: std::__valarray_get_memory(unsigned int) (in /home/bruel/tmp/a.out)
==1547==    by 0x80488E6: bool* restrict std::__valarray_get_storage<bool>(unsigned int) (in /home/bruel/tmp/a.out)
==1547==    by 0x80486EE: std::valarray<bool>::valarray(bool const&, unsigned int) (in /home/bruel/tmp/a.out)
==1547==    by 0x804857B: main (in /home/bruel/tmp/a.out)
==1547== 
==1547== Invalid write of size 4
==1547==    at 0x80489E3: void std::__valarray_copy<int>(std::_Array<int>, unsigned int, std::_Array<int>, std::_Array<bool>) (in /home/bruel/tmp/a.out)
==1547==    by 0x804889A: std::mask_array<int>::operator=(std::valarray<int> const&) const (in /home/bruel/tmp/a.out)
==1547==    by 0x80485C9: main (in /home/bruel/tmp/a.out)
==1547==  Address 0x4129108 is not stack'd, malloc'd or (recently) free'd
==1547== 
==1547== 
==1547== HEAP SUMMARY:
==1547==     in use at exit: 0 bytes in 0 blocks
==1547==   total heap usage: 3 allocs, 3 frees, 9 bytes allocated
==1547== 
==1547== All heap blocks were freed -- no leaks are possible
==1547== 
==1547== For counts of detected and suppressed errors, rerun with: -v
==1547== ERROR SUMMARY: 56 errors from 2 contexts (suppressed: 19 from 10)
Comment 1 Christian Bruel 2009-11-26 11:32:57 UTC
Created attachment 19153 [details]
reduced case
Comment 2 Christian Bruel 2009-11-26 11:35:20 UTC
Created attachment 19154 [details]
computed assignment test case

compiled with g++ -O0

gnx2439$ valgrind ./a.out==1599== Memcheck, a memory error detector
==1599== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==1599== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==1599== Command: ./a.out
==1599== 
==1599== Invalid read of size 1
==1599==    at 0x8048A0B: void std::_Array_augmented___plus<int>(std::_Array<int>, std::_Array<bool>, std::_Array<int>, unsigned int) (in /home/bruel/tmp/a.out)
==1599==    by 0x80488D4: std::mask_array<int>::operator+=(std::valarray<int> const&) const (in /home/bruel/tmp/a.out)
==1599==    by 0x80485E9: main (in /home/bruel/tmp/a.out)
==1599==  Address 0x4129061 is 0 bytes after a block of size 1 alloc'd
==1599==    at 0x40061CC: operator new(unsigned int) (vg_replace_malloc.c:214)
==1599==    by 0x804867E: std::__valarray_get_memory(unsigned int) (in /home/bruel/tmp/a.out)
==1599==    by 0x8048920: bool* restrict std::__valarray_get_storage<bool>(unsigned int) (in /home/bruel/tmp/a.out)
==1599==    by 0x8048728: std::valarray<bool>::valarray(bool const&, unsigned int) (in /home/bruel/tmp/a.out)
==1599==    by 0x804857B: main (in /home/bruel/tmp/a.out)
==1599== 
==1599== Invalid read of size 4
==1599==    at 0x8048A18: void std::_Array_augmented___plus<int>(std::_Array<int>, std::_Array<bool>, std::_Array<int>, unsigned int) (in /home/bruel/tmp/a.out)
==1599==    by 0x80488D4: std::mask_array<int>::operator+=(std::valarray<int> const&) const (in /home/bruel/tmp/a.out)
==1599==    by 0x80485E9: main (in /home/bruel/tmp/a.out)
==1599==  Address 0x4129108 is not stack'd, malloc'd or (recently) free'd
==1599== 
==1599== Invalid write of size 4
==1599==    at 0x8048A24: void std::_Array_augmented___plus<int>(std::_Array<int>, std::_Array<bool>, std::_Array<int>, unsigned int) (in /home/bruel/tmp/a.out)
==1599==    by 0x80488D4: std::mask_array<int>::operator+=(std::valarray<int> const&) const (in /home/bruel/tmp/a.out)
==1599==    by 0x80485E9: main (in /home/bruel/tmp/a.out)
==1599==  Address 0x4129108 is not stack'd, malloc'd or (recently) free'd
Comment 3 Paolo Carlini 2009-11-26 11:35:46 UTC
Hey, this is pointless, the issue is well known and Gaby is the reference person in this area.
Comment 4 Christian Bruel 2009-11-26 11:38:13 UTC
Created attachment 19155 [details]
controversal testcase

compile with -O0, Segfaults on x86 with

gcc version 4.5.0 20091119 (experimental) [trunk revision 154332] (GCC)
Comment 5 Christian Bruel 2009-11-26 11:40:21 UTC
Created attachment 19156 [details]
Proposed patch

2009-11-23  Christian Bruel  <christian.bruel@st.com>

        * include/bits/mask_array.h (mask_array<_Tp>::operator=): Use _M_sz.
        (mask_array<_Tp>::operator _Op##): Likewise.
        * testsuite/26_numerics/valarray/mask_array.cc: New test.

This patch makes the valarray_copy interates on the number of true elements rather than on the array size.
Comment 6 Christian Bruel 2009-11-26 12:08:02 UTC
(In reply to comment #3)
> Hey, this is pointless, the issue is well known and Gaby is the reference
> person in this area.
> 

no problem, if the issue is known it can be recorded

for the reference, a link to Gaby authoritative answer
http://gcc.gnu.org/ml/libstdc++/2009-11/msg00099.html

Comment 7 Paolo Carlini 2009-11-26 12:15:21 UTC
What I meant, exactly, is that if any issue is well known to the concerned people, there is no need for a Bugzilla, in particular an invalid one ;)
Comment 8 Christian Bruel 2009-11-26 12:18:38 UTC
(In reply to comment #7)
> What I meant, exactly, is that if any issue is well known to the concerned
> people, there is no need for a Bugzilla, in particular an invalid one ;)
> 

Well I still need to be convinced that va0.cxx here is invalid : 
it is a 5 lines of simple conforming code that produces a segfault. 

Comment 9 Paolo Carlini 2009-11-26 12:57:57 UTC
The point is that the code is not conforming. In any case, conforming or not, in the future please do not open Bugzilla for issues already known to the maintainers, thanks in advance.
Comment 10 Christian Bruel 2009-11-26 17:44:17 UTC
Paolo, 

I entered this defect for user reference, since the problem that originates the thread occurs in many public places such as testsuites (perenial Sec26/P26774) or class books (http://www.linux-kheops.com/doc/casteyde/cours_cpp-1.40.1/htm/x5866.htm#AEN6045) 14-21.
Digging on the net, it's not difficult to find other instances of the same problem, All fail for the same reason: out of bound access from the mask array 
or exibit a valgrind error. FYU they pass with the proposed patch,

I'm glad that it was not a bug with the library, and to have opened the discussion. just wanted to avoid future duplicate reports on the same problem an extra analysis. I didn't want to add extra burden to the maintainers of course, sorry if it was the case.

 
Comment 11 Paolo Carlini 2009-11-26 18:00:30 UTC
Yes, but remember our policy in the future, thanks.