[clang] [clang][analyzer] Check initialization and argument passing in FixedAddressChecker (PR #110977)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Balázs Kéri (balazske)

<details>
<summary>Changes</summary>



---
Full diff: https://github.com/llvm/llvm-project/pull/110977.diff


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp (+45-18) 
- (modified) clang/test/Analysis/ptr-arith.c (+7-2) 


``````````diff
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}}

``````````

</details>


https://github.com/llvm/llvm-project/pull/110977