Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[analyzer] Add checker modeling potential C++ self-assignment
This checker checks copy and move assignment operators whether they are protected against self-assignment. Since C++ core guidelines discourages explicit checking for `&rhs==this` in general we take a different approach: in top-frame analysis we branch the exploded graph for two cases, where &rhs==this and &rhs!=this and let existing checkers (e.g. unix.Malloc) do the rest of the work. It is important that we check all copy and move assignment operator in top frame even if we checked them already since self-assignments may happen undetected even in the same translation unit (e.g. using random indices for an array what may or may not be the same). This reapplies r275820 after fixing a string-lifetime issue discovered by the bots. A patch by Ádám Balogh! Differential Revision: https://reviews.llvm.org/D19311 llvm-svn: 276365
- Loading branch information
1 parent
c107a48
commit f57f90d
Showing
8 changed files
with
234 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62 changes: 62 additions & 0 deletions
62
clang/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
//=== CXXSelfAssignmentChecker.cpp -----------------------------*- C++ -*--===// | ||
// | ||
// The LLVM Compiler Infrastructure | ||
// | ||
// This file is distributed under the University of Illinois Open Source | ||
// License. See LICENSE.TXT for details. | ||
// | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This file defines CXXSelfAssignmentChecker, which tests all custom defined | ||
// copy and move assignment operators for the case of self assignment, thus | ||
// where the parameter refers to the same location where the this pointer | ||
// points to. The checker itself does not do any checks at all, but it | ||
// causes the analyzer to check every copy and move assignment operator twice: | ||
// once for when 'this' aliases with the parameter and once for when it may not. | ||
// It is the task of the other enabled checkers to find the bugs in these two | ||
// different cases. | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#include "ClangSACheckers.h" | ||
#include "clang/StaticAnalyzer/Core/Checker.h" | ||
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" | ||
|
||
using namespace clang; | ||
using namespace ento; | ||
|
||
namespace { | ||
|
||
class CXXSelfAssignmentChecker : public Checker<check::BeginFunction> { | ||
public: | ||
CXXSelfAssignmentChecker(); | ||
void checkBeginFunction(CheckerContext &C) const; | ||
}; | ||
} | ||
|
||
CXXSelfAssignmentChecker::CXXSelfAssignmentChecker() {} | ||
|
||
void CXXSelfAssignmentChecker::checkBeginFunction(CheckerContext &C) const { | ||
if (!C.inTopFrame()) | ||
return; | ||
const auto *LCtx = C.getLocationContext(); | ||
const auto *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl()); | ||
if (!MD) | ||
return; | ||
if (!MD->isCopyAssignmentOperator() && !MD->isMoveAssignmentOperator()) | ||
return; | ||
auto &State = C.getState(); | ||
auto &SVB = C.getSValBuilder(); | ||
auto ThisVal = | ||
State->getSVal(SVB.getCXXThis(MD, LCtx->getCurrentStackFrame())); | ||
auto Param = SVB.makeLoc(State->getRegion(MD->getParamDecl(0), LCtx)); | ||
auto ParamVal = State->getSVal(Param); | ||
ProgramStateRef SelfAssignState = State->bindLoc(Param, ThisVal); | ||
C.addTransition(SelfAssignState); | ||
ProgramStateRef NonSelfAssignState = State->bindLoc(Param, ParamVal); | ||
C.addTransition(NonSelfAssignState); | ||
} | ||
|
||
void ento::registerCXXSelfAssignmentChecker(CheckerManager &Mgr) { | ||
Mgr.registerChecker<CXXSelfAssignmentChecker>(); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection %s -verify -analyzer-output=text | ||
|
||
extern "C" char *strdup(const char* s); | ||
extern "C" void free(void* ptr); | ||
|
||
namespace std { | ||
template<class T> struct remove_reference { typedef T type; }; | ||
template<class T> struct remove_reference<T&> { typedef T type; }; | ||
template<class T> struct remove_reference<T&&> { typedef T type; }; | ||
template<class T> typename remove_reference<T>::type&& move(T&& t); | ||
} | ||
|
||
void clang_analyzer_eval(int); | ||
|
||
class StringUsed { | ||
public: | ||
StringUsed(const char *s = "") : str(strdup(s)) {} | ||
StringUsed(const StringUsed &rhs) : str(strdup(rhs.str)) {} | ||
~StringUsed(); | ||
StringUsed& operator=(const StringUsed &rhs); | ||
StringUsed& operator=(StringUsed &&rhs); | ||
operator const char*() const; | ||
private: | ||
char *str; | ||
}; | ||
|
||
StringUsed::~StringUsed() { | ||
free(str); | ||
} | ||
|
||
StringUsed& StringUsed::operator=(const StringUsed &rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}} | ||
clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}} | ||
free(str); // expected-note{{Memory is released}} | ||
str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}} expected-note{{Use of memory after it is freed}} | ||
return *this; | ||
} | ||
|
||
StringUsed& StringUsed::operator=(StringUsed &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}} | ||
clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}} | ||
str = rhs.str; | ||
rhs.str = nullptr; // FIXME: An improved leak checker should warn here | ||
return *this; | ||
} | ||
|
||
StringUsed::operator const char*() const { | ||
return str; | ||
} | ||
|
||
class StringUnused { | ||
public: | ||
StringUnused(const char *s = "") : str(strdup(s)) {} | ||
StringUnused(const StringUnused &rhs) : str(strdup(rhs.str)) {} | ||
~StringUnused(); | ||
StringUnused& operator=(const StringUnused &rhs); | ||
StringUnused& operator=(StringUnused &&rhs); | ||
operator const char*() const; | ||
private: | ||
char *str; | ||
}; | ||
|
||
StringUnused::~StringUnused() { | ||
free(str); | ||
} | ||
|
||
StringUnused& StringUnused::operator=(const StringUnused &rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}} | ||
clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}} | ||
free(str); // expected-note{{Memory is released}} | ||
str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}} expected-note{{Use of memory after it is freed}} | ||
return *this; | ||
} | ||
|
||
StringUnused& StringUnused::operator=(StringUnused &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}} | ||
clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}} | ||
str = rhs.str; | ||
rhs.str = nullptr; // FIXME: An improved leak checker should warn here | ||
return *this; | ||
} | ||
|
||
StringUnused::operator const char*() const { | ||
return str; | ||
} | ||
|
||
|
||
int main() { | ||
StringUsed s1 ("test"), s2; | ||
s2 = s1; | ||
s2 = std::move(s1); | ||
return 0; | ||
} |