Page MenuHomePhabricator

Merge #13077: Add compile time checking for all cs_KeyStore runtime locking assertions
ClosedPublic

Authored by nakihito on Sat, Aug 31, 00:33.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC69b14ee35aff: Merge #13077: Add compile time checking for all cs_KeyStore runtime locking…
Summary

66dc662c8a Add compile time checking for all cs_KeyStore runtime locking assertions (practicalswift)

Pull request description:

Add compile time checking for all `cs_KeyStore` runtime locking assertions.

This PR is a subset of #12665. The PR was broken up to make reviewing easier.

The intention is that literally all `EXCLUSIVE_LOCKS_REQUIRED`/`LOCKS_EXCLUDED`:s added in this PR should follow either directly or indirectly from `AssertLockHeld(…)`/`AssertLockNotHeld(…)`:s already existing in the repo.

Consider the case where function `A(…)` contains `AssertLockHeld(cs_foo)` (without
first locking `cs_foo` in `A`), and that `B(…)` calls `A(…)` (without first locking `cs_main`):
* It _directly_ follows that: `A(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
* It _indirectly_ follows that: `B(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.

Tree-SHA512: 84ee5459e7f75f9affaa4275cb760576ab0e26da8a48b9a4a59b51a25418fe4b862ba832cb01312479a8c9b0e38ee8b6c4076bcbbd73213346d09ec2057fc9f1

Backport of Core PR13077
https://github.com/bitcoin/bitcoin/pull/13077/

Test Plan
../configure CXX=clang++ CC=clang
make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.Sat, Aug 31, 00:33
Owners added a reviewer: Restricted Owners Package.Sat, Aug 31, 00:34
Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Aug 31, 00:34
Fabien requested changes to this revision.Tue, Sep 3, 08:17

The test plan is incorrect. --enable-debug is aimed to detect races at runtime from the LOCK(), AssertLockHeld(), ... macros.
EXCLUSIVE_LOCKS_REQUIRED() and alike are compile-time directives used by Clang (only) when the -Wthread-safety-analysis compile flag is set.

This revision now requires changes to proceed.Tue, Sep 3, 08:17
nakihito requested review of this revision.Tue, Sep 3, 19:06
nakihito edited the test plan for this revision. (Show Details)

Fixed test method.

Fabien accepted this revision.Wed, Sep 4, 15:47
This revision is now accepted and ready to land.Wed, Sep 4, 15:47