Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ignore mutable and static warnings for std::atomic<>, std::mutex, std::r... #1241

Merged
merged 5 commits into from Oct 31, 2013
Merged

ignore mutable and static warnings for std::atomic<>, std::mutex, std::r... #1241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc9260c
ignore mutable and static warnings for std::atomic<>, std::mutex, std…
gartung Oct 30, 2013
d21e484
FunctionChecker should only report a function once per non-const stat…
gartung Oct 30, 2013
63ed419
use shorter function name in warning message
gartung Oct 30, 2013
71f612e
skip mutable checkers unless the parent class is a data class
gartung Oct 31, 2013
e10a542
remove isDataClass check for StaticLocalChecker
gartung Oct 31, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 30 additions & 16 deletions Utilities/StaticAnalyzers/src/ClassChecker.cpp
View file
Open in desktop
Expand Up @@ -15,7 +15,18 @@
// pass by const ref & pointer OK
//
//

#include <clang/AST/Decl.h>
#include <clang/AST/DeclTemplate.h>
#include <clang/AST/DeclCXX.h>
#include <clang/AST/StmtVisitor.h>
#include <clang/AST/ParentMap.h>
#include <clang/Analysis/CFGStmtMap.h>
#include <clang/StaticAnalyzer/Core/Checker.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugReporter.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugType.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h>
#include <llvm/Support/SaveAndRestore.h>
#include <llvm/ADT/SmallString.h>

#include "ClassChecker.h"

Expand Down Expand Up @@ -288,6 +299,7 @@ void WalkAST::VisitDeclRefExpr( clang::DeclRefExpr * DRE) {
if (clang::VarDecl * D = llvm::dyn_cast<clang::VarDecl>(DRE->getDecl()) ) {
clang::SourceLocation SL = DRE->getLocStart();
if (BR.getSourceManager().isInSystemHeader(SL) || BR.getSourceManager().isInExternCSystemHeader(SL)) return;
if ( support::isSafeClassName( D->getCanonicalDecl()->getQualifiedNameAsString() ) ) return;
ReportDeclRef( DRE );

// llvm::errs()<<"Declaration Ref Expr\t";
Expand Down Expand Up @@ -618,21 +630,23 @@ void ClassChecker::checkASTDecl(const clang::CXXRecordDecl *RD, clang::ento::Ana

llvm::SmallString<100> buf;
llvm::raw_svector_ostream os(buf);
clang::FileSystemOptions FSO;
clang::FileManager FM(FSO);
const char * pPath = std::getenv("LOCALRT");
std::string dname("");
if ( pPath != NULL ) dname = std::string(pPath);
std::string fname("/tmp/classes.txt");
std::string tname = dname + fname;
if (!FM.getFile(tname) ) {
llvm::errs()<<"\n\nChecker optional.ClassChecker cannot find $LOCALRT/tmp/classes.txt. Run 'scram b checker' with USER_LLVM_CHECKERS='-enable-checker optional.ClassDumperCT -enable-checker optional.ClassDumperFT' to create $LOCALRT/tmp/classes.txt.\n\n\n";
exit(1);
}
llvm::MemoryBuffer * buffer = FM.getBufferForFile(FM.getFile(tname));
os <<"class "<<RD->getQualifiedNameAsString()<<"\n";
llvm::StringRef Rname(os.str());
if (buffer->getBuffer().find(Rname) == llvm::StringRef::npos ) {return;}
// clang::FileSystemOptions FSO;
// clang::FileManager FM(FSO);
// const char * pPath = std::getenv("LOCALRT");
// std::string dname("");
// if ( pPath != NULL ) dname = std::string(pPath);
// std::string fname("/tmp/classes.txt");
// std::string tname = dname + fname;
// if (!FM.getFile(tname) ) {
// llvm::errs()<<"\n\nChecker optional.ClassChecker cannot find $LOCALRT/tmp/classes.txt. Run 'scram b checker' with USER_LLVM_CHECKERS='-enable-checker optional.ClassDumperCT -enable-checker optional.ClassDumperFT' to create $LOCALRT/tmp/classes.txt.\n\n\n";
// exit(1);
// }
// llvm::MemoryBuffer * buffer = FM.getBufferForFile(FM.getFile(tname));
// os <<"class "<<RD->getQualifiedNameAsString()<<"\n";
// llvm::StringRef Rname(os.str());
// if (buffer->getBuffer().find(Rname) == llvm::StringRef::npos ) {return;}
std::string name = RD->getQualifiedNameAsString();
if ( ! support::isDataClass(name) ) return;
clang::ento::PathDiagnosticLocation DLoc =clang::ento::PathDiagnosticLocation::createBegin( RD, SM );
if ( !m_exception.reportClass( DLoc, BR ) ) return;
// clangcms::WalkAST walker(BR, mgr.getAnalysisDeclContext(RD));
Expand Down
11 changes: 1 addition & 10 deletions Utilities/StaticAnalyzers/src/ClassChecker.h
View file
Open in desktop
@@ -1,17 +1,8 @@
#ifndef Utilities_StaticAnalyzers_MemberChecker_h
#define Utilities_StaticAnalyzers_MemberChecker_h
#include <clang/AST/DeclCXX.h>
#include <clang/AST/Decl.h>
#include <clang/AST/DeclTemplate.h>
#include <clang/AST/StmtVisitor.h>
#include <clang/AST/ParentMap.h>
#include <clang/Analysis/CFGStmtMap.h>
#include <llvm/Support/SaveAndRestore.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h>
#include <clang/StaticAnalyzer/Core/Checker.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugReporter.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugType.h>
#include <llvm/ADT/SmallString.h>

#include "CmsException.h"
#include "CmsSupport.h"
Expand Down
52 changes: 51 additions & 1 deletion Utilities/StaticAnalyzers/src/CmsSupport.cpp
View file
Open in desktop
Expand Up @@ -3,12 +3,19 @@
// by Shahzad Malik MUZAFFAR [ Shahzad.Malik.MUZAFFAR@cern.ch ]
//
//===----------------------------------------------------------------------===//

#include <clang/Basic/FileManager.h>
#include <clang/StaticAnalyzer/Core/Checker.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugType.h>
#include <llvm/ADT/SmallString.h>
#include "llvm/Support/raw_ostream.h"
#include "CmsSupport.h"
#include <cstdlib>
#include <cstring>

using namespace clangcms;
using namespace clang;
using namespace llvm;
bool support::isCmsLocalFile(const char* file)
{
static char* LocalDir=0;
Expand Down Expand Up @@ -74,4 +81,47 @@ std::string support::getQualifiedName(const clang::NamedDecl &d) {
return ret;
}

bool support::isSafeClassName(const std::string &name) {
std::string atomic = "std::atomic<";
std::string mutex = "std::mutex";
std::string rmutex = "std::recursive_mutex";
std::string btsp = "boost::thread_specific_ptr<";

if ( name.substr(0,atomic.length()) == atomic || name.substr(0,mutex.length()) == mutex
|| name.substr(0,rmutex.length()) == rmutex || name.substr(0,btsp.length()) == btsp )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly certain substr is a const member function so there is no need to copy 'd' to 'name'. Also, it would be better to move the immutable strings outside the function and declare them as class const statics

static const std::string kAtomic("std::atomic<");

I'd also recommend starting them with 'k' since that is often used to denote a 'konstant' in our code.

return true;
return false;
}

bool support::isDataClass(const std::string & name) {
std::string buf;
llvm::raw_string_ostream os(buf);
clang::FileSystemOptions FSO;
clang::FileManager FM(FSO);
const char * lPath = std::getenv("LOCALRT");
const char * rPath = std::getenv("CMSSW_RELEASE_BASE");
std::string lname("");
std::string rname("");
if ( lPath != NULL && rPath != NULL ) {
lname = std::string(lPath);
rname = std::string(rPath);
}

std::string tname("/tmp/classes.txt");
std::string sname("/Utilities/StaticAnalyzers/scripts/classes.txt");
std::string fname1 = lname + tname;
std::string fname2 = rname + sname;
if (!FM.getFile(fname1) && !FM.getFile(fname2) ) {
llvm::errs()<<"\n\nChecker cannot find classes.txt. Run \"USER_LLVM_CHECKERS='-enable-checker optional.ClassDumperCT -enable-checker optional.ClassDumperFT scram b checker to create $LOCALRT/tmp/classes.txt.\n\n\n";
exit(1);
}
llvm::MemoryBuffer * buffer;
if ( FM.getFile(fname1) )
buffer = FM.getBufferForFile(FM.getFile(fname1));
else
buffer = FM.getBufferForFile(FM.getFile(fname2));
os <<"class "<< name <<"\n";
llvm::StringRef Rname(os.str());
if ( buffer->getBuffer().find(Rname) == llvm::StringRef::npos ) return false;
return true;
}
3 changes: 3 additions & 0 deletions Utilities/StaticAnalyzers/src/CmsSupport.h
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@

#include <clang/AST/Type.h>
#include <clang/AST/Decl.h>
#include <clang/AST/DeclCXX.h>

namespace clangcms {

Expand Down Expand Up @@ -45,6 +46,8 @@ inline bool isConst( clang::QualType const& qt )

bool isCmsLocalFile(const char* file);
std::string getQualifiedName(const clang::NamedDecl &d);
bool isSafeClassName(const std::string &d);
bool isDataClass(const std::string &d);
}
}

Expand Down
55 changes: 40 additions & 15 deletions Utilities/StaticAnalyzers/src/FunctionChecker.cpp
View file
Open in desktop
@@ -1,4 +1,20 @@
#include <clang/AST/DeclCXX.h>
#include <clang/AST/Decl.h>
#include <clang/AST/DeclTemplate.h>
#include <clang/AST/StmtVisitor.h>
#include <clang/AST/ParentMap.h>
#include <clang/Analysis/CFGStmtMap.h>
#include <clang/Analysis/CallGraph.h>
#include <llvm/Support/SaveAndRestore.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h>
#include <clang/StaticAnalyzer/Core/Checker.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugReporter.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugType.h>
#include <llvm/ADT/SmallString.h>

#include "FunctionChecker.h"

using namespace clang;
using namespace ento;
using namespace llvm;
Expand Down Expand Up @@ -37,7 +53,8 @@ void FWalker::VisitChildren( clang::Stmt *S) {


void FWalker::VisitDeclRefExpr( clang::DeclRefExpr * DRE) {
if (clang::VarDecl * D = llvm::dyn_cast<clang::VarDecl>(DRE->getDecl()) ) {
if (clang::VarDecl * D = llvm::dyn_cast<clang::VarDecl>(DRE->getDecl()) ) {
if ( support::isSafeClassName(D->getCanonicalDecl()->getQualifiedNameAsString() ) ) return;
ReportDeclRef(DRE);
// llvm::errs()<<"Declaration Ref Expr\t";
// dyn_cast<Stmt>(DRE)->dumpPretty(AC->getASTContext());
Expand All @@ -57,19 +74,25 @@ void FWalker::ReportDeclRef ( const clang::DeclRefExpr * DRE) {
clang::PrintingPolicy Policy(LangOpts);
const Decl * PD = AC->getDecl();
std::string dname ="";
if (const NamedDecl * ND = llvm::dyn_cast<NamedDecl>(PD)) dname = support::getQualifiedName(*ND);
std::string sdname ="";
if (const NamedDecl * ND = llvm::dyn_cast<NamedDecl>(PD)) {
sdname = support::getQualifiedName(*ND);
dname = ND->getQualifiedNameAsString();
}

clang::ento::PathDiagnosticLocation CELoc = clang::ento::PathDiagnosticLocation::createBegin(DRE, BR.getSourceManager(),AC);
// clang::ento::PathDiagnosticLocation CELoc = clang::ento::PathDiagnosticLocation::createBegin(DRE, BR.getSourceManager(),AC);
clang::ento::PathDiagnosticLocation DLoc = clang::ento::PathDiagnosticLocation::createBegin(D, BR.getSourceManager());

if ( (D->isStaticLocal() && D->getTSCSpec() != clang::ThreadStorageClassSpecifier::TSCS_thread_local ) && ! clangcms::support::isConst( t ) )
{
std::string buf;
llvm::raw_string_ostream os(buf);
os << "function '"<<dname << "' accesses or modifies non-const static local variable '" << D->getNameAsString() << "'.\n";
BugType * BT = new BugType("FunctionChecker : non-const static local variable accessed or modified","ThreadSafety");
BugReport * R = new BugReport(*BT,os.str(),CELoc);
BR.emitReport(R);
llvm::errs() << "function '"<<dname << "' static variable '" << D->getNameAsString() << "'.\n\n";
// BugType * BT = new BugType("FunctionChecker : non-const static local variable accessed or modified","ThreadSafety");
// BugReport * R = new BugReport(*BT,os.str(),CELoc);
// BR.emitReport(R);
BR.EmitBasicReport(D, "FunctionChecker : non-const static local variable accessed or modified","ThreadSafety",os.str(), DLoc);
llvm::errs() << "function '"<<sdname << "' static variable '" << D->getNameAsString() << "'.\n\n";
return;
}

Expand All @@ -78,10 +101,11 @@ void FWalker::ReportDeclRef ( const clang::DeclRefExpr * DRE) {
std::string buf;
llvm::raw_string_ostream os(buf);
os << "function '"<<dname<< "' accesses or modifies non-const static member data variable '" << D->getNameAsString() << "'.\n";
BugType * BT = new BugType("FunctionChecker : non-const static member variable accessed or modified","ThreadSafety");
BugReport * R = new BugReport(*BT,os.str(),CELoc);
BR.emitReport(R);
llvm::errs() << "function '"<<dname << "' static variable '" << D->getNameAsString() << "'.\n\n";
// BugType * BT = new BugType("FunctionChecker : non-const static member variable accessed or modified","ThreadSafety");
// BugReport * R = new BugReport(*BT,os.str(),CELoc);
// BR.emitReport(R);
BR.EmitBasicReport(D, "FunctionChecker : non-const static local variable accessed or modified","ThreadSafety",os.str(), DLoc);
llvm::errs() << "function '"<<sdname << "' static variable '" << D->getNameAsString() << "'.\n\n";
return;
}

Expand All @@ -95,10 +119,11 @@ void FWalker::ReportDeclRef ( const clang::DeclRefExpr * DRE) {
std::string buf;
llvm::raw_string_ostream os(buf);
os << "function '"<<dname << "' accesses or modifies non-const global static variable '" << D->getNameAsString() << "'.\n";
BugType * BT = new BugType("FunctionChecker : non-const global static variable accessed or modified","ThreadSafety");
BugReport * R = new BugReport(*BT,os.str(),CELoc);
BR.emitReport(R);
llvm::errs() << "function '"<<dname << "' static variable '" << D->getNameAsString() << "'.\n\n";
// BugType * BT = new BugType("FunctionChecker : non-const global static variable accessed or modified","ThreadSafety");
// BugReport * R = new BugReport(*BT,os.str(),CELoc);
// BR.emitReport(R);
BR.EmitBasicReport(D, "FunctionChecker : non-const static local variable accessed or modified","ThreadSafety",os.str(), DLoc);
llvm::errs() << "function '"<<sdname << "' static variable '" << D->getNameAsString() << "'.\n\n";
return;

}
Expand Down
13 changes: 1 addition & 12 deletions Utilities/StaticAnalyzers/src/FunctionChecker.h
View file
Open in desktop
@@ -1,19 +1,8 @@
#ifndef Utilities_StaticAnalyzers_FunctionChecker_h
#define Utilities_StaticAnalyzers_FunctionChecker_h
#include <clang/AST/DeclCXX.h>
#include <clang/AST/Decl.h>
#include <clang/AST/DeclTemplate.h>
#include <clang/AST/StmtVisitor.h>
#include <clang/AST/ParentMap.h>
#include <clang/Analysis/CFGStmtMap.h>
#include <clang/Analysis/CallGraph.h>
#include <llvm/Support/SaveAndRestore.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h>
#include <clang/StaticAnalyzer/Core/Checker.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugReporter.h>
#include <clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h>
#include <clang/StaticAnalyzer/Core/BugReporter/BugType.h>
#include <llvm/ADT/SmallString.h>

#include "CmsException.h"
#include "CmsSupport.h"
Expand Down
3 changes: 2 additions & 1 deletion Utilities/StaticAnalyzers/src/GlobalStaticChecker.cpp
View file
Open in desktop
Expand Up @@ -27,10 +27,11 @@ void GlobalStaticChecker::checkASTDecl(const clang::VarDecl *D,

if ( ! m_exception.reportGlobalStaticForType( t, DLoc, BR ) )
return;
if ( support::isSafeClassName( D->getCanonicalDecl()->getQualifiedNameAsString() ) ) return;

std::string buf;
llvm::raw_string_ostream os(buf);
os << "Non-const variable '" << *D << "' is static and might be thread-unsafe";
os << "Non-const variable '" << t.getAsString()<<" "<<*D << "' is static and might be thread-unsafe";

BR.EmitBasicReport(D, "Possibly Thread-Unsafe: non-const static variable",
"ThreadSafety",
Expand Down
8 changes: 4 additions & 4 deletions Utilities/StaticAnalyzers/src/MutableMemberChecker.cpp
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@
//===----------------------------------------------------------------------===//

#include "MutableMemberChecker.h"

using namespace clang;
namespace clangcms {

void MutableMemberChecker::checkASTDecl(const clang::FieldDecl *D,
Expand All @@ -21,11 +21,11 @@ void MutableMemberChecker::checkASTDecl(const clang::FieldDecl *D,

if ( ! m_exception.reportMutableMember( t, DLoc, BR ) )
return;

if ( support::isSafeClassName( t.getAsString() ) ) return;
if ( ! support::isDataClass( D->getParent()->getQualifiedNameAsString() ) ) return;
std::string buf;
llvm::raw_string_ostream os(buf);
os << "Mutable member'" << *D << "' in class, might be thread-unsafe when accessing via a const handle.";

os << "Mutable member'" <<t.getAsString()<<" "<<*D << "' in class '"<<D->getParent()->getQualifiedNameAsString()<<"', might be thread-unsafe when accessing via a const handle.";
BR.EmitBasicReport(D, "Possibly Thread-Unsafe: Mutable member",
"ThreadSafety",
os.str(), DLoc);
Expand Down
1 change: 1 addition & 0 deletions Utilities/StaticAnalyzers/src/MutableMemberChecker.h
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
#include <clang/StaticAnalyzer/Core/BugReporter/BugType.h>

#include "CmsException.h"
#include "CmsSupport.h"

namespace clangcms {
class MutableMemberChecker : public clang::ento::Checker< clang::ento::check::ASTDecl< clang::FieldDecl> > {
Expand Down
9 changes: 7 additions & 2 deletions Utilities/StaticAnalyzers/src/StaticLocalChecker.cpp
View file
Open in desktop
Expand Up @@ -24,12 +24,17 @@ void StaticLocalChecker::checkASTDecl(const clang::VarDecl *D,

if ( ! m_exception.reportGlobalStaticForType( t, DLoc, BR ) )
return;
std::string vname = D->getCanonicalDecl()->getQualifiedNameAsString();
unsigned found = vname.find_last_of("::");
std::string cname = vname.substr(0,found);
// if ( ! support::isDataClass( cname) ) return;
if ( support::isSafeClassName( vname ) ) return;

std::string buf;
llvm::raw_string_ostream os(buf);
os << "Non-const variable '" << *D << "' is local static and might be thread-unsafe";
os << "Non-const variable '" <<t.getAsString()<<" "<< *D << "' is static local or static member data and might be thread-unsafe";

BR.EmitBasicReport(D, "Possibly Thread-Unsafe: non-const static local variable",
BR.EmitBasicReport(D, "Possibly Thread-Unsafe: non-const static variable",
"ThreadSafety",
os.str(), DLoc);
return;
Expand Down