[PATCH] D75356: [Analyzer][StreamChecker] Introduction of stream error state handling.

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked 5 inline comments as done.
balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:37-40
+  // NoError: No error flag is set or stream is not open.
+  // EofError: EOF condition (feof returns true)
+  // OtherError: other (non-EOF) error (ferror returns true)
+  // AnyError: EofError or OtherError
----------------
Szelethus wrote:
> These too. Also, I'm not yet sure whether we need `OtherError` and `AnyError`, as stated in a later inline.
I plan to use `AnyError` for failing functions that can have **EOF** and other error as result. At a later `ferror` or `feof` call a new state split follows to differentiate the error (instead of having to add 3 new states after the function, for success, EOF error and other error). If other operation is called we know anyway that some error happened.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:333-335
+  // Ignore the call if the stream is is not tracked.
+  if (!State->get<StreamMap>(StreamSym))
+    return;
----------------
balazske wrote:
> Szelethus wrote:
> > If we check in `preCall` whether the stream is opened why don't we conservatively assume it to be open?
> If we do that we get a resource leak error for example in the test function `pr8081` (there is only a call to `fseek`). We can assume that if the function gets the file pointer as argument it does not "own" it so the close is not needed. Otherwise the false positive resource leaks come always in functions that take a file argument and use file operations. Or this case (the file pointer is passed as input argument, the file is not opened by the function that is analyzed) can be handled specially, we can assume that there is no error initially and closing the file is not needed.
This can be done in a next change. It involves more changes at other places. I think of inserting the state for the stream if it was not there before. But we need to save if this was such an insert or a normal `fopen` (and do not report resource leak for the "insert" case).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75356/new/

https://reviews.llvm.org/D75356