Skip to content

Commit 666c66a

Browse files
author
Jean-François DEVERGE
committed
Fix #15000: Propagate integral cast ranges with path-safe value flow
1 parent a573ac3 commit 666c66a

28 files changed

Lines changed: 4233 additions & 493 deletions

‎Makefile‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,8 @@ $(libcppdir)/vf_analyzers.o: lib/vf_analyzers.cpp lib/analyzer.h lib/astutils.h
691691
$(libcppdir)/vf_common.o: lib/vf_common.cpp lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vf_common.h lib/vf_settokenvalue.h lib/vfvalue.h
692692
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_common.cpp
693693

694-
$(libcppdir)/vf_settokenvalue.o: lib/vf_settokenvalue.cpp lib/astutils.h lib/calculate.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/vf_common.h lib/vf_settokenvalue.h lib/vfvalue.h
694+
$(libcppdir)/vf_settokenvalue.o: lib/vf_settokenvalue.cpp lib/astutils.h lib/calculate.h lib/checkers.h lib/config.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vf_common.h lib/vf_settokenvalue.h lib/vfvalue.h
695+
test/testvalueflow.o: test/testvalueflow.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueptr.h lib/vf_common.h lib/vfvalue.h test/fixture.h test/helpers.h
695696
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_settokenvalue.cpp
696697

697698
$(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathlib.h lib/smallvector.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h
@@ -961,7 +962,7 @@ test/testutils.o: test/testutils.cpp lib/check.h lib/checkers.h lib/color.h lib/
961962
test/testvaarg.o: test/testvaarg.cpp lib/check.h lib/checkers.h lib/checkimpl.h lib/checkvaarg.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/refthunk.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
962963
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvaarg.cpp
963964

964-
test/testvalueflow.o: test/testvalueflow.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
965+
test/testvalueflow.o: test/testvalueflow.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/infer.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueptr.h lib/vf_common.h lib/vfvalue.h test/fixture.h test/helpers.h
965966
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvalueflow.cpp
966967

967968
test/testvarid.o: test/testvarid.cpp lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/refthunk.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h

‎lib/astutils.cpp‎

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include <functional>
4343
#include <initializer_list>
4444
#include <iterator>
45+
#include <limits>
4546
#include <list>
4647
#include <set>
4748
#include <type_traits>
@@ -956,29 +957,58 @@ bool extractForLoopValues(const Token *forToken,
956957
nonneg int &varid,
957958
bool &knownInitValue,
958959
MathLib::bigint &initValue,
960+
const ValueFlow::Value*& initValueSource,
961+
const ValueFlow::Value*& initValueWitnessSource,
959962
bool &partialCond,
960963
MathLib::bigint &stepValue,
961964
MathLib::bigint &lastValue)
962965
{
966+
initValueSource = nullptr;
967+
initValueWitnessSource = nullptr;
963968
if (!Token::simpleMatch(forToken, "for (") || !Token::simpleMatch(forToken->next()->astOperand2(), ";"))
964969
return false;
965970
const Token *initExpr = forToken->next()->astOperand2()->astOperand1();
966971
const Token *condExpr = forToken->next()->astOperand2()->astOperand2()->astOperand1();
967972
const Token *incExpr = forToken->next()->astOperand2()->astOperand2()->astOperand2();
968973
if (!initExpr || !initExpr->isBinaryOp() || initExpr->str() != "=" || !Token::Match(initExpr->astOperand1(), "%var%"))
969974
return false;
970-
std::vector<MathLib::bigint> minInitValue =
971-
getMinValue(makeIntegralInferModel(), initExpr->astOperand2()->values());
975+
const Token* const initToken = initExpr->astOperand2();
976+
const std::vector<MathLib::bigint> minInitValue =
977+
getMinValue(makeIntegralInferModel(), initToken->values(), &initValueSource);
972978
if (minInitValue.empty()) {
973-
const ValueFlow::Value* v = initExpr->astOperand2()->getMinValue(true);
974-
if (v)
975-
minInitValue.push_back(v->intvalue);
979+
knownInitValue = false;
980+
initValue = 0;
981+
} else {
982+
knownInitValue = initToken->hasKnownIntValue();
983+
initValue = minInitValue.front();
984+
if (knownInitValue && initValueSource && initValueSource->bound != ValueFlow::Value::Bound::Point) {
985+
const auto it = std::find_if(initToken->values().cbegin(),
986+
initToken->values().cend(),
987+
[initValue](const ValueFlow::Value& value) {
988+
return value.isIntValue() && !value.isImpossible() && !value.isInconclusive() &&
989+
value.bound == ValueFlow::Value::Bound::Point && value.intvalue == initValue;
990+
});
991+
if (it != initToken->values().cend())
992+
initValueSource = &*it;
993+
}
976994
}
977-
if (minInitValue.empty())
995+
const auto isNegativeEvidence = [](const ValueFlow::Value& candidate) {
996+
return candidate.isIntValue() && !candidate.isImpossible() && !candidate.isInconclusive() &&
997+
(candidate.isKnown() || candidate.isPossible()) && candidate.path >= 0 && candidate.intvalue < 0 &&
998+
(candidate.bound == ValueFlow::Value::Bound::Point || candidate.bound == ValueFlow::Value::Bound::Upper);
999+
};
1000+
if (initValueSource && isNegativeEvidence(*initValueSource)) {
1001+
initValueWitnessSource = initValueSource;
1002+
} else {
1003+
const auto it = std::find_if(initToken->values().cbegin(),
1004+
initToken->values().cend(),
1005+
isNegativeEvidence);
1006+
if (it != initToken->values().cend())
1007+
initValueWitnessSource = &*it;
1008+
}
1009+
if (minInitValue.empty() && !initValueWitnessSource)
9781010
return false;
9791011
varid = initExpr->astOperand1()->varId();
980-
knownInitValue = initExpr->astOperand2()->hasKnownIntValue();
981-
initValue = minInitValue.front();
9821012
partialCond = Token::Match(condExpr, "%oror%|&&");
9831013
visitAstNodes(condExpr, [varid, &condExpr](const Token *tok) {
9841014
if (Token::Match(tok, "%oror%|&&"))
@@ -994,10 +1024,14 @@ bool extractForLoopValues(const Token *forToken,
9941024
if (!incExpr || !incExpr->isUnaryOp("++") || incExpr->astOperand1()->varId() != varid)
9951025
return false;
9961026
stepValue = 1;
997-
if (condExpr->str() == "<")
998-
lastValue = condExpr->astOperand2()->getKnownIntValue() - 1;
999-
else
1027+
if (condExpr->str() == "<") {
1028+
const MathLib::bigint limit = condExpr->astOperand2()->getKnownIntValue();
1029+
if (limit == std::numeric_limits<MathLib::bigint>::min())
1030+
return false;
1031+
lastValue = limit - 1;
1032+
} else {
10001033
lastValue = condExpr->astOperand2()->getKnownIntValue();
1034+
}
10011035
return true;
10021036
}
10031037

‎lib/astutils.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,16 @@ const Token* getCondTokFromEnd(const Token* endBlock);
230230
const Token *findNextTokenFromBreak(const Token *breakToken);
231231

232232
/**
233-
* Extract for loop values: loopvar varid, init value, step value, last value (inclusive)
233+
* Extract for loop values: loopvar varid, init value, step value, last value (inclusive).
234+
* `initValueSource` identifies the fact supplying the minimum; `initValueWitnessSource`
235+
* identifies an actual negative point or upper bound, if present.
234236
*/
235237
bool extractForLoopValues(const Token *forToken,
236238
nonneg int &varid,
237239
bool &knownInitValue,
238240
MathLib::bigint &initValue,
241+
const ValueFlow::Value*& initValueSource,
242+
const ValueFlow::Value*& initValueWitnessSource,
239243
bool &partialCond,
240244
MathLib::bigint &stepValue,
241245
MathLib::bigint &lastValue);

‎lib/checkbufferoverrun.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,7 @@ void CheckBufferOverrunImpl::arrayIndex()
355355
arrayIndexError(tok, dimensions, indexValues);
356356
}
357357

358+
358359
// Negative index
359360
bool neg = false;
360361
std::vector<ValueFlow::Value> negativeIndexes;

0 commit comments

Comments
 (0)