zaks.anna added a comment. Thanks for the patch!
Have you run this on a large codebase? ================ Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:406 @@ +405,3 @@ +def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">, + HelpText<"Check for blocks in critical section">, + DescFile<"BlockInCriticalSectionChecker.cpp">; ---------------- "blocks" sounds a lot like blocks ObjC feature. I'd just be more explicit and say "Check for calls to blocking functions inside a critical section". Please, change the name of the class as well. ================ Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:54 @@ +53,3 @@ + +REGISTER_TRAIT_WITH_PROGRAMSTATE(Counter, unsigned) + ---------------- Counter -> MutexCounter or MutexState ================ Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:61 @@ +60,3 @@ + BlockInCritSectionBugType.reset( + new BugType(this, "Block in critical section", "Unix Stream API Error")); +} ---------------- Same here: "Block" sounds like an ObjC language feature. Also, the category name ("Unix Stream API Error") should be different. ================ Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:85 @@ +84,3 @@ + +void BlockInCriticalSectionChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { ---------------- Why is handling of unlock in preCall? ================ Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:96 @@ +95,3 @@ + +void BlockInCriticalSectionChecker::reportBlockInCritSection( + SymbolRef BlockDescSym, const CallEvent &Call, CheckerContext &C) const { ---------------- It would be great if you could implement a BugReporterVisitor that walks the path and explains where the lock has occurred. (This is not blocking for this commit.) ================ Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:103 @@ +102,3 @@ + auto R = llvm::make_unique<BugReport>(*BlockInCritSectionBugType, + "Block in a critical section", ErrNode); + R->addRange(Call.getSourceRange()); ---------------- "Block in a critical section" -> "A blocking function %s is called inside a critical section." ================ Comment at: test/Analysis/block-in-critical-section.cpp:12 @@ +11,3 @@ + +void testBlockInCriticalSection() { + std::mutex m; ---------------- Please, add inter-procedural examples like in the description of the checker. Repository: rL LLVM http://reviews.llvm.org/D21506 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits