[clang] [analyzer] Assume the result of 'fopen' can't alias with 'std{in,out,err}' (PR #100085)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 23 04:22:22 PDT 2024


================
@@ -451,6 +462,10 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   /// The built-in va_list type is platform-specific
   mutable QualType VaListType;
 
+  mutable const VarDecl *StdinDecl = nullptr;
+  mutable const VarDecl *StdoutDecl = nullptr;
+  mutable const VarDecl *StderrDecl = nullptr;
----------------
steakhal wrote:

I tried to go fancy by following your suggestion, but overall, I find this as an unnecessary addition.
It would look like this:
```diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 53770532609d..9eef1c3d221f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -462,9 +462,8 @@ private:
   /// The built-in va_list type is platform-specific
   mutable QualType VaListType;
 
-  mutable const VarDecl *StdinDecl = nullptr;
-  mutable const VarDecl *StdoutDecl = nullptr;
-  mutable const VarDecl *StderrDecl = nullptr;
+  /// For example: stdin, stdout, stderr.
+  mutable std::array<const VarDecl *, 3> StdFileStreamDecls = {};
 
   void evalFopen(const FnDescription *Desc, const CallEvent &Call,
                  CheckerContext &C) const;
@@ -919,9 +918,8 @@ ProgramStateRef StreamChecker::assumeNoAliasingWithStdStreams(
   };
 
   assert(State);
-  State = assumeRetNE(State, StdinDecl);
-  State = assumeRetNE(State, StdoutDecl);
-  State = assumeRetNE(State, StderrDecl);
+  for (const VarDecl *VD : StdFileStreamDecls)
+    State = assumeRetNE(State, VD);
   assert(State);
   return State;
 }
@@ -2082,9 +2080,11 @@ getGlobalStreamPointerByName(const TranslationUnitDecl *TU, StringRef VarName) {
 
 void StreamChecker::checkASTDecl(const TranslationUnitDecl *TU,
                                  AnalysisManager &, BugReporter &) const {
-  StdinDecl = getGlobalStreamPointerByName(TU, "stdin");
-  StdoutDecl = getGlobalStreamPointerByName(TU, "stdout");
-  StderrDecl = getGlobalStreamPointerByName(TU, "stderr");
+  std::array<llvm::StringLiteral, 3> DeclNames{"stdin", "stdout", "stderr"};
+  for (const auto &[DeclSlot, DeclName] :
+       llvm::zip_equal(StdFileStreamDecls, DeclNames)) {
+    DeclSlot = getGlobalStreamPointerByName(TU, DeclName);
+  }
 }
 
 //===----------------------------------------------------------------------===//
```

In total, it replaces 9 lines with other 9 lines of code that is a lot more complicated and does an indirection. I'd say, not worth it.

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


More information about the cfe-commits mailing list