[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)

[Ben Shi via cfe-commits]

https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/76207

>From 0a46fedc497a124d3aca4295b8c18ae60df186e9 Mon Sep 17 00:00:00 2001
From: Ben Shi <bennshi at tencent.com>
Date: Fri, 22 Dec 2023 14:47:48 +0800
Subject: [PATCH] [clang][analyzer] Improve modeling of `fileno` in the
 StreamChecker

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 30 ++++++++++++-------
 clang/test/Analysis/stream-errno.c            |  7 +++--
 clang/test/Analysis/stream-noopen.c           | 12 ++++++++
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 254b36ed03968d..80ac910060720f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -265,7 +265,11 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
       {{{"fseek"}, 3},
        {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
       {{{"ftell"}, 1},
-       {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+       {&StreamChecker::preDefault,
+        std::bind(&StreamChecker::evalFtellFileno, _1, _2, _3, _4, true), 0}},
+      {{{"fileno"}, 1},
+       {&StreamChecker::preDefault,
+        std::bind(&StreamChecker::evalFtellFileno, _1, _2, _3, _4, false), 0}},
       {{{"fflush"}, 1},
        {&StreamChecker::preFflush, &StreamChecker::evalFflush, 0}},
       {{{"rewind"}, 1},
@@ -284,7 +288,6 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
        {&StreamChecker::preDefault,
         std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError),
         0}},
-      {{{"fileno"}, 1}, {&StreamChecker::preDefault, nullptr, 0}},
   };
 
   CallDescriptionMap<FnDescription> FnTestDescriptions = {
@@ -342,8 +345,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   void evalFsetpos(const FnDescription *Desc, const CallEvent &Call,
                    CheckerContext &C) const;
 
-  void evalFtell(const FnDescription *Desc, const CallEvent &Call,
-                 CheckerContext &C) const;
+  void evalFtellFileno(const FnDescription *Desc, const CallEvent &Call,
+                       CheckerContext &C, bool IsRetLongTy) const;
 
   void evalRewind(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
@@ -1049,8 +1052,9 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc,
   C.addTransition(StateFailed);
 }
 
-void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
-                              CheckerContext &C) const {
+void StreamChecker::evalFtellFileno(const FnDescription *Desc,
+                                    const CallEvent &Call, CheckerContext &C,
+                                    bool IsRetLongTy) const {
   ProgramStateRef State = C.getState();
   SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
   if (!Sym)
@@ -1067,10 +1071,12 @@ void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
   NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
   ProgramStateRef StateNotFailed =
       State->BindExpr(CE, C.getLocationContext(), RetVal);
-  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
-                            SVB.makeZeroVal(C.getASTContext().LongTy),
-                            SVB.getConditionType())
-                  .getAs<DefinedOrUnknownSVal>();
+  auto Cond =
+      SVB.evalBinOp(State, BO_GE, RetVal,
+                    SVB.makeZeroVal(IsRetLongTy ? C.getASTContext().LongTy
+                                                : C.getASTContext().IntTy),
+                    SVB.getConditionType())
+          .getAs<DefinedOrUnknownSVal>();
   if (!Cond)
     return;
   StateNotFailed = StateNotFailed->assume(*Cond, true);
@@ -1078,7 +1084,9 @@ void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
     return;
 
   ProgramStateRef StateFailed = State->BindExpr(
-      CE, C.getLocationContext(), SVB.makeIntVal(-1, C.getASTContext().LongTy));
+      CE, C.getLocationContext(),
+      SVB.makeIntVal(-1, IsRetLongTy ? C.getASTContext().LongTy
+                                     : C.getASTContext().IntTy));
 
   // This function does not affect the stream state.
   // Still we add success and failure state with the appropriate return value.
diff --git a/clang/test/Analysis/stream-errno.c b/clang/test/Analysis/stream-errno.c
index bf0a61db2424f9..62befbc78e5a75 100644
--- a/clang/test/Analysis/stream-errno.c
+++ b/clang/test/Analysis/stream-errno.c
@@ -216,9 +216,10 @@ void check_fileno(void) {
   int N = fileno(F);
   if (N == -1) {
     clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
-    if (errno) {} // no-warning
-    fclose(F);
-    return;
+    if (errno) {}                    // no-warning
+  } else {
+    clang_analyzer_eval(N >= 0);     // expected-warning{{TRUE}}
   }
   if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+  fclose(F);
 }
diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..198b5dc45e35da 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -129,6 +129,18 @@ void check_ftell(FILE *F) {
   clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
 }
 
+void check_fileno(FILE *F) {
+  int Ret = fileno(F);
+  clang_analyzer_eval(F != NULL);    // expected-warning{{TRUE}}
+  if (!(Ret >= 0)) {
+    clang_analyzer_eval(Ret == -1);  // expected-warning{{TRUE}}
+    clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+    if (errno) {}                    // no-warning
+  }
+  clang_analyzer_eval(feof(F));      // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(ferror(F));    // expected-warning{{UNKNOWN}}
+}
+
 void test_rewind(FILE *F) {
   errno = 0;
   rewind(F);