Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/checkbufferoverrun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,9 @@ static std::vector<ValueFlow::Value> getOverrunIndexValues(const Token* tok,
overflow = true;
indexValues.push_back(values.front());
}
if (overflow)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks more weird imho.

return indexValues;
return {};
if (!overflow)
indexValues.clear();
return indexValues;
}

void CheckBufferOverrun::arrayIndex()
Expand Down
2 changes: 1 addition & 1 deletion lib/checkstl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ static std::string indexValueString(const ValueFlow::Value& indexValue, const st
indexString += "+" + MathLib::toString(indexValue.intvalue);
}
if (indexValue.bound == ValueFlow::Value::Bound::Lower)
return "greater or equal to " + indexString;
indexString = "greater or equal to " + indexString;
return indexString;
}

Expand Down
76 changes: 39 additions & 37 deletions lib/errorlogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1055,47 +1055,49 @@ std::string getGuideline(const std::string &errId, ReportType reportType,
const std::map<std::string, std::string> &guidelineMapping,
Severity severity)
{
std::string guideline;
{
std::string guideline;

switch (reportType) {
case ReportType::autosar:
if (errId.rfind("premium-autosar-", 0) == 0) {
guideline = errId.substr(16);
switch (reportType) {
case ReportType::autosar:
if (errId.rfind("premium-autosar-", 0) == 0) {
guideline = errId.substr(16);
break;
}
if (errId.rfind("premium-misra-cpp-2008-", 0) == 0)
guideline = "M" + errId.substr(23);
break;
case ReportType::certC:
case ReportType::certCpp:
if (errId.rfind("premium-cert-", 0) == 0) {
guideline = errId.substr(13);
std::transform(guideline.begin(), guideline.end(),
guideline.begin(), static_cast<int (*)(int)>(std::toupper));
}
break;
case ReportType::misraC2012:
case ReportType::misraC2023:
case ReportType::misraC2025:
if (errId.rfind("misra-c20", 0) == 0 || errId.rfind("premium-misra-c-20", 0) == 0)
guideline = errId.substr(errId.rfind('-') + 1);
break;
case ReportType::misraCpp2008:
if (errId.rfind("premium-misra-cpp-2008", 0) == 0)
guideline = errId.substr(23);
break;
case ReportType::misraCpp2023:
if (errId.rfind("premium-misra-cpp-2023", 0) == 0)
guideline = errId.substr(errId.rfind('-') + 1);
break;
default:
break;
}
if (errId.rfind("premium-misra-cpp-2008-", 0) == 0)
guideline = "M" + errId.substr(23);
break;
case ReportType::certC:
case ReportType::certCpp:
if (errId.rfind("premium-cert-", 0) == 0) {
guideline = errId.substr(13);
std::transform(guideline.begin(), guideline.end(),
guideline.begin(), static_cast<int (*)(int)>(std::toupper));
}
break;
case ReportType::misraC2012:
case ReportType::misraC2023:
case ReportType::misraC2025:
if (errId.rfind("misra-c20", 0) == 0 || errId.rfind("premium-misra-c-20", 0) == 0)
guideline = errId.substr(errId.rfind('-') + 1);
break;
case ReportType::misraCpp2008:
if (errId.rfind("premium-misra-cpp-2008", 0) == 0)
guideline = errId.substr(23);
break;
case ReportType::misraCpp2023:
if (errId.rfind("premium-misra-cpp-2023", 0) == 0)
guideline = errId.substr(errId.rfind('-') + 1);
break;
default:
break;
}

if (!guideline.empty()) {
if (errId.find("-dir-") != std::string::npos)
guideline = "Dir " + guideline;
return guideline;
if (!guideline.empty()) {
if (errId.find("-dir-") != std::string::npos)
guideline = "Dir " + guideline;
return guideline;
}
}

auto it = guidelineMapping.find(errId);
Expand Down
10 changes: 6 additions & 4 deletions lib/fwdanalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,12 @@ FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *
if (!opTok)
opTok = tok->next();
std::pair<const Token*, const Token*> startEndTokens = opTok->findExpressionStartEndTokens();
FwdAnalysis::Result result =
checkRecursive(expr, startEndTokens.first, startEndTokens.second->next(), exprVarIds, local, true, depth);
if (result.type != Result::Type::NONE)
return result;
{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is quite pedantic by clang imho. FwdAnalysis::Result is small and simple data. But yes if we want to turn on the warning then you have to fix all warnings so I guess there's not much we can do.

FwdAnalysis::Result result =
checkRecursive(expr, startEndTokens.first, startEndTokens.second->next(), exprVarIds, local, true, depth);
if (result.type != Result::Type::NONE)
return result;
}

// #9167: if the return is inside an inner class, it does not tell us anything
if (!inInnerClass) {
Expand Down
4 changes: 2 additions & 2 deletions lib/library.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1213,8 +1213,8 @@ std::string Library::getFunctionName(const Token *ftok) const
const Token * tok = ftok->astParent()->isUnaryOp("&") ? ftok->astParent()->astOperand1() : ftok->next()->astOperand1();
std::string ret = getFunctionName(tok, error);
if (error)
return {};
if (startsWith(ret, "::"))
ret.clear();
else if (startsWith(ret, "::"))
ret.erase(0, 2);
return ret;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/mathlib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,9 @@ template<> std::string MathLib::toString<double>(double value)
result << value;
std::string s = result.str();
if (s == "-0")
return "0.0";
if (s.find_first_of(".e") == std::string::npos)
return s + ".0";
s = "0.0";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I like the old code better here from a readability point of view.

else if (s.find_first_of(".e") == std::string::npos)
s += ".0";
return s;
}

Expand Down
26 changes: 14 additions & 12 deletions lib/programmemory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1394,18 +1394,20 @@ namespace {
if (conditions1.empty())
return unknown();
std::unordered_map<nonneg int, ValueFlow::Value> condValues = executeAll(conditions1, &b);
bool allNegated = true;
ValueFlow::Value negatedValue = unknown();
for (const auto& p : condValues) {
const ValueFlow::Value& v = p.second;
if (isTrueOrFalse(v, b))
return v;
allNegated &= isTrueOrFalse(v, !b);
if (allNegated && negatedValue.isUninitValue())
negatedValue = v;
{
bool allNegated = true;
ValueFlow::Value negatedValue = unknown();
for (const auto& p : condValues) {
const ValueFlow::Value& v = p.second;
if (isTrueOrFalse(v, b))
return v;
allNegated &= isTrueOrFalse(v, !b);
if (allNegated && negatedValue.isUninitValue())
negatedValue = v;
}
if (condValues.size() == conditions1.size() && allNegated)
return negatedValue;
}
if (condValues.size() == conditions1.size() && allNegated)
return negatedValue;
if (n > 4)
return unknown();
if (!sortConditions(conditions1))
Expand Down Expand Up @@ -1805,7 +1807,7 @@ namespace {
if (elseStart)
result = execute(elseStart->scope());
} else {
return {unknown()};
result = {unknown()};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is less obvious what happens here. if the loop continues then other results can be returned.. so I want to see an unconditional return.

}
if (!result.empty())
return result;
Expand Down
5 changes: 4 additions & 1 deletion lib/symboldatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8300,10 +8300,13 @@ bool ValueType::fromLibraryType(const std::string &typestr, const Settings &sett

std::string ValueType::dump() const
{
if (type == UNKNOWN_TYPE)
return "";

std::string ret;
switch (type) {
case UNKNOWN_TYPE:
return "";
break; // never happens
case NONSTD:
ret += "valueType-type=\"nonstd\"";
break;
Expand Down
8 changes: 5 additions & 3 deletions lib/token.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2438,9 +2438,11 @@ std::pair<const Token*, const Token*> Token::typeDecl(const Token* tok, bool poi
varTok = varTok->next();
while (Token::Match(varTok, "%name% ::"))
varTok = varTok->tokAt(2);
std::pair<const Token*, const Token*> r = typeDecl(varTok);
if (r.first)
return r;
{
std::pair<const Token*, const Token*> r = typeDecl(varTok);
if (r.first)
return r;
}

if (pointedToType && tok2->astOperand1() && Token::simpleMatch(tok2, "new")) {
if (Token::simpleMatch(tok2->astOperand1(), "("))
Expand Down
89 changes: 49 additions & 40 deletions lib/valueflow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -781,9 +781,11 @@ static void valueFlowTypeTraits(TokenList& tokenlist, const Settings& settings)
eval["is_reference"] = [&](const std::vector<std::vector<const Token*>>& args) {
if (args.size() != 1)
return ValueFlow::Value::unknown();
ValueFlow::Value isRValue = eval["is_rvalue_reference"](args);
if (isRValue.isUninitValue() || isRValue.intvalue == 1)
return isRValue;
{
ValueFlow::Value isRValue = eval["is_rvalue_reference"](args);
if (isRValue.isUninitValue() || isRValue.intvalue == 1)
return isRValue;
}
return eval["is_lvalue_reference"](args);
};
for (Token* tok = tokenlist.front(); tok; tok = tok->next()) {
Expand Down Expand Up @@ -5116,21 +5118,22 @@ static void valueFlowCondition(const ValuePtr<ConditionHandler>& handler,

struct SimpleConditionHandler : ConditionHandler {
std::vector<Condition> parse(const Token* tok, const Settings& /*settings*/) const override {

std::vector<Condition> conds;
parseCompareEachInt(tok, [&](const Token* vartok, ValueFlow::Value true_value, ValueFlow::Value false_value) {
if (vartok->hasKnownIntValue())
return;
if (vartok->str() == "=" && vartok->astOperand1() && vartok->astOperand2())
vartok = vartok->astOperand1();
Condition cond;
cond.true_values.push_back(std::move(true_value));
cond.false_values.push_back(std::move(false_value));
cond.vartok = vartok;
conds.push_back(std::move(cond));
});
if (!conds.empty())
return conds;
{
std::vector<Condition> conds;
parseCompareEachInt(tok, [&](const Token* vartok, ValueFlow::Value true_value, ValueFlow::Value false_value) {
if (vartok->hasKnownIntValue())
return;
if (vartok->str() == "=" && vartok->astOperand1() && vartok->astOperand2())
vartok = vartok->astOperand1();
Condition cond;
cond.true_values.push_back(std::move(true_value));
cond.false_values.push_back(std::move(false_value));
cond.vartok = vartok;
conds.push_back(std::move(cond));
});
if (!conds.empty())
return conds;
}

const Token* vartok = nullptr;

Expand Down Expand Up @@ -6637,9 +6640,11 @@ static std::vector<ValueFlow::Value> getContainerSizeFromConstructorArgs(const s
} else if (astIsContainer(args[0]) && args.size() == 1) { // copy constructor
return getContainerValues(args[0]);
} else if (isIteratorPair(args)) {
std::vector<ValueFlow::Value> result = getContainerValues(args[0]);
if (!result.empty())
return result;
{
std::vector<ValueFlow::Value> result = getContainerValues(args[0]);
if (!result.empty())
return result;
}
// (ptr, ptr + size)
if (astIsPointer(args[0]) && args[0]->exprId() != 0) {
// (ptr, ptr) is empty
Expand Down Expand Up @@ -6940,21 +6945,23 @@ static void valueFlowContainerSize(const TokenList& tokenlist,
struct ContainerConditionHandler : ConditionHandler {
std::vector<Condition> parse(const Token* tok, const Settings& settings) const override
{
std::vector<Condition> conds;
parseCompareEachInt(tok, [&](const Token* vartok, ValueFlow::Value true_value, ValueFlow::Value false_value) {
vartok = settings.library.getContainerFromYield(vartok, Library::Container::Yield::SIZE);
if (!vartok)
return;
true_value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
false_value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
Condition cond;
cond.true_values.push_back(std::move(true_value));
cond.false_values.push_back(std::move(false_value));
cond.vartok = vartok;
conds.push_back(std::move(cond));
});
if (!conds.empty())
return conds;
{
std::vector<Condition> conds;
parseCompareEachInt(tok, [&](const Token* vartok, ValueFlow::Value true_value, ValueFlow::Value false_value) {
vartok = settings.library.getContainerFromYield(vartok, Library::Container::Yield::SIZE);
if (!vartok)
return;
true_value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
false_value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
Condition cond;
cond.true_values.push_back(std::move(true_value));
cond.false_values.push_back(std::move(false_value));
cond.vartok = vartok;
conds.push_back(std::move(cond));
});
if (!conds.empty())
return conds;
}

const Token* vartok = nullptr;

Expand Down Expand Up @@ -7645,10 +7652,12 @@ std::vector<ValueFlow::Value> ValueFlow::isOutOfBounds(const Value& size, const
ValueFlow::Value inBoundsValue = inferCondition("<", indexTok, size.intvalue);
if (inBoundsValue.isKnown() && inBoundsValue.intvalue != 0)
return {};
std::vector<ValueFlow::Value> result = isOutOfBoundsImpl(size, indexTok, false);
if (!result.empty())
return result;
{
std::vector<ValueFlow::Value> result = isOutOfBoundsImpl(size, indexTok, false);
if (!result.empty())
return result;
}
if (!possible)
return result;
return {};
return isOutOfBoundsImpl(size, indexTok, true);
}
12 changes: 7 additions & 5 deletions lib/vf_analyzers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,9 +581,11 @@ struct ValueFlowAnalyzer : Analyzer {
(Token::Match(parent, "*|[") || (parent && parent->originalName() == "->")) && getIndirect(tok) <= 0)
return Action::Read;

Action w = isWritable(tok, d);
if (w != Action::None)
return w;
{
Action w = isWritable(tok, d);
if (w != Action::None)
return w;
}

// Check for modifications by function calls
return isModified(tok);
Expand Down Expand Up @@ -624,7 +626,7 @@ struct ValueFlowAnalyzer : Analyzer {
if (Token::Match(tok->astParent(), "%assign%") && astIsLHS(tok))
a |= Action::Invalid;
if (inconclusiveRef && a.isModified())
return Action::Inconclusive;
a = Action::Inconclusive;
return a;
}
if (la.isRead()) {
Expand All @@ -636,7 +638,7 @@ struct ValueFlowAnalyzer : Analyzer {
inconclusive |= inconclusiveRef;
Action a = isAliasModified(tok);
if (inconclusive && a.isModified())
return Action::Inconclusive;
a = Action::Inconclusive;
return a;
}
if (isSameSymbolicValue(ref))
Expand Down
14 changes: 8 additions & 6 deletions tools/dmake/dmake.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,14 @@ static void compilefiles(std::ostream &fout, const std::vector<std::string> &fil
static std::string getCppFiles(std::vector<std::string> &files, const std::string &path, bool recursive)
{
std::list<FileWithDetails> filelist;
const std::set<std::string> extra;
const std::vector<std::string> masks;
const PathMatch matcher(masks, Path::getCurrentPath());
std::string err = FileLister::addFiles(filelist, path, extra, recursive, matcher);
if (!err.empty())
return err;
{
const std::set<std::string> extra;
const std::vector<std::string> masks;
const PathMatch matcher(masks, Path::getCurrentPath());
std::string err = FileLister::addFiles(filelist, path, extra, recursive, matcher);
if (!err.empty())
return err;
}

// add *.cpp files to the "files" vector..
for (const auto& file : filelist) {
Expand Down