Skip to content

Commit 3732f50

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

28 files changed

Lines changed: 4210 additions & 495 deletions

‎.github/workflows/CI-cygwin.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ jobs:
4444
- name: Set up Cygwin
4545
uses: cygwin/cygwin-install-action@v6
4646
with:
47-
site: https://mirrors.cicku.me/cygwin/
4847
platform: ${{ matrix.platform }}
4948
packages: ${{ matrix.packages }}
5049

‎Makefile‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ $(libcppdir)/templatesimplifier.o: lib/templatesimplifier.cpp lib/checkers.h lib
675675
$(libcppdir)/timer.o: lib/timer.cpp lib/config.h lib/timer.h
676676
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/timer.cpp
677677

678-
$(libcppdir)/token.o: lib/token.cpp externals/simplecpp/simplecpp.h lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.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/tokenlist.h lib/tokenrange.h lib/utils.h lib/valueflow.h lib/vfvalue.h
678+
$(libcppdir)/token.o: lib/token.cpp externals/simplecpp/simplecpp.h lib/astutils.h lib/checkers.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.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/tokenlist.h lib/tokenrange.h lib/utils.h lib/valueflow.h lib/vf_common.h lib/vfvalue.h
679679
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/token.cpp
680680

681681
$(libcppdir)/tokenlist.o: lib/tokenlist.cpp externals/simplecpp/simplecpp.h lib/astutils.h lib/checkers.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/keywords.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/vfvalue.h
@@ -690,7 +690,7 @@ $(libcppdir)/vf_analyzers.o: lib/vf_analyzers.cpp lib/analyzer.h lib/astutils.h
690690
$(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/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
691691
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_common.cpp
692692

693-
$(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/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
693+
$(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/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
694694
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_settokenvalue.cpp
695695

696696
$(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
@@ -957,7 +957,7 @@ test/testutils.o: test/testutils.cpp lib/check.h lib/checkers.h lib/color.h lib/
957957
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/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
958958
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvaarg.cpp
959959

960-
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/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
960+
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
961961
$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testvalueflow.cpp
962962

963963
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/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
@@ -41,6 +41,7 @@
4141
#include <functional>
4242
#include <initializer_list>
4343
#include <iterator>
44+
#include <limits>
4445
#include <list>
4546
#include <set>
4647
#include <type_traits>
@@ -955,29 +956,58 @@ bool extractForLoopValues(const Token *forToken,
955956
nonneg int &varid,
956957
bool &knownInitValue,
957958
MathLib::bigint &initValue,
959+
const ValueFlow::Value*& initValueSource,
960+
const ValueFlow::Value*& initValueWitnessSource,
958961
bool &partialCond,
959962
MathLib::bigint &stepValue,
960963
MathLib::bigint &lastValue)
961964
{
965+
initValueSource = nullptr;
966+
initValueWitnessSource = nullptr;
962967
if (!Token::simpleMatch(forToken, "for (") || !Token::simpleMatch(forToken->next()->astOperand2(), ";"))
963968
return false;
964969
const Token *initExpr = forToken->next()->astOperand2()->astOperand1();
965970
const Token *condExpr = forToken->next()->astOperand2()->astOperand2()->astOperand1();
966971
const Token *incExpr = forToken->next()->astOperand2()->astOperand2()->astOperand2();
967972
if (!initExpr || !initExpr->isBinaryOp() || initExpr->str() != "=" || !Token::Match(initExpr->astOperand1(), "%var%"))
968973
return false;
969-
std::vector<MathLib::bigint> minInitValue =
970-
getMinValue(makeIntegralInferModel(), initExpr->astOperand2()->values());
974+
const Token* const initToken = initExpr->astOperand2();
975+
const std::vector<MathLib::bigint> minInitValue =
976+
getMinValue(makeIntegralInferModel(), initToken->values(), &initValueSource);
971977
if (minInitValue.empty()) {
972-
const ValueFlow::Value* v = initExpr->astOperand2()->getMinValue(true);
973-
if (v)
974-
minInitValue.push_back(v->intvalue);
978+
knownInitValue = false;
979+
initValue = 0;
980+
} else {
981+
knownInitValue = initToken->hasKnownIntValue();
982+
initValue = minInitValue.front();
983+
if (knownInitValue && initValueSource && initValueSource->bound != ValueFlow::Value::Bound::Point) {
984+
const auto it = std::find_if(initToken->values().cbegin(),
985+
initToken->values().cend(),
986+
[initValue](const ValueFlow::Value& value) {
987+
return value.isIntValue() && !value.isImpossible() && !value.isInconclusive() &&
988+
value.bound == ValueFlow::Value::Bound::Point && value.intvalue == initValue;
989+
});
990+
if (it != initToken->values().cend())
991+
initValueSource = &*it;
992+
}
975993
}
976-
if (minInitValue.empty())
994+
const auto isNegativeEvidence = [](const ValueFlow::Value& candidate) {
995+
return candidate.isIntValue() && !candidate.isImpossible() && !candidate.isInconclusive() &&
996+
(candidate.isKnown() || candidate.isPossible()) && candidate.path >= 0 && candidate.intvalue < 0 &&
997+
(candidate.bound == ValueFlow::Value::Bound::Point || candidate.bound == ValueFlow::Value::Bound::Upper);
998+
};
999+
if (initValueSource && isNegativeEvidence(*initValueSource)) {
1000+
initValueWitnessSource = initValueSource;
1001+
} else {
1002+
const auto it = std::find_if(initToken->values().cbegin(),
1003+
initToken->values().cend(),
1004+
isNegativeEvidence);
1005+
if (it != initToken->values().cend())
1006+
initValueWitnessSource = &*it;
1007+
}
1008+
if (minInitValue.empty() && !initValueWitnessSource)
9771009
return false;
9781010
varid = initExpr->astOperand1()->varId();
979-
knownInitValue = initExpr->astOperand2()->hasKnownIntValue();
980-
initValue = minInitValue.front();
9811011
partialCond = Token::Match(condExpr, "%oror%|&&");
9821012
visitAstNodes(condExpr, [varid, &condExpr](const Token *tok) {
9831013
if (Token::Match(tok, "%oror%|&&"))
@@ -993,10 +1023,14 @@ bool extractForLoopValues(const Token *forToken,
9931023
if (!incExpr || !incExpr->isUnaryOp("++") || incExpr->astOperand1()->varId() != varid)
9941024
return false;
9951025
stepValue = 1;
996-
if (condExpr->str() == "<")
997-
lastValue = condExpr->astOperand2()->getKnownIntValue() - 1;
998-
else
1026+
if (condExpr->str() == "<") {
1027+
const MathLib::bigint limit = condExpr->astOperand2()->getKnownIntValue();
1028+
if (limit == std::numeric_limits<MathLib::bigint>::min())
1029+
return false;
1030+
lastValue = limit - 1;
1031+
} else {
9991032
lastValue = condExpr->astOperand2()->getKnownIntValue();
1033+
}
10001034
return true;
10011035
}
10021036

‎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)