balazske marked an inline comment as done. balazske added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:393-394 + + // Make the return value accordingly to the error. + State = State->assume(RetVal, (SS->*IsOfError)()); + assert(State && "Return value should not be constrained already."); ---------------- Szelethus wrote: > Is this correct? We set the `feof()` return value whether we know the state > of the stream to be EOF, but in this case we would incorrectly mark it as > non-EOF: > ```lang=bash > $ cat a.txt # contains a single newline > > $ cat test.cpp > ``` > ```lang=cpp > #include <cstdio> > > void tryRead(FILE *F) { > char C = fgetc(F); > > if (feof(F)) > printf("The stream is EOF!\n"); > else > printf("The stream is good! Read '%c'\n", C); > } > > int main() { > FILE *F = fopen("a.txt", "r"); > tryRead(F); > tryRead(F); // Incorrectly mark F to be non-EOF > } > ``` > ```lang=bash > $ clang test.cpp -fsanitize=address && ./a.out > The stream is good! Read ' > ' > The stream is EOF! > ``` > > Wouldn't it be safer to assume it to be `true` if we **know** its EOF and do > nothing otherwise? How about a state split? > > (I know this functions also handler FError, but you see what my point is.) The problem in this code seems to be that `fgetc` is not handled (yet) by the checker. The plan is to handle every possible file operation. The handler of `fgetc` would split the state to EOF and non-EOF case. In the current state this is a real "bug", it could be handled somehow by checking for escaped stream (the `fgetc` here should cause escape of the file pointer if not recognized as file operation). But this means again a new patch before this one. Or add every file operation to this patch with some simple implementation or with "forgetting" the stream? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75682/new/ https://reviews.llvm.org/D75682 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits