[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

balazske abandoned this revision. balazske added a comment. Herald added subscribers: manas, steakhal. The idea of this change still applies but because checker and other code changes it is better to start again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

balazske planned changes to this revision. balazske added a comment. Herald added subscribers: ASDenysPetrov, whisperity. Something like this can be added to StreamChecker but it has changed in many places so a new review would be needed. The main problem is with this `fgetc` check is that it wo

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

baloghadamsoftware added inline comments. Comment at: clang/test/Analysis/stream.c:188 +C = fgetc(fp); +fread(0, 0, 0, fp); // expected-warning {{Should call}} +fread(0, 0, 0, fp); Szelethus wrote: > balazske wrote: > > Szelethus wrote: > > > Are we s

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

balazske marked an inline comment as done. balazske added a comment. So, I think a different approach can be to store the stream state for the stream instead of `ShouldCallX` variables. (The state can be no-error, eof-error, other-error.) Optionally a warning can be made if any (modeled) stream

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

Szelethus added a comment. In D75045#1891064 , @balazske wrote: > In D75045#1891056 , @NoQ wrote: > > > In D75045#1891055 , @NoQ wrote: > > > > > How many such platforms can

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

balazske added a comment. In D75045#1891056 , @NoQ wrote: > In D75045#1891055 , @NoQ wrote: > > > How many such platforms can you name? > > > I mean, does Clang even support such targets? The problem can occur wit

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

NoQ added a comment. In D75045#1891055 , @NoQ wrote: > How many such platforms can you name? I mean, does Clang even support such targets? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75045/new/ https://

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

NoQ added a comment. In D75045#1891032 , @balazske wrote: > An improvement can be made if the check runs only when size of `char` and > `int` are equal. How many such platforms can you name? Please correct me if i'm wrong but it sounds like we're addin

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

balazske marked 4 inline comments as done. balazske added a comment. An improvement can be made if the check runs only when size of `char` and `int` are equal. And it is possible to avoid the warning if `fgetc` is called after `fgetc` (with no functions in between). (More complicated thing is to

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. I'm a bit confused as to what this patch aims to do. Once again, I'd kindly ask you to discuss for a bit before updating this patch. --- The rule states that after reaching `E

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:356 +State = State->set( +StreamSym, StreamState{SS->K, false, SS->ShouldCallFerror}); + } Maybe adding a comment like `false /*Feof*/` could make this mor

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

balazske created this revision. Herald added subscribers: cfe-commits, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. A warning is produced if the `