From 6dd66ed959b7f60749dd0040507b3f304183a1b6 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Sun, 14 Feb 2010 02:45:18 +0000 Subject: [PATCH] Add new static analyzer for checking LLVM coding conventions: -analyzer-check-llvm-conventions Currently these checks are intended to be largely syntactical, but may get more sophisticated over time. As an initial foray into this brave new world, emit a static analyzer warning when binding a temporary 'std::string' to an 'llvm::StringRef' where the lifetime of the 'std::string' does not outlive the 'llvm::StringRef'. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@96147 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Checker/Checkers/LocalCheckers.h | 2 +- include/clang/Driver/CC1Options.td | 2 + include/clang/Frontend/Analyses.def | 4 + lib/Checker/CMakeLists.txt | 1 + lib/Checker/LLVMConventionsChecker.cpp | 127 ++++++++++++++++++ lib/Frontend/AnalysisConsumer.cpp | 8 ++ 6 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 lib/Checker/LLVMConventionsChecker.cpp diff --git a/include/clang/Checker/Checkers/LocalCheckers.h b/include/clang/Checker/Checkers/LocalCheckers.h index b179c8eb6c..a262aaa1fb 100644 --- a/include/clang/Checker/Checkers/LocalCheckers.h +++ b/include/clang/Checker/Checkers/LocalCheckers.h @@ -50,8 +50,8 @@ void RegisterAppleChecks(GRExprEngine& Eng, const Decl &D); void RegisterExperimentalChecks(GRExprEngine &Eng); void RegisterExperimentalInternalChecks(GRExprEngine &Eng); +void CheckLLVMConventions(const Decl *D, BugReporter &BR); void CheckSecuritySyntaxOnly(const Decl *D, BugReporter &BR); - void CheckSizeofPointer(const Decl *D, BugReporter &BR); void RegisterCallInliner(GRExprEngine &Eng); diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td index bd243d681f..047363ea59 100644 --- a/include/clang/Driver/CC1Options.td +++ b/include/clang/Driver/CC1Options.td @@ -38,6 +38,8 @@ def analysis_CFGView : Flag<"-cfg-view">, HelpText<"View Control-Flow Graphs using GraphViz">; def analysis_DisplayLiveVariables : Flag<"-dump-live-variables">, HelpText<"Print results of live variable analysis">; +def analysis_LLVMConventionChecker : Flag<"-analyzer-check-llvm-conventions">, + HelpText<"Check code for LLVM codebase conventions (domain-specific)">; def analysis_SecuritySyntacticChecks : Flag<"-analyzer-check-security-syntactic">, HelpText<"Perform quick security checks that require no data flow">; def analysis_WarnDeadStores : Flag<"-analyzer-check-dead-stores">, diff --git a/include/clang/Frontend/Analyses.def b/include/clang/Frontend/Analyses.def index fc1030fa43..6ea43f7341 100644 --- a/include/clang/Frontend/Analyses.def +++ b/include/clang/Frontend/Analyses.def @@ -28,6 +28,10 @@ ANALYSIS(SecuritySyntacticChecks, "analyzer-check-security-syntactic", "Perform quick security checks that require no data flow", Code) +ANALYSIS(LLVMConventionChecker, "analyzer-check-llvm-conventions", + "Check code for LLVM codebase conventions (domain-specific)", + Code) + ANALYSIS(WarnDeadStores, "analyzer-check-dead-stores", "Warn about stores to dead variables", Code) diff --git a/lib/Checker/CMakeLists.txt b/lib/Checker/CMakeLists.txt index 130378a247..2aca818caa 100644 --- a/lib/Checker/CMakeLists.txt +++ b/lib/Checker/CMakeLists.txt @@ -34,6 +34,7 @@ add_clang_library(clangChecker GRExprEngine.cpp GRExprEngineExperimentalChecks.cpp GRState.cpp + LLVMConventionsChecker.cpp MallocChecker.cpp ManagerRegistry.cpp MemRegion.cpp diff --git a/lib/Checker/LLVMConventionsChecker.cpp b/lib/Checker/LLVMConventionsChecker.cpp new file mode 100644 index 0000000000..82f6d2c7bd --- /dev/null +++ b/lib/Checker/LLVMConventionsChecker.cpp @@ -0,0 +1,127 @@ +//=== LLVMConventionsChecker.cpp - Check LLVM codebase conventions ---*- C++ -*- +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This defines LLVMConventionsChecker, a bunch of small little checks +// for checking specific coding conventions in the LLVM/Clang codebase. +// +//===----------------------------------------------------------------------===// + +#include "clang/AST/DeclCXX.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/Checker/Checkers/LocalCheckers.h" +#include "clang/Checker/BugReporter/BugReporter.h" +#include +#include + +using namespace clang; + +//===----------------------------------------------------------------------===// +// Check if an llvm::StringRef is bound to temporary std::string whose lifetime +// is shorter than the StringRef's. +//===----------------------------------------------------------------------===// + +namespace { +class StringRefCheckerVisitor : public StmtVisitor { + BugReporter &BR; +public: + StringRefCheckerVisitor(BugReporter &br) : BR(br) {} + void VisitChildren(Stmt *S) { + for (Stmt::child_iterator I = S->child_begin(), E = S->child_end() ; + I != E; ++I) + if (Stmt *child = *I) + Visit(child); + } + void VisitStmt(Stmt *S) { VisitChildren(S); } + void VisitDeclStmt(DeclStmt *DS); +private: + void VisitVarDecl(VarDecl *VD); +}; +} // end anonymous namespace + +static void CheckStringRefAssignedTemporary(const Decl *D, BugReporter &BR) { + StringRefCheckerVisitor walker(BR); + walker.Visit(D->getBody()); +} + +void StringRefCheckerVisitor::VisitDeclStmt(DeclStmt *S) { + for (DeclStmt::decl_iterator I = S->decl_begin(), E = S->decl_end();I!=E; ++I) + if (VarDecl *VD = dyn_cast(*I)) + VisitVarDecl(VD); +} + +static bool IsStringRef(QualType T) { + const RecordType *RT = T->getAs(); + if (!RT) + return false; + + return llvm::StringRef(QualType(RT, 0).getAsString()) == + "class llvm::StringRef"; +} + +static bool IsStdString(QualType T) { + if (const QualifiedNameType *QT = T->getAs()) + T = QT->getNamedType(); + + const TypedefType *TT = T->getAs(); + if (!TT) + return false; + + const TypedefDecl *TD = TT->getDecl(); + const NamespaceDecl *ND = dyn_cast(TD->getDeclContext()); + if (!ND) + return false; + const IdentifierInfo *II = ND->getIdentifier(); + if (!II || II->getName() != "std") + return false; + + DeclarationName N = TD->getDeclName(); + return llvm::StringRef(N.getAsString()) == "string"; +} + +void StringRefCheckerVisitor::VisitVarDecl(VarDecl *VD) { + Expr *Init = VD->getInit(); + if (!Init) + return; + + // Pattern match for: + // llvm::StringRef x = call() (where call returns std::string) + if (!IsStringRef(VD->getType())) + return; + CXXExprWithTemporaries *Ex1 = dyn_cast(Init); + if (!Ex1) + return; + CXXConstructExpr *Ex2 = dyn_cast(Ex1->getSubExpr()); + if (!Ex2 || Ex2->getNumArgs() != 1) + return; + ImplicitCastExpr *Ex3 = dyn_cast(Ex2->getArg(0)); + if (!Ex3) + return; + CXXConstructExpr *Ex4 = dyn_cast(Ex3->getSubExpr()); + if (!Ex4 || Ex4->getNumArgs() != 1) + return; + ImplicitCastExpr *Ex5 = dyn_cast(Ex4->getArg(0)); + if (!Ex5) + return; + CXXBindTemporaryExpr *Ex6 = dyn_cast(Ex5->getSubExpr()); + if (!Ex6 || !IsStdString(Ex6->getType())) + return; + + // Okay, badness! Report an error. + BR.EmitBasicReport("StringRef should not be bound to temporary " + "std::string that it outlives", "LLVM Conventions", + VD->getLocStart(), Init->getSourceRange()); +} + +//===----------------------------------------------------------------------===// +// Entry point for all checks. +//===----------------------------------------------------------------------===// + +void clang::CheckLLVMConventions(const Decl *D, BugReporter &BR) { + CheckStringRefAssignedTemporary(D, BR); +} diff --git a/lib/Frontend/AnalysisConsumer.cpp b/lib/Frontend/AnalysisConsumer.cpp index 62073269c1..64fa1d24f7 100644 --- a/lib/Frontend/AnalysisConsumer.cpp +++ b/lib/Frontend/AnalysisConsumer.cpp @@ -446,6 +446,14 @@ static void ActionSecuritySyntacticChecks(AnalysisConsumer &C, CheckSecuritySyntaxOnly(D, BR); } +static void ActionLLVMConventionChecker(AnalysisConsumer &C, + AnalysisManager &mgr, + Decl *D) { + C.DisplayFunction(D); + BugReporter BR(mgr); + CheckLLVMConventions(D, BR); +} + static void ActionWarnObjCDealloc(AnalysisConsumer &C, AnalysisManager& mgr, Decl *D) { if (mgr.getLangOptions().getGCMode() == LangOptions::GCOnly) -- 2.40.0