test: Remove spurious double lock tsan suppressions by bumping to clang-12 #21669

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2104-tsanSupp changing 3 files +3 −22
  1. MarcoFalke commented at 6:07 PM on April 13, 2021: member

    The double lock warnings appeared in #19041, but they didn't make any sense. Also, our sync module would detect double locks, if there were any.

    Bumping to clang-12 allows us to remove the spurious suppressions needed to run the tests, so do that.

  2. test: Remove spurious double lock tsan suppressions by bumping to clang-12 fadbd99885
  3. hebasto commented at 6:10 PM on April 13, 2021: member

    Concept ACK. Any links to clang developers notes (if known)?

  4. DrahtBot added the label Tests on Apr 13, 2021
  5. MarcoFalke force-pushed on Apr 13, 2021
  6. MarcoFalke commented at 6:47 PM on April 13, 2021: member

    No, sorry not aware of any links

  7. fanquake requested review from practicalswift on Apr 14, 2021
  8. Revert "test: Add tsan supp for leveldb::DBImpl::DeleteObsoleteFiles"
    I can no longer observe the need for this suppression.

This reverts commit fa1fc536bb26471fd2a6fe8d12f98cf53c646306.
    fadea0bf37
  9. practicalswift approved
  10. practicalswift commented at 7:15 AM on April 14, 2021: contributor

    Very happy to see that these suppressions are no longer needed: especially the mutex:UpdateTip and race:ProcessNewBlock suppressions.

    Every file or function level suppression risks hiding future issues in the same file or function, so I think it makes sense to continually try to prune the list of suppression and keep it as short as possible. Thanks for doing that @MarcoFalke!

    cr ACK fadea0bf371a38620b7f1f93f87d1da76d3314e0 assuming CI passes and more specifically that newer Clang agrees that these TSan suppressions are no longer needed.

    Aside:

    Great TSan field report posted in April 2021: "Eliminating Data Races in Firefox – A Technical Report". Recommended reading for all sanitizer fans! :)

  11. MarcoFalke merged this on Apr 14, 2021
  12. MarcoFalke closed this on Apr 14, 2021
  13. MarcoFalke deleted the branch on Apr 14, 2021
  14. hebasto commented at 8:16 AM on April 14, 2021: member

    @practicalswift

    Great TSan field report posted in April 2021: "Eliminating Data Races in Firefox – A Technical Report". Recommended reading for all sanitizer fans! :)

    Thanks! :+1:

  15. hebasto commented at 8:43 AM on April 14, 2021: member

    It makes local TSan tests a bit complicated as clang-12 is not available in the Focal repo for now.

  16. hebasto commented at 8:45 AM on April 14, 2021: member

    https://cirrus-ci.com/task/4889293647773696

  17. hebasto commented at 10:12 AM on April 14, 2021: member

    Tested locally:

    $ lsb_release -c
Codename:	hirsute
$ clang --version | head -1
Ubuntu clang version 12.0.0-++rc4-1ubuntu1
$ git rev-parse HEAD
b8e5bbdf93e5b3b35557d3b0e57a426492d568c1
$ echo $TSAN_OPTIONS 
suppressions=/home/hebasto/bitcoin/test/sanitizer_suppressions/tsan:second_deadlock_stack=1
$ make -C depends CC=clang CXX='clang++ -stdlib=libc++'
$ CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure --with-sanitizers=thread CC=clang CXX='clang++ -stdlib=libc++' CXXFLAGS=-g
$ make clean
$ make check
...
Running 2 test cases...
Entering test module "Bitcoin Core Test Suite"
test/sync_tests.cpp(79): Entering test suite "sync_tests"
test/sync_tests.cpp(81): Entering test case "potential_deadlock_detected"
==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=16889)
  Cycle in lock order graph: M131693 (0x7ffce5bda6f8) => M131694 (0x7ffce5bda6d0) => M131693

  Mutex M131694 acquired here while holding mutex M131693 in main thread:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::recursive_mutex::lock() <null> (libc++.so.1+0x48b05)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:89:5 (test_bitcoin+0x673d8e)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

  Mutex M131693 previously acquired by the same thread here:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::recursive_mutex::lock() <null> (libc++.so.1+0x48b05)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:89:5 (test_bitcoin+0x673d8e)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

  Mutex M131693 acquired here while holding mutex M131694 in main thread:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::recursive_mutex::lock() <null> (libc++.so.1+0x48b05)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:89:5 (test_bitcoin+0x673d8e)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

  Mutex M131694 previously acquired by the same thread here:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::recursive_mutex::lock() <null> (libc++.so.1+0x48b05)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:89:5 (test_bitcoin+0x673d8e)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) (/home/hebasto/bitcoin/src/test/test_bitcoin+0xf5036) in pthread_mutex_lock
==================
==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=16889)
  Cycle in lock order graph: M131695 (0x7ffce5bda6a8) => M131696 (0x7ffce5bda680) => M131695

  Mutex M131696 acquired here while holding mutex M131695 in main thread:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::mutex::lock() <null> (libc++.so.1+0x48a15)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:94:5 (test_bitcoin+0x673dce)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

  Mutex M131695 previously acquired by the same thread here:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::mutex::lock() <null> (libc++.so.1+0x48a15)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:94:5 (test_bitcoin+0x673dce)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

  Mutex M131695 acquired here while holding mutex M131696 in main thread:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::mutex::lock() <null> (libc++.so.1+0x48a15)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:94:5 (test_bitcoin+0x673dce)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

  Mutex M131696 previously acquired by the same thread here:
    [#0](/bitcoin-bitcoin/0/) pthread_mutex_lock <null> (test_bitcoin+0xf5036)
    [#1](/bitcoin-bitcoin/1/) std::__1::mutex::lock() <null> (libc++.so.1+0x48a15)
    [#2](/bitcoin-bitcoin/2/) sync_tests::potential_deadlock_detected::test_method() /home/hebasto/bitcoin/src/test/sync_tests.cpp:94:5 (test_bitcoin+0x673dce)
    [#3](/bitcoin-bitcoin/3/) sync_tests::potential_deadlock_detected_invoker() /home/hebasto/bitcoin/src/test/sync_tests.cpp:81:1 (test_bitcoin+0x67368b)
    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x1ba82d)
    [#5](/bitcoin-bitcoin/5/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) <null> (test_bitcoin+0xfe42da)

SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) (/home/hebasto/bitcoin/src/test/test_bitcoin+0xf5036) in pthread_mutex_lock
==================
2021-04-14T10:04:13Z Seed: Setting random seed for current tests to RANDOM_CTX_SEED=7acdb16613e39bc474d4c35d8c1ffd9b76f45604614957bf721d9ba4c98f544a
2021-04-14T10:04:13.840447Z [test] [init.cpp:817] [InitLogging] Bitcoin Core version v21.99.0-b8e5bbdf93e5 (release build)
2021-04-14T10:04:13.840753Z [test] [init.cpp:1011] [AppInitParameterInteraction] Assuming ancestors of block 0000000000000000000b9d2ec5a352ecba0592946514a92f14319dc2b367fc72 have valid signatures.
2021-04-14T10:04:13.840810Z [test] [init.cpp:1024] [AppInitParameterInteraction] Setting nMinimumChainWork=00000000000000000000000000000000000000001533efd8d716a517fe2c5008
2021-04-14T10:04:13.914128Z [test] [script/sigcache.cpp:102] [InitSignatureCache] Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
2021-04-14T10:04:13.996929Z [test] [validation.cpp:1355] [InitScriptExecutionCache] Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
test/sync_tests.cpp(81): Leaving test case "potential_deadlock_detected"; testing time: 1185317us
test/sync_tests.cpp(118): Entering test case "inconsistent_lock_order_detected"
2021-04-14T10:04:15.027918Z [test] [test/util/setup_common.cpp:63] [Seed] Seed: Setting random seed for current tests to RANDOM_CTX_SEED=7acdb16613e39bc474d4c35d8c1ffd9b76f45604614957bf721d9ba4c98f544a
2021-04-14T10:04:15.028075Z [test] [init.cpp:817] [InitLogging] Bitcoin Core version v21.99.0-b8e5bbdf93e5 (release build)
2021-04-14T10:04:15.028348Z [test] [init.cpp:1011] [AppInitParameterInteraction] Assuming ancestors of block 0000000000000000000b9d2ec5a352ecba0592946514a92f14319dc2b367fc72 have valid signatures.
2021-04-14T10:04:15.028413Z [test] [init.cpp:1024] [AppInitParameterInteraction] Setting nMinimumChainWork=00000000000000000000000000000000000000001533efd8d716a517fe2c5008
2021-04-14T10:04:15.059007Z [test] [script/sigcache.cpp:102] [InitSignatureCache] Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
2021-04-14T10:04:15.087074Z [test] [validation.cpp:1355] [InitScriptExecutionCache] Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
test/sync_tests.cpp(118): Leaving test case "inconsistent_lock_order_detected"; testing time: 66114us
test/sync_tests.cpp(79): Leaving test suite "sync_tests"; testing time: 1251550us
Leaving test module "Bitcoin Core Test Suite"; testing time: 1251692us

*** No errors detected
ThreadSanitizer: reported 2 warnings
...
  18. MarcoFalke commented at 1:45 PM on April 14, 2021: member

    It makes local TSan tests a bit complicated as clang-12 is not available in the Focal repo for now.

    What is the point of running a sanitizer with known bugs in the first place?

    If you really want to run a thread sanitizer in Focal, you can use the gcc one. Apparently it is not recommended, but it doesn't need the spurious suppressions either.

  19. hebasto commented at 1:48 PM on April 14, 2021: member

    If you really want to run a thread sanitizer in Focal...

    I did not mean it. Just stating a fact that I need a hirsute distro now.

  20. MarcoFalke commented at 1:53 PM on April 14, 2021: member

    It is also possible to install another clang version on Focal, see https://apt.llvm.org/. Though, personally I use vms or docker/podman to run the version of Ubuntu I want.

  21. hebasto commented at 1:58 PM on April 14, 2021: member 
    $ VBoxManage list vms
"macOS Big Sur 11" {62a53114-1691-4f71-93e2-5dbaada8734c}
"CentOS 8" {cda31622-2a2d-41e6-b466-866f0026f748}
"Windows 10" {ccc55f7a-f1bd-4827-b637-e504b3d16f3f}
"Ubuntu Focal 20.04" {f578346f-1e2b-4a0c-944b-aa21e52f3b52}
"Windows 10 (32-bit)" {75c8de41-2512-4ee9-a2bf-0bdc17106a49}
"macOS Sierra 10.12" {3b573a9e-b6f7-4902-afab-7ee627495203}
"macOS Mojave 10.14" {99b5315c-f984-4e82-b9b7-3ce3666231d4}
"Ubuntu Hirsute 21.04" {e49d3b0f-fdcc-4872-b824-d4443a537f0b}
"Fedora" {18b824ec-b9b0-4c61-b765-b3678fe2e468}
"Ubuntu Bionic 18.04" {48450886-47f1-410d-b3e2-fa2c8df5c94a}
"macOS Catalina 10.15" {147c760d-c665-4108-aae3-a6a1d50a5725}
"Debian 9 Stretch" {7c1e869a-d875-4b15-b819-df2cea423524}

    :)

  22. sidhujag referenced this in commit 2216a229f9 on Apr 14, 2021
  23. MarcoFalke referenced this in commit 773f8c1a7d on Apr 14, 2021
  24. sidhujag referenced this in commit 10eb1422e0 on Apr 14, 2021
  25. PastaPastaPasta referenced this in commit 0a276d388b on Sep 17, 2021
  26. PastaPastaPasta referenced this in commit b69d657855 on Sep 19, 2021
  27. thelazier referenced this in commit ae05394685 on Sep 25, 2021
  28. DrahtBot locked this on Aug 18, 2022
Contributors
MarcoFalkehebastopracticalswift
Labels
Tests
Linked (view graph)
#19041 ci: tsan with -stdlib=libc++-10#21678 test: Fix TestPotentialDeadLockDetected suppression
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-17 06:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me