This is an archive of the discontinued LLVM Phabricator instance.

[clang][analyzer] Add report of NULL stream to StreamChecker.
ClosedPublic

Authored by balazske on Jun 5 2023, 7:47 AM.

Details

Summary

The report of NULL stream was removed in commit 570bf97.
The old reason is not actual any more because the checker dependencies are changed.
It is not good to eliminate a failure state (where the stream is NULL) without
generating a bug report because other checkers are not able to find it later.
The checker did this with the NULL stream pointer, and because this checker
runs now before other checkers that can detect NULL pointers, the null pointer
bug was not found at all.

Diff Detail

Event Timeline

balazske created this revision.Jun 5 2023, 7:47 AM
Herald added a reviewer: NoQ. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
balazske requested review of this revision.Jun 5 2023, 7:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2023, 7:47 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
steakhal accepted this revision.Jun 5 2023, 8:09 AM

Ah yes, finally this sorts itself out.
We also had to revert 570bf972f5adf05438c7e08d693bf4b96bfd510a because we use the Stream checker but not the StdLibFunc checker, so in our case after a rebase suddenly some important reports "disappeared", thus we reverted the cause.
As more and more patches came to the stdlibfunc checker, I worried about maintaining that revert. Finally, this patch will resolve it, so we don't have to do it ourselves.

Thanks!

This revision is now accepted and ready to land.Jun 5 2023, 8:09 AM
steakhal added inline comments.Jun 5 2023, 8:21 AM
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
553

What's the purpose of this hunk?

balazske added inline comments.Jun 5 2023, 8:53 AM
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
553

The intent was that NonNullParamChecker should find a null pointer problem before StreamChecker and have a fixed priority of the warnings. But NonNullParamChecker probably does not apply to the stream functions because there are no nonnull attributes (this may be possible?) and no references. No tests fail if this line is removed.

steakhal added inline comments.Jun 5 2023, 9:02 AM
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
553

Okay, I see. Choose as you want. I don't mind.