https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/110977
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 1/3] [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}} From 4bdffb0cd83672a0fea6b7caa3914d1494e076cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 3 Oct 2024 17:10:13 +0200 Subject: [PATCH 2/3] fixed variable names, using PreCall --- .../Checkers/FixedAddressChecker.cpp | 36 +++++++++---------- clang/test/Analysis/ptr-arith.c | 3 ++ 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp index aee85c59ddd630..5d611a795f763c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp @@ -18,6 +18,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" using namespace clang; @@ -26,16 +27,16 @@ using namespace ento; namespace { class FixedAddressChecker : public Checker<check::PreStmt<BinaryOperator>, check::PreStmt<DeclStmt>, - check::PreStmt<CallExpr>> { + check::PreCall> { 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 checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const; + void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; }; } @@ -67,32 +68,27 @@ void FixedAddressChecker::checkUseOfFixedAddress(QualType DstType, } } -void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, +void FixedAddressChecker::checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const { - if (B->getOpcode() != BO_Assign) + if (BO->getOpcode() != BO_Assign) return; - checkUseOfFixedAddress(B->getType(), B->getRHS(), C); + checkUseOfFixedAddress(BO->getType(), BO->getRHS(), C); } -void FixedAddressChecker::checkPreStmt(const DeclStmt *D, +void FixedAddressChecker::checkPreStmt(const DeclStmt *DS, 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); + for (const auto *D : DS->decls()) { + if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->hasInit()) + checkUseOfFixedAddress(VD->getType(), VD->getInit(), C); } } -void FixedAddressChecker::checkPreStmt(const CallExpr *CE, +void FixedAddressChecker::checkPreCall(const CallEvent &Call, 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); + for (auto Parm : enumerate(Call.parameters())) + checkUseOfFixedAddress(Parm.value()->getType(), + Call.getArgExpr(Parm.index()), C); } void ento::registerFixedAddressChecker(CheckerManager &mgr) { diff --git a/clang/test/Analysis/ptr-arith.c b/clang/test/Analysis/ptr-arith.c index 1a3e18a64dedc5..9964513f3a2c8c 100644 --- a/clang/test/Analysis/ptr-arith.c +++ b/clang/test/Analysis/ptr-arith.c @@ -60,6 +60,9 @@ void f4(void) { 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 (*f_p)(void *) = f_ptr_param; + f_p((void *) -2); // 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) { From 51a2e4470e935f35d06961f0d284afaeae31356b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 4 Oct 2024 10:30:33 +0200 Subject: [PATCH 3/3] updated failing tests --- clang/test/Analysis/concrete-address.c | 4 ++-- clang/test/Analysis/misc-ps.m | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/Analysis/concrete-address.c b/clang/test/Analysis/concrete-address.c index 346f5093e44f7a..56bfc4fa89124e 100644 --- a/clang/test/Analysis/concrete-address.c +++ b/clang/test/Analysis/concrete-address.c @@ -1,7 +1,7 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -verify %s -// expected-no-diagnostics void foo(void) { - int *p = (int*) 0x10000; // Should not crash here. + // Should not crash at next line. + int *p = (int*) 0x10000; // expected-warning{{Using a fixed address is not portable}} *p = 3; } diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m index 0a8a30cb6175cb..2e9a7a5bb2d204 100644 --- a/clang/test/Analysis/misc-ps.m +++ b/clang/test/Analysis/misc-ps.m @@ -1173,7 +1173,7 @@ void baz_pr8440(int n) // Support direct accesses to non-null memory. Reported in: // PR 5272 int test_direct_address_load(void) { - int *p = (int*) 0x4000; + int *p = (int*) 0x4000; // expected-warning{{Using a fixed address is not portable}} return *p; // no-warning } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits