On 08/05/18 16:17 +0100, Jonathan Wakely wrote:
On 8 May 2018 at 15:45, Marc Glisse <marc.gli...@inria.fr> wrote:
On Tue, 8 May 2018, Jonathan Wakely wrote:

On 8 May 2018 at 14:00, Jonathan Wakely wrote:

On 8 May 2018 at 13:44, Stephan Bergmann wrote:

I was recently bitten by the following issue (Linux, libstdc++ 8.0.1): A
process loads two dynamic libraries A and B both using std::regex, and A
is
compiled without -D_GLIBCXX_DEBUG while B is compiled with
-D_GLIBCXX_DEBUG.


This is only supported in very restricted cases.

B creates an instance of std::regex, which internally creates a
std::shared_ptr<std::__detail::_NFA<std::__cxx11::regex_traits<char>>>,
where _NFA has various members of std::__debug::vector type (but which
isn't
reflected in the mangled name of that _NFA instantiation itself).

Now, when that instance of std::regex is destroyed again in library B,
the

std::shared_ptr<std::__detail::_NFA<std::__cxx11::regex_traits<char>>>::~shared_ptr
destructor (and functions it in turn calls) that happens to get picked
is
the (inlined, and exported due to default visibility) instance from
library
A.  And that assumes that that _NFA instantiation has members of
non-debug
std::vector type, which causes a crash.

Should it be considered a bug that such mixture of debug and non-debug
std::regex usage causes ODR violations?


Yes, but my frank response is "don't do that".

The right fix here might be to ensure that _NFA always uses the
non-debug vector even in Debug Mode, but I'm fairly certain there are
other similar problems lurking.


N.B. I think this discussion belongs on the libstdc++ list.


Would it make sense to use the abi_tag attribute to help with that? (I
didn't really think about it, maybe it doesn't)

Yes, I think we could add it conditionally in debug mode, so that
types with members that are either std::xxx or __gnu_debug::xxx get a
different mangled name in debug mode.

For the regex _NFA type I don't think we want the debug mode checking,
because users can't access it directly so any errors are in the
libstdc++ implementation and we should have eliminated them ourselves,
not be pushing detection of those logic errors into users' programs.

I've committed this patch to do that.


For std::match_results (which derives from std::vector) it's possible
for users to use invalid iterators obtained from a match_results, so
Debug Mode can help. In that case we could decide whether to add the
abi_tag, or always derive from _GLIBCXX_STD_C::vector (i.e. the
non-debug mode one), or even provide an entire
__gnu_debug::match_results type.

"don't do that" remains the most sensible answer.

Yes, it's just asking for trouble.
commit 9e026542864d4ff5dd45ffdc43ec367e36aff8a6
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Tue May 8 16:39:33 2018 +0100

    Make std::regex automata use non-debug vector in Debug Mode
    
            * include/bits/regex_automaton.h (_NFA_base::_M_paren_stack, _NFA):
            Use normal std::vector even in Debug Mode.

diff --git a/libstdc++-v3/include/bits/regex_automaton.h b/libstdc++-v3/include/bits/regex_automaton.h
index bf51df79097..ff87dcc245d 100644
--- a/libstdc++-v3/include/bits/regex_automaton.h
+++ b/libstdc++-v3/include/bits/regex_automaton.h
@@ -210,7 +210,7 @@ namespace __detail
     _M_sub_count() const
     { return _M_subexpr_count; }
 
-    std::vector<size_t>       _M_paren_stack;
+    _GLIBCXX_STD_C::vector<size_t> _M_paren_stack;
     _FlagT                    _M_flags;
     _StateIdT                 _M_start_state;
     _SizeT                    _M_subexpr_count;
@@ -219,7 +219,7 @@ namespace __detail
 
   template<typename _TraitsT>
     struct _NFA
-    : _NFA_base, std::vector<_State<typename _TraitsT::char_type>>
+    : _NFA_base, _GLIBCXX_STD_C::vector<_State<typename _TraitsT::char_type>>
     {
       typedef typename _TraitsT::char_type	_Char_type;
       typedef _State<_Char_type>		_StateT;

Reply via email to