Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, balazske,
martong, rnkovacs, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso,
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D75430: [analyzer][NFC] Introduce
CXXDeallocatorCall, deploy it in MallocChecker.
Szelethus added a child revision: D77846: [analyzer][CallAndMessage][NFC] Split
up checkPreCall.
The following series of patches has something similar in mind with D77474
<https://reviews.llvm.org/D77474>, with the same goal to finally end incorrect
checker names for good. Despite CallAndMessage not suffering from this
particular issue, it //is// a dependency for many other checkers, which is
problematic, because we don't really want dependencies to also emit diagnostics
(reasoning for this is also more detailed in D77474
<https://reviews.llvm.org/D77474>).
CallAndMessage also has another problem, namely that it is responsible for a
lot of reports. You'll soon learn that this isn't really easy to solve for
compatibility reasons, but that is the topic of followup patches.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77845
Files:
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -110,7 +110,8 @@
static Optional<FunctionData> create(const CallEvent &Call,
const CheckerContext &C) {
- assert(Call.getDecl());
+ if (!Call.getDecl())
+ return None;
const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();
if (!FDecl || (FDecl->getKind() != Decl::Function &&
FDecl->getKind() != Decl::CXXMethod))
Index: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -11,9 +11,10 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/ParentMap.h"
#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -21,6 +22,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;
@@ -29,11 +31,8 @@
namespace {
class CallAndMessageChecker
- : public Checker< check::PreStmt<CallExpr>,
- check::PreStmt<CXXDeleteExpr>,
- check::PreObjCMessage,
- check::ObjCMessageNil,
- check::PreCall > {
+ : public Checker<check::PreObjCMessage, check::ObjCMessageNil,
+ check::PreCall> {
mutable std::unique_ptr<BugType> BT_call_null;
mutable std::unique_ptr<BugType> BT_call_undef;
mutable std::unique_ptr<BugType> BT_cxx_call_null;
@@ -48,11 +47,10 @@
mutable std::unique_ptr<BugType> BT_call_few_args;
public:
- DefaultBool Check_CallAndMessageUnInitRefArg;
- CheckerNameRef CheckName_CallAndMessageUnInitRefArg;
+ enum CheckKind { CK_CallAndMessageUnInitRefArg, CK_NumCheckKinds };
+
+ DefaultBool ChecksEnabled[CK_NumCheckKinds];
- void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
- void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;
/// Fill in the return value that results from messaging nil based on the
@@ -144,7 +142,7 @@
CheckerContext &C, const SVal &V, SourceRange ArgRange, const Expr *ArgEx,
std::unique_ptr<BugType> &BT, const ParmVarDecl *ParamDecl, const char *BD,
int ArgumentNumber) const {
- if (!Check_CallAndMessageUnInitRefArg)
+ if (!ChecksEnabled[CK_CallAndMessageUnInitRefArg])
return false;
// No parameter declaration available, i.e. variadic function argument.
@@ -311,63 +309,36 @@
return false;
}
-void CallAndMessageChecker::checkPreStmt(const CallExpr *CE,
- CheckerContext &C) const{
-
- const Expr *Callee = CE->getCallee()->IgnoreParens();
+void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
ProgramStateRef State = C.getState();
- const LocationContext *LCtx = C.getLocationContext();
- SVal L = State->getSVal(Callee, LCtx);
-
- if (L.isUndef()) {
- if (!BT_call_undef)
- BT_call_undef.reset(new BuiltinBug(
- this, "Called function pointer is an uninitialized pointer value"));
- emitBadCall(BT_call_undef.get(), C, Callee);
- return;
- }
-
- ProgramStateRef StNonNull, StNull;
- std::tie(StNonNull, StNull) = State->assume(L.castAs<DefinedOrUnknownSVal>());
-
- if (StNull && !StNonNull) {
- if (!BT_call_null)
- BT_call_null.reset(new BuiltinBug(
- this, "Called function pointer is null (null dereference)"));
- emitBadCall(BT_call_null.get(), C, Callee);
- return;
- }
-
- C.addTransition(StNonNull);
-}
+ if (const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr())) {
+ const Expr *Callee = CE->getCallee()->IgnoreParens();
+ const LocationContext *LCtx = C.getLocationContext();
+ SVal L = State->getSVal(Callee, LCtx);
+
+ if (L.isUndef()) {
+ if (!BT_call_undef)
+ BT_call_undef.reset(new BuiltinBug(
+ this, "Called function pointer is an uninitialized pointer value"));
+ emitBadCall(BT_call_undef.get(), C, Callee);
+ return;
+ }
-void CallAndMessageChecker::checkPreStmt(const CXXDeleteExpr *DE,
- CheckerContext &C) const {
+ ProgramStateRef StNonNull, StNull;
+ std::tie(StNonNull, StNull) =
+ State->assume(L.castAs<DefinedOrUnknownSVal>());
- SVal Arg = C.getSVal(DE->getArgument());
- if (Arg.isUndef()) {
- StringRef Desc;
- ExplodedNode *N = C.generateErrorNode();
- if (!N)
+ if (StNull && !StNonNull) {
+ if (!BT_call_null)
+ BT_call_null.reset(new BuiltinBug(
+ this, "Called function pointer is null (null dereference)"));
+ emitBadCall(BT_call_null.get(), C, Callee);
return;
- if (!BT_cxx_delete_undef)
- BT_cxx_delete_undef.reset(
- new BuiltinBug(this, "Uninitialized argument value"));
- if (DE->isArrayFormAsWritten())
- Desc = "Argument to 'delete[]' is uninitialized";
- else
- Desc = "Argument to 'delete' is uninitialized";
- BugType *BT = BT_cxx_delete_undef.get();
- auto R = std::make_unique<PathSensitiveBugReport>(*BT, Desc, N);
- bugreporter::trackExpressionValue(N, DE, *R);
- C.emitReport(std::move(R));
- return;
- }
-}
+ }
-void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
- CheckerContext &C) const {
- ProgramStateRef State = C.getState();
+ State = StNonNull;
+ }
// If this is a call to a C++ method, check if the callee is null or
// undefined.
@@ -425,6 +396,30 @@
}
}
+ if (const auto *DC = dyn_cast<CXXDeallocatorCall>(&Call)) {
+ const CXXDeleteExpr *DE = DC->getOriginExpr();
+ assert(DE);
+ SVal Arg = C.getSVal(DE->getArgument());
+ if (Arg.isUndef()) {
+ StringRef Desc;
+ ExplodedNode *N = C.generateErrorNode();
+ if (!N)
+ return;
+ if (!BT_cxx_delete_undef)
+ BT_cxx_delete_undef.reset(
+ new BuiltinBug(this, "Uninitialized argument value"));
+ if (DE->isArrayFormAsWritten())
+ Desc = "Argument to 'delete[]' is uninitialized";
+ else
+ Desc = "Argument to 'delete' is uninitialized";
+ BugType *BT = BT_cxx_delete_undef.get();
+ auto R = std::make_unique<PathSensitiveBugReport>(*BT, Desc, N);
+ bugreporter::trackExpressionValue(N, DE, *R);
+ C.emitReport(std::move(R));
+ return;
+ }
+ }
+
// Don't check for uninitialized field values in arguments if the
// caller has a body that is available and we have the chance to inline it.
// This is a hack, but is a reasonable compromise betweens sometimes warning
@@ -444,8 +439,8 @@
if(FD && i < FD->getNumParams())
ParamDecl = FD->getParamDecl(i);
if (PreVisitProcessArg(C, Call.getArgSVal(i), Call.getArgSourceRange(i),
- Call.getArgExpr(i), i,
- checkUninitFields, Call, *BT, ParamDecl))
+ Call.getArgExpr(i), i, checkUninitFields, Call, *BT,
+ ParamDecl))
return;
}
@@ -609,12 +604,13 @@
return true;
}
-void ento::registerCallAndMessageUnInitRefArg(CheckerManager &mgr) {
- CallAndMessageChecker *Checker = mgr.getChecker<CallAndMessageChecker>();
- Checker->Check_CallAndMessageUnInitRefArg = true;
- Checker->CheckName_CallAndMessageUnInitRefArg = mgr.getCurrentCheckerName();
-}
+#define REGISTER_CHECKER(name) \
+ void ento::register##name(CheckerManager &mgr) { \
+ CallAndMessageChecker *checker = mgr.getChecker<CallAndMessageChecker>(); \
+ checker->ChecksEnabled[CallAndMessageChecker::CK_##name] = true; \
+ \
+ } \
+ \
+ bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
-bool ento::shouldRegisterCallAndMessageUnInitRefArg(const CheckerManager &mgr) {
- return true;
-}
+REGISTER_CHECKER(CallAndMessageUnInitRefArg)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
