From 213d6e84ccb5b52c5cbb44f72881aff35965445b Mon Sep 17 00:00:00 2001 From: "John L. Singleton" Date: Fri, 26 May 2023 17:04:20 -0400 Subject: [PATCH 1/7] testing qcc --- .../CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql | 2 ++ .../src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql | 2 ++ 2 files changed, 4 insertions(+) diff --git a/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql b/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql index 6259ff67d3..fa8a24da1f 100644 --- a/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql +++ b/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql @@ -21,6 +21,8 @@ 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 + + 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..f98843aeb0 100644 --- a/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql +++ b/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql @@ -21,6 +21,8 @@ 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 + + n.(MutexFunctionCall).isSpeculativeLock() and not n.(MutexFunctionCall).isRecursive() select n, "(Audit) Attempt to speculatively lock a non-recursive mutex while it is $@.", From 67ae121568f119e01abed3e90cb88c21db486d64 Mon Sep 17 00:00:00 2001 From: "John L. Singleton" Date: Fri, 26 May 2023 18:58:34 -0400 Subject: [PATCH 2/7] logic fix --- cpp/common/src/codingstandards/cpp/Concurrency.qll | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index 66af30dbb9..93bc72feda 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -231,7 +231,7 @@ pragma[inline] ControlFlowNode getAThreadContextAwarePredecessor(ControlFlowNode start, ControlFlowNode end) { result = getAThreadContextAwareSuccessor(start) and not result = getAThreadContextAwareSuccessor(end) and - not result = end + not result = end } /** @@ -402,6 +402,13 @@ class LockProtectedControlFlowNode extends ThreadedCFN { unlock.(MutexFunctionCall).isUnlock() // 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 lock (to be actually) + // an unlock along the same path it must be the case that when we + // supply it as the starting point of the search it hits the try lock + and getAThreadContextAwareSuccessor(unlock) = this ) and (lock instanceof MutexFunctionCall implies not this.(MutexFunctionCall).isUnlock()) ) From e58596a1f95a58596bed3eb97d302c41d0f91dee Mon Sep 17 00:00:00 2001 From: "John L. Singleton" Date: Fri, 26 May 2023 19:07:38 -0400 Subject: [PATCH 3/7] format --- .../src/codingstandards/cpp/Concurrency.qll | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index 93bc72feda..f202e7adb7 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -231,7 +231,7 @@ pragma[inline] ControlFlowNode getAThreadContextAwarePredecessor(ControlFlowNode start, ControlFlowNode end) { result = getAThreadContextAwareSuccessor(start) and not result = getAThreadContextAwareSuccessor(end) and - not result = end + not result = end } /** @@ -399,16 +399,15 @@ 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 lock (to be actually) - // an unlock along the same path it must be the case that when we - // supply it as the starting point of the search it hits the try lock - and getAThreadContextAwareSuccessor(unlock) = this + // for operations PAST it to be technically part of the predecessors. + // Thus, we need to make sure that this lock (to be actually) + // an unlock along the same path it must be the case that when we + // supply it as the starting point of the search it hits the try lock + getAThreadContextAwareSuccessor(unlock) = this ) and (lock instanceof MutexFunctionCall implies not this.(MutexFunctionCall).isUnlock()) ) From cf2f486e3fd9f1e7da177c9994d502c6e6b33900 Mon Sep 17 00:00:00 2001 From: "John L. Singleton" Date: Fri, 26 May 2023 19:31:16 -0400 Subject: [PATCH 4/7] work --- .../DoNotSpeculativelyLockALockedNonRecursiveMutex.ql | 6 ++---- .../rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql b/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql index fa8a24da1f..94d23c8664 100644 --- a/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql +++ b/cpp/cert/src/rules/CON56-CPP/DoNotSpeculativelyLockALockedNonRecursiveMutex.ql @@ -19,10 +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 f98843aeb0..d3a5778f2c 100644 --- a/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql +++ b/cpp/cert/src/rules/CON56-CPP/LockedALockedNonRecursiveMutexAudit.ql @@ -19,10 +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 $@.", From 4c7c348cb966028f02370ce45411ec54e2976469 Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Mon, 29 May 2023 11:58:42 +0200 Subject: [PATCH 5/7] Fix OwnedPointerValueStoredInUnrelatedSmartPointer.expected.qcc removed extra spaces --- ...wnedPointerValueStoredInUnrelatedSmartPointer.expected.qcc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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_] | From 390f67003792c20242d6224b021ed0d00dd96044 Mon Sep 17 00:00:00 2001 From: "John L. Singleton" Date: Tue, 30 May 2023 09:19:33 -0400 Subject: [PATCH 6/7] Update cpp/common/src/codingstandards/cpp/Concurrency.qll Co-authored-by: Mauro Baluda --- cpp/common/src/codingstandards/cpp/Concurrency.qll | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index f202e7adb7..fa8ae5a615 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -404,9 +404,8 @@ class LockProtectedControlFlowNode extends ThreadedCFN { // 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 lock (to be actually) - // an unlock along the same path it must be the case that when we - // supply it as the starting point of the search it hits the try lock + // 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()) From 6e8ef103a7da254157f2509d458210c98175ba5e Mon Sep 17 00:00:00 2001 From: "John L. Singleton" Date: Tue, 30 May 2023 10:59:20 -0400 Subject: [PATCH 7/7] format fix --- cpp/common/src/codingstandards/cpp/Concurrency.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index fa8ae5a615..5162255de9 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -404,7 +404,7 @@ class LockProtectedControlFlowNode extends ThreadedCFN { // 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 + // Thus, we need to make sure that this node is a // successor of the unlock in the CFG getAThreadContextAwareSuccessor(unlock) = this ) and