Hi,
the below is what I ended up committing for this PR, very close to the
preliminary draft I recently attached to Bugzilla. All in all, in my
opinion the situation wrt the destructors was broken enough that we want
the patch in the release branch too, if we want to encourage people to
experiment with this special mode at all.
Tested x86_64-linux, normal, debug, and parallel-mode. Committed to
mainline for now.
Thanks,
Paolo.
//////////////////////////
2011-05-03 Paolo Carlini <paolo.carl...@oracle.com>
PR libstdc++/48750
* include/parallel/multiway_merge.h: Run _ValueType destructors.
* include/parallel/multiway_mergesort.h: Likewise.
* include/parallel/quicksort.h: Likewise.
* include/parallel/random_shuffle.h: Likewise.
* include/parallel/partial_sum.h: Likewise.
* include/parallel/losertree.h: Run destructors; minor tweaks.
* include/parallel/par_loop.h: Run destructors, fix memory
allocations and deallocations.
* testsuite/26_numerics/accumulate/48750.cc: New.
* testsuite/ext/profile/mutex_extensions_neg.cc: Do not run in
parallel-mode to avoid spurious multiple errors.
Index: include/parallel/multiway_merge.h
===================================================================
--- include/parallel/multiway_merge.h (revision 173297)
+++ include/parallel/multiway_merge.h (working copy)
@@ -1045,11 +1045,12 @@
_ValueType;
// __k sequences.
- _SeqNumber __k = static_cast<_SeqNumber>(__seqs_end - __seqs_begin);
+ const _SeqNumber __k
+ = static_cast<_SeqNumber>(__seqs_end - __seqs_begin);
- _ThreadIndex __num_threads = omp_get_num_threads();
+ const _ThreadIndex __num_threads = omp_get_num_threads();
- _DifferenceType __num_samples =
+ const _DifferenceType __num_samples =
__gnu_parallel::_Settings::get().merge_oversampling * __num_threads;
_ValueType* __samples = static_cast<_ValueType*>
@@ -1096,6 +1097,10 @@
__pieces[__slab][__seq].second =
_GLIBCXX_PARALLEL_LENGTH(__seqs_begin[__seq]);
}
+
+ for (_SeqNumber __s = 0; __s < __k; ++__s)
+ for (_DifferenceType __i = 0; __i < __num_samples; ++__i)
+ __samples[__s * __num_samples + __i].~_ValueType();
::operator delete(__samples);
}
@@ -1258,10 +1263,10 @@
__length = std::min<_DifferenceTp>(__length, __total_length);
if (__total_length == 0 || __k == 0)
- {
- delete[] __ne_seqs;
- return __target;
- }
+ {
+ delete[] __ne_seqs;
+ return __target;
+ }
std::vector<std::pair<_DifferenceType, _DifferenceType> >* __pieces;
Index: include/parallel/losertree.h
===================================================================
--- include/parallel/losertree.h (revision 173297)
+++ include/parallel/losertree.h (working copy)
@@ -1,6 +1,6 @@
// -*- C++ -*-
-// Copyright (C) 2007, 2008, 2009 Free Software Foundation, Inc.
+// Copyright (C) 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the terms
@@ -116,7 +116,11 @@
* @brief The destructor.
*/
~_LoserTreeBase()
- { ::operator delete(_M_losers); }
+ {
+ for (unsigned int __i = 0; __i < (2 * _M_k); ++__i)
+ _M_losers[__i].~_Loser();
+ ::operator delete(_M_losers);
+ }
/**
* @brief Initializes the sequence "_M_source" with the element "__key".
@@ -131,15 +135,15 @@
{
unsigned int __pos = _M_k + __source;
- if(_M_first_insert)
+ if (_M_first_insert)
{
- // Construct all keys, so we can easily deconstruct them.
+ // Construct all keys, so we can easily destruct them.
for (unsigned int __i = 0; __i < (2 * _M_k); ++__i)
- new(&(_M_losers[__i]._M_key)) _Tp(__key);
+ ::new(&(_M_losers[__i]._M_key)) _Tp(__key);
_M_first_insert = false;
}
else
- new(&(_M_losers[__pos]._M_key)) _Tp(__key);
+ _M_losers[__pos]._M_key = __key;
_M_losers[__pos]._M_sup = __sup;
_M_losers[__pos]._M_source = __source;
@@ -379,7 +383,7 @@
}
~_LoserTreePointerBase()
- { ::operator delete[](_M_losers); }
+ { delete[] _M_losers; }
int __get_min_source()
{ return _M_losers[0]._M_source; }
@@ -577,7 +581,7 @@
_Compare _M_comp;
public:
- _LoserTreeUnguardedBase(unsigned int __k, const _Tp __sentinel,
+ _LoserTreeUnguardedBase(unsigned int __k, const _Tp& __sentinel,
_Compare __comp = std::less<_Tp>())
: _M_comp(__comp)
{
@@ -590,15 +594,24 @@
_M_losers = static_cast<_Loser*>(::operator new(2 * _M_k
* sizeof(_Loser)));
- for (unsigned int __i = _M_k + _M_ik - 1; __i < (2 * _M_k); ++__i)
- {
- _M_losers[__i]._M_key = __sentinel;
+ for (unsigned int __i = 0; __i < _M_k; ++__i)
+ {
+ ::new(&(_M_losers[__i]._M_key)) _Tp(__sentinel);
_M_losers[__i]._M_source = -1;
}
+ for (unsigned int __i = _M_k + _M_ik - 1; __i < (2 * _M_k); ++__i)
+ {
+ ::new(&(_M_losers[__i]._M_key)) _Tp(__sentinel);
+ _M_losers[__i]._M_source = -1;
+ }
}
~_LoserTreeUnguardedBase()
- { ::operator delete(_M_losers); }
+ {
+ for (unsigned int __i = 0; __i < (2 * _M_k); ++__i)
+ _M_losers[__i].~_Loser();
+ ::operator delete(_M_losers);
+ }
int
__get_min_source()
@@ -615,7 +628,7 @@
{
unsigned int __pos = _M_k + __source;
- new(&(_M_losers[__pos]._M_key)) _Tp(__key);
+ ::new(&(_M_losers[__pos]._M_key)) _Tp(__key);
_M_losers[__pos]._M_source = __source;
}
};
@@ -634,7 +647,7 @@
using _Base::_M_losers;
public:
- _LoserTreeUnguarded(unsigned int __k, const _Tp __sentinel,
+ _LoserTreeUnguarded(unsigned int __k, const _Tp& __sentinel,
_Compare __comp = std::less<_Tp>())
: _Base::_LoserTreeUnguardedBase(__k, __sentinel, __comp)
{ }
@@ -721,7 +734,7 @@
using _Base::_M_losers;
public:
- _LoserTreeUnguarded(unsigned int __k, const _Tp __sentinel,
+ _LoserTreeUnguarded(unsigned int __k, const _Tp& __sentinel,
_Compare __comp = std::less<_Tp>())
: _Base::_LoserTreeUnguardedBase(__k, __sentinel, __comp)
{ }
Index: include/parallel/par_loop.h
===================================================================
--- include/parallel/par_loop.h (revision 173297)
+++ include/parallel/par_loop.h (working copy)
@@ -91,8 +91,7 @@
_ThreadIndex __iam = omp_get_thread_num();
// Neutral element.
- _Result* __reduct = static_cast<_Result*>
- (::operator new(sizeof(_Result)));
+ _Result* __reduct;
_DifferenceType
__start = __equally_split_point(__length, __num_threads, __iam),
@@ -100,7 +99,7 @@
if (__start < __stop)
{
- new(__reduct) _Result(__f(__o, __begin + __start));
+ __reduct = new _Result(__f(__o, __begin + __start));
++__start;
__constructed[__iam] = true;
}
@@ -110,18 +109,26 @@
for (; __start < __stop; ++__start)
*__reduct = __r(*__reduct, __f(__o, __begin + __start));
- __thread_results[__iam] = *__reduct;
+ if (__constructed[__iam])
+ {
+ ::new(&__thread_results[__iam]) _Result(*__reduct);
+ delete __reduct;
+ }
} //parallel
for (_ThreadIndex __i = 0; __i < __num_threads; ++__i)
if (__constructed[__i])
- __output = __r(__output, __thread_results[__i]);
+ {
+ __output = __r(__output, __thread_results[__i]);
+ __thread_results[__i].~_Result();
+ }
// Points to last element processed (needed as return value for
// some algorithms like transform).
__f._M_finish_iterator = __begin + __length;
- delete[] __thread_results;
+ ::operator delete(__thread_results);
+
delete[] __constructed;
return __o;
Index: include/parallel/quicksort.h
===================================================================
--- include/parallel/quicksort.h (revision 173297)
+++ include/parallel/quicksort.h (working copy)
@@ -1,6 +1,6 @@
// -*- C++ -*-
-// Copyright (C) 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+// Copyright (C) 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the terms
@@ -82,6 +82,8 @@
_DifferenceType __split = __parallel_partition(__begin, __end,
__pred, __num_threads);
+ for (_DifferenceType __s = 0; __s < __num_samples; ++__s)
+ __samples[__s].~_ValueType();
::operator delete(__samples);
return __split;
Index: include/parallel/random_shuffle.h
===================================================================
--- include/parallel/random_shuffle.h (revision 173297)
+++ include/parallel/random_shuffle.h (working copy)
@@ -1,6 +1,6 @@
// -*- C++ -*-
-// Copyright (C) 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+// Copyright (C) 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the terms
@@ -209,7 +209,7 @@
_ThreadIndex __target_p = __bin_proc[__target_bin];
// Last column [__d->_M_num_threads] stays unchanged.
- ::new(&(__temporaries[__target_p][__dist[__target_bin + 1]++]))
+ ::new(&(__temporaries[__target_p][__dist[__target_bin + 1]++]))
_ValueType(*(__source + __i + __start));
}
@@ -227,8 +227,8 @@
(__sd->_M_temporaries[__iam]
+ (__b == __d->_M_bins_begin
? 0 : __sd->_M_dist[__b][__d->_M_num_threads])),
- * __end = (__sd->_M_temporaries[__iam]
- + __sd->_M_dist[__b + 1][__d->_M_num_threads]);
+ *__end = (__sd->_M_temporaries[__iam]
+ + __sd->_M_dist[__b + 1][__d->_M_num_threads]);
__sequential_random_shuffle(__begin, __end, __rng);
std::copy(__begin, __end, __sd->_M_source + __global_offset
@@ -236,6 +236,8 @@
? 0 : __sd->_M_dist[__b][__d->_M_num_threads]));
}
+ for (_SequenceIndex __i = 0; __i < __offset; ++__i)
+ __sd->_M_temporaries[__iam][__i].~_ValueType();
::operator delete(__sd->_M_temporaries[__iam]);
}
@@ -501,6 +503,9 @@
delete[] __dist0;
delete[] __dist1;
delete[] __oracles;
+
+ for (_DifferenceType __i = 0; __i < __n; ++__i)
+ __target[__i].~_ValueType();
::operator delete(__target);
}
else
Index: include/parallel/multiway_mergesort.h
===================================================================
--- include/parallel/multiway_mergesort.h (revision 173297)
+++ include/parallel/multiway_mergesort.h (working copy)
@@ -378,6 +378,8 @@
# pragma omp barrier
+ for (_DifferenceType __i = 0; __i < __length_local; ++__i)
+ __sd->_M_temporary[__iam][__i].~_ValueType();
::operator delete(__sd->_M_temporary[__iam]);
}
@@ -413,6 +415,7 @@
// shared variables
_PMWMSSortingData<_RAIter> __sd;
_DifferenceType* __starts;
+ _DifferenceType __size;
# pragma omp parallel num_threads(__num_threads)
{
@@ -427,7 +430,7 @@
if (!__exact)
{
- _DifferenceType __size =
+ __size =
(_Settings::get().sort_mwms_oversampling * __num_threads - 1)
* __num_threads;
__sd._M_samples = static_cast<_ValueType*>
@@ -463,7 +466,11 @@
delete[] __sd._M_temporary;
if (!__exact)
- ::operator delete(__sd._M_samples);
+ {
+ for (_DifferenceType __i = 0; __i < __size; ++__i)
+ __sd._M_samples[__i].~_ValueType();
+ ::operator delete(__sd._M_samples);
+ }
delete[] __sd._M_offsets;
delete[] __sd._M_pieces;
Index: include/parallel/partial_sum.h
===================================================================
--- include/parallel/partial_sum.h (revision 173297)
+++ include/parallel/partial_sum.h (working copy)
@@ -184,7 +184,10 @@
__bin_op, __sums[__iam]);
} //parallel
+ for (_ThreadIndex __i = 0; __i < __num_threads; ++__i)
+ __sums[__i].~_ValueType();
::operator delete(__sums);
+
delete[] __borders;
return __result + __n;
Index: testsuite/26_numerics/accumulate/48750.cc
===================================================================
--- testsuite/26_numerics/accumulate/48750.cc (revision 0)
+++ testsuite/26_numerics/accumulate/48750.cc (revision 0)
@@ -0,0 +1,70 @@
+// Copyright (C) 2011 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <vector>
+#include <numeric>
+
+class NaturalParameters
+{
+public:
+
+ NaturalParameters()
+ : m_data(2)
+ { }
+
+ std::vector<double>::const_iterator
+ begin() const
+ { return m_data.begin(); }
+
+ std::vector<double>::const_iterator
+ end() const
+ { return m_data.begin(); }
+
+ NaturalParameters&
+ operator+=(const NaturalParameters&)
+ { return *this; }
+
+private:
+ std::vector<double> m_data;
+};
+
+inline
+NaturalParameters
+operator+(const NaturalParameters& a, const NaturalParameters& b)
+{
+ NaturalParameters tmp = a;
+ return tmp += b;
+}
+
+// libstdc++/48750
+void test01()
+{
+ // Used to fail in parallel-mode with a segfault.
+ for (std::size_t i = 0; i < 1000; ++i)
+ {
+ std::vector<NaturalParameters> ChildrenNP(1000);
+ NaturalParameters init;
+ NaturalParameters NP = std::accumulate(ChildrenNP.begin(),
+ ChildrenNP.end(), init);
+ }
+}
+
+int main()
+{
+ test01();
+ return 0;
+}
Index: testsuite/ext/profile/mutex_extensions_neg.cc
===================================================================
--- testsuite/ext/profile/mutex_extensions_neg.cc (revision 173297)
+++ testsuite/ext/profile/mutex_extensions_neg.cc (working copy)
@@ -3,6 +3,9 @@
// -*- C++ -*-
+// Otherwise we may get *multiple* errors.
+#undef _GLIBCXX_PARALLEL
+
// Copyright (C) 2006, 2007, 2009, 2010, 2011 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
