Skip to content

Commit dcae601

Browse files
committed
Fix false negatives when OOB occurs inside a conditional check
1 parent dbffd0a commit dcae601

File tree

1 file changed

+10
-31
lines changed

1 file changed

+10
-31
lines changed

clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -686,37 +686,16 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
686686
C.addTransition(ExceedsUpperBound, SUR.createNoteTag(C));
687687
return;
688688
}
689-
690-
BadOffsetKind Problem = AlsoMentionUnderflow
691-
? BadOffsetKind::Indeterminate
692-
: BadOffsetKind::Overflowing;
693-
Messages Msgs =
694-
getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset,
695-
*KnownSize, Location, Problem);
696-
reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize);
697-
return;
698-
}
699-
// ...and it can be valid as well...
700-
if (isTainted(State, ByteOffset)) {
701-
// ...but it's tainted, so report an error.
702-
703-
// Diagnostic detail: saying "tainted offset" is always correct, but
704-
// the common case is that 'idx' is tainted in 'arr[idx]' and then it's
705-
// nicer to say "tainted index".
706-
const char *OffsetName = "offset";
707-
if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
708-
if (isTainted(State, ASE->getIdx(), C.getLocationContext()))
709-
OffsetName = "index";
710-
711-
Messages Msgs =
712-
getTaintMsgs(Space, Reg, OffsetName, AlsoMentionUnderflow);
713-
reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize,
714-
/*IsTaintBug=*/true);
715-
return;
716689
}
717-
// ...and it isn't tainted, so the checker will (optimistically) assume
718-
// that the offset is in bounds and mention this in the note tag.
719-
SUR.recordUpperBoundAssumption(*KnownSize);
690+
691+
BadOffsetKind Problem = AlsoMentionUnderflow
692+
? BadOffsetKind::Indeterminate
693+
: BadOffsetKind::Overflowing;
694+
Messages Msgs =
695+
getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset,
696+
*KnownSize, Location, Problem);
697+
reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize);
698+
return;
720699
}
721700

722701
// Actually update the state. The "if" only fails in the extremely unlikely
@@ -758,7 +737,7 @@ void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState,
758737
std::optional<NonLoc> Extent,
759738
bool IsTaintBug /*=false*/) const {
760739

761-
ExplodedNode *ErrorNode = C.generateErrorNode(ErrorState);
740+
ExplodedNode *ErrorNode = C.generateNonFatalErrorNode(ErrorState);
762741
if (!ErrorNode)
763742
return;
764743

0 commit comments

Comments
 (0)