[analyzer] Add 'freopen' support to SimpleStreamChecker.

[balazske]

Summary

The checker now recognizes 'freopen' as file opening function.
Added support for binding the return value to the passed stream.

[NoQ]

SimpleStreamChecker is a historical tutorial example, i don't think we should be updating it other than for modernizing checker API use. It was supposed to handle a few simple examples correctly but it wasn't supposed to support all sorts of different APIs.

I'd recommend improving StreamChecker instead if you're interested in having a production-quality checker for everyday use.

[balazske]

Code of StreamChecker does not look much better, it uses deprecated eval::Call and does not check for escape. But it handles more functions and error at open (still not freopen). A mixture of both would be a better option.

[balazske]

I try to improve StreamChecker instead.

[NoQ]

Btw, evalCall is not deprecated. In fact, there are no alternatives for it in many cases.

clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp

void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.AddInteger(K);
   }
 };
 
 class SimpleStreamChecker : public Checker<check::PostCall,
                                            check::PreCall,
                                            check::DeadSymbols,
                                            check::PointerEscape> {
-  CallDescription OpenFn, CloseFn;
+  CallDescription OpenFn, CloseFn, ReopenFn;
 
   std::unique_ptr<BugType> DoubleCloseBugType;
   std::unique_ptr<BugType> LeakBugType;
 
   void reportDoubleClose(SymbolRef FileDescSym,
                          const CallEvent &Call,
                          CheckerContext &C) const;
 
   void reportLeaks(ArrayRef<SymbolRef> LeakedStreams, CheckerContext &C,
                    ExplodedNode *ErrNode) const;
 
   bool guaranteedNotToCloseFile(const CallEvent &Call) const;
 
 public:
   SimpleStreamChecker();
 
-  /// Process fopen.
+  /// Process fopen and freopen.
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   /// Process fclose.
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
   /// Stop tracking addresses which escape.
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
public:
   bool VisitSymbol(SymbolRef sym) override {
     state = state->remove<StreamMap>(sym);
     return true;
   }
 };
 } // end anonymous namespace
 
 SimpleStreamChecker::SimpleStreamChecker()
-    : OpenFn("fopen"), CloseFn("fclose", 1) {
+    : OpenFn("fopen"), CloseFn("fclose", 1), ReopenFn("freopen", 3) {
   // Initialize the bug types.
   DoubleCloseBugType.reset(
       new BugType(this, "Double fclose", "Unix Stream API Error"));
 
   // Sinks are higher importance bugs as well as calls to assert() or exit(0).
   LeakBugType.reset(
       new BugType(this, "Resource Leak", "Unix Stream API Error",
                   /*SuppressOnSink=*/true));
 }
 
+static ProgramStateRef tryBindReopenResult(const CallEvent &Call,
+                                           CheckerContext &C,
+                                           ProgramStateRef State) {
+  // Get value of file descriptor input argument.
+  SymbolRef InputDesc = Call.getArgSVal(2).getAsSymbol();
+  if (!InputDesc)
+    return State;
+
+  // Get the return value.
+  SymbolRef RetDesc = Call.getReturnValue().getAsSymbol();
+  if (!RetDesc)
+    return State;
+
+  // Bind the input to the return value.
+  const Expr *E = Call.getOriginExpr();
+  if (!E)
+    return State;
+
+  return State->BindExpr(E, C.getLocationContext(),
+                         C.getSValBuilder().makeSymbolVal(InputDesc));
+}
+
 void SimpleStreamChecker::checkPostCall(const CallEvent &Call,
                                         CheckerContext &C) const {
   if (!Call.isGlobalCFunction())
     return;
 
-  if (!Call.isCalled(OpenFn))
-    return;
+  ProgramStateRef State = C.getState();
 
+  if (Call.isCalled(OpenFn)) {
     // Get the symbolic value corresponding to the file handle.
     SymbolRef FileDesc = Call.getReturnValue().getAsSymbol();
     if (!FileDesc)
       return;
 
     // Generate the next transition (an edge in the exploded graph).
-  ProgramStateRef State = C.getState();
     State = State->set<StreamMap>(FileDesc, StreamState::getOpened());
+  } else if (Call.isCalled(ReopenFn)) {
+    // Try to tell the analyzer that the return value of 'freopen' is the same
+    // as argument 2 (if no error occurs).
+    State = tryBindReopenResult(Call, C, State);
+
+    // Set the state to opened regardless of what is was before.
+    SymbolRef InputDesc = Call.getArgSVal(2).getAsSymbol();
+    if (InputDesc) {
+      const StreamState *SS = State->get<StreamMap>(InputDesc);
+      if (!SS || !SS->isOpened())
+        State = State->set<StreamMap>(InputDesc, StreamState::getOpened());
+    }
+  } else {
+    return;
+  }
+
+  // Generate the next transition (an edge in the exploded graph).
   C.addTransition(State);
 }
 
 void SimpleStreamChecker::checkPreCall(const CallEvent &Call,
                                        CheckerContext &C) const {
   if (!Call.isGlobalCFunction())
     return;
 

clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h

 // Like the compiler, the static analyzer treats some functions differently if
 // they come from a system header -- for example, it is assumed that system
 // functions do not arbitrarily free() their parameters, and that some bugs
 // found in system headers cannot be fixed by the user and should be
 // suppressed.
 #pragma clang system_header
 
 typedef struct __sFILE {
   unsigned char *_p;
 } FILE;
 FILE *fopen(const char * restrict, const char * restrict) __asm("_" "fopen" );
+FILE *freopen(const char *restrict, const char *restrict, FILE *restrict);
 int fputc(int, FILE *);
 int fputs(const char * restrict, FILE * restrict) __asm("_" "fputs" );
 int fclose(FILE *);
 void exit(int);
 
 // The following is a fake system header function
 typedef struct __FileStruct {
   FILE * p;
 } FileStruct;
 void fakeSystemHeaderCall(FileStruct *);

clang/test/Analysis/simple-stream-checks.c

void testPassToSystemHeaderFunctionIndirectly() {
   fs.p = fopen("myfile.txt", "w");
   fakeSystemHeaderCall(&fs); // invalidates fs, making fs.p unreachable
 }  // no-warning
 
 void testOverwrite() {
   FILE *fp = fopen("myfile.txt", "w");
   fp = 0;
 } // expected-warning {{Opened file is never closed; potential resource leak}}
+
+void testReopenAndClose() {
+  FILE *F = fopen("myfile.txt", "w");
+  freopen("myfile1.txt", "w", F);
+  fclose(F);
+} // no-warning
+
+void testReopenAndNoClose() {
+  FILE *F = fopen("myfile.txt", "w");
+  freopen("myfile1.txt", "w", F);
+} // expected-warning {{Opened file is never closed; potential resource leak}}
+
+void testCloseAndReopen() {
+  FILE *F = fopen("myfile.txt", "w");
+  fclose(F);
+  freopen("myfile1.txt", "w", F);
+} // expected-warning {{Opened file is never closed; potential resource leak}}
+
+void testCloseAndReopenAndClose() {
+  FILE *F = fopen("myfile.txt", "w");
+  fclose(F);
+  freopen("myfile1.txt", "w", F);
+  fclose(F);
+} // no-warning
+
+void testBindingOfReopenReturnValue() {
+  FILE *F = fopen("myfile.txt", "w");
+  FILE *F1 = freopen("", "r", F);
+  fclose(F);
+  fclose(F1); // expected-warning {{Closing a previously closed file stream}}
+}
+
+void testEscapeAfterReopen() {
+  FILE *F = fopen("myfile.txt", "w");
+  FILE *F1 = freopen("", "r", F);
+  myfclose(F);
+} // no-warning