[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> In this case function `fileno` returns -1 because of failure, but this is not indicated in a `NoteTag`. This is a correct result, only the note is missing. This problem can be solved if a note is displayed on every branch ("case") of the standard C functions. But this leads to many notes at un-interesting places. If the note is displayed only at "interesting" values another difficulty shows up: The note disappears from places where it should be shown because the "interestingness" is not set, for example at conditions of `if` statement. So the solution may require more work. This case with function `fileno` occurs 13 times in all the tested projects.

Extra notes along the path are fine as long as their `isPrunable()` flag is correctly set. It's perfectly ok to say //"(15) Hey btw we've just ran over this statement and here's what we assume it did"// in a section of a path that's already displayed to the user. Even if you say that on every line, it's probably ok.

But if the note causes a new nested stack frame to be displayed (which was otherwise pruned from the report), there better be a good reason for this, because this can easily increase the complexity of the bug report by a factor of 100.

It's definitely a requirement for a non-alpha checker to make sure that there are enough non-prunable pieces in the report for the user to understand the report. A lot of our existing on-by-default checkers (and even some `core` checkers) don't really hold up to this expectation (looking at you null dereference), but almost every time they don't, that's a false positive. If you need to pass `-analyzer-config prune-paths=false` in order to see a key step in the bug report, that's a false positive even if your path simulation was perfect.

So if any of these notes are essential to understanding any bug report (by this checker or by another checker, eg. like `getenv()` returning null is essential for null dereference checker), there needs to be a way for the note tag to learn that (eg., the `getenv()` should be able to ask whether the return value is being tracked by `trackExpressionValue()`) and if so, the note has to be marked as unprunable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152436