diff --git a/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql b/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql index 6259ff67d3..94d23c8664 100644 --- a/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql +++ b/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql @@ -19,8 +19,8 @@ import codingstandards.cpp.Concurrency from LockProtectedControlFlowNode n where not isExcluded(n, ConcurrencyPackage::doNotSpeculativelyLockALockedNonRecursiveMutexQuery()) and - // problematic nodes are ones where a lock is active and there is an attempt to - // call a speculative locking function + // problematic nodes are ones where a lock is active and there is an attempt + // to call a speculative locking function n.(MutexFunctionCall).isSpeculativeLock() and not n.(MutexFunctionCall).isRecursive() and n.getAProtectingLock() = n.(MutexFunctionCall).getLock() diff --git a/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql b/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql index 87000ecfb3..d3a5778f2c 100644 --- a/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql +++ b/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql @@ -19,8 +19,8 @@ import codingstandards.cpp.Concurrency from LockProtectedControlFlowNode n where not isExcluded(n, ConcurrencyPackage::lockedALockedNonRecursiveMutexAuditQuery()) and - // problematic nodes are ones where a lock is active and there is an attempt to - // call a speculative locking function + // problematic nodes are ones where a lock is active and there is an attempt + // to call a speculative locking function n.(MutexFunctionCall).isSpeculativeLock() and not n.(MutexFunctionCall).isRecursive() select n, "(Audit) Attempt to speculatively lock a non-recursive mutex while it is $@.", diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index 66af30dbb9..5162255de9 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -399,9 +399,14 @@ class LockProtectedControlFlowNode extends ThreadedCFN { not exists(ControlFlowNode unlock | // it's an unlock unlock = getAThreadContextAwarePredecessor(lock, this) and - unlock.(MutexFunctionCall).isUnlock() + unlock.(MutexFunctionCall).isUnlock() and // note that we don't check that it's the same lock -- this is left // to the caller to enforce this condition. + // Because of the way that `getAThreadContextAwarePredecessor` works, it is possible + // for operations PAST it to be technically part of the predecessors. + // Thus, we need to make sure that this node is a + // successor of the unlock in the CFG + getAThreadContextAwareSuccessor(unlock) = this ) and (lock instanceof MutexFunctionCall implies not this.(MutexFunctionCall).isUnlock()) ) diff --git a/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected.qcc b/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected.qcc index 1155f0a056..a579781f95 100644 --- a/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected.qcc +++ b/cpp/common/test/rules/ownedpointervaluestoredinunrelatedsmartpointer/OwnedPointerValueStoredInUnrelatedSmartPointer.expected.qcc @@ -51,7 +51,7 @@ edges | test.cpp:17:27:17:28 | v1 | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:3757:34:3757:36 | __p | | test.cpp:17:27:17:28 | v1 | test.cpp:17:27:17:28 | ref arg v1 | | test.cpp:19:6:19:7 | v1 | test.cpp:3:14:3:15 | v1 | - nodes +nodes | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:2469:31:2469:33 | __p | semmle.label | __p | | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:2469:31:2469:33 | __p | semmle.label | __p | | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:3611:30:3611:32 | __p | semmle.label | __p | @@ -92,7 +92,7 @@ edges | test.cpp:17:27:17:28 | v1 | semmle.label | v1 | | test.cpp:17:27:17:28 | v1 | semmle.label | v1 | | test.cpp:19:6:19:7 | v1 | semmle.label | v1 | - subpaths +subpaths | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:4065:28:4065:30 | __p | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:2469:31:2469:33 | __p | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:2469:31:2469:33 | __p | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:4065:28:4065:30 | ref arg __p | | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:4068:30:4068:32 | __p | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:3611:30:3611:32 | __p | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:3611:30:3611:32 | __p | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:4068:30:4068:32 | ref arg __p | | test.cpp:5:27:5:28 | v1 | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:3757:34:3757:36 | __p | file:///opt/qcc/qnx-sdp/target/qnx7/usr/include/c++/v1/memory:4063:7:4063:17 | constructor init of field __ptr_ [post-this] [__ptr_] | test.cpp:5:27:5:29 | call to shared_ptr [__ptr_] |