================
@@ -538,21 +536,35 @@ void
CheckerManager::runCheckersForBeginFunction(ExplodedNodeSet &Dst,
expandGraphWithCheckers(C, Dst, Src);
}
-/// Run checkers for end of path.
-// Note, We do not chain the checker output (like in expandGraphWithCheckers)
-// for this callback since end of path nodes are expected to be final.
+/// Run checkers for end of a function (either the entrypoint or another
+/// function that was inlined). Note that this function places the
+/// checker activations on separate execution paths:
+/// /-[checker1]-> N1 ...
+/// Pred --[checker2]-> N2 ...
+/// \-[checker3]-> N3 ...
+/// (If none of the checkers produce a transition, we continue with 'Pred'.)
+///
+/// This differs from the handling of all the other checker callbacks, where
+/// the checker activations are chained sequentially on a single path:
+/// Pred --[checker1]-> N1 --[checker2]-> N2 --[checker3]-> N3 ...
+///
+/// This difference has historical reasons: originally this callback was called
+/// 'EndPath' and only activated at the end of an execution paths, and
+/// (according to an old comment) those 'EndPath' checkers expected that they
+/// create an "end of path" node which will be final.
+/// TODO: Check whether this exceptional behavior is still justified.
void CheckerManager::runCheckersForEndFunction(ExplodedNodeSet &Dst,
ExplodedNode *Pred,
ExprEngine &Eng,
const ReturnStmt *RS) {
----------------
NagyDonat wrote:
@steakhal @Xazax-hun @haoNoQ Did you know that the `EndFunction` has this
unusual behavior? (It isn't mentioned in `CheckerDocumentation.cpp` and I
didn't spot comments referencing it in `check::EndFunction` checkers.)
Should we preserve it or should we align this with the behavior of all other
callbacks?
I briefly reviewed that we have roughly seven "real" `check::EndFunction`
checkers and my impression is that most of them just emits leak reports, so the
"activate in parallel" logic is mostly irrelevant for them.
However, the `UnconditionalVAArgChecker` which was written by me earlier this
year uses an `EndFunction` callback to clean up a state trait that is no longer
relevant (but not harmful); here the parallel checker execution is technically
incorrect, because it can avoid the cleanup step when it follows a transition
by another checker.
https://github.com/llvm/llvm-project/pull/217319
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
