trixirt updated this revision to Diff 150003.
trixirt added a comment.
cleanup of last patch
Repository:
rC Clang
https://reviews.llvm.org/D47554
Files:
include/clang/AST/ExprCXX.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/AST/ExprCXX.cpp
lib/ASTMatchers/Dynamic/Registry.cpp
lib/CodeGen/CGExprCXX.cpp
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
test/Analysis/dead-status-new-nullptr.cpp
Index: test/Analysis/dead-status-new-nullptr.cpp
===================================================================
--- /dev/null
+++ test/Analysis/dead-status-new-nullptr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=deadcode.DeadStatus -verify %s
+
+void *t1(unsigned n, bool *fail);
+void *t1(unsigned n, bool *fail) {
+ char *m = new char[n];
+ if (nullptr == m) // expected-warning{{This variable can never be a nullptr}}
+ *fail = true;
+ else
+ *fail = false;
+ return m;
+}
+
+void *t2(unsigned n, bool *fail);
+void *t2(unsigned n, bool *fail) {
+ char *m = new char[n];
+ if (m == nullptr) // expected-warning{{This variable can never be a nullptr}}
+ *fail = true;
+ else
+ *fail = false;
+ return m;
+}
+
+// a variant of nullptr
+void *t3(unsigned n, bool *fail);
+void *t3(unsigned n, bool *fail) {
+ char *m = new char[n];
+ if (m == 0) // expected-warning{{This variable can never be a nullptr}}
+ *fail = true;
+ else
+ *fail = false;
+ return m;
+}
+
+// a variant of nullptr
+#define NULL __null
+void *t4(unsigned n, bool *fail);
+void *t4(unsigned n, bool *fail) {
+ char *m = new char[n];
+ if (m == NULL) // expected-warning{{This variable can never be a nullptr}}
+ *fail = true;
+ else
+ *fail = false;
+ return m;
+}
Index: lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
@@ -0,0 +1,140 @@
+//==- DeadStatusChecker.cpp - Check for impossible status -*- 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 a DeadStatus, a checker that looks for impossible
+// status checks.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class DeadStatusChecker : public Checker<check::ASTCodeBody> {
+ class Walker : public RecursiveASTVisitor<Walker> {
+ BugReporter &BR;
+ const DeadStatusChecker *Checker;
+ AnalysisDeclContext *AC;
+ llvm::SmallPtrSet<const VarDecl *, 8> throwingNews;
+
+ const Expr *hasNullptr(const BinaryOperator *BO) const {
+ const Expr *ret = nullptr;
+ const Expr *E[2] = {BO->getLHS()->IgnoreParenCasts(),
+ BO->getRHS()->IgnoreParenCasts()};
+ const Expr *EO[2] = {E[1], E[0]};
+ for (int i = 0; i < 2; i++) {
+ if (isa<CXXNullPtrLiteralExpr>(E[i]) || isa<GNUNullExpr>(E[i]) ||
+ (isa<IntegerLiteral>(E[i]) &&
+ dyn_cast<IntegerLiteral>(E[i])->getValue() == 0)) {
+ ret = EO[i];
+ break;
+ }
+ }
+ return ret;
+ }
+
+ const Expr *hasThrowingNew(const BinaryOperator *BO) const {
+ const Expr *ret = nullptr;
+ const Expr *rhs = BO->getRHS()->IgnoreParenCasts();
+ if (isa<CXXNewExpr>(rhs)) {
+ auto NE = static_cast<const CXXNewExpr *>(rhs);
+ if (!NE->shouldNullCheckAllocation()) {
+ ret = BO->getLHS()->IgnoreParenCasts();
+ }
+ }
+ return ret;
+ }
+
+ bool hasThrowingNew(const VarDecl *VD) const {
+ bool ret = false;
+ const Expr *E = VD->getInit();
+ if (E != nullptr) {
+ E = E->IgnoreParenCasts();
+ if (isa<CXXNewExpr>(E)) {
+ auto NE = static_cast<const CXXNewExpr *>(E);
+ if (!NE->shouldNullCheckAllocation()) {
+ ret = true;
+ }
+ }
+ }
+ return ret;
+ }
+
+ public:
+ explicit Walker(BugReporter &BR, const DeadStatusChecker *C,
+ AnalysisDeclContext *AC)
+ : BR(BR), Checker(C), AC(AC) {}
+ bool VisitBinaryOperator(const BinaryOperator *BO) {
+ const Expr *E = nullptr;
+ BinaryOperator::Opcode Op = BO->getOpcode();
+ if (BinaryOperator::isEqualityOp(Op)) {
+ E = hasNullptr(BO);
+ } else if (BinaryOperator::isAssignmentOp(Op)) {
+ E = hasThrowingNew(BO);
+ }
+ if (E != nullptr) {
+ if (isa<DeclRefExpr>(E)) {
+ auto DR = static_cast<const DeclRefExpr *>(E);
+ if (DR->getType()->isPointerType()) {
+ auto D = dyn_cast<VarDecl>(DR->getDecl());
+ if (D) {
+ if (BinaryOperator::isEqualityOp(Op)) {
+ if (throwingNews.count(D)) {
+ auto DSC = dyn_cast<DeadStatusChecker>(Checker);
+ if (DSC != nullptr)
+ DSC->reportThrowingNewStatusBug(BR, AC, BO, D);
+ }
+ } else {
+ // assignment
+ throwingNews.insert(D);
+ }
+ }
+ }
+ }
+ }
+ return true;
+ }
+ bool VisitVarDecl(const VarDecl *D) {
+ if (hasThrowingNew(D))
+ throwingNews.insert(D);
+ return true;
+ }
+ };
+
+ void reportThrowingNewStatusBug(BugReporter &BR, AnalysisDeclContext *AC,
+ const BinaryOperator *BO,
+ const VarDecl *D) const {
+ PathDiagnosticLocation L =
+ PathDiagnosticLocation::createBegin(BO, BR.getSourceManager(), AC);
+
+ BR.EmitBasicReport(D, this, "Dead nullptr check", categories::LogicError,
+ "This variable can never be a nullptr", L,
+ BO->getSourceRange());
+ }
+
+public:
+ DeadStatusChecker() {}
+ void checkASTCodeBody(const Decl *D, AnalysisManager &M,
+ BugReporter &BR) const {
+ Walker V(BR, this, M.getAnalysisDeclContext(D));
+ V.TraverseDecl(const_cast<Decl *>(D));
+ }
+};
+} // namespace
+
+void ento::registerDeadStatusChecker(CheckerManager &mgr) {
+ mgr.registerChecker<DeadStatusChecker>();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -27,6 +27,7 @@
CloneChecker.cpp
ConversionChecker.cpp
CXXSelfAssignmentChecker.cpp
+ DeadStatusChecker.cpp
DeadStoresChecker.cpp
DebugCheckers.cpp
DeleteWithNonVirtualDtorChecker.cpp
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -1649,7 +1649,7 @@
// function is allowed to return null (because it has a non-throwing
// exception spec or is the reserved placement new) and we have an
// interesting initializer.
- bool nullCheck = E->shouldNullCheckAllocation(getContext()) &&
+ bool nullCheck = E->shouldNullCheckAllocation() &&
(!allocType.isPODType(getContext()) || E->hasInitializer());
llvm::BasicBlock *nullCheckBB = nullptr;
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===================================================================
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -435,6 +435,7 @@
REGISTER_MATCHER(returns);
REGISTER_MATCHER(returnStmt);
REGISTER_MATCHER(rValueReferenceType);
+ REGISTER_MATCHER(shouldNullCheckAllocation);
REGISTER_MATCHER(sizeOfExpr);
REGISTER_MATCHER(specifiesNamespace);
REGISTER_MATCHER(specifiesType);
Index: lib/AST/ExprCXX.cpp
===================================================================
--- lib/AST/ExprCXX.cpp
+++ lib/AST/ExprCXX.cpp
@@ -168,7 +168,7 @@
SubExprs = new (C) Stmt*[TotalSize];
}
-bool CXXNewExpr::shouldNullCheckAllocation(const ASTContext &Ctx) const {
+bool CXXNewExpr::shouldNullCheckAllocation() const {
return getOperatorNew()->getType()->castAs<FunctionProtoType>()
->isNothrow() &&
!getOperatorNew()->isReservedGlobalPlacementOperator();
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -346,6 +346,11 @@
def DeadStoresChecker : Checker<"DeadStores">,
HelpText<"Check for values stored to variables that are never read afterwards">,
DescFile<"DeadStoresChecker.cpp">;
+
+def DeadStatusChecker : Checker<"DeadStatus">,
+ HelpText<"Check for impossible status">,
+ DescFile<"DeadStatusChecker.cpp">;
+
} // end DeadCode
let ParentPackage = DeadCodeAlpha in {
Index: include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -5973,6 +5973,18 @@
InnerMatcher.matches(*Node.getArraySize(), Finder, Builder);
}
+/// Matches nothrow new expressions.
+///
+/// Given:
+/// \code
+/// MyClass *p1 = new MyClass;
+/// \endcode
+/// cxxNewExpr(shouldNullCheckAllocation())
+/// matches the expression 'new (std::nothrow) MyClass'.
+AST_MATCHER(CXXNewExpr, shouldNullCheckAllocation) {
+ return Node.shouldNullCheckAllocation();
+}
+
/// Matches a class declaration that is defined.
///
/// Example matches x (matcher = cxxRecordDecl(hasDefinition()))
Index: include/clang/AST/ExprCXX.h
===================================================================
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -1940,7 +1940,7 @@
/// has a non-throwing exception-specification. The '03 rule is
/// identical except that the definition of a non-throwing
/// exception specification is just "is it throw()?".
- bool shouldNullCheckAllocation(const ASTContext &Ctx) const;
+ bool shouldNullCheckAllocation() const;
FunctionDecl *getOperatorNew() const { return OperatorNew; }
void setOperatorNew(FunctionDecl *D) { OperatorNew = D; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
