Skip to content

Commit 0bd4cac

Browse files
author
Your Name
committed
Merge branch 'main' into check-algo-out-of-bounds
2 parents c2d3e56 + bcd74c7 commit 0bd4cac

17 files changed

Lines changed: 540 additions & 209 deletions

lib/analyzer.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,13 @@ struct Analyzer {
153153
struct Assume {
154154
enum Flags : std::uint8_t {
155155
None = 0,
156-
Quiet = (1 << 0),
157-
Absolute = (1 << 1),
158-
ContainerEmpty = (1 << 2),
156+
Quiet = (1u << 0),
157+
Absolute = (1u << 1),
158+
ContainerEmpty = (1u << 2),
159+
// The branch this condition guards is not traversed yet (a separate path walks it), so
160+
// the assume must not record the program state at the branch boundaries - they would be
161+
// premature. When unset, the branch has been traversed and control is leaving it.
162+
Pending = (1u << 3),
159163
};
160164
};
161165

lib/astutils.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2230,6 +2230,8 @@ bool isEscapeFunction(const Token* ftok, const Library& library)
22302230
{
22312231
if (!Token::Match(ftok, "%name% ("))
22322232
return false;
2233+
if (Token::Match(ftok, "exit|abort"))
2234+
return true;
22332235
const Function* function = ftok->function();
22342236
if (function) {
22352237
if (function->isEscapeFunction())

lib/checkbufferoverrun.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,8 +1088,12 @@ void CheckBufferOverrunImpl::objectIndex()
10881088
for (const ValueFlow::Value& v:values) {
10891089
if (v.lifetimeKind != ValueFlow::Value::LifetimeKind::Address && v.lifetimeKind != ValueFlow::Value::LifetimeKind::Object)
10901090
continue;
1091-
const Token* varTok = nextAfterAstRightmostLeaf(v.tokvalue->astParent());
1092-
varTok = varTok ? varTok->previous() : nullptr;
1091+
const Token* varTok = v.tokvalue;
1092+
if (Token::simpleMatch(varTok->astParent(), ".")) {
1093+
varTok = varTok->astParent();
1094+
while (Token::simpleMatch(varTok, "."))
1095+
varTok = varTok->astOperand2();
1096+
}
10931097
const Variable *var = varTok ? varTok->variable() : nullptr;
10941098
if (!var)
10951099
continue;

lib/forwardanalyzer.cpp

Lines changed: 121 additions & 104 deletions
Large diffs are not rendered by default.

lib/programmemory.cpp

Lines changed: 144 additions & 65 deletions
Large diffs are not rendered by default.

lib/programmemory.h

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <map>
2929
#include <memory>
3030
#include <string>
31+
#include <tuple>
3132
#include <unordered_map>
3233
#include <utility>
3334
#include <vector>
@@ -161,20 +162,40 @@ struct CPPCHECKLIB ProgramMemory {
161162
};
162163

163164
struct ProgramMemoryState {
165+
struct ChangedKeyHash {
166+
std::size_t operator()(const std::tuple<const Token*, const Token*, const Token*>& t) const
167+
{
168+
const std::hash<const Token*> h;
169+
std::size_t seed = h(std::get<0>(t));
170+
seed ^= h(std::get<1>(t)) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
171+
seed ^= h(std::get<2>(t)) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
172+
return seed;
173+
}
174+
};
175+
using ChangedCache =
176+
std::unordered_map<std::tuple<const Token*, const Token*, const Token*>, const Token*, ChangedKeyHash>;
177+
// The token modifying expr between start and end, or nullptr.
178+
using FindChangedFn = std::function<const Token*(const Token* expr, const Token* start, const Token* end)>;
179+
164180
ProgramMemory state;
165181
std::map<nonneg int, const Token*> origins;
166182
const Settings& settings;
183+
// Memoized findExpressionChanged() pre-filter; structural, so never invalidated.
184+
std::shared_ptr<ChangedCache> changedCache;
167185

168186
explicit ProgramMemoryState(const Settings& s);
169187

170188
void replace(ProgramMemory pm, const Token* origin = nullptr);
171189

172190
void addState(const Token* tok, const ProgramMemory::Map& vars);
173191

174-
void assume(const Token* tok, bool b, bool isEmpty = false);
192+
void assume(const Token* tok, bool b, bool isEmpty = false, const Token* origin = nullptr);
175193

176194
void removeModifiedVars(const Token* tok);
177195

196+
// A findExpressionChanged() closure memoized in changedCache
197+
FindChangedFn getCachedFindExpressionChanged(bool skipDeadCode) const;
198+
178199
ProgramMemory get(const Token* tok, const Token* ctx, const ProgramMemory::Map& vars) const;
179200
};
180201

@@ -184,21 +205,30 @@ void execute(const Token* expr,
184205
ProgramMemory& programMemory,
185206
MathLib::bigint* result,
186207
bool* error,
187-
const Settings& settings);
208+
const Settings& settings,
209+
const ProgramMemory::Map& vars = {});
188210

189211
/**
190212
* Is condition always false when variable has given value?
191213
* \param condition top ast token in condition
192214
* \param pm program memory
215+
* \param vars optional tracked values that take precedence over the program memory
193216
*/
194-
bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings);
217+
bool conditionIsFalse(const Token* condition,
218+
ProgramMemory pm,
219+
const Settings& settings,
220+
const ProgramMemory::Map& vars = {});
195221

196222
/**
197223
* Is condition always true when variable has given value?
198224
* \param condition top ast token in condition
199225
* \param pm program memory
226+
* \param vars optional tracked values that take precedence over the program memory
200227
*/
201-
bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings);
228+
bool conditionIsTrue(const Token* condition,
229+
ProgramMemory pm,
230+
const Settings& settings,
231+
const ProgramMemory::Map& vars = {});
202232

203233
/**
204234
* Get program memory by looking backwards from given token.

lib/settings.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ void Settings::setCheckLevel(CheckLevel level)
335335
vfOptions.maxIfCount = 100;
336336
vfOptions.doConditionExpressionAnalysis = false;
337337
vfOptions.maxForwardBranches = 4;
338+
vfOptions.maxForwardConditionForkDepth = 0;
338339
vfOptions.maxIterations = 1;
339340
}
340341
else if (level == CheckLevel::normal) {
@@ -344,6 +345,7 @@ void Settings::setCheckLevel(CheckLevel level)
344345
vfOptions.maxIfCount = 100;
345346
vfOptions.doConditionExpressionAnalysis = false;
346347
vfOptions.maxForwardBranches = 4;
348+
vfOptions.maxForwardConditionForkDepth = 1;
347349
}
348350
else if (level == CheckLevel::exhaustive) {
349351
// Checking can take a little while. ~ 10 times slower than normal analysis is OK.
@@ -352,6 +354,8 @@ void Settings::setCheckLevel(CheckLevel level)
352354
vfOptions.maxSubFunctionArgs = 256;
353355
vfOptions.doConditionExpressionAnalysis = true;
354356
vfOptions.maxForwardBranches = -1;
357+
vfOptions.maxForwardConditionForkDepth = 4;
358+
vfOptions.maxForwardConditionForks = 256;
355359
}
356360
}
357361

lib/settings.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,15 @@ class CPPCHECKLIB WARN_UNUSED Settings {
512512
/** @brief Maximum performed forward branches */
513513
int maxForwardBranches = -1;
514514

515+
/** @brief Maximum depth of nested condition-fork continuations in the forward analyzer.
516+
Bounds the exponential fan-out of carrying condition state forward at exhaustive level (where
517+
maxForwardBranches is unlimited); past it the forward analysis continues on a single linear path
518+
without skipping any branch. 0 disables forking (linear); a negative value means unlimited. */
519+
int maxForwardConditionForkDepth = 4;
520+
521+
/** @brief Maximum total condition-fork continuations in one forward traversal. */
522+
int maxForwardConditionForks = 256;
523+
515524
/** @brief Maximum performed alignof recursion */
516525
int maxAlignOfRecursion = 100;
517526

lib/vf_analyzers.cpp

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -678,16 +678,20 @@ struct ValueFlowAnalyzer : Analyzer {
678678
if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::INT))
679679
return {v->intvalue};
680680
std::vector<MathLib::bigint> result;
681-
ProgramMemory pm = getProgramMemoryFunc();
681+
// Pass the tracked values so a cached program-memory value that depends on one (e.g. 'h(p)'
682+
// after 'p' was reassigned) is re-evaluated rather than served stale. The memory is built
683+
// from the same state, so compute it once and hand it to the builder.
684+
const ProgramState vars = getProgramState();
685+
ProgramMemory pm = getProgramMemoryFunc(vars);
682686
if (Token::Match(tok, "&&|%oror%")) {
683-
if (conditionIsTrue(tok, pm, getSettings()))
687+
if (conditionIsTrue(tok, pm, getSettings(), vars))
684688
result.push_back(1);
685-
if (conditionIsFalse(tok, std::move(pm), getSettings()))
689+
if (conditionIsFalse(tok, std::move(pm), getSettings(), vars))
686690
result.push_back(0);
687691
} else {
688692
MathLib::bigint out = 0;
689693
bool error = false;
690-
execute(tok, pm, &out, &error, getSettings());
694+
execute(tok, pm, &out, &error, getSettings(), vars);
691695
if (!error)
692696
result.push_back(out);
693697
}
@@ -696,16 +700,16 @@ struct ValueFlowAnalyzer : Analyzer {
696700

697701
std::vector<MathLib::bigint> evaluateInt(const Token* tok) const
698702
{
699-
return evaluateInt(tok, [&] {
700-
return ProgramMemory{getProgramState()};
703+
return evaluateInt(tok, [](const ProgramState& vars) {
704+
return ProgramMemory{vars};
701705
});
702706
}
703707

704708
std::vector<MathLib::bigint> evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const override
705709
{
706710
if (e == Evaluate::Integral) {
707-
return evaluateInt(tok, [&] {
708-
return pms.get(tok, ctx, getProgramState());
711+
return evaluateInt(tok, [&](const ProgramState& vars) {
712+
return pms.get(tok, ctx, vars);
709713
});
710714
}
711715
if (e == Evaluate::ContainerEmpty) {
@@ -723,30 +727,43 @@ struct ValueFlowAnalyzer : Analyzer {
723727
return {};
724728
}
725729

726-
void assume(const Token* tok, bool state, unsigned int flags) override {
727-
// Update program state
728-
pms.removeModifiedVars(tok);
729-
pms.addState(tok, getProgramState());
730-
pms.assume(tok, state, flags & Assume::ContainerEmpty);
731-
730+
void assume(const Token* tok, bool state, unsigned int flags) override
731+
{
732732
bool isCondBlock = false;
733733
const Token* parent = tok->astParent();
734734
if (parent) {
735735
isCondBlock = Token::Match(parent->previous(), "if|while (");
736736
}
737737

738+
const Token* endBlock = nullptr;
738739
if (isCondBlock) {
739740
const Token* startBlock = parent->link()->next();
740741
if (Token::simpleMatch(startBlock, ";") && Token::simpleMatch(parent->tokAt(-2), "} while ("))
741742
startBlock = parent->linkAt(-2);
742-
const Token* endBlock = startBlock->link();
743-
if (state) {
744-
pms.removeModifiedVars(endBlock);
745-
pms.addState(endBlock->previous(), getProgramState());
746-
} else {
747-
if (Token::simpleMatch(endBlock, "} else {"))
748-
pms.addState(endBlock->linkAt(2)->previous(), getProgramState());
749-
}
743+
endBlock = startBlock->link();
744+
}
745+
746+
// Without Pending the 'then' block has been traversed and control is leaving it, so anchor
747+
// the assumed state at the block end instead of the condition. That keeps assumptions on
748+
// variables modified inside the block (e.g. an 'if' narrowing a value computed there) from
749+
// being discarded as "modified" once control leaves the block.
750+
const bool scopeEnd = !(flags & Assume::Pending) && state && endBlock;
751+
const Token* anchor = scopeEnd ? endBlock : tok;
752+
const Token* origin = scopeEnd ? endBlock : nullptr;
753+
754+
// Update program state
755+
pms.removeModifiedVars(anchor);
756+
pms.addState(anchor, getProgramState());
757+
pms.assume(tok, state, flags & Assume::ContainerEmpty, origin);
758+
759+
// The false path (the true path uses scopeEnd above): record the assumed state where control
760+
// continues - the end of the else block, or the closing brace when there is no else - so it
761+
// reaches the enclosing scope.
762+
if (isCondBlock && !(flags & Assume::Pending) && !state) {
763+
if (Token::simpleMatch(endBlock, "} else {"))
764+
pms.addState(endBlock->linkAt(2)->previous(), getProgramState());
765+
else
766+
pms.addState(endBlock, getProgramState());
750767
}
751768

752769
if (!(flags & Assume::Quiet)) {

test/testautovariables.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4941,7 +4941,9 @@ class TestAutoVariables : public TestFixture {
49414941
" return *iPtr;\n"
49424942
" return 0;\n"
49434943
"}");
4944-
ASSERT_EQUALS("[test.cpp:5:16] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n", errout_str());
4944+
ASSERT_EQUALS(
4945+
"[test.cpp:5:16] -> [test.cpp:7:10] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n",
4946+
errout_str());
49454947

49464948
// #11753
49474949
check("int main(int argc, const char *argv[]) {\n"

0 commit comments

Comments
 (0)