[clang] [analyzer] Use explicit call description mode in iterator checkers (PR #88913)

via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 16 08:23:54 PDT 2024


https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/88913

This commit explicitly specifies the matching mode (C library function, any non-method function, or C++ method) for the `CallDescription`s constructed in the iterator/container checkers.

This change won't cause major functional changes, but isn't NFC because it ensures that e.g. call descriptions for a non-method function won't accidentally match a method that has the same name.

Separate commits will perform (or have already performed) this change in other checkers. My goal is to ensure that the call description mode is always explicitly specified and eliminate (or strongly restrict) the vague "may be either a method or a simple function" mode that's the current default.

I'm handling the iterator checkers in this separate commit because they're infamously complex; but I don't expect any trouble because this transition doesn't interact with the "central" logic of iterator handling.

>From b18e8aa88b7ef5b4cd131ba0908ecc6134d8caae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 16 Apr 2024 13:47:58 +0200
Subject: [PATCH] [analyzer] Use explicit call description mode in iterator
 checkers

This commit explicitly specifies the matching mode (C library function,
any non-method function, or C++ method) for the `CallDescription`s
constructed in the iterator/container checkers.

This change won't cause major functional changes, but isn't NFC because
it ensures that e.g. call descriptions for a non-method function won't
accidentally match a method that has the same name.

Separate commits will perform (or have already performed) this change in
other checkers. My goal is to ensure that the call description mode is
always explicitly specified and eliminate (or strongly restrict) the
vague "may be either a method or a simple function" mode that's the
current default.

I'm handling the iterator checkers in this separate commit because
they're infamously complex; but I don't expect any trouble because this
transition doesn't interact with the "central" logic of iterator
handling.
---
 .../Checkers/ContainerModeling.cpp            | 33 +++++----
 .../Checkers/DebugContainerModeling.cpp       |  4 +-
 .../Checkers/DebugIteratorModeling.cpp        |  6 +-
 .../Checkers/IteratorModeling.cpp             |  7 +-
 .../Checkers/IteratorRangeChecker.cpp         | 11 ++-
 .../Checkers/STLAlgorithmModeling.cpp         | 67 ++++++++++++-------
 6 files changed, 80 insertions(+), 48 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index 009c0d3fb93686..55ed809bfed6ce 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -72,26 +72,31 @@ class ContainerModeling
                                                    SVal) const;
 
   CallDescriptionMap<NoItParamFn> NoIterParamFunctions = {
-      {{{"clear"}, 0}, &ContainerModeling::handleClear},
-      {{{"assign"}, 2}, &ContainerModeling::handleAssign},
-      {{{"push_back"}, 1}, &ContainerModeling::handlePushBack},
-      {{{"emplace_back"}, 1}, &ContainerModeling::handlePushBack},
-      {{{"pop_back"}, 0}, &ContainerModeling::handlePopBack},
-      {{{"push_front"}, 1}, &ContainerModeling::handlePushFront},
-      {{{"emplace_front"}, 1}, &ContainerModeling::handlePushFront},
-      {{{"pop_front"}, 0}, &ContainerModeling::handlePopFront},
+      {{CDM::CXXMethod, {"clear"}, 0}, &ContainerModeling::handleClear},
+      {{CDM::CXXMethod, {"assign"}, 2}, &ContainerModeling::handleAssign},
+      {{CDM::CXXMethod, {"push_back"}, 1}, &ContainerModeling::handlePushBack},
+      {{CDM::CXXMethod, {"emplace_back"}, 1},
+       &ContainerModeling::handlePushBack},
+      {{CDM::CXXMethod, {"pop_back"}, 0}, &ContainerModeling::handlePopBack},
+      {{CDM::CXXMethod, {"push_front"}, 1},
+       &ContainerModeling::handlePushFront},
+      {{CDM::CXXMethod, {"emplace_front"}, 1},
+       &ContainerModeling::handlePushFront},
+      {{CDM::CXXMethod, {"pop_front"}, 0}, &ContainerModeling::handlePopFront},
   };
 
   CallDescriptionMap<OneItParamFn> OneIterParamFunctions = {
-      {{{"insert"}, 2}, &ContainerModeling::handleInsert},
-      {{{"emplace"}, 2}, &ContainerModeling::handleInsert},
-      {{{"erase"}, 1}, &ContainerModeling::handleErase},
-      {{{"erase_after"}, 1}, &ContainerModeling::handleEraseAfter},
+      {{CDM::CXXMethod, {"insert"}, 2}, &ContainerModeling::handleInsert},
+      {{CDM::CXXMethod, {"emplace"}, 2}, &ContainerModeling::handleInsert},
+      {{CDM::CXXMethod, {"erase"}, 1}, &ContainerModeling::handleErase},
+      {{CDM::CXXMethod, {"erase_after"}, 1},
+       &ContainerModeling::handleEraseAfter},
   };
 
   CallDescriptionMap<TwoItParamFn> TwoIterParamFunctions = {
-      {{{"erase"}, 2}, &ContainerModeling::handleErase},
-      {{{"erase_after"}, 2}, &ContainerModeling::handleEraseAfter},
+      {{CDM::CXXMethod, {"erase"}, 2}, &ContainerModeling::handleErase},
+      {{CDM::CXXMethod, {"erase_after"}, 2},
+       &ContainerModeling::handleEraseAfter},
   };
 };
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
index 72186a99d94358..d3830a01dd0cbd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
@@ -42,9 +42,9 @@ class DebugContainerModeling
                                                  CheckerContext &) const;
 
   CallDescriptionMap<FnCheck> Callbacks = {
-      {{{"clang_analyzer_container_begin"}, 1},
+      {{CDM::SimpleFunc, {"clang_analyzer_container_begin"}, 1},
        &DebugContainerModeling::analyzerContainerBegin},
-      {{{"clang_analyzer_container_end"}, 1},
+      {{CDM::SimpleFunc, {"clang_analyzer_container_end"}, 1},
        &DebugContainerModeling::analyzerContainerEnd},
   };
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
index 79ab71d7829db7..203743dacda636 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
@@ -43,11 +43,11 @@ class DebugIteratorModeling
                                                  CheckerContext &) const;
 
   CallDescriptionMap<FnCheck> Callbacks = {
-      {{{"clang_analyzer_iterator_position"}, 1},
+      {{CDM::SimpleFunc, {"clang_analyzer_iterator_position"}, 1},
        &DebugIteratorModeling::analyzerIteratorPosition},
-      {{{"clang_analyzer_iterator_container"}, 1},
+      {{CDM::SimpleFunc, {"clang_analyzer_iterator_container"}, 1},
        &DebugIteratorModeling::analyzerIteratorContainer},
-      {{{"clang_analyzer_iterator_validity"}, 1},
+      {{CDM::SimpleFunc, {"clang_analyzer_iterator_validity"}, 1},
        &DebugIteratorModeling::analyzerIteratorValidity},
   };
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
index a95e811c2a4181..5649454b4cd47e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -129,19 +129,20 @@ class IteratorModeling
   CallDescriptionMap<AdvanceFn> AdvanceLikeFunctions = {
       // template<class InputIt, class Distance>
       // void advance(InputIt& it, Distance n);
-      {{{"std", "advance"}, 2}, &IteratorModeling::handleAdvance},
+      {{CDM::SimpleFunc, {"std", "advance"}, 2},
+       &IteratorModeling::handleAdvance},
 
       // template<class BidirIt>
       // BidirIt prev(
       //   BidirIt it,
       //   typename std::iterator_traits<BidirIt>::difference_type n = 1);
-      {{{"std", "prev"}, 2}, &IteratorModeling::handlePrev},
+      {{CDM::SimpleFunc, {"std", "prev"}, 2}, &IteratorModeling::handlePrev},
 
       // template<class ForwardIt>
       // ForwardIt next(
       //   ForwardIt it,
       //   typename std::iterator_traits<ForwardIt>::difference_type n = 1);
-      {{{"std", "next"}, 2}, &IteratorModeling::handleNext},
+      {{CDM::SimpleFunc, {"std", "next"}, 2}, &IteratorModeling::handleNext},
   };
 
 public:
diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
index d2b61fb92483c3..4dd2f700a2a0eb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -56,10 +56,15 @@ class IteratorRangeChecker
   using AdvanceFn = void (IteratorRangeChecker::*)(CheckerContext &, SVal,
                                                    SVal) const;
 
+  // FIXME: these three functions are also listed in IteratorModeling.cpp,
+  // perhaps unify their handling?
   CallDescriptionMap<AdvanceFn> AdvanceFunctions = {
-      {{{"std", "advance"}, 2}, &IteratorRangeChecker::verifyAdvance},
-      {{{"std", "prev"}, 2}, &IteratorRangeChecker::verifyPrev},
-      {{{"std", "next"}, 2}, &IteratorRangeChecker::verifyNext},
+      {{CDM::SimpleFunc, {"std", "advance"}, 2},
+       &IteratorRangeChecker::verifyAdvance},
+      {{CDM::SimpleFunc, {"std", "prev"}, 2},
+       &IteratorRangeChecker::verifyPrev},
+      {{CDM::SimpleFunc, {"std", "next"}, 2},
+       &IteratorRangeChecker::verifyNext},
   };
 };
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
index a5173a05636a09..e037719b902986 100644
--- a/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
@@ -33,29 +33,50 @@ class STLAlgorithmModeling : public Checker<eval::Call> {
                                                 const CallExpr *) const;
 
   const CallDescriptionMap<FnCheck> Callbacks = {
-    {{{"std", "find"}, 3}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_if"}, 3}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_if"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_if_not"}, 3}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_if_not"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_first_of"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_first_of"}, 5}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_first_of"}, 6}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_end"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_end"}, 5}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "find_end"}, 6}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "lower_bound"}, 3}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "lower_bound"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "upper_bound"}, 3}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "upper_bound"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "search"}, 3}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "search"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "search"}, 5}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "search"}, 6}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "search_n"}, 4}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "search_n"}, 5}, &STLAlgorithmModeling::evalFind},
-    {{{"std", "search_n"}, 6}, &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find"}, 3}, &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find"}, 4}, &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_if"}, 3},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_if"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_if_not"}, 3},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_if_not"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_first_of"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_first_of"}, 5},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_first_of"}, 6},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_end"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_end"}, 5},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "find_end"}, 6},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "lower_bound"}, 3},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "lower_bound"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "upper_bound"}, 3},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "upper_bound"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "search"}, 3},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "search"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "search"}, 5},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "search"}, 6},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "search_n"}, 4},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "search_n"}, 5},
+       &STLAlgorithmModeling::evalFind},
+      {{CDM::SimpleFunc, {"std", "search_n"}, 6},
+       &STLAlgorithmModeling::evalFind},
   };
 
 public:



More information about the cfe-commits mailing list