walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

New check: Writing overlapping data, detect undefined behavior

This commit is contained in:
Daniel Marjamäki 2021-06-12 21:16:52 +02:00
parent 3985e548c5
commit 6234b5438e
7 changed files with 190 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
cfg/cppcheck-cfg.rng
View File

@ -165,6 +165,20 @@
<empty/>
</element>
</optional>
<optional>
<element name="not-overlapping-data">
<optional>
<attribute name="ptr1-arg"><data type="positiveInteger"/></attribute>
</optional>
<optional>
<attribute name="ptr2-arg"><data type="positiveInteger"/></attribute>
</optional>
<optional>
<attribute name="size-arg"><data type="positiveInteger"/></attribute>
</optional>
<empty/>
</element>
</optional>
<optional>
<element name="warn">
<attribute name="severity">

1
cfg/std.cfg
View File

@ -3863,6 +3863,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
<returnValue type="void *"/>
<noreturn>false</noreturn>
<leak-ignore/>
<not-overlapping-data ptr1-arg="1" ptr2-arg="2" size-arg="3"/>
<arg nr="1" direction="out">
<not-null/>
<minsize type="argvalue" arg="3"/>

113
lib/checkother.cpp
View File

@ -3358,3 +3358,116 @@ void CheckOther::checkModuloOfOneError(const Token *tok)
{
reportError(tok, Severity::style, "moduloofone", "Modulo of one is always equal to zero");
}
//-----------------------------------------------------------------------------
// Overlapping write (undefined behavior)
//-----------------------------------------------------------------------------
static bool getBufAndOffset(const Token *expr, const Token **buf, MathLib::bigint *offset)
{
if (!expr)
return false;
const Token *bufToken, *offsetToken;
if (expr->isUnaryOp("&") && Token::simpleMatch(expr->astOperand1(), "[")) {
bufToken = expr->astOperand1()->astOperand1();
offsetToken = expr->astOperand1()->astOperand2();
} else if (Token::Match(expr, "+|-") && expr->isBinaryOp()) {
const bool pointer1 = (expr->astOperand1()->valueType() && expr->astOperand1()->valueType()->pointer > 0);
const bool pointer2 = (expr->astOperand2()->valueType() && expr->astOperand2()->valueType()->pointer > 0);
if (pointer1 && !pointer2) {
bufToken = expr->astOperand1();
offsetToken = expr->astOperand2();
} else if (!pointer1 && pointer2) {
bufToken = expr->astOperand2();
offsetToken = expr->astOperand1();
} else {
return false;
}
} else if (expr->valueType() && expr->valueType()->pointer > 0) {
*buf = expr;
*offset = 0;
return true;
} else {
return false;
}
if (!bufToken->valueType() || !bufToken->valueType()->pointer)
return false;
if (!offsetToken->hasKnownIntValue())
return false;
*buf = bufToken;
*offset = offsetToken->getKnownIntValue();
return true;
}
void CheckOther::checkOverlappingWrite()
{
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope *functionScope : symbolDatabase->functionScopes) {
for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
if (tok->isAssignmentOp()) {
// check if LHS is a union member..
const Token * const lhs = tok->astOperand1();
if (!Token::simpleMatch(lhs, ".") || !lhs->isBinaryOp())
continue;
const Variable * const lhsvar = lhs->astOperand1()->variable();
if (!lhsvar || !lhsvar->typeScope() || lhsvar->typeScope()->type != Scope::ScopeType::eUnion)
continue;
const Token* const lhsmember = lhs->astOperand2();
if (!lhsmember)
continue;
// Is other union member used in RHS?
const Token *errorToken = nullptr;
visitAstNodes(tok->astOperand2(), [lhsvar, lhsmember, &errorToken](const Token *rhs) {
if (!Token::simpleMatch(rhs, "."))
return ChildrenToVisit::op1_and_op2;
if (!rhs->isBinaryOp() || rhs->astOperand1()->variable() != lhsvar)
return ChildrenToVisit::none;
if (lhsmember->str() == rhs->astOperand2()->str())
return ChildrenToVisit::none;
errorToken = rhs->astOperand2();
return ChildrenToVisit::done;
});
if (errorToken)
overlappingWriteUnion(tok);
} else if (Token::Match(tok, "%name% (")) {
const Library::NonOverlappingData *nonOverlappingData = mSettings->library.getNonOverlappingData(tok);
if (!nonOverlappingData)
continue;
const std::vector<const Token *> args = getArguments(tok);
if (nonOverlappingData->ptr1Arg <= 0 || nonOverlappingData->ptr1Arg > args.size())
continue;
if (nonOverlappingData->ptr2Arg <= 0 || nonOverlappingData->ptr2Arg > args.size())
continue;
if (nonOverlappingData->sizeArg <= 0 || nonOverlappingData->sizeArg > args.size())
continue;
if (!args[nonOverlappingData->sizeArg-1]->hasKnownIntValue())
continue;
const Token *buf1, *buf2;
MathLib::bigint offset1, offset2;
if (!getBufAndOffset(args[nonOverlappingData->ptr1Arg-1], &buf1, &offset1))
continue;
if (!getBufAndOffset(args[nonOverlappingData->ptr2Arg-1], &buf2, &offset2))
continue;
ErrorPath errorPath;
const bool macro = true;
const bool pure = true;
const bool follow = true;
if (!isSameExpression(mTokenizer->isCPP(), macro, buf1, buf2, mSettings->library, pure, follow, &errorPath))
continue;
overlappingWriteFunction(tok);
}
}
}
}
void CheckOther::overlappingWriteUnion(const Token *tok)
{
reportError(tok, Severity::error, "overlappingWriteUnion", "Overlapping read/write of union is undefined behavior");
}
void CheckOther::overlappingWriteFunction(const Token *tok)
{
const std::string funcname = tok ? tok->str() : "";
reportError(tok, Severity::error, "overlappingWriteFunction", "Overlapping read/write in " + funcname + "() is undefined behavior");
}

8
lib/checkother.h
View File

@ -102,6 +102,7 @@ public:
checkOther.checkMisusedScopedObject();
checkOther.checkAccessOfMovedVariable();
checkOther.checkModuloOfOne();
checkOther.checkOverlappingWrite();
}
/** Is expression a comparison that checks if a nonzero (unsigned/pointer) expression is less than zero? */
@ -226,6 +227,10 @@ public:
void checkModuloOfOne();
void checkOverlappingWrite();
void overlappingWriteUnion(const Token *tok);
void overlappingWriteFunction(const Token *tok);
private:
// Error messages..
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &functionName, const std::string &varName, const bool result);
@ -298,6 +303,8 @@ private:
c.checkPipeParameterSizeError(nullptr, "varname", "dimension");
c.raceAfterInterlockedDecrementError(nullptr);
c.invalidFreeError(nullptr, "malloc", false);
c.overlappingWriteUnion(nullptr);
c.overlappingWriteFunction(nullptr);
//performance
c.redundantCopyError(nullptr, "varname");
@ -376,6 +383,7 @@ private:
"- cast the return values of getc(),fgetc() and getchar() to character and compare it to EOF\n"
"- race condition with non-interlocked access after InterlockedDecrement() call\n"
"- expression 'x = x++;' depends on order of evaluation of side effects\n"
"- overlapping write of union\n"
// warning
"- either division by zero or useless condition\n"

16
lib/library.cpp
View File

@ -640,7 +640,13 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
func.isconst = true; // a constant function is pure
} else if (functionnodename == "leak-ignore")
func.leakignore = true;
else if (functionnodename == "use-retval") {
else if (functionnodename == "not-overlapping-data") {
NonOverlappingData nonOverlappingData;
nonOverlappingData.ptr1Arg = functionnode->IntAttribute("ptr1-arg", -1);
nonOverlappingData.ptr2Arg = functionnode->IntAttribute("ptr2-arg", -1);
nonOverlappingData.sizeArg = functionnode->IntAttribute("size-arg", -1);
mNonOverlappingData[name] = nonOverlappingData;
} else if (functionnodename == "use-retval") {
func.useretval = Library::UseRetValType::DEFAULT;
if (const char *type = functionnode->Attribute("type"))
if (std::strcmp(type, "error-code") == 0)
@ -1265,6 +1271,14 @@ bool Library::formatstr_secure(const Token* ftok) const
return functions.at(getFunctionName(ftok)).formatstr_secure;
}
const Library::NonOverlappingData* Library::getNonOverlappingData(const Token *ftok) const
{
if (isNotLibraryFunction(ftok))
return nullptr;
const std::unordered_map<std::string, NonOverlappingData>::const_iterator it = mNonOverlappingData.find(getFunctionName(ftok));
return (it != mNonOverlappingData.cend()) ? &it->second : nullptr;
}
Library::UseRetValType Library::getUseRetValType(const Token *ftok) const
{
if (isNotLibraryFunction(ftok))

8
lib/library.h
View File

@ -168,6 +168,13 @@ public:
bool formatstr_scan(const Token* ftok) const;
bool formatstr_secure(const Token* ftok) const;
struct NonOverlappingData {
int ptr1Arg;
int ptr2Arg;
int sizeArg;
};
const NonOverlappingData* getNonOverlappingData(const Token *ftok) const;
struct WarnInfo {
std::string message;
Standards standards;
@ -581,6 +588,7 @@ private:
std::map<std::string, PlatformType> mPlatformTypes; // platform independent typedefs
std::map<std::string, Platform> mPlatforms; // platform dependent typedefs
std::map<std::pair<std::string,std::string>, TypeCheck> mTypeChecks;
std::unordered_map<std::string, NonOverlappingData> mNonOverlappingData;
const ArgumentChecks * getarg(const Token *ftok, int argnr) const;

31
test/testother.cpp
View File

@ -252,6 +252,8 @@ private:
TEST_CASE(moduloOfOne);
TEST_CASE(sameExpressionPointers);
TEST_CASE(checkOverlappingWrite);
}
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, bool verbose=false, Settings* settings = nullptr) {
@ -9312,6 +9314,35 @@ private:
"}\n");
ASSERT_EQUALS("", errout.str());
}
void checkOverlappingWrite() {
// union
check("void foo() {\n"
" union { int i; float f; } u;\n"
" u.i = 0;\n"
" u.i = u.f;\n" // <- error
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Overlapping read/write of union is undefined behavior\n", errout.str());
// memcpy
check("void foo() {\n"
" char a[10];\n"
" memcpy(&a[5], &a[4], 2u);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Overlapping read/write in memcpy() is undefined behavior\n", errout.str());
check("void foo() {\n"
" char a[10];\n"
" memcpy(a+5, a+4, 2u);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Overlapping read/write in memcpy() is undefined behavior\n", errout.str());
check("void foo() {\n"
" char a[10];\n"
" memcpy(a, a+1, 2u);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Overlapping read/write in memcpy() is undefined behavior\n", errout.str());
}
};
REGISTER_TEST(TestOther)