Szelethus added a reviewer: xazax.hun.
Szelethus added a comment.
Herald added a subscriber: rnkovacs.
Alright, I'm sold. How about we add a checker option for it? I don't actually
insist, just an idea. @xazax.hun, how has this feature played out?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:394
+const ExplodedNode *StreamChecker::getAcquireSite(const ExplodedNode *N,
+ SymbolRef Sym,
----------------
How about `getAcquisitionSite`, and a line of comment:
> Searches for the `ExplodedNode` where the file descriptor was acquired for
> `Sym`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:398-399
+ ProgramStateRef State = N->getState();
+ // When bug type is resource leak, exploded node N may not have state info
+ // for leaked file descriptor, but predecessor should have it.
+ if (!State->get<StreamMap>(Sym))
----------------
I see what you mean, but I'd phrase this differently, and place it...
>Resource leaks can result in multiple warning that describe the same kind of
>programming error:
>```
>void f() {
> FILE *F = fopen("a.txt");
> if (rand()) // state split
> return; // warning
>} // warning
>```
>While this isn't necessarily true (leaking the same stream could result from a
>different kinds of errors), the reduction in redundant reports makes this a
>worthwhile heuristic.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:961
- C.emitReport(std::make_unique<PathSensitiveBugReport>(
- BT_ResourceLeak, BT_ResourceLeak.getDescription(), N));
+ ;
+ const ExplodedNode *AcquireNode = getAcquireSite(N, Sym, C);
----------------
...here!
================
Comment at: clang/test/Analysis/stream-note.c:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store
region -analyzer-output text -verify %s
+
----------------
`core` package! Also, we don't specify `-analyzer-store region` explicitly, we
even wondered whether we should just remove the option altogether. Fun fact,
`clang_analyze_cc1` actually expands to an invocation that contains it anyways.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81407/new/
https://reviews.llvm.org/D81407
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
