False Positive: Break out of for loop

[Aaron Danen]

Hi all, Aaron again :)

on the code:

void f() {
    int idx;
    int arr[3];
    for (idx = 0; idx < 3; idx++) {
        break;
    }
    arr[idx] = 0;
}

cppcheck reports
examples/breakfor.c:7:8: error: Array 'arr[3]' accessed at index 3, which is out of bounds. [arrayIndexOutOfBounds]
arr[idx] = 0;
^
examples/breakfor.c:4:23: note: Assuming that condition 'idx<3' is not redundant
for (idx = 0; idx < 3; idx++) {
^
examples/breakfor.c:7:8: note: Array index out of bounds
arr[idx] = 0;
^
This is a false positive because in the first iteration of the for loop, we would break out and idx would stay 0.

towards a solution:
I originally thought it had to do with valueFlowForLoop with this condition:

if (extractForLoopValues(tok, varid, knownInitValue, initValue, partialCond, stepValue, lastValue)) {
    const bool executeBody = !knownInitValue || initValue <= lastValue;
    const Token* vartok = Token::findmatch(tok, "%varid%", bodyStart, varid);
    if (executeBody && vartok) {
        std::list<ValueFlow::Value> initValues;
        initValues.emplace_back(initValue, ValueFlow::Value::Bound::Lower);
        initValues.push_back(ValueFlow::asImpossible(initValues.back()));
        Analyzer::Result result = valueFlowForward(bodyStart, bodyStart->link(), vartok, std::move(initValues), tokenlist, errorLogger, settings);

        if (!result.action.isModified()) {
            std::list<ValueFlow::Value> lastValues;
            lastValues.emplace_back(lastValue, ValueFlow::Value::Bound::Upper);
            lastValues.back().conditional = true;
            lastValues.push_back(ValueFlow::asImpossible(lastValues.back()));
            if (stepValue != 1)
                lastValues.pop_front();
            valueFlowForward(bodyStart, bodyStart->link(), vartok, std::move(lastValues), tokenlist, errorLogger, settings);
        }
    }
    const MathLib::bigint afterValue = executeBody ? lastValue + stepValue : initValue;
    valueFlowForLoopSimplifyAfter(tok, varid, afterValue, tokenlist, errorLogger, settings);

afterValue gets set to 3, not considering the break statement. Then I thought valueFlowForLoopSimplifAfter would find the out of bounds array access using the 3, but when I hardcoded it to use 2 instead that didn't seem to do anything. There is some work to findEscapePaths() and then bail out of for loop analysis, but that code path (valueFlowForLoopSimplify->findEscapePaths->bail) only happens if extractForLoopValues fails. Maybe extractForLoopValues should try to findEscapePaths before returning true, or valueFlowForLoopSimplifyAfter should try to find escape paths before proceeding.

However, while I think this might be a different issue, I don't believe its causing this issue. It should produce an error like "after the for loop, idx=3", not "assuming the condition idx < 3 is not redundant". I tried to do some debugging but I failed to find the exact issue. I think it has something to do with ConditionHandler::afterCondition not checking if the "for" hasEscapePaths()

Kind regards,
Aaron

[CHR]

Thanks for reporting, ticket is here: https://trac.cppcheck.net/ticket/14956