diff mbox

[libstdc++/64584,libstdc++/64585] Clear basic_regex after imbue and make assign exception tolerant

Message ID CAG4ZjNkK3jqvVc5=mpwpz8+jMXVKGHSDTWwa3cJsJ0KLGw+_QA@mail.gmail.com
State New
Headers show

Commit Message

Tim Shen Jan. 15, 2015, 5:43 a.m. UTC
On Wed, Jan 14, 2015 at 8:53 PM, Tim Shen <timshen@google.com> wrote:
> I'll make a 4.9 patch later.

Here it is. :)

Bootstrapped and tested.

Comments

Jonathan Wakely Jan. 16, 2015, 12:14 p.m. UTC | #1
>@@ -675,12 +681,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	assign(const basic_string<_Ch_type, _Ch_typeraits, _Alloc>& __s,
> 	       flag_type __flags = ECMAScript)
> 	{
>+	  auto __traits = _M_traits;
>+	  auto __f = _M_flags;
> 	  _M_flags = __flags;
>-	  _M_original_str.assign(__s.begin(), __s.end());
>-	  auto __p = _M_original_str.c_str();
>-	  _M_automaton = __detail::__compile_nfa(__p,
>-						 __p + _M_original_str.size(),
>-						 _M_traits, _M_flags);
>+	  _M_traits = __traits;

What is this assignnment for?

>+	  __try
>+	    {
>+	      _M_automaton = __detail::__compile_nfa(
>+	        __s.data(), __s.data() + __s.size(), _M_traits, _M_flags);
>+	      _M_original_str = __s;
>+	    }
>+	  __catch (...)
>+	    {
>+	      _M_traits = __traits;
>+	      _M_flags = __f;
>+	      __throw_exception_again;
>+	    }
> 	  return *this;
> 	}
>
>@@ -767,8 +786,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       swap(basic_regex& __rhs)
>       {
> 	std::swap(_M_flags, __rhs._M_flags);
>-	std::swap(_M_original_str, __rhs._M_original_str);
>-	this->imbue(__rhs.imbue(this->getloc()));
>+	std::swap(_M_traits, __rhs._M_traits);
>+	auto tmp = std::move(_M_original_str);

Please rename this to __tmp to avoid user-defined macros.

>+	this->assign(__rhs._M_original_str, _M_flags);
>+	__rhs.assign(tmp, __rhs._M_flags);
>       }
>
> #ifdef _GLIBCXX_DEBUG
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h
index 3cbec3c..efb38e5 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -476,7 +476,10 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        */
       basic_regex(const basic_regex& __rhs)
       : _M_flags(__rhs._M_flags), _M_original_str(__rhs._M_original_str)
-      { this->imbue(__rhs.getloc()); }
+      {
+	_M_traits.imbue(__rhs.getloc());
+	this->assign(_M_original_str, _M_flags);
+      }
 
       /**
        * @brief Move-constructs a basic regular expression.
@@ -490,7 +493,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : _M_flags(__rhs._M_flags),
       _M_original_str(std::move(__rhs._M_original_str))
       {
-	this->imbue(__rhs.getloc());
+	_M_traits.imbue(__rhs.getloc());
+	this->assign(_M_original_str, _M_flags);
 	__rhs._M_automaton.reset();
       }
 
@@ -604,7 +608,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
 	_M_flags = __rhs._M_flags;
 	_M_original_str = __rhs._M_original_str;
-	this->imbue(__rhs.getloc());
+	_M_traits.imbue(__rhs.getloc());
+	this->assign(_M_original_str, _M_flags);
 	return *this;
       }
 
@@ -622,7 +627,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_flags = __rhs._M_flags;
 	_M_original_str = std::move(__rhs._M_original_str);
 	__rhs._M_automaton.reset();
-	this->imbue(__rhs.getloc());
+	_M_traits.imbue(__rhs.getloc());
+	this->assign(_M_original_str, _M_flags);
       }
 
       /**
@@ -675,12 +681,22 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	assign(const basic_string<_Ch_type, _Ch_typeraits, _Alloc>& __s,
 	       flag_type __flags = ECMAScript)
 	{
+	  auto __traits = _M_traits;
+	  auto __f = _M_flags;
 	  _M_flags = __flags;
-	  _M_original_str.assign(__s.begin(), __s.end());
-	  auto __p = _M_original_str.c_str();
-	  _M_automaton = __detail::__compile_nfa(__p,
-						 __p + _M_original_str.size(),
-						 _M_traits, _M_flags);
+	  _M_traits = __traits;
+	  __try
+	    {
+	      _M_automaton = __detail::__compile_nfa(
+	        __s.data(), __s.data() + __s.size(), _M_traits, _M_flags);
+	      _M_original_str = __s;
+	    }
+	  __catch (...)
+	    {
+	      _M_traits = __traits;
+	      _M_flags = __f;
+	      __throw_exception_again;
+	    }
 	  return *this;
 	}
 
@@ -725,7 +741,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        */
       unsigned int
       mark_count() const
-      { return _M_automaton->_M_sub_count() - 1; }
+      {
+	if (_M_automaton)
+	  return _M_automaton->_M_sub_count() - 1;
+	return 0;
+      }
 
       /**
        * @brief Gets the flags used to construct the regular expression
@@ -744,9 +764,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       locale_type
       imbue(locale_type __loc)
       {
-	auto __ret = _M_traits.imbue(__loc);
-	this->assign(_M_original_str, _M_flags);
-	return __ret;
+	_M_automaton = nullptr;
+	return _M_traits.imbue(__loc);
       }
 
       /**
@@ -767,8 +786,10 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       swap(basic_regex& __rhs)
       {
 	std::swap(_M_flags, __rhs._M_flags);
-	std::swap(_M_original_str, __rhs._M_original_str);
-	this->imbue(__rhs.imbue(this->getloc()));
+	std::swap(_M_traits, __rhs._M_traits);
+	auto tmp = std::move(_M_original_str);
+	this->assign(__rhs._M_original_str, _M_flags);
+	__rhs.assign(tmp, __rhs._M_flags);
       }
 
 #ifdef _GLIBCXX_DEBUG
@@ -777,7 +798,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       { _M_automaton->_M_dot(__ostr); }
 #endif
 
-    protected:
+    private:
       typedef std::shared_ptr<__detail::_NFA<_Rx_traits>> _AutomatonPtr;
 
       template<typename _Bp, typename _Ap, typename _Cp, typename _Rp,
diff --git a/libstdc++-v3/testsuite/28_regex/basic_regex/assign/char/string.cc b/libstdc++-v3/testsuite/28_regex/basic_regex/assign/char/string.cc
index 0a32ab5..7d4db8f1 100644
--- a/libstdc++-v3/testsuite/28_regex/basic_regex/assign/char/string.cc
+++ b/libstdc++-v3/testsuite/28_regex/basic_regex/assign/char/string.cc
@@ -1,4 +1,3 @@ 
-// { dg-do compile }
 // { dg-options "-std=gnu++0x" }
 
 // 2007-03-12  Stephen M. Webb  <stephen.webb@bregmasoft.com>
@@ -29,6 +28,7 @@ 
 // Tests C++ string assignment of the basic_regex class.  
 void test01()
 {
+  bool test __attribute__((unused)) = true;
   typedef std::basic_regex<char> test_type;
 
   std::string s("a*b");
@@ -36,9 +36,27 @@  void test01()
   re.assign(s);
 }
 
+// libstdc++/64584
+void test02()
+{
+  bool test __attribute__((unused)) = true;
+  std::regex re("", std::regex_constants::extended);
+  auto flags = re.flags();
+  try
+    {
+      re.assign("(", std::regex_constants::icase);
+      VERIFY(false);
+    }
+  catch (const std::regex_error& e)
+    {
+      VERIFY(flags == re.flags());
+    }
+}
+
 int
 main()
 { 
   test01();
+  test02();
   return 0;
 }
diff --git a/libstdc++-v3/testsuite/28_regex/basic_regex/imbue/string.cc b/libstdc++-v3/testsuite/28_regex/basic_regex/imbue/string.cc
new file mode 100644
index 0000000..d4d4f47
--- /dev/null
+++ b/libstdc++-v3/testsuite/28_regex/basic_regex/imbue/string.cc
@@ -0,0 +1,44 @@ 
+// { dg-options "-std=gnu++11" }
+
+// Copyright (C) 2015 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/>.
+
+// [28.8.5] class template basic_regex locale
+
+#include <string>
+#include <regex>
+#include <testsuite_hooks.h>
+
+// libstdc++/64585
+void test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  static const char s[] = "a";
+  std::regex re("a");
+  VERIFY(std::regex_search(s, re));
+
+  auto loc = re.imbue(re.getloc());
+  VERIFY(!std::regex_search(s, re));
+}
+
+int
+main()
+{
+  test01();
+  return 0;
+}