https://github.com/balazske created https://github.com/llvm/llvm-project/pull/110977
None From 36d99fc59b675737ce952087b7a71ec6e4b579a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Mon, 30 Sep 2024 16:51:35 +0200 Subject: [PATCH] [clang][analyzer] Check initialization and argument passing in FixedAddressChecker --- .../Checkers/FixedAddressChecker.cpp | 63 +++++++++++++------ clang/test/Analysis/ptr-arith.c | 9 ++- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp index f7fd92db7757e5..aee85c59ddd630 100644 --- a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp @@ -8,6 +8,8 @@ // // This files defines FixedAddressChecker, a builtin checker that checks for // assignment of a fixed address to a pointer. +// Using a fixed address is not portable because that address will probably +// not be valid in all environments or platforms. // This check corresponds to CWE-587. // //===----------------------------------------------------------------------===// @@ -23,38 +25,35 @@ using namespace ento; namespace { class FixedAddressChecker - : public Checker< check::PreStmt<BinaryOperator> > { + : public Checker<check::PreStmt<BinaryOperator>, check::PreStmt<DeclStmt>, + check::PreStmt<CallExpr>> { const BugType BT{this, "Use fixed address"}; + void checkUseOfFixedAddress(QualType DstType, const Expr *SrcExpr, + CheckerContext &C) const; + public: void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const; + void checkPreStmt(const DeclStmt *D, CheckerContext &C) const; + void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; }; } -void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, - CheckerContext &C) const { - // Using a fixed address is not portable because that address will probably - // not be valid in all environments or platforms. - - if (B->getOpcode() != BO_Assign) - return; - - QualType T = B->getType(); - if (!T->isPointerType()) +void FixedAddressChecker::checkUseOfFixedAddress(QualType DstType, + const Expr *SrcExpr, + CheckerContext &C) const { + if (!DstType->isPointerType()) return; - // Omit warning if the RHS has already pointer type. Without this passing - // around one fixed value in several pointer variables would produce several - // redundant warnings. - if (B->getRHS()->IgnoreParenCasts()->getType()->isPointerType()) + if (SrcExpr->IgnoreParenCasts()->getType()->isPointerType()) return; - SVal RV = C.getSVal(B->getRHS()); + SVal RV = C.getSVal(SrcExpr); if (!RV.isConstant() || RV.isZeroConstant()) return; - if (C.getSourceManager().isInSystemMacro(B->getRHS()->getBeginLoc())) + if (C.getSourceManager().isInSystemMacro(SrcExpr->getBeginLoc())) return; if (ExplodedNode *N = C.generateNonFatalErrorNode()) { @@ -63,11 +62,39 @@ void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, "Using a fixed address is not portable because that address will " "probably not be valid in all environments or platforms."; auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); - R->addRange(B->getRHS()->getSourceRange()); + R->addRange(SrcExpr->getSourceRange()); C.emitReport(std::move(R)); } } +void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, + CheckerContext &C) const { + if (B->getOpcode() != BO_Assign) + return; + + checkUseOfFixedAddress(B->getType(), B->getRHS(), C); +} + +void FixedAddressChecker::checkPreStmt(const DeclStmt *D, + CheckerContext &C) const { + for (const auto *D1 : D->decls()) { + if (const auto *VD1 = dyn_cast<VarDecl>(D1); VD1 && VD1->hasInit()) + checkUseOfFixedAddress(VD1->getType(), VD1->getInit(), C); + } +} + +void FixedAddressChecker::checkPreStmt(const CallExpr *CE, + CheckerContext &C) const { + const FunctionDecl *Callee = CE->getDirectCallee(); + if (!Callee) + return; + if (CE->getNumArgs() != Callee->getNumParams()) + return; + + for (auto [Arg, Param] : zip_equal(CE->arguments(), Callee->parameters())) + checkUseOfFixedAddress(Param->getType(), Arg, C); +} + void ento::registerFixedAddressChecker(CheckerManager &mgr) { mgr.registerChecker<FixedAddressChecker>(); } diff --git a/clang/test/Analysis/ptr-arith.c b/clang/test/Analysis/ptr-arith.c index b5ccd2c207ead1..1a3e18a64dedc5 100644 --- a/clang/test/Analysis/ptr-arith.c +++ b/clang/test/Analysis/ptr-arith.c @@ -39,6 +39,8 @@ domain_port (const char *domain_b, const char *domain_e, #define FIXED_VALUE (int*) 0x1111 +void f_ptr_param(void *); + void f4(void) { int *p; p = (int*) 0x10000; // expected-warning{{Using a fixed address is not portable because that address will probably not be valid in all environments or platforms}} @@ -55,6 +57,9 @@ void f4(void) { sigaction(SIGINT, &sa, NULL); p = FIXED_VALUE; // expected-warning{{Using a fixed address is not portable because that address will probably not be valid in all environments or platforms}} + + int *p2 = (int*) 1; // expected-warning{{Using a fixed address is not portable because that address will probably not be valid in all environments or platforms}} + f_ptr_param((void *)-1); // expected-warning{{Using a fixed address is not portable because that address will probably not be valid in all environments or platforms}} } void f5(void) { @@ -104,8 +109,8 @@ void null_operand(int *a) { } void const_locs(void) { - char *a = (char*)0x1000; - char *b = (char*)0x1100; + char *a = (char*)0x1000; // expected-warning{{Using a fixed address is not portable because that address will probably not be valid in all environments or platforms}} + char *b = (char*)0x1100; // expected-warning{{Using a fixed address is not portable because that address will probably not be valid in all environments or platforms}} start: clang_analyzer_eval(a != b); // expected-warning{{TRUE}} clang_analyzer_eval(a < b); // expected-warning{{TRUE}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits