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

clangSA update symbols.py script and fix potential bug in fixAnonNS(). #12332

Merged
merged 4 commits into from
Nov 10, 2015
Merged

clangSA update symbols.py script and fix potential bug in fixAnonNS(). #12332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e3a4530
use the output of ldd to limit which libraries the symbols can come from
gartung Nov 5, 2015
525cbad
produce graphviz graphs if pydot is available
gartung Nov 5, 2015
195fd18
put back old symbols to library matching
gartung Nov 9, 2015
5dbb28a
move fixAnonNS to CmsSupport.cpp
gartung Nov 10, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 29 additions & 9 deletions Utilities/StaticAnalyzers/scripts/symbols.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
symbols_re_skip = re.compile("(@@)")
nm_line_re = re.compile(r"\s+".join([addr_re, code_re, symbol_re]) + "\s*$",
re.I)
ldd_line_re = re.compile(r"\s+(.*) => (.*) \(0x")

requires = collections.defaultdict(set)
provides = collections.defaultdict(set)
dependencies = collections.defaultdict(set)
libraries = collections.defaultdict(set)

def get_symbols(fname):
lines = subprocess.check_output(["nm", "-g", fname])
Expand All @@ -28,6 +30,15 @@ def get_symbols(fname):
else:
provides[symbol].add(fname)

def get_libraries(fname):
lines = subprocess.check_output(["ldd",fname])
for l in lines.splitlines():
m = ldd_line_re.match(l)
if not m: continue
library = m.group(2)
libraries[fname].add(library.rstrip('\r\n'))


paths=os.environ['LD_LIBRARY_PATH'].split(':')

for p in paths:
Expand All @@ -37,28 +48,26 @@ def get_symbols(fname):
filetype = subprocess.check_output(["file", fpth])
if filetype.find("dynamically linked") >= 0 :
get_symbols(fpth)
get_libraries(fpth)

def pick(symbols):
# If several files provide a symbol, choose the one with the shortest name.
def pick(symbols,libraries):
best = None
for s in symbols:
if best is None or len(s) < len(best):
if best is None or s in libraries :
best = s
# if len(symbols) > 1:
# best = "*" + best
return best

for fname, symbols in requires.items():
dependencies[fname] = set(pick(provides[s]) for s in symbols if s in provides)
# print fname + ': ' + ' '.join(sorted(dependencies[fname]))
dependencies[fname] = set(pick(provides[s],libraries[fname]) for s in symbols if s in provides)
print fname + ' : primary dependencies : ' + ', '.join(sorted(dependencies[fname]))+'\n'
unmet = set()
demangled = set()
for s in symbols:
if s not in provides and not symbols_re_skip.search(s) : unmet.add(s)
for u in sorted(unmet):
dm = subprocess.check_output(["c++filt",u])
demangled.add(dm.rstrip('\r\n'))
# if demangled : print fname + ': undefined : ' + ' '.join(sorted(demangled))
if demangled : print fname + ': undefined : ' + ', '.join(sorted(demangled))

import networkx as nx
G=nx.DiGraph()
Expand All @@ -74,4 +83,15 @@ def pick(symbols):
if key != node : deps.add(key)
for v in vals :
deps.add(v)
print node + ': ' + ','.join(sorted(deps))
print node + ': primary and secondary dependencies :' + ', '.join(sorted(deps))

import pydot

H=nx.DiGraph()
for key,values in dependencies.items():
H.add_node(os.path.basename(key))
for val in values: H.add_edge(os.path.basename(key),os.path.basename(val))
for node in nx.nodes_iter(H):
T = nx.dfs_tree(H,node)
name = node + ".dot"
nx.write_dot(T,name)
95 changes: 36 additions & 59 deletions Utilities/StaticAnalyzers/src/ClassChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ void writeLog(std::string ostring) {
return;
}



class WalkAST : public clang::StmtVisitor<WalkAST> {
const CheckerBase *Checker;
clang::ento::BugReporter &BR;
Expand Down Expand Up @@ -82,35 +80,21 @@ class WalkAST : public clang::StmtVisitor<WalkAST> {
/// generating bug reports. This is null while visiting the body of a
/// constructor or destructor.
const clang::CXXMemberCallExpr *visitingCallExpr;

const char *sfile;
public:
WalkAST(const CheckerBase *checker, clang::ento::BugReporter &br, clang::AnalysisDeclContext *ac, const CXXMethodDecl * fd)
WalkAST(const CheckerBase *checker, clang::ento::BugReporter &br, clang::AnalysisDeclContext *ac, const CXXMethodDecl * fd, const char* file)
: Checker(checker),
BR(br),
AC(ac),
AD(fd),
visitingCallExpr(0) {}

void fixAnonNS(std::string & name) {
const std::string anon_ns = "(anonymous namespace)";
if (name.substr(0, anon_ns.size()) == anon_ns ) {
const char* fname = BR.getSourceManager().getPresumedLoc(AD->getLocation()).getFilename();
const char* sname = "/src/";
const char* filename = std::strstr(fname, sname);
if (filename != NULL) name = name.substr(0, anon_ns.size() - 1)+" in "+filename+")"+name.substr(anon_ns.size());
}
return;
}

visitingCallExpr(0),sfile(file) {}

bool hasWork() const { return !WList.empty(); }

/// This method adds a CallExpr to the worklist
void Enqueue(WorkListUnit WLUnit) {
Kind &K = VisitedFunctions[WLUnit];
if (K = Visiting) {
return;
}
if ((K = Visiting)) return;
K = Visiting;
WList.push_back(WLUnit);
}
Expand Down Expand Up @@ -253,7 +237,6 @@ void WalkAST::CheckCXXOperatorCallExpr(const clang::CXXOperatorCallExpr *OCE,con


void WalkAST::CheckExplicitCastExpr(const clang::ExplicitCastExpr * CE,const clang::MemberExpr *ME){

if (! ( clang::CStyleCastExpr::classof(CE) || clang::CXXConstCastExpr::classof(CE) )) return;
const clang::Expr *E = CE->getSubExpr();
clang::ASTContext &Ctx = AC->getASTContext();
Expand Down Expand Up @@ -296,9 +279,9 @@ void WalkAST::CheckReturnStmt(const clang::ReturnStmt * RS, const clang::MemberE
std::string buf;
llvm::raw_string_ostream os(buf);
std::string mname = support::getQualifiedName(*MD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string pname = support::getQualifiedName(*(MD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
os << mname << " is a const member function that returns a const std::vector<*> or const std::vector<*>& "<<rtname<<"\n";
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
writeLog(tolog);
Expand All @@ -318,9 +301,9 @@ void WalkAST::VisitCXXConstCastExpr(clang::CXXConstCastExpr *CCE) {
llvm::raw_string_ostream os(buf);
os <<"const_cast used\n";
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str()+".";
writeLog(tolog);
BugType * BT = new BugType(Checker,"const_cast used in const function ","Data Class Const Correctness");
Expand All @@ -330,12 +313,11 @@ void WalkAST::VisitCXXConstCastExpr(clang::CXXConstCastExpr *CCE) {
}

void WalkAST::VisitDeclRefExpr( clang::DeclRefExpr * DRE) {
if (clang::VarDecl * D = llvm::dyn_cast_or_null<clang::VarDecl>(DRE->getDecl()) ) {
if (clang::VarDecl * D = llvm::dyn_cast_or_null<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 );

}
}

Expand All @@ -360,9 +342,9 @@ void WalkAST::ReportDeclRef( const clang::DeclRefExpr * DRE) {
PS->printPretty(os,0,Policy);
os << "'.\n";
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
writeLog(tolog);
BugType * BT = new BugType(Checker,"ClassChecker : non-const static local variable accessed","Data Class Const Correctness");
Expand All @@ -379,9 +361,9 @@ void WalkAST::ReportDeclRef( const clang::DeclRefExpr * DRE) {
PS->printPretty(os,0,Policy);
os << "'.\n";
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
writeLog(tolog);
BugType * BT = new BugType(Checker,"Non-const static member variable accessed","Data Class Const Correctness");
Expand All @@ -403,9 +385,9 @@ void WalkAST::ReportDeclRef( const clang::DeclRefExpr * DRE) {
PS->printPretty(os,0,Policy);
os << "'.\n";
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
writeLog(tolog);
BugType * BT = new BugType(Checker,"Non-const global static variable accessed","Data Class Const Correctness");
Expand All @@ -423,7 +405,6 @@ void WalkAST::VisitMemberExpr( clang::MemberExpr *ME) {

clang::SourceLocation SL = ME->getExprLoc();
if (BR.getSourceManager().isInSystemHeader(SL) || BR.getSourceManager().isInExternCSystemHeader(SL)) return;

const ValueDecl * D = ME->getMemberDecl();
if ( D->hasAttr<CMSThreadGuardAttr>() || D->hasAttr<CMSThreadSafeAttr>()) return;
if (!(ME->isImplicitAccess())) return;
Expand Down Expand Up @@ -502,9 +483,9 @@ void WalkAST::ReportMember(const clang::MemberExpr *ME) {
ME->printPretty(os,0,Policy);
os << "' is directly or indirectly modified in const function\n";
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: " + os.str();
writeLog(tolog);
BR.EmitBasicReport(AD,Checker,"Member data modified in const function","Data Class Const Correctness",os.str(),CELoc);
Expand All @@ -521,7 +502,6 @@ void WalkAST::ReportCall(const clang::CXXMemberCallExpr *CE) {
clang::LangOptions LangOpts;
LangOpts.CPlusPlus = true;
clang::PrintingPolicy Policy(LangOpts);

os << "call expr '";
CE->printPretty(os,0,Policy);
os << "' with implicit object argument '";
Expand All @@ -530,9 +510,9 @@ void WalkAST::ReportCall(const clang::CXXMemberCallExpr *CE) {
os<<"' is a non-const member function '"<<support::getQualifiedName(*MD);
os<<"' that could modify member data object of type '"<<support::getQualifiedName(*RD)<<"'\n";
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+ pname +"' const function '" + mname + "' Warning: "+os.str();
if ( support::isSafeClassName(support::getQualifiedName(*MD)) ) return;
writeLog(tolog);
Expand All @@ -556,9 +536,9 @@ void WalkAST::ReportCast(const clang::ExplicitCastExpr *CE) {
os << "Const qualifier of member data object";
os <<" was removed via cast expression '";
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
clang::ento::PathDiagnosticLocation CELoc =
clang::ento::PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(),AC);
Expand Down Expand Up @@ -588,9 +568,9 @@ void WalkAST::ReportCallArg(const clang::CXXMemberCallExpr *CE,const int i) {
os << "\n";

std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);

std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();

Expand Down Expand Up @@ -619,9 +599,9 @@ void WalkAST::ReportCallReturn(const clang::ReturnStmt * RS) {
clang::ento::PathDiagnosticLocation CELoc =
clang::ento::PathDiagnosticLocation::createBegin(RS, BR.getSourceManager(),AC);
std::string pname = support::getQualifiedName(*(AD->getParent()));
fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*AD);
fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
writeLog(tolog);
clang::ASTContext &Ctx = AC->getASTContext();
Expand Down Expand Up @@ -650,35 +630,32 @@ void ClassChecker::checkASTDecl(const clang::CXXRecordDecl *RD, clang::ento::Ana
clang::ento::BugReporter &BR) const {

const clang::SourceManager &SM = BR.getSourceManager();
const char *sfile=SM.getPresumedLoc(RD->getLocation()).getFilename();
const char *sfile=SM.getPresumedLoc(RD->getLocation()).getFilename();
if (!support::isCmsLocalFile(sfile)) return;

std::string buf;
llvm::raw_string_ostream os(buf);
std::string name = RD->getQualifiedNameAsString();
if ( ! support::isDataClass(name) ) return;

for ( auto I = RD->field_begin(), E = RD->field_end(); I != E; ++I)
{
const FieldDecl * D = (*I) ;
if ( D->hasAttr<CMSThreadGuardAttr>() || D->hasAttr<CMSThreadSafeAttr>()) return;
if ( D->isMutable() )
if ( D->hasAttr<CMSThreadGuardAttr>() || D->hasAttr<CMSThreadSafeAttr>()) return;
if ( D->isMutable() )
{
clang::QualType t = D->getType();
clang::ento::PathDiagnosticLocation DLoc =
clang::ento::PathDiagnosticLocation::createBegin(D, BR.getSourceManager());
if ( support::isSafeClassName( t.getCanonicalType().getAsString() ) ) return;
if ( ! support::isDataClass( support::getQualifiedName(*RD) ) ) return;
WalkAST walker(this,BR, mgr.getAnalysisDeclContext(RD), (*(RD->ctor_begin()))->getMostRecentDecl() ) ;
if ( ! support::isDataClass( support::getQualifiedName(*RD) ) ) return;
std::string buf;
llvm::raw_string_ostream os(buf);
os << "Mutable member '" <<t.getCanonicalType().getAsString()<<" "<<*D << "' in data class '"<<support::getQualifiedName(*RD)<<"', might be thread-unsafe when accessing via a const handle.";
BR.EmitBasicReport(D, this, "Mutable member in data class",
"Data Class Const Correctness", os.str(), DLoc);
std::string pname = support::getQualifiedName(*(RD));
walker.fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*D);
walker.fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' mutable member '" + mname + "' Warning: "+os.str();
writeLog(tolog);

Expand All @@ -695,7 +672,7 @@ void ClassChecker::checkASTDecl(const clang::CXXRecordDecl *RD, clang::ento::Ana
if ( MD->hasAttr<CMSThreadGuardAttr>() || MD->hasAttr<CMSThreadSafeAttr>()) continue;
Copy link
Member Author

Choose a reason for hiding this comment

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

This caused a crash because RD->ctor_begin() returned a null pointer. Making fixAnonNS a helper function removed the need to instantiate a WalkAST to get access to fixAnonNS.

if ( MD->hasBody() ) {
clang::Stmt *Body = MD->getBody();
WalkAST walker(this,BR, mgr.getAnalysisDeclContext(MD),MD);
WalkAST walker(this,BR, mgr.getAnalysisDeclContext(MD),MD,sfile);
walker.Visit(Body);
clang::QualType RQT = MD->getCallResultType();
clang::ASTContext &Ctx = BR.getContext();
Expand All @@ -707,9 +684,9 @@ void ClassChecker::checkASTDecl(const clang::CXXRecordDecl *RD, clang::ento::Ana
llvm::raw_string_ostream os(buf);
os << MD->getQualifiedNameAsString() << " is a const member function that returns a pointer or reference to a non-const object \n";
std::string pname = support::getQualifiedName(*(MD->getParent()));
walker.fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*MD);
walker.fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
writeLog(tolog);
BR.EmitBasicReport(MD,this, "Const function returns pointer or reference to non-const object.","Data Class Const Correctness",os.str(),ELoc);
Expand All @@ -728,9 +705,9 @@ void ClassChecker::checkASTDecl(const clang::CXXRecordDecl *RD, clang::ento::Ana
std::string buf;
llvm::raw_string_ostream os(buf);
std::string pname = support::getQualifiedName(*(MD->getParent()));
walker.fixAnonNS(pname);
support::fixAnonNS(pname,sfile);
std::string mname = support::getQualifiedName(*MD);
walker.fixAnonNS(mname);
support::fixAnonNS(mname,sfile);
os << mname << " is a const member function that returns an object of type const std::vector<*> or const std::vector<*>& "<<rtname<<"\n";
std::string tolog = "data class '"+pname+"' const function '" + mname + "' Warning: "+os.str();
writeLog(tolog);
Expand Down
Loading