[clang][analyzer] Support 'getdelim' and 'getline' in StreamChecker

[benshi001]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ben Shi (benshi001)

Changes

---

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

4 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+66)
• (modified) clang/test/Analysis/Inputs/system-header-simulator.h (+3)
• (modified) clang/test/Analysis/stream-error.c (+44)
• (modified) clang/test/Analysis/taint-tester.c (-1)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 95c7503e49e0d3..1a5b9b892163cb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -269,6 +269,12 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
       {{{"ungetc"}, 2},
        {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
         std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
+      {{{"getdelim"}, 4},
+       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+        std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 3}},
+      {{{"getline"}, 3},
+       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
+        std::bind(&StreamChecker::evalGetdelim, _1, _2, _3, _4), 2}},
       {{{"fseek"}, 3},
        {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
       {{{"fseeko"}, 3},
@@ -348,6 +354,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   void evalUngetc(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
 
+  void evalGetdelim(const FnDescription *Desc, const CallEvent &Call,
+                    CheckerContext &C) const;
+
   void preFseek(const FnDescription *Desc, const CallEvent &Call,
                 CheckerContext &C) const;
   void evalFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -1014,6 +1023,63 @@ void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
   C.addTransition(StateFailed);
 }
 
+void StreamChecker::evalGetdelim(const FnDescription *Desc,
+                                 const CallEvent &Call,
+                                 CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!StreamSym)
+    return;
+
+  const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  const StreamState *OldSS = State->get<StreamMap>(StreamSym);
+  if (!OldSS)
+    return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Upon successful completion, the getline() and getdelim() functions shall
+  // return the number of bytes written into the buffer.
+  // If the end-of-file indicator for the stream is set, the function shall
+  // return -1.
+  // If an error occurs, the function shall return -1 and set 'errno'.
+
+  // Add transition for the successful state.
+  if (OldSS->ErrorState != ErrorFEof) {
+    NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+    ProgramStateRef StateNotFailed =
+        State->BindExpr(CE, C.getLocationContext(), RetVal);
+    SValBuilder &SVB = C.getSValBuilder();
+    ASTContext &ASTC = C.getASTContext();
+    auto Cond =
+        SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(CE->getType()),
+                      SVB.getConditionType())
+            .getAs<DefinedOrUnknownSVal>();
+    if (!Cond)
+      return;
+    StateNotFailed = StateNotFailed->assume(*Cond, true);
+    if (!StateNotFailed)
+      return;
+    C.addTransition(StateNotFailed);
+  }
+
+  // Add transition for the failed state.
+  // If a (non-EOF) error occurs, the resulting value of the file position
+  // indicator for the stream is indeterminate.
+  ProgramStateRef StateFailed = bindInt(-1, State, C, CE);
+  StreamErrorState NewES =
+      OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError;
+  StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
+  StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
+  if (OldSS->ErrorState != ErrorFEof)
+    C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+  else
+    C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
                              CheckerContext &C) const {
   ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index f8e3e546a7aed5..96072741a8abc1 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -14,6 +14,7 @@ typedef long long __int64_t;
 typedef __int64_t __darwin_off_t;
 typedef __darwin_off_t fpos_t;
 typedef int off_t;
+typedef long ssize_t;
 
 typedef struct _FILE FILE;
 #define SEEK_SET 0 /* Seek from beginning of file. */
@@ -55,6 +56,8 @@ char *fgets(char *restrict str, int count, FILE *restrict stream);
 int fputc(int ch, FILE *stream);
 int fputs(const char *restrict s, FILE *restrict stream);
 int ungetc(int c, FILE *stream);
+ssize_t getdelim(char **restrict lineptr, size_t *restrict n, int delimiter, FILE *restrict stream);
+ssize_t getline(char **restrict lineptr, size_t *restrict n, FILE *restrict stream);
 int fseek(FILE *__stream, long int __off, int __whence);
 int fseeko(FILE *__stream, off_t __off, int __whence);
 long int ftell(FILE *__stream);
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 0f7fdddc0dd4cd..a3c0f9629dffc2 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -224,6 +224,50 @@ void error_ungetc() {
   ungetc('A', F); // expected-warning {{Stream might be already closed}}
 }
 
+void error_getdelim(char *P, size_t Sz) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  ssize_t Ret = getdelim(&P, &Sz, '\t', F);
+  if (Ret >= 0) {
+    clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+    clang_analyzer_eval(Ret == -1);            // expected-warning {{TRUE}}
+    clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{TRUE}}
+    if (feof(F)) {
+      clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+      getdelim(&P, &Sz, '\n', F);     // expected-warning {{Read function called when stream is in EOF state}}
+    } else {
+      clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+      getdelim(&P, &Sz, '\n', F);     // expected-warning {{might be 'indeterminate'}}
+    }
+  }
+  fclose(F);
+  getdelim(&P, &Sz, '\n', F);         // expected-warning {{Stream might be already closed}}
+}
+
+void error_getline(char *P, size_t Sz) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  ssize_t Ret = getline(&P, &Sz, F);
+  if (Ret >= 0) {
+    clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+    clang_analyzer_eval(Ret == -1);            // expected-warning {{TRUE}}
+    clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{TRUE}}
+    if (feof(F)) {
+      clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+      getline(&P, &Sz, F);            // expected-warning {{Read function called when stream is in EOF state}}
+    } else {
+      clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+      getline(&P, &Sz, F);            // expected-warning {{might be 'indeterminate'}}
+    }
+  }
+  fclose(F);
+  getline(&P, &Sz, F);                // expected-warning {{Stream might be already closed}}
+}
+
 void write_after_eof_is_allowed(void) {
   FILE *F = tmpfile();
   if (!F)
diff --git a/clang/test/Analysis/taint-tester.c b/clang/test/Analysis/taint-tester.c
index ddfa91021825f3..302349fb662ddb 100644
--- a/clang/test/Analysis/taint-tester.c
+++ b/clang/test/Analysis/taint-tester.c
@@ -154,7 +154,6 @@ void getwTest(void) {
   int i = getw(stdin); // expected-warning + {{tainted}}
 }
 
-typedef long ssize_t;
 ssize_t getline(char ** __restrict, size_t * __restrict, FILE * __restrict);
 int  printf(const char * __restrict, ...);
 void free(void *ptr);

[NagyDonat]

Looks reasonable.

My only concern is that several recent changes introduced similar, but separate evalXXX methods that are used for modeling just one or two standard functions. I have a vague suspicion that these could be "standardized" (e.g. by moving their common parts into helper functions) to reduce the length and complexity of the code.

[balazske]

I have already a working code to simplify code repetitions, but want to merge first patches #76979 and #78180 (@NagyDonat could you look at these too?).

[benshi001]

I have already a working code to simplify code repetitions, but want to merge first patches #76979 and #78180 (@NagyDonat could you look at these too?).

How about merge this patch first, then apply your simplification one? Since my current one also is affected.

[balazske]

OK, change can be merged before the simplification.