google / sanitizers

AddressSanitizer, ThreadSanitizer, MemorySanitizer
Other
11.49k stars 1.04k forks source link

TSAN false positive in libstdc++ cow_string.h with C++03 ABI #1811

Open rustyx opened 4 days ago

rustyx commented 4 days ago

It seems that enabling the legacy ABI with _GLIBCXX_USE_CXX11_ABI=0 causes TSAN to report false positives in libstdc++/libgcc copy-on-write strings.

Consider the following simple reproducer

#include <string>
#include <thread>

int main(int argc, char** argv) {

    std::string *s_orig = new std::string("test");

    std::thread t([s_copy = *s_orig]() mutable {
        s_copy[0]++;
    });

    delete s_orig;

    t.join();
}

When compiled with c++ -g -std=c++17 -fsanitize=thread -D_GLIBCXX_USE_CXX11_ABI=0 repro.cpp and run, produces:

==================
WARNING: ThreadSanitizer: data race (pid=1234418)
  Write of size 8 at 0x7b0800000038 by main thread:
    #0 operator delete(void*) ../../../../libsanitizer/tsan/tsan_new_delete.cpp:126 (libtsan.so.2+0x88e79)
    #1 std::string::_Rep::_M_dispose(std::allocator<char> const&) /gcc-12.3.0/libstdc++-v3/include/bits/cow_string.h:292 (libstdc++.so.6+0xf2d3c)
    #2 std::string::_Rep::_M_dispose(std::allocator<char> const&) /gcc-12.3.0/libstdc++-v3/include/bits/cow_string.h:272 (libstdc++.so.6+0xf2d3c)
    #3 std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() /gcc-12.3.0/libstdc++-v3/include/bits/cow_string.h:718 (libstdc++.so.6+0xf2d3c)

  Previous read of size 1 at 0x7b080000003b by thread T1:
    #0 memcpy ../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 (libtsan.so.2+0x606a2)
    #1 memcpy ../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:819 (libtsan.so.2+0x606a2)
    #2 std::char_traits<char>::copy(char*, char const*, unsigned long) /gcc-12.3.0/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:435 (libstdc++.so.6+0xf2274)
    #3 std::string::_M_copy(char*, char const*, unsigned long) /gcc-12.3.0/libstdc++-v3/include/bits/cow_string.h:404 (libstdc++.so.6+0xf2274)
    #4 std::string::_M_copy(char*, char const*, unsigned long) /gcc-12.3.0/libstdc++-v3/include/bits/cow_string.h:399 (libstdc++.so.6+0xf2274)
    #5 std::string::_M_mutate(unsigned long, unsigned long, unsigned long) /gcc-12.3.0/libstdc++-v3/include/bits/cow_string.h:3404 (libstdc++.so.6+0xf2274)
    #6 __invoke_impl<void, main(int, char**)::<lambda()> > /usr/local/include/c++/12.3.0/bits/invoke.h:61 (main+0x402ae6)
    #7 __invoke<main(int, char**)::<lambda()> > /usr/local/include/c++/12.3.0/bits/invoke.h:96 (main+0x402a61)
    #8 _M_invoke<0> /usr/local/include/c++/12.3.0/bits/std_thread.h:279 (main+0x4029c6)
    #9 operator() /usr/local/include/c++/12.3.0/bits/std_thread.h:286 (main+0x402970)
    #10 _M_run /usr/local/include/c++/12.3.0/bits/std_thread.h:231 (main+0x40292a)
    #11 execute_native_thread_routine ../../../../../libstdc++-v3/src/c++11/thread.cc:82 (libstdc++.so.6+0xdb23e)

  Thread T1 (tid=1234420, finished) created by main thread at:
    #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors_posix.cpp:1001 (libtsan.so.2+0x610a2)
    #1 __gthread_create /gcc-12.3.0/build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 (libstdc++.so.6+0xdb597)
    #2 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) ../../../../../libstdc++-v3/src/c++11/thread.cc:147 (libstdc++.so.6+0xdb597)
    #3 main ../main.cpp:34 (main+0x402434)

SUMMARY: ThreadSanitizer: data race ../../../../libsanitizer/tsan/tsan_new_delete.cpp:126 in operator delete(void*)

Tried with Clang 18.1.7, Clang 17.0.6, GCC 13.2.0 - same result.

For Clang need to compile with -stdlib=libstdc++ --rtlib=libgcc, the issue is not reproducible with libc++.

rustyx commented 4 days ago

If I look in cow_string.h, std::string::_Rep::_M_dispose is implemented as

    void
    _M_dispose(const _Alloc& __a) _GLIBCXX_NOEXCEPT
    {
#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
      if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
        {
          // Be race-detector-friendly.  For more info see bits/c++config.
          _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&this->_M_refcount);
          // Decrement of _M_refcount is acq_rel, because:
          // - all but last decrements need to release to synchronize with
          //   the last decrement that will delete the object.
          // - the last decrement needs to acquire to synchronize with
          //   all the previous decrements.
          // - last but one decrement needs to release to synchronize with
          //   the acquire load in _M_is_shared that will conclude that
          //   the object is not shared anymore.
          if (__gnu_cxx::__exchange_and_add_dispatch(&this->_M_refcount,
                             -1) <= 0)
        {
          _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&this->_M_refcount);
          _M_destroy(__a);
        }
        }
    }  // XXX MT

Could it be that legacy ABI breaks _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE/_GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER functionality somehow?