From 59785c52625a4dd49a813abe6ce0b215cbcff439 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Wed, 19 Nov 2025 15:59:31 -0800 Subject: [PATCH] Fix #972, CON51-CPP falsely reporting that std::lock_guard requires catch. --- ...exclude-raii-style-locks-from-con51-cpp.md | 2 ++ ...LocksAreReleasedOnExceptionalConditions.ql | 2 ++ cpp/cert/test/rules/CON51-CPP/test.cpp | 6 +++++ .../concurrency/LockProtectedControlFlow.qll | 5 ++-- .../cpp/concurrency/LockingOperation.qll | 25 +++++++++++++++++-- 5 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 change_notes/2025-11-19-exclude-raii-style-locks-from-con51-cpp.md diff --git a/change_notes/2025-11-19-exclude-raii-style-locks-from-con51-cpp.md b/change_notes/2025-11-19-exclude-raii-style-locks-from-con51-cpp.md new file mode 100644 index 0000000000..f7b9ce4836 --- /dev/null +++ b/change_notes/2025-11-19-exclude-raii-style-locks-from-con51-cpp.md @@ -0,0 +1,2 @@ + - `CON51-CPP` - `EnsureActivelyHeldLocksAreReleasedOnExceptionalConditions.ql`: + - Exclude RAII-style locks from query results, as they cannot be leaked, and are recommended to avoid alerts in this rule. \ No newline at end of file diff --git a/cpp/cert/src/rules/CON51-CPP/EnsureActivelyHeldLocksAreReleasedOnExceptionalConditions.ql b/cpp/cert/src/rules/CON51-CPP/EnsureActivelyHeldLocksAreReleasedOnExceptionalConditions.ql index ac09d41c42..13977e2c1d 100644 --- a/cpp/cert/src/rules/CON51-CPP/EnsureActivelyHeldLocksAreReleasedOnExceptionalConditions.ql +++ b/cpp/cert/src/rules/CON51-CPP/EnsureActivelyHeldLocksAreReleasedOnExceptionalConditions.ql @@ -36,6 +36,8 @@ where // To reduce the number of results we require that this is a direct child // of the lock within the same function lpn.coveredByLock().getASuccessor*() = lpn and + // Exclude RAII-style locks which cannot leak + lpn.coveredByLock().canLeak() and // report those expressions for which there doesn't exist a catch block not exists(CatchBlock cb | catches(cb, lpn, _) and diff --git a/cpp/cert/test/rules/CON51-CPP/test.cpp b/cpp/cert/test/rules/CON51-CPP/test.cpp index 7f5731e479..6d714f7e16 100644 --- a/cpp/cert/test/rules/CON51-CPP/test.cpp +++ b/cpp/cert/test/rules/CON51-CPP/test.cpp @@ -83,6 +83,11 @@ void f8(std::mutex *pm) { } } +void f9(std::mutex *pm) { + std::lock_guard lg(*pm); + mightThrow(); // COMPLIANT +} + void m() { std::mutex pm; std::thread t1 = std::thread(f1, &pm); @@ -93,4 +98,5 @@ void m() { std::thread t6 = std::thread(f6, &pm); std::thread t7 = std::thread(f7, &pm); std::thread t8 = std::thread(f8, &pm); + std::thread t9 = std::thread(f9, &pm); } diff --git a/cpp/common/src/codingstandards/cpp/concurrency/LockProtectedControlFlow.qll b/cpp/common/src/codingstandards/cpp/concurrency/LockProtectedControlFlow.qll index a828ec8768..5def491fb2 100644 --- a/cpp/common/src/codingstandards/cpp/concurrency/LockProtectedControlFlow.qll +++ b/cpp/common/src/codingstandards/cpp/concurrency/LockProtectedControlFlow.qll @@ -38,9 +38,10 @@ class LockProtectedControlFlowNode extends ThreadedCFN { } /** - * The `MutexFunctionCall` holding the lock that locks this node. + * The LockingOperation (for instance, a `MutexFunctionCall`, or RAII-style lock constructor) + * holding the lock that locks this node. */ - FunctionCall coveredByLock() { result = lockingFunction } + LockingOperation coveredByLock() { result = lockingFunction } /** * The lock underlying this `LockProtectedControlFlowNode`. diff --git a/cpp/common/src/codingstandards/cpp/concurrency/LockingOperation.qll b/cpp/common/src/codingstandards/cpp/concurrency/LockingOperation.qll index 0aab11a269..cfa263792e 100644 --- a/cpp/common/src/codingstandards/cpp/concurrency/LockingOperation.qll +++ b/cpp/common/src/codingstandards/cpp/concurrency/LockingOperation.qll @@ -22,6 +22,11 @@ abstract class LockingOperation extends FunctionCall { */ abstract predicate isUnlock(); + /** + * Holds if this locking operation can leak the lock. For example, RAII-style locks cannot leak. + */ + abstract predicate canLeak(); + /** * Holds if this locking operation is really a locking operation within a * designated locking operation. This library assumes the underlying locking @@ -33,11 +38,27 @@ abstract class LockingOperation extends FunctionCall { } } +/** + * A locking operation that can leak the lock. + * + * RAII style locks cannot leak, but other kinds of locks can. + */ +abstract class LeakableLockingOperation extends LockingOperation { + override predicate canLeak() { any() } +} + +/** + * A locking operation that cannot leak the lock, such as RAII-style locks. + */ +abstract class NonLeakableLockingOperation extends LockingOperation { + override predicate canLeak() { none() } +} + /** * Common base class providing an interface into function call * based mutex locks. */ -abstract class MutexFunctionCall extends LockingOperation { +abstract class MutexFunctionCall extends LeakableLockingOperation { abstract predicate isRecursive(); abstract predicate isSpeculativeLock(); @@ -180,7 +201,7 @@ class CMutexFunctionCall extends MutexFunctionCall { /** * Models a RAII-Style lock. */ -class RAIIStyleLock extends LockingOperation { +class RAIIStyleLock extends NonLeakableLockingOperation { VariableAccess lock; RAIIStyleLock() {