[clang] [clang][analyzer] Check initialization and argument passing in FixedAddressChecker (PR #110977)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 3 08:10:42 PDT 2024
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.keri at ericsson.com>
Date: Mon, 30 Sep 2024 16:51:35 +0200
Subject: [PATCH 1/2] [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.keri at ericsson.com>
Date: Thu, 3 Oct 2024 17:10:13 +0200
Subject: [PATCH 2/2] 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) {
More information about the cfe-commits
mailing list