[clang][Analyzer] Track null stream argument in alpha.unix.Stream .

[balazske]

Summary

The checker contains check for passing a NULL stream argument.
This change should make more easy to identify where the passed pointer
becomes NULL.

[NoQ]

Yes please!

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

838

Stream pointer *is* null. There's no doubt about that.

Path notes should look like one big complete sentence: "Assuming pointer value is null on line 83, ... stream pointer might be is null on line 88".

This isn't part of this patch so I'm just spreading awareness^^

[balazske]

Thanks for the quick accept.
The goal is to make this checker non-alpha.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

838

I do not like the messages in this checker but plan to improve it later. I observed that the last . character is removed from the messages, they can be like one sentence but multiple sentences.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

private:
   void evalSetFeofFerror(const FnDescription *Desc, const CallEvent &Call,
                          CheckerContext &C,
                          const StreamErrorState &ErrorKind) const;
 
   /// Check that the stream (in StreamVal) is not NULL.
   /// If it can only be NULL a fatal error is emitted and nullptr returned.
   /// Otherwise the return value is a new state where the stream is constrained
   /// to be non-null.
-  ProgramStateRef ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
+  ProgramStateRef ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
+                                      CheckerContext &C,
                                       ProgramStateRef State) const;
 
   /// Check that the stream is the opened state.
   /// If the stream is known to be not opened an error is generated
   /// and nullptr returned, otherwise the original state is returned.
   ProgramStateRef ensureStreamOpened(SVal StreamVal, CheckerContext &C,
                                      ProgramStateRef State) const;
 
C.addTransition(StateNotNull,
                   constructNoteTag(C, RetSym, "Stream opened here"));
   C.addTransition(StateNull);
 }
 
 void StreamChecker::preFreopen(const FnDescription *Desc, const CallEvent &Call,
                                CheckerContext &C) const {
   // Do not allow NULL as passed stream pointer but allow a closed stream.
   ProgramStateRef State = C.getState();
-  State = ensureStreamNonNull(getStreamArg(Desc, Call), C, State);
+  State = ensureStreamNonNull(getStreamArg(Desc, Call),
+                              Call.getArgExpr(Desc->StreamArgNo), C, State);
   if (!State)
     return;
 
   C.addTransition(State);
 }
 
 void StreamChecker::evalFreopen(const FnDescription *Desc,
                                 const CallEvent &Call,
void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
 
   C.addTransition(State);
 }
 
 void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
                              CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, C, State);
+  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+                              State);
   if (!State)
     return;
   State = ensureStreamOpened(StreamVal, C, State);
   if (!State)
     return;
   State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
   if (!State)
     return;
 
   SymbolRef Sym = StreamVal.getAsSymbol();
   if (Sym && State->get<StreamMap>(Sym)) {
     const StreamState *SS = State->get<StreamMap>(Sym);
     if (SS->ErrorState & ErrorFEof)
       reportFEofWarning(C, State);
   } else {
     C.addTransition(State);
   }
 }
 
 void StreamChecker::preFwrite(const FnDescription *Desc, const CallEvent &Call,
                               CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, C, State);
+  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+                              State);
   if (!State)
     return;
   State = ensureStreamOpened(StreamVal, C, State);
   if (!State)
     return;
   State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
   if (!State)
     return;
void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
   StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
   C.addTransition(StateFailed);
 }
 
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
                              CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, C, State);
+  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+                              State);
   if (!State)
     return;
   State = ensureStreamOpened(StreamVal, C, State);
   if (!State)
     return;
   State = ensureFseekWhenceCorrect(Call.getArgSVal(2), C, State);
   if (!State)
     return;
C.addTransition(FalseState->set<StreamMap>(
             Desc, NewES, SS->FilePositionIndeterminate && !NewES.isFEof())));
   }
 }
 
 void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call,
                                CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, C, State);
+  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
+                              State);
   if (!State)
     return;
   State = ensureStreamOpened(StreamVal, C, State);
   if (!State)
     return;
 
   C.addTransition(State);
 }
 
 void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
                                       const CallEvent &Call, CheckerContext &C,
                                       const StreamErrorState &ErrorKind) const {
   ProgramStateRef State = C.getState();
   SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
   assert(StreamSym && "Operation not permitted on non-symbolic stream value.");
   const StreamState *SS = State->get<StreamMap>(StreamSym);
   assert(SS && "Stream should be tracked by the checker.");
   State = State->set<StreamMap>(
       StreamSym, StreamState::getOpened(SS->LastOperation, ErrorKind));
   C.addTransition(State);
 }
 
 ProgramStateRef
-StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
+StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
+                                   CheckerContext &C,
                                    ProgramStateRef State) const {
   auto Stream = StreamVal.getAs<DefinedSVal>();
   if (!Stream)
     return State;
 
   ConstraintManager &CM = C.getConstraintManager();
 
   ProgramStateRef StateNotNull, StateNull;
   std::tie(StateNotNull, StateNull) = CM.assumeDual(C.getState(), *Stream);
 
   if (!StateNotNull && StateNull) {
     if (ExplodedNode *N = C.generateErrorNode(StateNull)) {
-      C.emitReport(std::make_unique<PathSensitiveBugReport>(
-          BT_FileNull, "Stream pointer might be NULL.", N));
+      auto R = std::make_unique<PathSensitiveBugReport>(
+          BT_FileNull, "Stream pointer might be NULL.", N);
+      if (StreamE)
+        bugreporter::trackExpressionValue(N, StreamE, *R);
+      C.emitReport(std::move(R));
     }
     return nullptr;
   }
 
   return StateNotNull;
 }
 
 ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal,

clang/test/Analysis/stream-note.c

if (c)
     return;
   // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
   // expected-note@-2 {{Opened stream never closed. Potential resource leak}}
   // expected-warning@-3 {{Opened stream never closed. Potential resource leak}}
   // expected-note@-4 {{Opened stream never closed. Potential resource leak}}
   fclose(F1);
   fclose(F2);
 }
+
+void check_track_null() {
+  FILE *F;
+  F = fopen("foo1.c", "r"); // expected-note {{Value assigned to 'F'}} expected-note {{Assuming pointer value is null}}
+  if (F != NULL) {          // expected-note {{Taking false branch}} expected-note {{'F' is equal to NULL}}
+    fclose(F);
+    return;
+  }
+  fclose(F); // expected-warning {{Stream pointer might be NULL}}
+  // expected-note@-1 {{Stream pointer might be NULL}}
+}