public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe.
@ 2015-11-14 19:45 Torvald Riegel
  2015-12-16 21:06 ` Jonathan Wakely
  2015-12-17 20:04 ` Jonathan Wakely
  0 siblings, 2 replies; 7+ messages in thread
From: Torvald Riegel @ 2015-11-14 19:45 UTC (permalink / raw)
  To: libstdc++, GCC Patches; +Cc: Jonathan Wakely, Jason Merrill

[-- Attachment #1: Type: text/plain, Size: 2300 bytes --]

The attached patch makes some exceptions transaction-safe, as require by
the Transactional Memory TS.  It has some rough edges, but I'm hoping I
can sort them out quickly using your feedback.  It only supports
logic_error and exception/bad_exception, but the other exceptions that
the TM TS specifies as transaction-safe basically require the same
support (notably, runtime_error is the same as logic_error, and their
subclasses don't add anything).

There are two things that complicate such support.  First, it seems
better to not rely on -fgnu-tm of libstdc++ code for now (or at least we
tried to avoid this so far).  Therefore, the transactional clones in
this patch are all manually instrumented (ie, the functions are C
functions with names matching the mangled names of the respective C++
functions, and the _ZGTt prefix signaling that they are txnal clones).

Second, exceptions still use a COW string internally, which cannot be
made transaction-safe with just compiler support because of the
reference counting implementation inside of COW strings, which uses
atomics.  One would need something custom for that nonetheless.

Thus, the patch adds txnal clones as required.  They are new exported
symbols, but not visible to nontransactional code.  The only changes to
headers is transaction_safe[_dynamic] annotations where required by the
TS, and a few friend declarations.  The annotaitons are only enabled if
a user compiles with -fgnu-tm.  IOW, the changes are pretty much
invisible when not using the TM TS.

The patch doesn't include tests yet, but tests like this one seem to run
successfully on x86_64:

template<typename T> void thrower4(const string& what)
{
  try
    {
      // Creating a temporary inside of the txn ICEs.
      T t(what);
      atomic_commit
      {
	_ITM_hackOrTxnProp (pr_atomicCancel);
	dontoptimize (t.what());
	throw T (what);
      }
    }
  catch (T ex)
    {
      if (what != ex.what()) abort ();
    }
}
thrower4<logic_error> ("test");

I can't yet test the destructors because of issues on the compiler side.
There are also commented-out calls to _ITM_setAssociatedException in the
code, which exist to show how we plan to support transaction
cancellation through exceptions (which needs some more libitm support
and bugfixes on the compiler side).

[-- Attachment #2: txn-safe-exceptions.patch --]
[-- Type: text/x-patch, Size: 16051 bytes --]

commit e81080a01ab0daf2949a400c1a2d5077d37ba515
Author: Torvald Riegel <triegel@redhat.com>
Date:   Fri Nov 13 01:00:52 2015 +0100

    libstdc++: Make certain exceptions transaction_safe.

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 1b3184a..d902b03 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1876,6 +1876,12 @@ GLIBCXX_3.4.22 {
     _ZNSt6thread6_StateD[012]Ev;
     _ZNSt6thread15_M_start_threadESt10unique_ptrINS_6_StateESt14default_deleteIS1_EEPFvvE;
 
+    # Support for the Transactional Memory TS (N4514)
+    _ZGTtNSt11logic_errorC1EPKc;
+    _ZGTtNSt11logic_errorC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNKSt11logic_error4whatEv;
+    _ZGTtNSt11logic_errorD1Ev;
+
 } GLIBCXX_3.4.21;
 
 # Symbols in the support library (libsupc++) have their own tag.
@@ -2107,6 +2113,12 @@ CXXABI_1.3.9 {
     # operator delete[](void*, std::size_t)
     _ZdaPv[jmy];
 
+    # Support for the Transactional Memory TS (N4514)
+    _ZGTtNKSt9exceptionD1Ev;
+    _ZGTtNKSt9exception4whatEv;
+    _ZGTtNKSt13bad_exceptionD1Ev;
+    _ZGTtNKSt13bad_exception4whatEv;
+
 } CXXABI_1.3.8;
 
 # Symbols in the support library (libsupc++) supporting transactional memory.
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index b3853cd..47d9f2f 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -4904,6 +4904,18 @@ _GLIBCXX_END_NAMESPACE_CXX11
       int
       compare(size_type __pos, size_type __n1, const _CharT* __s,
 	      size_type __n2) const;
+
+# ifdef _GLIBCXX_TM_TS_INTERNAL
+      friend void
+      ::_txnal_cow_string_C1_for_exceptions(void* that, const char* s,
+					    void* exc);
+      friend const char*
+      ::_txnal_cow_string_c_str(const void *that);
+      friend void
+      ::_txnal_cow_string_D1(void *that);
+      friend void
+      ::_txnal_cow_string_D1_commit(void *that);
+# endif
   };
 #endif  // !_GLIBCXX_USE_CXX11_ABI
 
diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 723feb1..0e66bb0 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -481,6 +481,17 @@ namespace std
 # define _GLIBCXX_BEGIN_EXTERN_C extern "C" {
 # define _GLIBCXX_END_EXTERN_C }
 
+// Conditionally enable annotations for the Transactional Memory TS on C++11.
+#if __cplusplus >= 201103L && \
+  _GLIBCXX_USE_CXX11_ABI && _GLIBCXX_USE_DUAL_ABI && \
+  defined(__cpp_transactional_memory) && __cpp_transactional_memory >= 201505L
+#define _GLIBCXX_TXN_SAFE transaction_safe
+#define _GLIBCXX_TXN_SAFE_DYN transaction_safe_dynamic
+#else
+#define _GLIBCXX_TXN_SAFE
+#define _GLIBCXX_TXN_SAFE_DYN
+#endif
+
 #else // !__cplusplus
 # define _GLIBCXX_BEGIN_EXTERN_C
 # define _GLIBCXX_END_EXTERN_C
diff --git a/libstdc++-v3/include/std/stdexcept b/libstdc++-v3/include/std/stdexcept
index 2428919..620a2f4 100644
--- a/libstdc++-v3/include/std/stdexcept
+++ b/libstdc++-v3/include/std/stdexcept
@@ -117,11 +117,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   public:
     /** Takes a character string describing the error.  */
     explicit 
-    logic_error(const string& __arg);
+    logic_error(const string& __arg) _GLIBCXX_TXN_SAFE;
 
 #if __cplusplus >= 201103L
     explicit
-    logic_error(const char*);
+    logic_error(const char*) _GLIBCXX_TXN_SAFE;
 #endif
 
 #if _GLIBCXX_USE_CXX11_ABI || _GLIBCXX_DEFINE_STDEXCEPT_COPY_OPS
@@ -129,12 +129,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     logic_error& operator=(const logic_error&) _GLIBCXX_USE_NOEXCEPT;
 #endif
 
-    virtual ~logic_error() _GLIBCXX_USE_NOEXCEPT;
+    virtual ~logic_error() _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT;
 
     /** Returns a C-style character string describing the general cause of
      *  the current error (the same string passed to the ctor).  */
     virtual const char* 
-    what() const _GLIBCXX_USE_NOEXCEPT;
+    what() const _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT;
+# ifdef _GLIBCXX_TM_TS_INTERNAL
+    friend void*
+    ::_txnal_logic_error_get_msg(void* e);
+# endif
   };
 
   /** Thrown by the library, or by you, to report domain errors (domain in
diff --git a/libstdc++-v3/libsupc++/eh_exception.cc b/libstdc++-v3/libsupc++/eh_exception.cc
index 49f4632..cce813c 100644
--- a/libstdc++-v3/libsupc++/eh_exception.cc
+++ b/libstdc++-v3/libsupc++/eh_exception.cc
@@ -26,16 +26,18 @@
 #include "exception"
 #include <cxxabi.h>
 
-std::exception::~exception() _GLIBCXX_USE_NOEXCEPT { }
+std::exception::~exception() _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT { }
 
-std::bad_exception::~bad_exception() _GLIBCXX_USE_NOEXCEPT { }
+std::bad_exception::~bad_exception() _GLIBCXX_TXN_SAFE_DYN
+    _GLIBCXX_USE_NOEXCEPT
+{ }
 
 abi::__forced_unwind::~__forced_unwind() throw() { }
 
 abi::__foreign_exception::~__foreign_exception() throw() { }
 
 const char* 
-std::exception::what() const _GLIBCXX_USE_NOEXCEPT
+std::exception::what() const _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT
 {
   // NB: Another elegant option would be returning typeid(*this).name()
   // and not overriding what() in bad_exception, bad_alloc, etc.  In
@@ -44,7 +46,36 @@ std::exception::what() const _GLIBCXX_USE_NOEXCEPT
 }
 
 const char* 
-std::bad_exception::what() const _GLIBCXX_USE_NOEXCEPT
+std::bad_exception::what() const _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT
 {
   return "std::bad_exception";
 }
+
+// Transactional clones for the destructors and what().
+// what() is effectively transaction_pure, but we do not want to annotate it
+// as such; thus, we call exactly the respective nontransactional function.
+extern "C" {
+
+void
+_ZGTtNKSt9exceptionD1Ev(const std::exception*)
+{ }
+
+const char*
+_ZGTtNKSt9exception4whatEv(const std::exception* that)
+{
+  return that->std::exception::what();
+}
+
+void
+_ZGTtNKSt13bad_exceptionD1Ev(
+    const std::bad_exception*)
+{ }
+
+const char*
+_ZGTtNKSt13bad_exception4whatEv(
+    const std::bad_exception* that)
+{
+  return that->std::bad_exception::what();
+}
+
+}
diff --git a/libstdc++-v3/libsupc++/exception b/libstdc++-v3/libsupc++/exception
index 5571fd9..e61975c 100644
--- a/libstdc++-v3/libsupc++/exception
+++ b/libstdc++-v3/libsupc++/exception
@@ -61,11 +61,12 @@ namespace std
   {
   public:
     exception() _GLIBCXX_USE_NOEXCEPT { }
-    virtual ~exception() _GLIBCXX_USE_NOEXCEPT;
+    virtual ~exception() _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT;
 
     /** Returns a C-style character string describing the general cause
      *  of the current error.  */
-    virtual const char* what() const _GLIBCXX_USE_NOEXCEPT;
+    virtual const char* what() const _GLIBCXX_TXN_SAFE_DYN
+	_GLIBCXX_USE_NOEXCEPT;
   };
 
   /** If an %exception is thrown which is not listed in a function's
@@ -77,10 +78,11 @@ namespace std
 
     // This declaration is not useless:
     // http://gcc.gnu.org/onlinedocs/gcc-3.0.2/gcc_6.html#SEC118
-    virtual ~bad_exception() _GLIBCXX_USE_NOEXCEPT;
+    virtual ~bad_exception() _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT;
 
     // See comment in eh_exception.cc.
-    virtual const char* what() const _GLIBCXX_USE_NOEXCEPT;
+    virtual const char* what() const _GLIBCXX_TXN_SAFE_DYN
+	_GLIBCXX_USE_NOEXCEPT;
   };
 
   /// If you write a replacement %terminate handler, it must be of this type.
diff --git a/libstdc++-v3/src/c++11/cow-stdexcept.cc b/libstdc++-v3/src/c++11/cow-stdexcept.cc
index d24dfe6..c14d5af 100644
--- a/libstdc++-v3/src/c++11/cow-stdexcept.cc
+++ b/libstdc++-v3/src/c++11/cow-stdexcept.cc
@@ -26,6 +26,19 @@
 // ISO C++ 14882: 19.1  Exception classes
 //
 
+// Enable hooks for support for the Transactional Memory TS (N4514).
+#define _GLIBCXX_TM_TS_INTERNAL
+void
+_txnal_cow_string_C1_for_exceptions(void* that, const char* s, void *exc);
+const char*
+_txnal_cow_string_c_str(const void *that);
+void
+_txnal_cow_string_D1(void *that);
+void
+_txnal_cow_string_D1_commit(void *that);
+void*
+_txnal_logic_error_get_msg(void* e);
+
 // All exception classes still use the classic COW std::string.
 #define _GLIBCXX_USE_CXX11_ABI 0
 #define _GLIBCXX_DEFINE_STDEXCEPT_COPY_OPS 1
@@ -151,3 +164,220 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
+
+// Support for the Transactional Memory TS (N4514).
+//
+// logic_error and runtime_error both carry a message in the form of a COW
+// string.  This COW string is never made visible to users of the exception
+// because what() returns a C string.  The COW string can be constructed as
+// either a copy of a COW string of another logic_error/runtime_error, or
+// using a C string or SSO string; thus, the COW string's _Rep is only
+// accessed by logic_error operations.  We control all txnal clones of those
+// operations and thus can ensure that _Rep is never accessed transactionally.
+// Furthermore, _Rep will always have been allocated or deallocated via
+// global new or delete, so nontransactional writes we do to _Rep cannot
+// interfere with transactional accesses.
+extern "C" {
+
+// Declare all libitm symbols we rely on, but make them weak so that we do
+// not depend on libitm.
+#define WEAK  __attribute__((weak))
+
+// FIXME copy over libitm's configury for MANGLE_SIZE_T?
+#define MANGLE_SIZE_T m
+#define CONCAT1(x,y)		x##y
+#define CONCAT(x,y)		CONCAT1(x,y)
+#define _ZGTtnaX		CONCAT(_ZGTtna,MANGLE_SIZE_T)
+
+#ifdef __i386__
+/* Only for 32-bit x86.  */
+# define ITM_REGPARM	__attribute__((regparm(2)))
+#else
+# define ITM_REGPARM
+#endif
+
+extern void *_ZGTtnaX (size_t sz)
+  _GLIBCXX_WEAK_DEFINITION;
+extern uint8_t _ITM_RU1(const uint8_t *p)
+  ITM_REGPARM _GLIBCXX_WEAK_DEFINITION;
+extern uint32_t _ITM_RU4(const uint32_t *p)
+  ITM_REGPARM _GLIBCXX_WEAK_DEFINITION;
+extern uint64_t _ITM_RU8(const uint64_t *p)
+  ITM_REGPARM _GLIBCXX_WEAK_DEFINITION;
+extern void _ITM_memcpyRtWn(void *, const void *, size_t)
+  ITM_REGPARM _GLIBCXX_WEAK_DEFINITION;
+extern void _ITM_memcpyRnWt(void *, const void *, size_t)
+  ITM_REGPARM _GLIBCXX_WEAK_DEFINITION;
+extern void _ITM_addUserCommitAction(void (*)(void *), uint64_t, void *)
+  ITM_REGPARM _GLIBCXX_WEAK_DEFINITION;
+
+// If there is no support for weak, create dummies.
+// FIXME really needed for libstdc++?
+//#if !defined (HAVE_ELF_STYLE_WEAKREF)
+//void *_ZGTtnaX (size_t) { return NULL; }
+//uint8_t _ITM_RU1(const uint8_t *) { return 0; }
+//uint32_t _ITM_RU4(const uint32_t *) { return 0; }
+//uint64_t _ITM_RU8(const uint64_t *) { return 0; }
+//void _ITM_memcpyRtWn(void *, const void *, size_t) { }
+//void _ITM_memcpyRnWt(void *, const void *, size_t) { }
+//void _ITM_addUserCommitAction(void (*)(void *), uint64_t, void *) { };
+//#endif
+
+}
+
+// A transactional version of basic_string::basic_string(const char *s)
+// that also notifies the TM runtime about allocations belonging to this
+// exception.
+void
+_txnal_cow_string_C1_for_exceptions(void* that, const char* s, void *exc)
+{
+  typedef std::basic_string<char> bs_type;
+  bs_type *bs = (bs_type*) that;
+
+  // First, do a transactional strlen, but including the trailing zero.
+  bs_type::size_type len = 1;
+  for (const char *ss = s; _ITM_RU1((const uint8_t*) ss) != 0; ss++, len++);
+
+
+  // Allocate memory for the string and the refcount.  We use the
+  // transactional clone of global new[]; if this throws, it will do so in a
+  // transaction-compatible way.
+  // The allocation belongs to this exception, so tell the runtime about it.
+  // TODO associate exception
+//  void *prev = _ITM_setAssociatedException(exc);
+  bs_type::_Rep *rep;
+  try
+    {
+      rep = (bs_type::_Rep*) _ZGTtnaX (len + sizeof (bs_type::_Rep));
+    }
+  catch (...)
+    {
+      // Pop the association with this exception.
+//      _ITM_setAssociatedException(prev);
+      // We do not need to instrument a rethrow.
+      throw;
+    }
+  // Pop the association with this exception.
+//  _ITM_setAssociatedException(prev);
+
+  // Now initialize the rest of the string and copy the C string.  The memory
+  // will be freshly allocated, so nontransactional accesses are sufficient,
+  // including the writes when copying the string (see above).
+  rep->_M_set_sharable();
+  rep->_M_length = rep->_M_capacity = len - 1;
+  _ITM_memcpyRtWn(rep->_M_refdata(), s, len);
+  new (&bs->_M_dataplus) bs_type::_Alloc_hider(rep->_M_refdata(),
+					       bs_type::allocator_type());
+}
+
+static void* txnal_read_ptr(void* const * ptr)
+{
+  static_assert(sizeof(uint64_t) == sizeof(void*)
+		|| sizeof(uint32_t) == sizeof(void*));
+  // FIXME make a true compile-time choice to prevent warnings.
+  if (sizeof(uint64_t)== sizeof(void*))
+    return (void*)_ITM_RU8((const uint64_t*)ptr);
+  else
+    return (void*)_ITM_RU4((const uint32_t*)ptr);
+}
+
+// We must access the data pointer in the COW string transactionally because
+// another transaction can delete the string and reuse the memory.
+const char*
+_txnal_cow_string_c_str(const void *that)
+{
+  typedef std::basic_string<char> bs_type;
+  const bs_type *bs = (const bs_type*) that;
+
+  return (const char*) txnal_read_ptr((void**)&bs->_M_dataplus._M_p);
+}
+
+const char*
+_txnal_sso_string_c_str(const void *that)
+{
+  return (const char*) txnal_read_ptr(
+      (void* const*)const_cast<char* const*>(
+	  &((const std::__sso_string*) that)->_M_s._M_p));
+}
+
+void
+_txnal_cow_string_D1_commit(void *data)
+{
+  typedef std::basic_string<char> bs_type;
+  bs_type::_Rep *rep = (bs_type::_Rep*) data;
+  rep->_M_dispose(bs_type::allocator_type());
+}
+
+void
+_txnal_cow_string_D1(void *that)
+{
+  typedef std::basic_string<char> bs_type;
+  bs_type::_Rep *rep = reinterpret_cast<bs_type::_Rep*>(
+      const_cast<char*>(_txnal_cow_string_c_str(that))) - 1;
+
+  // The string can be shared, in which case we would need to decrement the
+  // reference count.  We cannot undo that because we might loose the string
+  // otherwise.  Therefore, we register a commit action that will dispose of
+  // the string's _Rep.
+  enum {_ITM_noTransactionId  = 1};
+  _ITM_addUserCommitAction(_txnal_cow_string_D1_commit, _ITM_noTransactionId,
+			   rep);
+}
+
+void*
+_txnal_logic_error_get_msg(void* e)
+{
+  std::logic_error* le = (std::logic_error*) e;
+  return &le->_M_msg;
+}
+
+extern "C" {
+
+void
+_ZGTtNSt11logic_errorC1EPKc (std::logic_error *that, const char* s)
+{
+  // This will use the singleton _Rep for an empty string and just point
+  // to it instead of allocating memory.  Thus, we can use it as source, copy
+  // it into the object we are constructing, and then construct the COW string
+  // in the latter manually.
+  std::logic_error le("");
+  _ITM_memcpyRnWt(that, &le, sizeof(std::logic_error));
+  _txnal_cow_string_C1_for_exceptions(_txnal_logic_error_get_msg(that),
+				      s, that);
+}
+// TODO C2 and C3?
+
+void
+_ZGTtNSt11logic_errorC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE(
+    std::logic_error *that, const std::__sso_string& s)
+{
+  std::logic_error le("");
+  _ITM_memcpyRnWt(that, &le, sizeof(std::logic_error));
+  // This constructor is only declared transaction-safe if the C++11 ABI is
+  // used for std::string yet logic_error used a COW string internally.  A
+  // user must not call this constructor otherwise; although we can still
+  // compile and provide it, calling it would result in undefined behavior,
+  // which is in this case not initializing the message.
+#if _GLIBCXX_USE_DUAL_ABI
+  // Get the C string from the SSO string.
+  _txnal_cow_string_C1_for_exceptions(_txnal_logic_error_get_msg(that),
+				      _txnal_sso_string_c_str(&s), that);
+#endif
+}
+// TODO C2 and C3?
+
+const char*
+_ZGTtNKSt11logic_error4whatEv(const std::logic_error* that)
+{
+  return _txnal_cow_string_c_str(_txnal_logic_error_get_msg(
+      const_cast<std::logic_error*>(that)));
+}
+
+void
+_ZGTtNSt11logic_errorD1Ev(std::logic_error *that)
+{
+  _txnal_cow_string_D1(_txnal_logic_error_get_msg(that));
+}
+// TODO D0 and D2?
+
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe.
  2015-11-14 19:45 [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe Torvald Riegel
@ 2015-12-16 21:06 ` Jonathan Wakely
  2015-12-23 17:35   ` Torvald Riegel
  2015-12-17 20:04 ` Jonathan Wakely
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2015-12-16 21:06 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: libstdc++, GCC Patches, Jason Merrill

Sorry for the delay finishing this review, some of the code kept
melting my brain ;-)


On 14/11/15 20:45 +0100, Torvald Riegel wrote:
>diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
>index 1b3184a..d902b03 100644
>--- a/libstdc++-v3/config/abi/pre/gnu.ver
>+++ b/libstdc++-v3/config/abi/pre/gnu.ver
>@@ -1876,6 +1876,12 @@ GLIBCXX_3.4.22 {
>     _ZNSt6thread6_StateD[012]Ev;
>     _ZNSt6thread15_M_start_threadESt10unique_ptrINS_6_StateESt14default_deleteIS1_EEPFvvE;
> 
>+    # Support for the Transactional Memory TS (N4514)
>+    _ZGTtNSt11logic_errorC1EPKc;
>+    _ZGTtNSt11logic_errorC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
>+    _ZGTtNKSt11logic_error4whatEv;
>+    _ZGTtNSt11logic_errorD1Ev;
>+
> } GLIBCXX_3.4.21;

This is OK but ...

> # Symbols in the support library (libsupc++) have their own tag.
>@@ -2107,6 +2113,12 @@ CXXABI_1.3.9 {
>     # operator delete[](void*, std::size_t)
>     _ZdaPv[jmy];
> 
>+    # Support for the Transactional Memory TS (N4514)
>+    _ZGTtNKSt9exceptionD1Ev;
>+    _ZGTtNKSt9exception4whatEv;
>+    _ZGTtNKSt13bad_exceptionD1Ev;
>+    _ZGTtNKSt13bad_exception4whatEv;
>+
> } CXXABI_1.3.8;

That symbol version was already used in the gcc-5 release and so is
frozen, you'll need CXXABI_1.3.10 for these new symbols (similar to
https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00258.html so if
Catherine's already added that version you can just add them there).


>diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
>index 723feb1..0e66bb0 100644
>--- a/libstdc++-v3/include/bits/c++config
>+++ b/libstdc++-v3/include/bits/c++config
>@@ -481,6 +481,17 @@ namespace std
> # define _GLIBCXX_BEGIN_EXTERN_C extern "C" {
> # define _GLIBCXX_END_EXTERN_C }
> 
>+// Conditionally enable annotations for the Transactional Memory TS on C++11.
>+#if __cplusplus >= 201103L && \
>+  _GLIBCXX_USE_CXX11_ABI && _GLIBCXX_USE_DUAL_ABI && \

It's possible we can make this work for the old ABI too, but this is
OK for now. The old ABI always uses COW strings, but that's what the
code you've written deals with anyway.

>+  defined(__cpp_transactional_memory) && __cpp_transactional_memory >= 201505L

The defined(__cpp_transactional_memory) check is redundant, isn't it?

Users aren't allowed to define it, so it will either be defined to an
integer value or undefined and evaluate to zero.

>+#define _GLIBCXX_TXN_SAFE transaction_safe
>+#define _GLIBCXX_TXN_SAFE_DYN transaction_safe_dynamic
>+#else
>+#define _GLIBCXX_TXN_SAFE
>+#define _GLIBCXX_TXN_SAFE_DYN
>+#endif
>+
> #else // !__cplusplus
> # define _GLIBCXX_BEGIN_EXTERN_C
> # define _GLIBCXX_END_EXTERN_C


>@@ -44,7 +46,36 @@ std::exception::what() const _GLIBCXX_USE_NOEXCEPT
> }
> 
> const char* 
>-std::bad_exception::what() const _GLIBCXX_USE_NOEXCEPT
>+std::bad_exception::what() const _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT
> {
>   return "std::bad_exception";
> }
>+
>+// Transactional clones for the destructors and what().
>+// what() is effectively transaction_pure, but we do not want to annotate it
>+// as such; thus, we call exactly the respective nontransactional function.
>+extern "C" {
>+
>+void
>+_ZGTtNKSt9exceptionD1Ev(const std::exception*)
>+{ }
>+
>+const char*
>+_ZGTtNKSt9exception4whatEv(const std::exception* that)
>+{
>+  return that->std::exception::what();
>+}

This makes a non-virtual call, is that correct?

If users derive from std::exception and override what() they will
expect a call to what() to dispatch to their override in the derived
class, but IIUC in a transactional block they would call this
function, which would call the base what(), not their override.

>+_ZGTtNKSt13bad_exception4whatEv(
>+    const std::bad_exception* that)
>+{
>+  return that->std::bad_exception::what();
>+}

Ditto.

>@@ -151,3 +164,220 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace
>+
>+// Support for the Transactional Memory TS (N4514).
>+//
>+// logic_error and runtime_error both carry a message in the form of a COW
>+// string.  This COW string is never made visible to users of the exception
>+// because what() returns a C string.  The COW string can be constructed as
>+// either a copy of a COW string of another logic_error/runtime_error, or
>+// using a C string or SSO string; thus, the COW string's _Rep is only
>+// accessed by logic_error operations.  We control all txnal clones of those
>+// operations and thus can ensure that _Rep is never accessed transactionally.
>+// Furthermore, _Rep will always have been allocated or deallocated via
>+// global new or delete, so nontransactional writes we do to _Rep cannot

Hmm, will it always be global new/delete? It uses std::allocator,
which by default uses new/delete but libstdc++ can be configured to
use a different std::allocator implementation. If they always use new
at some point maybe we're OK, but I'd have to check the alternative
allocators. Maybe we just say only new_allocator is supported for TM.

I assume we want to avoid making a txnal std::allocator.

>+extern "C" {
>+
>+void
>+_ZGTtNSt11logic_errorC1EPKc (std::logic_error *that, const char* s)
>+{
>+  // This will use the singleton _Rep for an empty string and just point
>+  // to it instead of allocating memory.  Thus, we can use it as source, copy
>+  // it into the object we are constructing, and then construct the COW string
>+  // in the latter manually.
>+  std::logic_error le("");
>+  _ITM_memcpyRnWt(that, &le, sizeof(std::logic_error));
>+  _txnal_cow_string_C1_for_exceptions(_txnal_logic_error_get_msg(that),
>+				      s, that);

The shared empty _Rep is also only used conditionally, it can be
disabled with --enable-fully-dynamic-string. Maybe another thing we
just don't deal with for now.

Otherwise this looks good to me.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe.
  2015-11-14 19:45 [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe Torvald Riegel
  2015-12-16 21:06 ` Jonathan Wakely
@ 2015-12-17 20:04 ` Jonathan Wakely
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2015-12-17 20:04 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: libstdc++, GCC Patches, Jason Merrill

On 14/11/15 20:45 +0100, Torvald Riegel wrote:
>+void
>+_txnal_cow_string_D1(void *that)
>+{
>+  typedef std::basic_string<char> bs_type;
>+  bs_type::_Rep *rep = reinterpret_cast<bs_type::_Rep*>(
>+      const_cast<char*>(_txnal_cow_string_c_str(that))) - 1;
>+
>+  // The string can be shared, in which case we would need to decrement the
>+  // reference count.  We cannot undo that because we might loose the string
>+  // otherwise.  Therefore, we register a commit action that will dispose of
>+  // the string's _Rep.
>+  enum {_ITM_noTransactionId  = 1};
>+  _ITM_addUserCommitAction(_txnal_cow_string_D1_commit, _ITM_noTransactionId,
>+			   rep);
>+}

s/loose/lose/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe.
  2015-12-16 21:06 ` Jonathan Wakely
@ 2015-12-23 17:35   ` Torvald Riegel
  2015-12-23 20:56     ` Jason Merrill
  2015-12-24 13:34     ` Jonathan Wakely
  0 siblings, 2 replies; 7+ messages in thread
From: Torvald Riegel @ 2015-12-23 17:35 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, GCC Patches, Jason Merrill

[-- Attachment #1: Type: text/plain, Size: 9759 bytes --]

On Wed, 2015-12-16 at 21:05 +0000, Jonathan Wakely wrote:
> Sorry for the delay finishing this review, some of the code kept
> melting my brain ;-)

I know what you mean :)  Thanks for the review!

> On 14/11/15 20:45 +0100, Torvald Riegel wrote:
> >diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
> >index 1b3184a..d902b03 100644
> >--- a/libstdc++-v3/config/abi/pre/gnu.ver
> >+++ b/libstdc++-v3/config/abi/pre/gnu.ver
> >@@ -1876,6 +1876,12 @@ GLIBCXX_3.4.22 {
> >     _ZNSt6thread6_StateD[012]Ev;
> >     _ZNSt6thread15_M_start_threadESt10unique_ptrINS_6_StateESt14default_deleteIS1_EEPFvvE;
> > 
> >+    # Support for the Transactional Memory TS (N4514)
> >+    _ZGTtNSt11logic_errorC1EPKc;
> >+    _ZGTtNSt11logic_errorC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
> >+    _ZGTtNKSt11logic_error4whatEv;
> >+    _ZGTtNSt11logic_errorD1Ev;
> >+
> > } GLIBCXX_3.4.21;
> 
> This is OK but ...
> 
> > # Symbols in the support library (libsupc++) have their own tag.
> >@@ -2107,6 +2113,12 @@ CXXABI_1.3.9 {
> >     # operator delete[](void*, std::size_t)
> >     _ZdaPv[jmy];
> > 
> >+    # Support for the Transactional Memory TS (N4514)
> >+    _ZGTtNKSt9exceptionD1Ev;
> >+    _ZGTtNKSt9exception4whatEv;
> >+    _ZGTtNKSt13bad_exceptionD1Ev;
> >+    _ZGTtNKSt13bad_exception4whatEv;
> >+
> > } CXXABI_1.3.8;
> 
> That symbol version was already used in the gcc-5 release and so is
> frozen, you'll need CXXABI_1.3.10 for these new symbols (similar to
> https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00258.html so if
> Catherine's already added that version you can just add them there).

Fixed.

> >diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
> >index 723feb1..0e66bb0 100644
> >--- a/libstdc++-v3/include/bits/c++config
> >+++ b/libstdc++-v3/include/bits/c++config
> >@@ -481,6 +481,17 @@ namespace std
> > # define _GLIBCXX_BEGIN_EXTERN_C extern "C" {
> > # define _GLIBCXX_END_EXTERN_C }
> > 
> >+// Conditionally enable annotations for the Transactional Memory TS on C++11.
> >+#if __cplusplus >= 201103L && \
> >+  _GLIBCXX_USE_CXX11_ABI && _GLIBCXX_USE_DUAL_ABI && \
> 
> It's possible we can make this work for the old ABI too, but this is
> OK for now. The old ABI always uses COW strings, but that's what the
> code you've written deals with anyway.

OK.  I would have to write the wrappers for the new strings too then,
and I wanted to avoid that for now.

> >+  defined(__cpp_transactional_memory) && __cpp_transactional_memory >= 201505L
> 
> The defined(__cpp_transactional_memory) check is redundant, isn't it?
> 
> Users aren't allowed to define it, so it will either be defined to an
> integer value or undefined and evaluate to zero.

Won't we get an undefined warning when using -Wundef if we remove the
first part?  That's what I wanted to avoid.

> >+#define _GLIBCXX_TXN_SAFE transaction_safe
> >+#define _GLIBCXX_TXN_SAFE_DYN transaction_safe_dynamic
> >+#else
> >+#define _GLIBCXX_TXN_SAFE
> >+#define _GLIBCXX_TXN_SAFE_DYN
> >+#endif
> >+
> > #else // !__cplusplus
> > # define _GLIBCXX_BEGIN_EXTERN_C
> > # define _GLIBCXX_END_EXTERN_C
> 
> 
> >@@ -44,7 +46,36 @@ std::exception::what() const _GLIBCXX_USE_NOEXCEPT
> > }
> > 
> > const char* 
> >-std::bad_exception::what() const _GLIBCXX_USE_NOEXCEPT
> >+std::bad_exception::what() const _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT
> > {
> >   return "std::bad_exception";
> > }
> >+
> >+// Transactional clones for the destructors and what().
> >+// what() is effectively transaction_pure, but we do not want to annotate it
> >+// as such; thus, we call exactly the respective nontransactional function.
> >+extern "C" {
> >+
> >+void
> >+_ZGTtNKSt9exceptionD1Ev(const std::exception*)
> >+{ }
> >+
> >+const char*
> >+_ZGTtNKSt9exception4whatEv(const std::exception* that)
> >+{
> >+  return that->std::exception::what();
> >+}
> 
> This makes a non-virtual call, is that correct?
> 
> If users derive from std::exception and override what() they will
> expect a call to what() to dispatch to their override in the derived
> class, but IIUC in a transactional block they would call this
> function, which would call the base what(), not their override.

Yes.  I added this comment to _ZGTtNKSt9exception4whatEv and also
referenced it from a comment in _ZGTtNKSt13bad_exception4whatEv.

  // We really want the non-virtual call here.  We already executed the
  // indirect call representing the virtual call, and the TM runtime or the
  // compiler resolved it to this transactional clone.  In the clone, we want
  // to do the same as for the nontransactional original, so we just call it.


> >+_ZGTtNKSt13bad_exception4whatEv(
> >+    const std::bad_exception* that)
> >+{
> >+  return that->std::bad_exception::what();
> >+}
> 
> Ditto.

See above.

> >@@ -151,3 +164,220 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > 
> > _GLIBCXX_END_NAMESPACE_VERSION
> > } // namespace
> >+
> >+// Support for the Transactional Memory TS (N4514).
> >+//
> >+// logic_error and runtime_error both carry a message in the form of a COW
> >+// string.  This COW string is never made visible to users of the exception
> >+// because what() returns a C string.  The COW string can be constructed as
> >+// either a copy of a COW string of another logic_error/runtime_error, or
> >+// using a C string or SSO string; thus, the COW string's _Rep is only
> >+// accessed by logic_error operations.  We control all txnal clones of those
> >+// operations and thus can ensure that _Rep is never accessed transactionally.
> >+// Furthermore, _Rep will always have been allocated or deallocated via
> >+// global new or delete, so nontransactional writes we do to _Rep cannot
> 
> Hmm, will it always be global new/delete? It uses std::allocator,
> which by default uses new/delete but libstdc++ can be configured to
> use a different std::allocator implementation. If they always use new
> at some point maybe we're OK, but I'd have to check the alternative
> allocators. Maybe we just say only new_allocator is supported for TM.

I wasn't aware that a different std::allocator can be hooked in at
compile time.  Can we check for this (I assume there must be some build
flag for the new allocator?), and simply disable support for the TM TS
if this is the case?

An alternative would be to call the transactional clones of the
std::allocator allocate/deallocate functions -- but even if we did that,
and created txnal clones for those of new_allocator so that we
eventually reach the txnal global new/delete functions that I'm calling
directly in this patch, we'd still need to check whether other
allocators provide them.

What would you like to see?  Or should we just document that limitation
somewhere for now?

> I assume we want to avoid making a txnal std::allocator.

I'm not quite sure what you mean, but the TM support should work with
the default std::allocator at least.

> >+extern "C" {
> >+
> >+void
> >+_ZGTtNSt11logic_errorC1EPKc (std::logic_error *that, const char* s)
> >+{
> >+  // This will use the singleton _Rep for an empty string and just point
> >+  // to it instead of allocating memory.  Thus, we can use it as source, copy
> >+  // it into the object we are constructing, and then construct the COW string
> >+  // in the latter manually.
> >+  std::logic_error le("");
> >+  _ITM_memcpyRnWt(that, &le, sizeof(std::logic_error));
> >+  _txnal_cow_string_C1_for_exceptions(_txnal_logic_error_get_msg(that),
> >+				      s, that);
> 
> The shared empty _Rep is also only used conditionally, it can be
> disabled with --enable-fully-dynamic-string. Maybe another thing we
> just don't deal with for now.

What would you like to see?

Also, can you give me some guidance on the remaining FIXME's in this
patch, please?  In particular, these:

+// FIXME copy over libitm's configury for MANGLE_SIZE_T?
Is there some way to get this from libstdc++ already, or how do you
prefer we handle this?

+// If there is no support for weak, create dummies.
+// FIXME really needed for libstdc++?
+//#if !defined (HAVE_ELF_STYLE_WEAKREF)
Can I assume weak refs to be supported, or how do I check for whether
they are?  What's your preference?

+  // FIXME make a true compile-time choice to prevent warnings.
+  if (sizeof(uint64_t)== sizeof(void*))
+    return (void*)_ITM_RU8((const uint64_t*)ptr);
+  else
+    return (void*)_ITM_RU4((const uint32_t*)ptr);

I've attached a patch with changes for V2 of this (for ease of review,
this is on top of the patch I sent).  This now makes the other
exceptions besides logic_error transaction-safe too.  Please have a
look. 

I tested this on x86_64-linux by running the libitm test that is added
by the V2 changes.  I haven't run a full test of the library so far.

And now with changelogs too! ;)

libstdc++-v3:
2015-12-23  Torvald Riegel  <triegel@redhat.com>

	* include/bits/basic_string.h (basic_string): Declare friends.
	* include/bits/c++config (_GLIBCXX_TXN_SAFE,
	_GLIBCXX_TXN_SAFE_DYN): New.
	* include/std/stdexcept (logic_error, domain_error, invalid_argument,
	length_error, out_of_range, runtime_error, range_error,
	underflow_error, overflow_error): Declare members as transaction-safe.
	(logic_error, runtime_error): Declare friend functions.
	* libsupc++/exception (exception, bad_exception): Declare members as
	transaction-safe.
	* src/c++11/cow-stdexcept.cc: Define transactional clones for the
	transaction-safe members of exceptions and helper functions.
	* libsupc++/eh_exception.cc: Adjust and define transactional clones.
	* config/abi/pre/gnu.ver (GLIBCXX_3.4.22) Add transactional clones.
	(CXXABI_1.3.10): New.

libitm:
2015-12-23  Torvald Riegel  <triegel@redhat.com>

	testsuite/libitm.c++/libstdc++-safeexc.C: New.


[-- Attachment #2: tmts-safeexc-v2changes.patch --]
[-- Type: text/x-patch, Size: 16035 bytes --]

commit c8457910bf92772e60f77935f687dad1b4cf2b6a
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed Dec 23 18:19:32 2015 +0100

    v2 changes

diff --git a/libitm/testsuite/libitm.c++/libstdc++-safeexc.C b/libitm/testsuite/libitm.c++/libstdc++-safeexc.C
new file mode 100644
index 0000000..3e1655e
--- /dev/null
+++ b/libitm/testsuite/libitm.c++/libstdc++-safeexc.C
@@ -0,0 +1,89 @@
+// Tests that the exceptions declared by the TM TS (N4514) as transaction_safe
+// are indeed that.  Thus, this also tests the transactional clones in
+// libstdc++ and libsupc++.
+
+// { dg-do run }
+
+#include <iostream>
+#include <exception>
+#include <stdexcept>
+#include <string>
+
+using namespace std;
+
+template<typename T> void thrower(const T& t)
+{
+  try
+    {
+      atomic_commit
+      {
+	throw t;
+      }
+    }
+  catch (T ex)
+    {
+      if (ex != t) abort ();
+    }
+}
+
+template<typename T> void thrower1(const string& what)
+{
+  try
+    {
+      atomic_commit
+      {
+	throw T ();
+      }
+    }
+  catch (T ex)
+    {
+      if (what != ex.what()) abort ();
+    }
+}
+
+template<typename T> void thrower2(const string& what)
+{
+  try
+    {
+      atomic_commit
+      {
+	throw T (what);
+      }
+    }
+  catch (T ex)
+    {
+      if (what != ex.what()) abort ();
+    }
+}
+
+
+int main ()
+{
+  thrower<unsigned int> (23);
+  thrower<int> (23);
+  thrower<unsigned short> (23);
+  thrower<short> (23);
+  thrower<unsigned char> (23);
+  thrower<char> (23);
+  thrower<unsigned long int> (42);
+  thrower<long int> (42);
+  thrower<unsigned long long int> (42);
+  thrower<long long int> (42);
+  thrower<double> (23.42);
+  thrower<long double> (23.42);
+  thrower<float> (23.42);
+  thrower<void*> (0);
+  thrower<void**> (0);
+  thrower1<exception> ("std::exception");
+  thrower1<bad_exception> ("std::bad_exception");
+  thrower2<logic_error> ("test");
+  thrower2<domain_error> ("test");
+  thrower2<invalid_argument> ("test");
+  thrower2<length_error> ("test");
+  thrower2<out_of_range> ("test");
+  thrower2<runtime_error> ("test");
+  thrower2<range_error> ("test");
+  thrower2<overflow_error> ("test");
+  thrower2<underflow_error> ("test");
+  return 0;
+}
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 2777c49..730d339 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1881,6 +1881,31 @@ GLIBCXX_3.4.22 {
     _ZGTtNSt11logic_errorC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
     _ZGTtNKSt11logic_error4whatEv;
     _ZGTtNSt11logic_errorD[012]Ev;
+    _ZGTtNSt12domain_errorC[12]EPKc;
+    _ZGTtNSt12domain_errorC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNSt12domain_errorD[012]Ev;
+    _ZGTtNSt16invalid_argumentC[12]EPKc;
+    _ZGTtNSt16invalid_argumentC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNSt16invalid_argumentD[012]Ev;
+    _ZGTtNSt12length_errorC[12]EPKc;
+    _ZGTtNSt12length_errorC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNSt12length_errorD[012]Ev;
+    _ZGTtNSt12out_of_rangeC[12]EPKc;
+    _ZGTtNSt12out_of_rangeC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNSt12out_of_rangeD[012]Ev;
+    _ZGTtNSt13runtime_errorC[12]EPKc;
+    _ZGTtNSt13runtime_errorC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNKSt13runtime_error4whatEv;
+    _ZGTtNSt13runtime_errorD[012]Ev;
+    _ZGTtNSt11range_errorC[12]EPKc;
+    _ZGTtNSt11range_errorC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNSt11range_errorD[012]Ev;
+    _ZGTtNSt14overflow_errorC[12]EPKc;
+    _ZGTtNSt14overflow_errorC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNSt14overflow_errorD[012]Ev;
+    _ZGTtNSt15underflow_errorC[12]EPKc;
+    _ZGTtNSt15underflow_errorC[12]ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE;
+    _ZGTtNSt15underflow_errorD[012]Ev;
 
 } GLIBCXX_3.4.21;
 
diff --git a/libstdc++-v3/include/std/stdexcept b/libstdc++-v3/include/std/stdexcept
index 620a2f4..51ea91a 100644
--- a/libstdc++-v3/include/std/stdexcept
+++ b/libstdc++-v3/include/std/stdexcept
@@ -146,9 +146,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class domain_error : public logic_error 
   {
   public:
-    explicit domain_error(const string& __arg);
+    explicit domain_error(const string& __arg) _GLIBCXX_TXN_SAFE;
 #if __cplusplus >= 201103L
-    explicit domain_error(const char*);
+    explicit domain_error(const char*) _GLIBCXX_TXN_SAFE;
 #endif
     virtual ~domain_error() _GLIBCXX_USE_NOEXCEPT;
   };
@@ -157,9 +157,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class invalid_argument : public logic_error 
   {
   public:
-    explicit invalid_argument(const string& __arg);
+    explicit invalid_argument(const string& __arg) _GLIBCXX_TXN_SAFE;
 #if __cplusplus >= 201103L
-    explicit invalid_argument(const char*);
+    explicit invalid_argument(const char*) _GLIBCXX_TXN_SAFE;
 #endif
     virtual ~invalid_argument() _GLIBCXX_USE_NOEXCEPT;
   };
@@ -169,9 +169,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class length_error : public logic_error 
   {
   public:
-    explicit length_error(const string& __arg);
+    explicit length_error(const string& __arg) _GLIBCXX_TXN_SAFE;
 #if __cplusplus >= 201103L
-    explicit length_error(const char*);
+    explicit length_error(const char*) _GLIBCXX_TXN_SAFE;
 #endif
     virtual ~length_error() _GLIBCXX_USE_NOEXCEPT;
   };
@@ -181,9 +181,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class out_of_range : public logic_error 
   {
   public:
-    explicit out_of_range(const string& __arg);
+    explicit out_of_range(const string& __arg) _GLIBCXX_TXN_SAFE;
 #if __cplusplus >= 201103L
-    explicit out_of_range(const char*);
+    explicit out_of_range(const char*) _GLIBCXX_TXN_SAFE;
 #endif
     virtual ~out_of_range() _GLIBCXX_USE_NOEXCEPT;
   };
@@ -200,11 +200,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   public:
     /** Takes a character string describing the error.  */
     explicit 
-    runtime_error(const string& __arg);
+    runtime_error(const string& __arg) _GLIBCXX_TXN_SAFE;
 
 #if __cplusplus >= 201103L
     explicit
-    runtime_error(const char*);
+    runtime_error(const char*) _GLIBCXX_TXN_SAFE;
 #endif
 
 #if _GLIBCXX_USE_CXX11_ABI || _GLIBCXX_DEFINE_STDEXCEPT_COPY_OPS
@@ -212,21 +212,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     runtime_error& operator=(const runtime_error&) _GLIBCXX_USE_NOEXCEPT;
 #endif
 
-    virtual ~runtime_error() _GLIBCXX_USE_NOEXCEPT;
+    virtual ~runtime_error() _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT;
 
     /** Returns a C-style character string describing the general cause of
      *  the current error (the same string passed to the ctor).  */
     virtual const char* 
-    what() const _GLIBCXX_USE_NOEXCEPT;
+    what() const _GLIBCXX_TXN_SAFE_DYN _GLIBCXX_USE_NOEXCEPT;
+# ifdef _GLIBCXX_TM_TS_INTERNAL
+    friend void*
+    ::_txnal_runtime_error_get_msg(void* e);
+# endif
   };
 
   /** Thrown to indicate range errors in internal computations.  */
   class range_error : public runtime_error 
   {
   public:
-    explicit range_error(const string& __arg);
+    explicit range_error(const string& __arg) _GLIBCXX_TXN_SAFE;
 #if __cplusplus >= 201103L
-    explicit range_error(const char*);
+    explicit range_error(const char*) _GLIBCXX_TXN_SAFE;
 #endif
     virtual ~range_error() _GLIBCXX_USE_NOEXCEPT;
   };
@@ -235,9 +239,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class overflow_error : public runtime_error 
   {
   public:
-    explicit overflow_error(const string& __arg);
+    explicit overflow_error(const string& __arg) _GLIBCXX_TXN_SAFE;
 #if __cplusplus >= 201103L
-    explicit overflow_error(const char*);
+    explicit overflow_error(const char*) _GLIBCXX_TXN_SAFE;
 #endif
     virtual ~overflow_error() _GLIBCXX_USE_NOEXCEPT;
   };
@@ -246,9 +250,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   class underflow_error : public runtime_error 
   {
   public:
-    explicit underflow_error(const string& __arg);
+    explicit underflow_error(const string& __arg) _GLIBCXX_TXN_SAFE;
 #if __cplusplus >= 201103L
-    explicit underflow_error(const char*);
+    explicit underflow_error(const char*) _GLIBCXX_TXN_SAFE;
 #endif
     virtual ~underflow_error() _GLIBCXX_USE_NOEXCEPT;
   };
diff --git a/libstdc++-v3/src/c++11/cow-stdexcept.cc b/libstdc++-v3/src/c++11/cow-stdexcept.cc
index b158f7b..b2a912f 100644
--- a/libstdc++-v3/src/c++11/cow-stdexcept.cc
+++ b/libstdc++-v3/src/c++11/cow-stdexcept.cc
@@ -38,6 +38,8 @@ void
 _txnal_cow_string_D1_commit(void* that);
 void*
 _txnal_logic_error_get_msg(void* e);
+void*
+_txnal_runtime_error_get_msg(void* e);
 
 // All exception classes still use the classic COW std::string.
 #define _GLIBCXX_USE_CXX11_ABI 0
@@ -244,8 +246,8 @@ _txnal_cow_string_C1_for_exceptions(void* that, const char* s, void *exc)
   // transactional clone of global new[]; if this throws, it will do so in a
   // transaction-compatible way.
   // The allocation belongs to this exception, so tell the runtime about it.
-  // TODO associate exception
-//  void *prev = _ITM_setAssociatedException(exc);
+  // TODO Once this is supported, link the following allocation to this
+  // exception: void *prev = _ITM_setAssociatedException(exc);
   bs_type::_Rep *rep;
   try
     {
@@ -254,12 +256,14 @@ _txnal_cow_string_C1_for_exceptions(void* that, const char* s, void *exc)
   catch (...)
     {
       // Pop the association with this exception.
-//      _ITM_setAssociatedException(prev);
+      // TODO Once this is supported, link the following allocation to this
+      // exception: _ITM_setAssociatedException(prev);
       // We do not need to instrument a rethrow.
       throw;
     }
   // Pop the association with this exception.
-//  _ITM_setAssociatedException(prev);
+  // TODO Once this is supported, link the following allocation to this
+  // exception: _ITM_setAssociatedException(prev);
 
   // Now initialize the rest of the string and copy the C string.  The memory
   // will be freshly allocated, so nontransactional accesses are sufficient,
@@ -332,45 +336,78 @@ _txnal_logic_error_get_msg(void* e)
   return &le->_M_msg;
 }
 
-extern "C" {
-
-void
-_ZGTtNSt11logic_errorC1EPKc (std::logic_error* that, const char* s)
+void*
+_txnal_runtime_error_get_msg(void* e)
 {
-  // This will use the singleton _Rep for an empty string and just point
-  // to it instead of allocating memory.  Thus, we can use it as source, copy
-  // it into the object we are constructing, and then construct the COW string
-  // in the latter manually.
-  std::logic_error le("");
-  _ITM_memcpyRnWt(that, &le, sizeof(std::logic_error));
-  _txnal_cow_string_C1_for_exceptions(_txnal_logic_error_get_msg(that),
-				      s, that);
+  std::runtime_error* le = (std::runtime_error*) e;
+  return &le->_M_msg;
 }
-void
-_ZGTtNSt11logic_errorC2EPKc (std::logic_error*, const char*)
-  __attribute__((alias ("_ZGTtNSt11logic_errorC1EPKc")));
 
-void
-_ZGTtNSt11logic_errorC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE(
-    std::logic_error* that, const std::__sso_string& s)
-{
-  std::logic_error le("");
-  _ITM_memcpyRnWt(that, &le, sizeof(std::logic_error));
-  // This constructor is only declared transaction-safe if the C++11 ABI is
-  // used for std::string yet logic_error used a COW string internally.  A
-  // user must not call this constructor otherwise; although we can still
-  // compile and provide it, calling it would result in undefined behavior,
-  // which is in this case not initializing the message.
+// The constructors are only declared transaction-safe if the C++11 ABI is
+// used for std::string and the exception classes use a COW string internally.
+// A user must not call these constructors otherwise; if they do, it will
+// result in undefined behavior, which is in this case not initializing this
+// string.
 #if _GLIBCXX_USE_DUAL_ABI
-  // Get the C string from the SSO string.
-  _txnal_cow_string_C1_for_exceptions(_txnal_logic_error_get_msg(that),
-				      _txnal_sso_string_c_str(&s), that);
+#define CTORDTORSTRINGCSTR(s) _txnal_sso_string_c_str((s))
+#else
+#define CTORDTORSTRINGCSTR(s) ""
 #endif
+
+// This macro defines transaction constructors and destructors for a specific
+// exception class.  NAME is the variable part of the mangled name, CLASS is
+// the class name, and BASE must be logic_error or runtime_error (which is
+// then used to call the proper friend function that can return a pointer to
+// the _M_msg member declared by the given (base) class).
+#define CTORDTOR(NAME, CLASS, BASE)					\
+void									\
+_ZGTtNSt##NAME##C1EPKc (CLASS* that, const char* s)			\
+{									\
+  /* This will use the singleton _Rep for an empty string and just	\
+     point to it instead of allocating memory.  Thus, we can use it as	\
+     source, copy it into the object we are constructing, and then	\
+     construct the COW string in the latter manually.  */		\
+  CLASS e("");								\
+  _ITM_memcpyRnWt(that, &e, sizeof(CLASS));				\
+  _txnal_cow_string_C1_for_exceptions(_txnal_##BASE##_get_msg(that),	\
+				      s, that);				\
+}									\
+void									\
+_ZGTtNSt##NAME##C2EPKc (CLASS*, const char*)				\
+  __attribute__((alias ("_ZGTtNSt" #NAME "C1EPKc")));			\
+void									\
+_ZGTtNSt##NAME##C1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE( \
+    CLASS* that, const std::__sso_string& s)				\
+{									\
+  CLASS e("");								\
+  _ITM_memcpyRnWt(that, &e, sizeof(CLASS));				\
+  /* Get the C string from the SSO string.  */				\
+  _txnal_cow_string_C1_for_exceptions(_txnal_##BASE##_get_msg(that),	\
+				      CTORDTORSTRINGCSTR(&s), that);	\
+}									\
+void									\
+_ZGTtNSt##NAME##C2ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE( \
+    CLASS*, const std::__sso_string&) __attribute__((alias		\
+("_ZGTtNSt" #NAME							\
+  "C1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE")));	\
+void									\
+_ZGTtNSt##NAME##D1Ev(CLASS* that)					\
+{ _txnal_cow_string_D1(_txnal_##BASE##_get_msg(that)); }		\
+void									\
+_ZGTtNSt##NAME##D2Ev(CLASS*)						\
+__attribute__((alias ("_ZGTtNSt" #NAME "D1Ev")));			\
+void									\
+_ZGTtNSt##NAME##D0Ev(CLASS* that)					\
+{									\
+  _ZGTtNSt##NAME##D1Ev(that);						\
+  _ZGTtdlPv(that);							\
 }
-void
-_ZGTtNSt11logic_errorC2ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE(
-    std::logic_error*, const std::__sso_string&) __attribute__((alias
-("_ZGTtNSt11logic_errorC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE")));
+
+// Now create all transactional constructors and destructors, as well as the
+// two virtual what() functions.
+extern "C" {
+
+CTORDTOR(11logic_error, std::logic_error, logic_error)
 
 const char*
 _ZGTtNKSt11logic_error4whatEv(const std::logic_error* that)
@@ -379,19 +416,23 @@ _ZGTtNKSt11logic_error4whatEv(const std::logic_error* that)
       const_cast<std::logic_error*>(that)));
 }
 
-void
-_ZGTtNSt11logic_errorD1Ev(std::logic_error* that)
-{
-  _txnal_cow_string_D1(_txnal_logic_error_get_msg(that));
-}
-void
-_ZGTtNSt11logic_errorD2Ev(std::logic_error*)
-__attribute__((alias ("_ZGTtNSt11logic_errorD1Ev")));
-void
-_ZGTtNSt11logic_errorD0Ev(std::logic_error* that)
+CTORDTOR(12domain_error, std::domain_error, logic_error)
+CTORDTOR(16invalid_argument, std::invalid_argument, logic_error)
+CTORDTOR(12length_error, std::length_error, logic_error)
+CTORDTOR(12out_of_range, std::out_of_range, logic_error)
+
+
+CTORDTOR(13runtime_error, std::runtime_error, runtime_error)
+
+const char*
+_ZGTtNKSt13runtime_error4whatEv(const std::runtime_error* that)
 {
-  _ZGTtNSt11logic_errorD1Ev(that);
-  _ZGTtdlPv(that);
+  return _txnal_cow_string_c_str(_txnal_runtime_error_get_msg(
+      const_cast<std::runtime_error*>(that)));
 }
 
+CTORDTOR(11range_error, std::range_error, runtime_error)
+CTORDTOR(14overflow_error, std::overflow_error, runtime_error)
+CTORDTOR(15underflow_error, std::underflow_error, runtime_error)
+
 }

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe.
  2015-12-23 17:35   ` Torvald Riegel
@ 2015-12-23 20:56     ` Jason Merrill
  2015-12-24 13:34       ` Jonathan Wakely
  2015-12-24 13:34     ` Jonathan Wakely
  1 sibling, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2015-12-23 20:56 UTC (permalink / raw)
  To: Torvald Riegel, Jonathan Wakely; +Cc: libstdc++, GCC Patches

On 12/23/2015 12:35 PM, Torvald Riegel wrote:
> +//#if !defined (HAVE_ELF_STYLE_WEAKREF)
> Can I assume weak refs to be supported, or how do I check for whether
> they are?  What's your preference?

G++ does support targets without weak symbols, but I imagine we can 
decide not to support libitm on such targets.

> +  // FIXME make a true compile-time choice to prevent warnings.
> +  if (sizeof(uint64_t)== sizeof(void*))
> +    return (void*)_ITM_RU8((const uint64_t*)ptr);
> +  else
> +    return (void*)_ITM_RU4((const uint32_t*)ptr);

#include <stdint.h> and compare UINTPTR_MAX to UINT64_MAX?

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe.
  2015-12-23 20:56     ` Jason Merrill
@ 2015-12-24 13:34       ` Jonathan Wakely
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2015-12-24 13:34 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Torvald Riegel, libstdc++, GCC Patches

On 23/12/15 15:56 -0500, Jason Merrill wrote:
>On 12/23/2015 12:35 PM, Torvald Riegel wrote:
>>+//#if !defined (HAVE_ELF_STYLE_WEAKREF)
>>Can I assume weak refs to be supported, or how do I check for whether
>>they are?  What's your preference?
>
>G++ does support targets without weak symbols, but I imagine we can 
>decide not to support libitm on such targets.
>
>>+  // FIXME make a true compile-time choice to prevent warnings.
>>+  if (sizeof(uint64_t)== sizeof(void*))
>>+    return (void*)_ITM_RU8((const uint64_t*)ptr);
>>+  else
>>+    return (void*)_ITM_RU4((const uint32_t*)ptr);
>
>#include <stdint.h> and compare UINTPTR_MAX to UINT64_MAX?

Presumably that header is already included to use uint32_t and
uint64_t, but even if not there are pre-defined versions of those
macros that don't need any header, __UINTPTR_MAX__ and __UINT64_MAX__.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe.
  2015-12-23 17:35   ` Torvald Riegel
  2015-12-23 20:56     ` Jason Merrill
@ 2015-12-24 13:34     ` Jonathan Wakely
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2015-12-24 13:34 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: libstdc++, GCC Patches, Jason Merrill

On 23/12/15 18:35 +0100, Torvald Riegel wrote:
>> It's possible we can make this work for the old ABI too, but this is
>> OK for now. The old ABI always uses COW strings, but that's what the
>> code you've written deals with anyway.
>
>OK.  I would have to write the wrappers for the new strings too then,
>and I wanted to avoid that for now.

Really? When the dual ABI is disabled there are no new strings, and
when dual ABI is enabled but you're compiling with the old ABI new
strings shouldn't be involved here either. Maybe I'm missing
something.

>> >+  defined(__cpp_transactional_memory) && __cpp_transactional_memory >= 201505L
>>
>> The defined(__cpp_transactional_memory) check is redundant, isn't it?
>>
>> Users aren't allowed to define it, so it will either be defined to an
>> integer value or undefined and evaluate to zero.
>
>Won't we get an undefined warning when using -Wundef if we remove the
>first part?  That's what I wanted to avoid.

Only when -Wsystem-headers is also used. We don't care about -Wundef
in libstdc++ (I don't care about it anyway, we already have loads of
such uses of macros and nobody has ever complained).

>> This makes a non-virtual call, is that correct?
>>
>> If users derive from std::exception and override what() they will
>> expect a call to what() to dispatch to their override in the derived
>> class, but IIUC in a transactional block they would call this
>> function, which would call the base what(), not their override.
>
>Yes.  I added this comment to _ZGTtNKSt9exception4whatEv and also
>referenced it from a comment in _ZGTtNKSt13bad_exception4whatEv.
>
>  // We really want the non-virtual call here.  We already executed the
>  // indirect call representing the virtual call, and the TM runtime or the
>  // compiler resolved it to this transactional clone.  In the clone, we want
>  // to do the same as for the nontransactional original, so we just call it.

Gotcha, I was misunderstanding when we ended up calling the clone

>> Hmm, will it always be global new/delete? It uses std::allocator,
>> which by default uses new/delete but libstdc++ can be configured to
>> use a different std::allocator implementation. If they always use new
>> at some point maybe we're OK, but I'd have to check the alternative
>> allocators. Maybe we just say only new_allocator is supported for TM.
>
>I wasn't aware that a different std::allocator can be hooked in at
>compile time.  Can we check for this (I assume there must be some build
>flag for the new allocator?), and simply disable support for the TM TS
>if this is the case?

The build-time configuration is done by GLIBCXX_ENABLE_ALLOCATOR in
libstdc++-v3/acinclude.m4 but it doesn't currently set anything that
can be used to test whether the chosen config is new_allocator or not.

>An alternative would be to call the transactional clones of the
>std::allocator allocate/deallocate functions -- but even if we did that,
>and created txnal clones for those of new_allocator so that we
>eventually reach the txnal global new/delete functions that I'm calling
>directly in this patch, we'd still need to check whether other
>allocators provide them.
>
>What would you like to see?  Or should we just document that limitation
>somewhere for now?

I'm OK with only providing TM support for the default configuration.

We might need to add something to acinclude.m4 that says we're using
new_allocator and therefore it's OK to enable things that rely on it.

>> I assume we want to avoid making a txnal std::allocator.
>
>I'm not quite sure what you mean, but the TM support should work with
>the default std::allocator at least.
>
>> >+extern "C" {
>> >+
>> >+void
>> >+_ZGTtNSt11logic_errorC1EPKc (std::logic_error *that, const char* s)
>> >+{
>> >+  // This will use the singleton _Rep for an empty string and just point
>> >+  // to it instead of allocating memory.  Thus, we can use it as source, copy
>> >+  // it into the object we are constructing, and then construct the COW string
>> >+  // in the latter manually.
>> >+  std::logic_error le("");
>> >+  _ITM_memcpyRnWt(that, &le, sizeof(std::logic_error));
>> >+  _txnal_cow_string_C1_for_exceptions(_txnal_logic_error_get_msg(that),
>> >+				      s, that);
>>
>> The shared empty _Rep is also only used conditionally, it can be
>> disabled with --enable-fully-dynamic-string. Maybe another thing we
>> just don't deal with for now.
>
>What would you like to see?

Disable TM support if _GLIBCXX_FULLY_DYNAMIC_STRING is defined to a
non-zero value.

>Also, can you give me some guidance on the remaining FIXME's in this
>patch, please?  In particular, these:
>
>+// FIXME copy over libitm's configury for MANGLE_SIZE_T?
>Is there some way to get this from libstdc++ already, or how do you
>prefer we handle this?
>
>+// If there is no support for weak, create dummies.
>+// FIXME really needed for libstdc++?
>+//#if !defined (HAVE_ELF_STYLE_WEAKREF)
>Can I assume weak refs to be supported, or how do I check for whether
>they are?  What's your preference?

Again, I'm OK with TM being unsupported where it's painful to do.

>+  // FIXME make a true compile-time choice to prevent warnings.
>+  if (sizeof(uint64_t)== sizeof(void*))
>+    return (void*)_ITM_RU8((const uint64_t*)ptr);
>+  else
>+    return (void*)_ITM_RU4((const uint32_t*)ptr);

These MAX values are pre-defined by the compiler:

#if __UINTPTR_MAX__ == __UINT64_MAX__

>I've attached a patch with changes for V2 of this (for ease of review,
>this is on top of the patch I sent).  This now makes the other
>exceptions besides logic_error transaction-safe too.  Please have a
>look.

I won't be able to review this properly until the new year.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-12-24 13:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-14 19:45 [PATCH][WIP] libstdc++: Make certain exceptions transaction_safe Torvald Riegel
2015-12-16 21:06 ` Jonathan Wakely
2015-12-23 17:35   ` Torvald Riegel
2015-12-23 20:56     ` Jason Merrill
2015-12-24 13:34       ` Jonathan Wakely
2015-12-24 13:34     ` Jonathan Wakely
2015-12-17 20:04 ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).