Szelethus added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953
if (FailureSt && !SuccessSt) {
- if (ExplodedNode *N = C.generateErrorNode(NewState))
+ if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))
reportBug(Call, N, Constraint.get(), Summary, C);
----------------
balazske wrote:
> Szelethus wrote:
> > Let me know if I got this right. The reason behind `generateErrorNode` not
> > behaving like it usually does for other checkers is because of the
> > explicitly supplied `NewState` parameter -- in its absence, the //current//
> > path of execution is sunk. With this parameter, a new parallel node is.
> > Correct?
> The `NewState` only sets the state of the new error node, if it is nullptr
> the current state is used. A new node is always added. The other new node
> functions (`addTransition`, `generateNonFatalErrorNode`, `generateSink` and
> `addSink`) have a version that can take a predecessor node, only
> `generateErrorNode` did not have this (and I can not find out why part of
> these is called "generate" and other part "add" instead of using only
> "generate" or "add").
>
> The new function is used when a node sequence `CurrentNode->A->B->ErrorNode`
> is needed. Without the new function it is only possible to make a
> `CurrentNode->ErrorNode` transition, and the following incorrect graph is
> created:
> ```
> CurrentNode->A->B
> |->ErrorNode
> ```
> The code here does exactly this (before the fix), in `NewNode` a sequence of
> nodes is appended (like A and B above), and if then an error node is created
> it is added to the CurrentNode. Not this is needed here, the error node
> should come after B. Otherwise analysis can continue after node B (that path
> is invalid because a constraint violation was found).
> (The "CurrentNode" is a `Pred` value that is stored in `CheckerContext` and
> not changed if other nodes are added.)
I've been wondering that, especially looking at the test case. Seems like this
loop runs only once, how come that new nodes are added on top of `CurrentNode`
(which, in this case refers to `C.getPredecessor()`, right?)? I checked the
checker's code, and I can't really see why `A` and `B` would ever appear. Isn't
that a bug?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137722/new/
https://reviews.llvm.org/D137722
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
