mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix some crashes in unusedfields plugin

Browse Source 
for some reason the insideMoveOrCopyDecl pointer to MethodDecl becomes
bad during AST traversal, but the pointers to RecordDecl seem stable?

Change-Id: Ida939f5ca4780e674b245411f7395f147258544e
This commit is contained in:
Noel Grandin
2017-06-29 13:40:11 +02:00
parent fb37189e84
commit acd8f0c344
1 changed files with 26 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
compilerplugins/clang/unusedfields.cxx
View File

@@ -84,8 +84,9 @@ public:
bool TraverseCXXMethodDecl( CXXMethodDecl* ); bool TraverseCXXMethodDecl( CXXMethodDecl* );
private: private:
MyFieldInfo niceName(const FieldDecl*); MyFieldInfo niceName(const FieldDecl*);
void checkTouched(const FieldDecl* fieldDecl, const Expr* memberExpr); void checkTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr);
CXXMethodDecl * insideMoveOrCopyDecl; void checkWriteOnly(const FieldDecl* fieldDecl, const Expr* memberExpr);
RecordDecl * insideMoveOrCopyDeclParent;
}; };
void UnusedFields::run() void UnusedFields::run()
@@ -200,19 +201,21 @@ bool startswith(const std::string& rStr, const char* pSubStr)
bool UnusedFields::TraverseCXXConstructorDecl(CXXConstructorDecl* cxxConstructorDecl) bool UnusedFields::TraverseCXXConstructorDecl(CXXConstructorDecl* cxxConstructorDecl)
{ {
if (cxxConstructorDecl->isCopyOrMoveConstructor()) if (!ignoreLocation(cxxConstructorDecl)
insideMoveOrCopyDecl = cxxConstructorDecl; && cxxConstructorDecl->isCopyOrMoveConstructor())
insideMoveOrCopyDeclParent = cxxConstructorDecl->getParent();
bool ret = RecursiveASTVisitor::TraverseCXXConstructorDecl(cxxConstructorDecl); bool ret = RecursiveASTVisitor::TraverseCXXConstructorDecl(cxxConstructorDecl);
insideMoveOrCopyDecl = nullptr; insideMoveOrCopyDeclParent = nullptr;
return ret; return ret;
} }
bool UnusedFields::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl) bool UnusedFields::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl)
{ {
if (cxxMethodDecl->isCopyAssignmentOperator() || cxxMethodDecl->isMoveAssignmentOperator()) if (!ignoreLocation(cxxMethodDecl)
insideMoveOrCopyDecl = cxxMethodDecl; && (cxxMethodDecl->isCopyAssignmentOperator() || cxxMethodDecl->isMoveAssignmentOperator()))
insideMoveOrCopyDeclParent = cxxMethodDecl->getParent();
bool ret = RecursiveASTVisitor::TraverseCXXMethodDecl(cxxMethodDecl); bool ret = RecursiveASTVisitor::TraverseCXXMethodDecl(cxxMethodDecl);
insideMoveOrCopyDecl = nullptr; insideMoveOrCopyDeclParent = nullptr;
return ret; return ret;
} }
@@ -232,21 +235,21 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
return true; return true;
} }
MyFieldInfo fieldInfo = niceName(fieldDecl); checkTouchedFromOutside(fieldDecl, memberExpr);
// for the touched-from-outside analysis checkWriteOnly(fieldDecl, memberExpr);
checkTouched(fieldDecl, memberExpr); return true;
}
// for the write-only analysis void UnusedFields::checkWriteOnly(const FieldDecl* fieldDecl, const Expr* memberExpr)
{
// we don't care about reads from a field when inside that field parent class copy/move constructor/operator= // we don't care about reads from a field when inside the copy/move constructor/operator= for that field
if (insideMoveOrCopyDecl) if (insideMoveOrCopyDeclParent)
{ {
auto cxxRecordDecl1 = dyn_cast<CXXRecordDecl>(fieldDecl->getDeclContext()); RecordDecl const * cxxRecordDecl1 = fieldDecl->getParent();
auto cxxRecordDecl2 = dyn_cast<CXXRecordDecl>(insideMoveOrCopyDecl->getDeclContext()); if (cxxRecordDecl1 && (cxxRecordDecl1 == insideMoveOrCopyDeclParent))
if (cxxRecordDecl1 && cxxRecordDecl1 == cxxRecordDecl2) return;
return true;
} }
auto parentsRange = compiler.getASTContext().getParents(*memberExpr); auto parentsRange = compiler.getASTContext().getParents(*memberExpr);
@@ -268,7 +271,7 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
bPotentiallyReadFrom = true; bPotentiallyReadFrom = true;
} }
if (!bPotentiallyReadFrom) if (!bPotentiallyReadFrom)
return true; return;
break; break;
} }
if (isa<CastExpr>(parent) || isa<MemberExpr>(parent) || isa<ParenExpr>(parent) || isa<ParenListExpr>(parent) if (isa<CastExpr>(parent) || isa<MemberExpr>(parent) || isa<ParenExpr>(parent) || isa<ParenListExpr>(parent)
@@ -404,10 +407,9 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
memberExpr->dump(); memberExpr->dump();
} }
MyFieldInfo fieldInfo = niceName(fieldDecl);
if (bPotentiallyReadFrom) if (bPotentiallyReadFrom)
readFromSet.insert(fieldInfo); readFromSet.insert(fieldInfo);
return true;
} }
bool UnusedFields::VisitDeclRefExpr( const DeclRefExpr* declRefExpr ) bool UnusedFields::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
@@ -425,11 +427,11 @@ bool UnusedFields::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) { if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) {
return true; return true;
} }
checkTouched(fieldDecl, declRefExpr); checkTouchedFromOutside(fieldDecl, declRefExpr);
return true; return true;
} }
void UnusedFields::checkTouched(const FieldDecl* fieldDecl, const Expr* memberExpr) { void UnusedFields::checkTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr) {
const FunctionDecl* memberExprParentFunction = parentFunctionDecl(memberExpr); const FunctionDecl* memberExprParentFunction = parentFunctionDecl(memberExpr);
const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(memberExprParentFunction); const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(memberExprParentFunction);