[clang][analyzer] Fix argument invalidations in StreamChecker.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

@@ -21,6 +21,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/Sequence.h"
 #include <functional>
 #include <optional>
 
@@ -629,6 +630,21 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
   return nullptr;
 }
 
+static ProgramStateRef escapeArgs(ProgramStateRef State, CheckerContext &C,
+                                  const CallEvent &Call,
+                                  ArrayRef<unsigned int> EscapingArgs) {
+  const auto *CE = Call.getOriginExpr();
+
+  SmallVector<SVal> EscapingVals;
+  EscapingVals.reserve(EscapingArgs.size());
+  for (auto EscArgIdx : EscapingArgs)
+    EscapingVals.push_back(Call.getArgSVal(EscArgIdx));
+  State = State->invalidateRegions(EscapingVals, CE, C.blockCount(),
+                                   C.getLocationContext(),
+                                   /*CausesPointerEscape=*/false);
+  return State;
+}
+
 //===----------------------------------------------------------------------===//
 // Methods of StreamChecker.
 //===----------------------------------------------------------------------===//
@@ -819,6 +835,11 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
     return;
   }
 
+  // At read, invalidate the buffer in any case of error or success,
+  // except if EOF was already present.
+  if (IsFread && !E.isStreamEof())
+    State = escapeArgs(State, C, Call, {0});
+
   // Generate a transition for the success state.
   // If we know the state to be FEOF at fread, do not add a success state.
   if (!IsFread || !E.isStreamEof()) {
@@ -863,6 +884,9 @@ void StreamChecker::evalFgetx(const FnDescription *Desc, const CallEvent &Call,
     return;
 
   if (!E.isStreamEof()) {
+    // If there was already EOF, assume that read buffer is not changed.
+    // Otherwise it may change at success or failure.
+    State = escapeArgs(State, C, Call, {0});
     if (SingleChar) {
       // Generate a transition for the success state of `fgetc`.
       NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
@@ -1011,6 +1035,14 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
         State->BindExpr(E.CE, C.getLocationContext(), RetVal);
     StateNotFailed =
         E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+    if (!StateNotFailed)
+      return;
+
+    SmallVector<unsigned int> EscArgs;
+    for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
+      EscArgs.push_back(EscArg);
+    StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
+
     if (StateNotFailed)
       C.addTransition(StateNotFailed);
   }
@@ -1073,8 +1105,12 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
   // return -1.
   // If an error occurs, the function shall return -1 and set 'errno'.
 
-  // Add transition for the successful state.
   if (!E.isStreamEof()) {
+    // Escape buffer and size (may change by the call).
+    // May happen even at error (partial read?).
+    State = escapeArgs(State, C, Call, {0, 1});
+
+    // Add transition for the successful state.
     NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
     ProgramStateRef StateNotFailed =
         State->BindExpr(E.CE, C.getLocationContext(), RetVal);
@@ -1161,6 +1197,7 @@ void StreamChecker::evalFgetpos(const FnDescription *Desc,
 
   ProgramStateRef StateNotFailed, StateFailed;
   std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
+  StateNotFailed = escapeArgs(StateNotFailed, C, Call, {1});
 
   // This function does not affect the stream state.
   // Still we add success and failure state with the appropriate return value.

clang/test/Analysis/stream-invalidate.c

@@ -0,0 +1,147 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=alpha.unix.Stream \
+// RUN: -analyzer-checker=debug.ExprInspection
+
+#include "Inputs/system-header-simulator.h"
+
+void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
+
+void test_fread(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+    return;
+
+  char Buf[3] = {10, 10, 10};
+  fread(Buf, 1, 3, F);
+  // The check applies to success and failure.
+  clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10.
+  clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}}
+  if (feof(F)) {
+    char Buf1[3] = {10, 10, 10};
+    fread(Buf1, 1, 3, F); // expected-warning {{is in EOF state}}
+    clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}}
+    clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}}
+  }
+
+  fclose(F);
+}
+
+void test_fwrite(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+    return;
+
+  char Buf[3] = {10, 10, 10};
+  fwrite(Buf, 1, 3, F);
+  // The check applies to success and failure.
+  clang_analyzer_dump(Buf[0]); // expected-warning {{10 S32b}}
+  clang_analyzer_dump(Buf[2]); // expected-warning {{10 S32b}}
+
+  fclose(F);
+}
+
+void test_fgets() {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+
+  char Buf[3] = {10, 10, 10};
+  fgets(Buf, 3, F);
+  // The check applies to success and failure.
+  clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10.
+  clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}}
+  if (feof(F)) {
+    char Buf1[3] = {10, 10, 10};
+    fgets(Buf1, 3, F); // expected-warning {{is in EOF state}}
+    clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}}
+    clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}}
+  }
+
+  fclose(F);
+}
+
+void test_fputs() {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+
+  char *Buf = "aaa";
+  fputs(Buf, F);
+  // The check applies to success and failure.
+  clang_analyzer_dump(Buf[0]); // expected-warning {{97 S32b}}
+  clang_analyzer_dump(Buf[2]); // expected-warning {{97 S32b}}
+  clang_analyzer_dump(Buf[3]); // expected-warning {{0 S32b}}
+
+  fclose(F);
+}
+
+void test_fscanf() {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+
+  int a = 1;
+  unsigned b;
+  int Ret = fscanf(F, "%d %u", &a, &b);
+  if (Ret == 0) {
+    clang_analyzer_dump(a); // expected-warning {{conj_$}}
+    // FIXME: should be {{1 S32b}}.
+    clang_analyzer_dump(b); // expected-warning {{conj_$}}
+    // FIXME: should be {{uninitialized value}}.
+  } else if (Ret == 1) {
+    clang_analyzer_dump(a); // expected-warning {{conj_$}}
+    clang_analyzer_dump(b); // expected-warning {{conj_$}}
+    // FIXME: should be {{uninitialized value}}.
+  } else if (Ret >= 2) {
+    clang_analyzer_dump(a); // expected-warning {{conj_$}}
+    clang_analyzer_dump(b); // expected-warning {{conj_$}}
+    clang_analyzer_eval(Ret == 2); // expected-warning {{FALSE}} expected-warning {{TRUE}}
+    // FIXME: should be only TRUE.
+  } else {
+    clang_analyzer_dump(a); // expected-warning {{1 S32b}}
+    clang_analyzer_dump(b); // expected-warning {{uninitialized value}}
+  }
+
+  fclose(F);
+}
+
+void test_getdelim(char *P, size_t Sz) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+
+  char *P1 = P;
+  size_t Sz1 = Sz;
+  ssize_t Ret = getdelim(&P, &Sz, '\t', F);
+  if (Ret < 0) {
+    clang_analyzer_eval(P == P1); // expected-warning {{FALSE}} \
+                                  // expected-warning {{TRUE}}
+    clang_analyzer_eval(Sz == Sz1); // expected-warning {{FALSE}} \
+                                    // expected-warning {{TRUE}}
+  } else {
+    clang_analyzer_eval(P == P1); // expected-warning {{FALSE}} \
+                                  // expected-warning {{TRUE}}
+    clang_analyzer_eval(Sz == Sz1); // expected-warning {{FALSE}} \
+                                    // expected-warning {{TRUE}}
+  }
+
+  fclose(F);
+}
+
+void test_fgetpos() {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+
+  fpos_t Pos = 1;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret == 0) {
+    clang_analyzer_dump(Pos); // expected-warning {{conj_$}}
+  } else {
+    clang_analyzer_dump(Pos); // expected-warning {{1 S32b}}
+  }
+
+  fclose(F);
+}