ASDenysPetrov updated this revision to Diff 283916.
ASDenysPetrov added a comment.
@vsavchenko
Made changes due to your remarks. Thanks you.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85431/new/
https://reviews.llvm.org/D85431
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp
Index: clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.ThreadPrimitives -verify %s
+
+namespace std {
+struct mutex {
+ void lock();
+ void unlock();
+};
+template <typename T>
+struct guard_lock {
+ T &t;
+ guard_lock(T &m) : t(m) {
+ t.lock();
+ }
+ ~guard_lock() {
+ t.unlock();
+ }
+};
+} // namespace std
+
+void correct_lock_unlock(std::mutex m) {
+ m.lock();
+ m.unlock();
+}
+
+void incorrect_lock_unlock(std::mutex m1, std::mutex m2) {
+ m1.lock();
+ m2.unlock(); // expected-warning{{unlock without lock}}
+}
+
+void incorrect_lock_unlock2(std::mutex m, bool b) {
+ m.lock();
+ if (b)
+ m.unlock();
+}
+
+void unlock_without_lock(std::mutex m) {
+ m.unlock(); // expected-warning{{unlock without lock}}
+}
+
+void twice_lock(std::mutex m) {
+ m.lock();
+ m.lock(); // expected-warning{{lock more than once}}
+}
+
+void twice_lock2(std::mutex m) {
+ while (true)
+ m.lock(); // expected-warning{{lock more than once}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
@@ -0,0 +1,111 @@
+//=== ConversionChecker.cpp -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This checker finds STL thread primitives misuse.
+// - std::mutex::unlock without std::mutex::lock
+// - std::mutex::lock twice
+//
+//===----------------------------------------------------------------------===//
+#include "clang/AST/ParentMap.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/APFloat.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+enum MutexFunc {
+
+};
+
+class ThreadPrimitivesChecker : public Checker<check::PostCall> {
+public:
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+private:
+ BugType BT{this, "STL thread primitives misuse", "std::mutex misuse"};
+ CallDescription LockFunc{{"std", "mutex", "lock"}, 0, 0};
+ CallDescription UnlockFunc{{"std", "mutex", "unlock"}, 0, 0};
+
+ std::pair<bool, bool> FindMutexLockOrUnlock(const CallEvent &Call,
+ CheckerContext &C) const;
+ void reportBug(CheckerContext &C, const char Msg[]) const;
+};
+
+} // namespace
+
+REGISTER_SET_WITH_PROGRAMSTATE(LockedMutexes, SVal)
+
+std::pair<bool, bool>
+ThreadPrimitivesChecker::FindMutexLockOrUnlock(const CallEvent &Call,
+ CheckerContext &C) const {
+
+ if (const auto *MCall = dyn_cast<CXXMemberCall>(&Call)) {
+ const bool IsLockFunc = MCall->isCalled(LockFunc);
+ const bool IsUnlockFunc = IsLockFunc ? false : MCall->isCalled(UnlockFunc);
+ return {IsLockFunc, IsUnlockFunc};
+ }
+ return {false, false};
+}
+
+void ThreadPrimitivesChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ // Find mutex::lock or mutex::unlock functions.
+ bool IsLockFunc;
+ bool IsUnlockFunc;
+ std::tie(IsLockFunc, IsUnlockFunc) = FindMutexLockOrUnlock(Call, C);
+
+ if (!IsLockFunc && !IsUnlockFunc)
+ return;
+
+ // We are sure about cast here, because mutex::lock/unlock met before.
+ const auto *MCall = cast<CXXMemberCall>(&Call);
+ SVal SymVal = MCall->getCXXThisVal();
+ const bool LockFound = C.getState()->contains<LockedMutexes>(SymVal);
+
+ if (LockFound) {
+ if (IsLockFunc) {
+ reportBug(C, "Call mutex::lock more than once.");
+ } else if (IsUnlockFunc) {
+ ProgramStateRef State = C.getState()->remove<LockedMutexes>(SymVal);
+ C.addTransition(State);
+ }
+ } else {
+ if (IsLockFunc) {
+ ProgramStateRef State = C.getState()->add<LockedMutexes>(SymVal);
+ C.addTransition(State);
+ } else if (IsUnlockFunc) {
+ reportBug(C, "unlock without lock");
+ }
+ }
+}
+
+void ThreadPrimitivesChecker::reportBug(CheckerContext &C,
+ const char Msg[]) const {
+ ExplodedNode *ErrNode = C.generateNonFatalErrorNode();
+ if (!ErrNode)
+ return;
+
+ // Generate a report for this bug.
+ C.emitReport(std::make_unique<PathSensitiveBugReport>(BT, Msg, ErrNode));
+}
+
+void ento::registerThreadPrimitivesChecker(CheckerManager &mgr) {
+ mgr.registerChecker<ThreadPrimitivesChecker>();
+}
+
+bool ento::shouldRegisterThreadPrimitivesChecker(const CheckerManager &mgr) {
+ return true;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -107,6 +107,7 @@
Taint.cpp
TaintTesterChecker.cpp
TestAfterDivZeroChecker.cpp
+ ThreadPrimitivesChecker.cpp
TraversalChecker.cpp
TrustNonnullChecker.cpp
UndefBranchChecker.cpp
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -133,6 +133,8 @@
return !(*this == R);
}
+ bool operator<(const SVal &R) const { return Data < R.Data; }
+
bool isUnknown() const {
return getRawKind() == UnknownValKind;
}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -193,7 +193,7 @@
const ProgramPointTag *Tag = nullptr) {
if (!State)
State = getState();
- addTransition(State, generateSink(State, getPredecessor()));
+ addTransition(State, generateSink(State, getPredecessor(), Tag));
}
/// Generate a transition to a node that will be used to report
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -743,6 +743,10 @@
"of the same container are expected">,
Dependencies<[IteratorModeling]>,
Documentation<HasAlphaDocumentation>;
+
+def ThreadPrimitivesChecker : Checker<"ThreadPrimitives">,
+ HelpText<"Check STD synchronization primitives">,
+ Documentation<HasAlphaDocumentation>;
def SmartPtrChecker: Checker<"SmartPtr">,
HelpText<"Find the dereference of null SmrtPtr">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
