[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D72705#2036066 <https://reviews.llvm.org/D72705#2036066>, @balazske wrote:

> It is a way of find some of the cases with the mentioned way of tracking what operations are allowed after what operations with or without error check, on a specific stream. Instead of a stream a symbol that is passed to the function at a specific argument can be used (to generalize for other than streams). If the code would work this way, the check to discover the "error check statement" is still needed, like now in the checker.
>
> But I think it is no problem to say that the return value should be checked always (for the specific functions). The fact that a function has failed and the return value was not checked is in itself a bug and should not discovered only when a later function is called. This is not always possible. For example at `strtol` type of functions we can only find if the return value is checked for special error return value. A failing `strtol` may not cause any discoverable abnormal conditions later.


Yes, absolutely. It makes sense what you suggest. I am still a bit worried about false positives. Nevertheless, this is absolutely a good work with added value. I suggest to move on and do some analysis on real projects with this checker (tmux, curl, redis, xerces, bitcoin, ...). And see if it finds any new bugs and try to find out whether they are false positives or not. Also, would be interesting to see if there is any performance regression in the overall analysis time (I am a bit concerned about `checkLocation`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705