Skip to content

Commit 0b2778d

Browse files
authored
[analyzer] Fix StdLibraryFunctionsChecker crash on surprising sink node (#66109)
Recent changes in StdLibraryFunctionsChecker introduced a situation where the checker sequentially performed two state transitions to add two separate note tags. In the unlikely case when the updated state (the variable `NewState`) was posteriorly overconstrained, the engine marked the node after the first state transition as a sink to stop the "natural" graph exploration after that point. However, in this particular case the checker tried to directly add a second node, and this triggered an assertion in the `addPredecessor()` method of `ExplodedNode`. This commit introduces an explicit `isSink()` check to avoid this crash. To avoid similar bugs in the future, perhaps it would be possible to tweak `addTransition()` and ensure that it returns `nullptr` when it would return a sink node (to unify the two possible error conditions). This crash was observed in an analysis of the curl project (in a very long and complex function), and there I validated that this is the root cause, but I don't have a self-contained testcase that can trigger the creation of a PosteriorlyOverconstrained node in this situation.
1 parent e873280 commit 0b2778d

File tree

1 file changed

+8
-3
lines changed

1 file changed

+8
-3
lines changed

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,8 +1387,8 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
13871387
if (!NewState)
13881388
continue;
13891389

1390-
// It is possible that NewState == State is true.
1391-
// It can occur if another checker has applied the state before us.
1390+
// Here it's possible that NewState == State, e.g. when other checkers
1391+
// already applied the same constraints (or stricter ones).
13921392
// Still add these note tags, the other checker should add only its
13931393
// specialized note tags. These general note tags are handled always by
13941394
// StdLibraryFunctionsChecker.
@@ -1427,7 +1427,12 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
14271427
});
14281428
Pred = C.addTransition(NewState, Pred, Tag);
14291429
}
1430-
if (!Pred)
1430+
1431+
// Pred may be:
1432+
// - a nullpointer, if we reach an already existing node (theoretically);
1433+
// - a sink, when NewState is posteriorly overconstrained.
1434+
// In these situations we cannot add the errno note tag.
1435+
if (!Pred || Pred->isSink())
14311436
continue;
14321437
}
14331438

0 commit comments

Comments
 (0)