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

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

After reading @martong's comment D72705#2035844 <https://reviews.llvm.org/D72705#2035844> and @balazske's comment D72705#2086994 <https://reviews.llvm.org/D72705#2086994>, I think there is a need to argue across all paths of execution within the function, and as such this is a dataflow problem.

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

>   c = fgetc(fd);
>   if (c == '+' || c == '*' || c == '|' || c == '>' || c == '@' || c == EOF || c == '\n') { ... }
>  
>
>
> The first `c == '+'` is found by the checker and reported as false positive (the later `c == EOF` is not found). Such a case can be found if the checker can collect expressions that are separated by `||` or `&&` and the symbol to check occurs in these and there is only a simple comparison.


Our current dataflow analyses definitely have to make judgement of which block (and in that, which `CFGStmt`s) **reads** or **writes** a variable/expression. You can actually register an observer for expression writes (or in other terms, //kills//), so I don't think it'd be too challenging to do this with reads.

`DeadStoresChecker` is a great example for a checker that utilizes dataflow and pathsensitive analyses.

In D72705#1863381 <https://reviews.llvm.org/D72705#1863381>, @Szelethus wrote:

> In D72705#1859711 <https://reviews.llvm.org/D72705#1859711>, @balazske wrote:
>
> > Generally, is it a problem if there are multiple checkers that may detect same defects but with different approach?
>
>
> If avoidable, yes. If not, I think its an acceptable sacrifice.


Mind that since D80905 <https://reviews.llvm.org/D80905> landed, the tone has shifted, if a more checker detects a specific bug (like double free) but a more general checker does as well (as passing garbage value to a function) we're okay with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705