On 01/02/17 11:42 +0000, Jonathan Wakely wrote:
On 27/01/17 16:16 +0000, Jonathan Wakely wrote:
This implements the strong exception-safety guarantee that is required
by [string.require] p2, which the new string can fail to meet when
propagate_on_container_copy_assignment (POCCA) is true.
The solution is to define a helper that takes ownership of the
string's memory (and also the associated allocator, length and
capacity) and either deallocates it after the assignment, or swaps it
back in if an exception happens (i.e. commit or rollback).
PR libstdc++/79254
* config/abi/pre/gnu.ver: Add new symbols.
* include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI]
(basic_string::_M_copy_assign): New overloaded functions to perform
copy assignment.
(basic_string::operator=(const basic_string&)): Dispatch to
_M_copy_assign.
* include/bits/basic_string.tcc [_GLIBCXX_USE_CXX11_ABI]
(basic_string::_M_copy_assign(const basic_string&, true_type)):
Define, performing rollback on exception.
* testsuite/21_strings/basic_string/allocator/char/copy_assign.cc:
Test exception-safety guarantee.
* testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc:
Likewise.
* testsuite/util/testsuite_allocator.h (uneq_allocator::swap): Make
std::swap visible.
The backports for the branches will be a bit different, as we can't
add new exports to closed symbol versions, so I'll keep everything in
operator= instead of tag dispatching. The POCCA code path will still
be dependent on a constant expression, so should be optimized away for
most allocators.
Whlie working on the backport of this I realised the RAII
commit-and-rollback approach is a lot more complicated than simply
doing the new allocation before making any changes to *this.
Here's the backport for the branches, which shows that the new
approach is much closer to the original code and much simpler.
Tested x86_64-linux, committed to gcc-6-branch and gcc-5-branch.
commit 90d239b147e4f72e9361e10c5d45470c4b52eb7f
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Wed Feb 1 04:27:20 2017 +0000
PR libstdc++/79254 fix exception-safety of std::string copy assignment
PR libstdc++/79254
* include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI]
(basic_string::operator=(const basic_string&)): If source object is
small just deallocate, otherwise perform new allocation before
making any changes.
* testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc:
Test exception-safety of copy assignment when allocator propagates.
* testsuite/21_strings/basic_string/allocator/char/copy_assign.cc:
Likewise.
* testsuite/util/testsuite_allocator.h (uneq_allocator::swap): Make
std::swap visible.
diff --git a/libstdc++-v3/include/bits/basic_string.h
b/libstdc++-v3/include/bits/basic_string.h
index f8f3f88..0352bf4 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -570,10 +570,25 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
if (!_Alloc_traits::_S_always_equal() && !_M_is_local()
&& _M_get_allocator() != __str._M_get_allocator())
{
- // replacement allocator cannot free existing storage
- _M_destroy(_M_allocated_capacity);
- _M_data(_M_local_data());
- _M_set_length(0);
+ // Propagating allocator cannot free existing storage so must
+ // deallocate it before replacing current allocator.
+ if (__str.size() <= _S_local_capacity)
+ {
+ _M_destroy(_M_allocated_capacity);
+ _M_data(_M_local_data());
+ _M_set_length(0);
+ }
+ else
+ {
+ const auto __len = __str.size();
+ auto __alloc = __str._M_get_allocator();
+ // If this allocation throws there are no effects:
+ auto __ptr = _Alloc_traits::allocate(__alloc, __len + 1);
+ _M_destroy(_M_allocated_capacity);
+ _M_data(__ptr);
+ _M_capacity(__len);
+ _M_set_length(__len);
+ }
}
std::__alloc_on_copy(_M_get_allocator(), __str._M_get_allocator());
}
diff --git
a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc
b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc
index 3c8e440..645e3cb 100644
---
a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc
+++
b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2016 Free Software Foundation, Inc.
+// Copyright (C) 2015-2017 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
@@ -20,6 +20,7 @@
#include <string>
#include <testsuite_hooks.h>
#include <testsuite_allocator.h>
+#include <ext/throw_allocator.h>
#if _GLIBCXX_USE_CXX11_ABI
using C = char;
@@ -100,10 +101,44 @@ void test02()
VERIFY(1 == v5.get_allocator().get_personality());
}
+void test03()
+{
+ // PR libstdc++/79254
+ using throw_alloc = __gnu_cxx::throw_allocator_limit<C>;
+ typedef propagating_allocator<C, true, throw_alloc> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+ alloc_type a1(1), a2(2);
+ throw_alloc::set_limit(2); // Throw on third allocation (during assignment).
+ const C* s1 = "a string that is longer than a small string";
+ const C* s2 = "another string that is longer than a small string";
+ test_type v1(s1, a1);
+ test_type v2(s2, a2);
+ bool caught = false;
+ try {
+ v1 = v2;
+ } catch (__gnu_cxx::forced_error&) {
+ caught = true;
+ }
+ VERIFY( caught );
+ VERIFY( v1 == s1 );
+ VERIFY( v1.get_allocator() == a1 );
+
+ throw_alloc::set_limit(1); // Allow one more allocation (and no more).
+ test_type v3(s1, a1);
+ // No allocation when allocators are equal and capacity is sufficient:
+ VERIFY( v1.capacity() >= v3.size() );
+ v1 = v3;
+ // No allocation when the contents fit in the small-string buffer:
+ v2 = "sso";
+ v1 = v2;
+ VERIFY( v1.get_allocator() == a2 );
+}
+
int main()
{
test01();
test02();
+ test03();
return 0;
}
#else
diff --git
a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc
b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc
index 3ac15bb..01cdba4 100644
---
a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc
+++
b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2016 Free Software Foundation, Inc.
+// Copyright (C) 2015-2017 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
@@ -20,6 +20,7 @@
#include <string>
#include <testsuite_hooks.h>
#include <testsuite_allocator.h>
+#include <ext/throw_allocator.h>
#if _GLIBCXX_USE_CXX11_ABI
using C = wchar_t;
@@ -100,10 +101,44 @@ void test02()
VERIFY(1 == v5.get_allocator().get_personality());
}
+void test03()
+{
+ // PR libstdc++/79254
+ using throw_alloc = __gnu_cxx::throw_allocator_limit<C>;
+ typedef propagating_allocator<C, true, throw_alloc> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+ alloc_type a1(1), a2(2);
+ throw_alloc::set_limit(2); // Throw on third allocation (during assignment).
+ const C* s1 = L"a string that is longer than a small string";
+ const C* s2 = L"another string that is longer than a small string";
+ test_type v1(s1, a1);
+ test_type v2(s2, a2);
+ bool caught = false;
+ try {
+ v1 = v2;
+ } catch (__gnu_cxx::forced_error&) {
+ caught = true;
+ }
+ VERIFY( caught );
+ VERIFY( v1 == s1 );
+ VERIFY( v1.get_allocator() == a1 );
+
+ throw_alloc::set_limit(1); // Allow one more allocation (and no more).
+ test_type v3(s1, a1);
+ // No allocation when allocators are equal and capacity is sufficient:
+ VERIFY( v1.capacity() >= v3.size() );
+ v1 = v3;
+ // No allocation when the contents fit in the small-string buffer:
+ v2 = L"sso";
+ v1 = v2;
+ VERIFY( v1.get_allocator() == a2 );
+}
+
int main()
{
test01();
test02();
+ test03();
return 0;
}
#else
diff --git a/libstdc++-v3/testsuite/util/testsuite_allocator.h
b/libstdc++-v3/testsuite/util/testsuite_allocator.h
index 4884600..f597a38 100644
--- a/libstdc++-v3/testsuite/util/testsuite_allocator.h
+++ b/libstdc++-v3/testsuite/util/testsuite_allocator.h
@@ -287,7 +287,7 @@ namespace __gnu_test
Alloc& base() { return *this; }
const Alloc& base() const { return *this; }
- void swap_base(Alloc& b) { swap(b, this->base()); }
+ void swap_base(Alloc& b) { using std::swap; swap(b, this->base()); }
public:
typedef typename check_consistent_alloc_value_type<Tp, Alloc>::value_type
