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

Wed Jun 21 06:34:06 PDT 2023

donat.nagy added a comment.

In D152436#4432736 <https://reviews.llvm.org/D152436#4432736>, @balazske wrote:

> [...]
> From these two solutions, which one is better? (Show many unnecessary notes, or show only necessary ones but lose some of the useful notes too.)

I think we must avoid polluting the environment with unnecessary notes (especially if we'd emit "many" of them), so I strongly suggest option 2.

Later it would be possible to fix the limitations of the interestingness system, e.g. by introducing a "mark all symbols in this expression as interesting" function. Perhaps we should open a separate discussion about this on discourse -- but this review and the de-alpha-ing of StdCLibraryFunctions should not be delayed by this tangentially related engine improvement!

Personally I think it's completely acceptable if the analyzer sometimes emits bug reports that are true positives but lack a message like "expected-note{{Function returns 1}}" as I'd guess that in the majority of these cases it wouldn't be terribly difficult for the user to manually derive this information from the context. (I'd say that even the previously highlighted fileno report <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=memcached_1.6.8_stdclibraryfunctions_test&is-unique=on&diff-type=New&checker-name=unix.StdCLibraryFunctions&report-hash=2bf08110160cdf74b43d1443a243c170&report-filepath=%2aauthfile.c&report-id=1930445> is acceptable -- it's surprising at first, but all bug reports come with an implicit //"On a certain execution path..."// prefix and in this case it's easy to see that "certain" = "when fileno returns -1".)

If these issues produce //lots// of issues that are //very// confusing, then we should put the affected functions behind off-by-default flags, finish this review process, and revisit them later when the interestingness system is improved; but based on the available information I don't think that's necessary.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152436