Skip to content

Commit 9cb299b

Browse files
danmardanmar
authored
Fix #13521 (Duplicate warnings nullPointerOutOfMemory and ctunullpointer) (#7289)
1 parent fd3e458 commit 9cb299b

File tree

8 files changed

+144
-35
lines changed

8 files changed

+144
-35
lines changed

‎lib/checkbufferoverrun.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -914,7 +914,7 @@ namespace
914914
};
915915
}
916916

917-
bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Settings &settings, const Token *argtok, MathLib::bigint *offset, int type)
917+
bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset, int type)
918918
{
919919
if (!offset)
920920
return false;
@@ -931,16 +931,16 @@ bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Settings &settings, const
931931
return false;
932932
if (!indexTok->hasKnownIntValue())
933933
return false;
934-
*offset = indexTok->getKnownIntValue() * argtok->valueType()->typeSize(settings.platform);
934+
offset->value = indexTok->getKnownIntValue() * argtok->valueType()->typeSize(settings.platform);
935935
return true;
936936
}
937937

938-
bool CheckBufferOverrun::isCtuUnsafeArrayIndex(const Settings &settings, const Token *argtok, MathLib::bigint *offset)
938+
bool CheckBufferOverrun::isCtuUnsafeArrayIndex(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset)
939939
{
940940
return isCtuUnsafeBufferUsage(settings, argtok, offset, 1);
941941
}
942942

943-
bool CheckBufferOverrun::isCtuUnsafePointerArith(const Settings &settings, const Token *argtok, MathLib::bigint *offset)
943+
bool CheckBufferOverrun::isCtuUnsafePointerArith(const Settings &settings, const Token *argtok, CTU::FileInfo::Value* offset)
944944
{
945945
return isCtuUnsafeBufferUsage(settings, argtok, offset, 2);
946946
}
@@ -1031,13 +1031,13 @@ bool CheckBufferOverrun::analyseWholeProgram1(const std::map<std::string, std::l
10311031
if (type == 1) {
10321032
errorId = "ctuArrayIndex";
10331033
if (unsafeUsage.value > 0)
1034-
errmsg = "Array index out of bounds; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue) + " and it is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".";
1034+
errmsg = "Array index out of bounds; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue.value) + " and it is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".";
10351035
else
10361036
errmsg = "Array index out of bounds; buffer '" + unsafeUsage.myArgumentName + "' is accessed at offset " + MathLib::toString(unsafeUsage.value) + ".";
10371037
cwe = (unsafeUsage.value > 0) ? CWE_BUFFER_OVERRUN : CWE_BUFFER_UNDERRUN;
10381038
} else {
10391039
errorId = "ctuPointerArith";
1040-
errmsg = "Pointer arithmetic overflow; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue);
1040+
errmsg = "Pointer arithmetic overflow; '" + unsafeUsage.myArgumentName + "' buffer size is " + MathLib::toString(functionCall->callArgValue.value);
10411041
cwe = CWE_POINTER_ARITHMETIC_OVERFLOW;
10421042
}
10431043

‎lib/checkbufferoverrun.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,9 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
107107
ValueFlow::Value getBufferSize(const Token *bufTok) const;
108108

109109
// CTU
110-
static bool isCtuUnsafeBufferUsage(const Settings &settings, const Token *argtok, MathLib::bigint *offset, int type);
111-
static bool isCtuUnsafeArrayIndex(const Settings &settings, const Token *argtok, MathLib::bigint *offset);
112-
static bool isCtuUnsafePointerArith(const Settings &settings, const Token *argtok, MathLib::bigint *offset);
110+
static bool isCtuUnsafeBufferUsage(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset, int type);
111+
static bool isCtuUnsafeArrayIndex(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset);
112+
static bool isCtuUnsafePointerArith(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset);
113113

114114
Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
115115
static bool analyseWholeProgram1(const std::map<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage,

‎lib/checknullpointer.cpp‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ void CheckNullPointer::redundantConditionWarning(const Token* tok, const ValueFl
583583
}
584584

585585
// NOLINTNEXTLINE(readability-non-const-parameter) - used as callback so we need to preserve the signature
586-
static bool isUnsafeUsage(const Settings &settings, const Token *vartok, MathLib::bigint *value)
586+
static bool isUnsafeUsage(const Settings &settings, const Token *vartok, CTU::FileInfo::Value *value)
587587
{
588588
(void)value;
589589
bool unknown = false;
@@ -659,22 +659,34 @@ bool CheckNullPointer::analyseWholeProgram(const CTU::FileInfo &ctu, const std::
659659
if (warning == 1 && !settings.severity.isEnabled(Severity::warning))
660660
break;
661661

662+
ValueFlow::Value::UnknownFunctionReturn unknownFunctionReturn = ValueFlow::Value::UnknownFunctionReturn::no;
662663
const std::list<ErrorMessage::FileLocation> &locationList =
663664
CTU::FileInfo::getErrorPath(CTU::FileInfo::InvalidValueType::null,
664665
unsafeUsage,
665666
callsMap,
666667
"Dereferencing argument ARG that is null",
667668
nullptr,
668669
warning,
669-
settings.maxCtuDepth);
670+
settings.maxCtuDepth,
671+
&unknownFunctionReturn);
670672
if (locationList.empty())
671673
continue;
672674

675+
std::string id = "ctunullpointer";
676+
std::string message = "Null pointer dereference: " + unsafeUsage.myArgumentName;
677+
if (unknownFunctionReturn == ValueFlow::Value::UnknownFunctionReturn::outOfMemory) {
678+
id += "OutOfMemory";
679+
message = "If memory allocation fails, then there is a possible null pointer dereference: " + unsafeUsage.myArgumentName;
680+
} else if (unknownFunctionReturn == ValueFlow::Value::UnknownFunctionReturn::outOfResources) {
681+
id += "OutOfResources";
682+
message = "If resource allocation fails, then there is a possible null pointer dereference: " + unsafeUsage.myArgumentName;
683+
}
684+
673685
const ErrorMessage errmsg(locationList,
674686
fi->file0,
675687
warning ? Severity::warning : Severity::error,
676-
"Null pointer dereference: " + unsafeUsage.myArgumentName,
677-
"ctunullpointer",
688+
message,
689+
id,
678690
CWE_NULL_POINTER_DEREFERENCE, Certainty::normal);
679691
errorLogger.reportErr(errmsg);
680692

‎lib/checkuninitvar.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,6 +1722,10 @@ namespace
17221722
};
17231723
}
17241724

1725+
static bool isVariableUsage(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *value) {
1726+
return isVariableUsage(settings, argtok, &value->value);
1727+
}
1728+
17251729
Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const
17261730
{
17271731
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, ::isVariableUsage);

‎lib/ctu.cpp‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ static constexpr char ATTR_CALL_ARGNR[] = "call-argnr";
4545
static constexpr char ATTR_CALL_ARGEXPR[] = "call-argexpr";
4646
static constexpr char ATTR_CALL_ARGVALUETYPE[] = "call-argvaluetype";
4747
static constexpr char ATTR_CALL_ARGVALUE[] = "call-argvalue";
48+
static constexpr char ATTR_CALL_UNKNOWN_FUNCTION_RETURN[] = "call-argvalue-ufr";
4849
static constexpr char ATTR_WARNING[] = "warning";
4950
static constexpr char ATTR_LOC_FILENAME[] = "file";
5051
static constexpr char ATTR_LOC_LINENR[] = "line";
@@ -102,7 +103,8 @@ std::string CTU::FileInfo::FunctionCall::toXmlString() const
102103
<< toBaseXmlString()
103104
<< " " << ATTR_CALL_ARGEXPR << "=\"" << ErrorLogger::toxml(callArgumentExpression) << "\""
104105
<< " " << ATTR_CALL_ARGVALUETYPE << "=\"" << static_cast<int>(callValueType) << "\""
105-
<< " " << ATTR_CALL_ARGVALUE << "=\"" << callArgValue << "\"";
106+
<< " " << ATTR_CALL_ARGVALUE << "=\"" << callArgValue.value << "\""
107+
<< " " << ATTR_CALL_UNKNOWN_FUNCTION_RETURN << "=\"" << (int)callArgValue.unknownFunctionReturn << "\"";
106108
if (warning)
107109
out << " " << ATTR_WARNING << "=\"true\"";
108110
if (callValuePath.empty())
@@ -201,7 +203,9 @@ bool CTU::FileInfo::FunctionCall::loadFromXml(const tinyxml2::XMLElement *xmlEle
201203
bool error=false;
202204
callArgumentExpression = readAttrString(xmlElement, ATTR_CALL_ARGEXPR, &error);
203205
callValueType = (ValueFlow::Value::ValueType)readAttrInt(xmlElement, ATTR_CALL_ARGVALUETYPE, &error);
204-
callArgValue = readAttrInt(xmlElement, ATTR_CALL_ARGVALUE, &error);
206+
callArgValue.value = readAttrInt(xmlElement, ATTR_CALL_ARGVALUE, &error);
207+
const auto ufr = readAttrInt(xmlElement, ATTR_CALL_UNKNOWN_FUNCTION_RETURN, &error);
208+
callArgValue.unknownFunctionReturn = (ValueFlow::Value::UnknownFunctionReturn)(ufr>=0 && ufr <=0xff ? ufr : 0xff);
205209
const char *w = xmlElement->Attribute(ATTR_WARNING);
206210
warning = w && std::strcmp(w, "true") == 0;
207211
for (const tinyxml2::XMLElement *e2 = xmlElement->FirstChildElement(); !error && e2; e2 = e2->NextSiblingElement()) {
@@ -342,7 +346,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer &tokenizer)
342346
functionCall.location = FileInfo::Location(tokenizer,tok);
343347
functionCall.callArgNr = argnr + 1;
344348
functionCall.callArgumentExpression = argtok->expressionString();
345-
functionCall.callArgValue = value.intvalue;
349+
functionCall.callArgValue = value;
346350
functionCall.warning = !value.errorSeverity();
347351
for (const ErrorPathItem &i : value.errorPath) {
348352
const std::string& file = tokenizer.list.file(i.first);
@@ -364,7 +368,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer &tokenizer)
364368
functionCall.callArgNr = argnr + 1;
365369
functionCall.callArgumentExpression = argtok->expressionString();
366370
const auto typeSize = argtok->valueType()->typeSize(tokenizer.getSettings().platform);
367-
functionCall.callArgValue = typeSize > 0 ? argtok->variable()->dimension(0) * typeSize : -1;
371+
functionCall.callArgValue.value = typeSize > 0 ? argtok->variable()->dimension(0) * typeSize : -1;
368372
functionCall.warning = false;
369373
fileInfo->functionCalls.push_back(std::move(functionCall));
370374
}
@@ -377,7 +381,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer &tokenizer)
377381
functionCall.location = FileInfo::Location(tokenizer, tok);
378382
functionCall.callArgNr = argnr + 1;
379383
functionCall.callArgumentExpression = argtok->expressionString();
380-
functionCall.callArgValue = argtok->astOperand1()->valueType()->typeSize(tokenizer.getSettings().platform);
384+
functionCall.callArgValue.value = argtok->astOperand1()->valueType()->typeSize(tokenizer.getSettings().platform);
381385
functionCall.warning = false;
382386
fileInfo->functionCalls.push_back(std::move(functionCall));
383387
}
@@ -411,7 +415,8 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer &tokenizer)
411415
functionCall.callFunctionName = tok->astOperand1()->expressionString();
412416
functionCall.location = FileInfo::Location(tokenizer, tok);
413417
functionCall.callArgNr = argnr + 1;
414-
functionCall.callArgValue = 0;
418+
functionCall.callArgValue = v;
419+
functionCall.callArgValue.value = 0;
415420
functionCall.callArgumentExpression = argtok->expressionString();
416421
functionCall.warning = false;
417422
fileInfo->functionCalls.push_back(std::move(functionCall));
@@ -436,9 +441,9 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer &tokenizer)
436441
return fileInfo;
437442
}
438443

439-
static std::list<std::pair<const Token *, MathLib::bigint>> getUnsafeFunction(const Settings &settings, const Scope *scope, int argnr, bool (*isUnsafeUsage)(const Settings &settings, const Token *argtok, MathLib::bigint *value))
444+
static std::vector<std::pair<const Token *, CTU::FileInfo::Value>> getUnsafeFunction(const Settings &settings, const Scope *scope, int argnr, bool (*isUnsafeUsage)(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *value))
440445
{
441-
std::list<std::pair<const Token *, MathLib::bigint>> ret;
446+
std::vector<std::pair<const Token *, CTU::FileInfo::Value>> ret;
442447
const Variable * const argvar = scope->function->getArgumentVar(argnr);
443448
if (!argvar->isArrayOrPointer() && !argvar->isReference())
444449
return ret;
@@ -459,7 +464,7 @@ static std::list<std::pair<const Token *, MathLib::bigint>> getUnsafeFunction(co
459464
}
460465
if (tok2->variable() != argvar)
461466
continue;
462-
MathLib::bigint value = 0;
467+
CTU::FileInfo::Value value;
463468
if (!isUnsafeUsage(settings, tok2, &value))
464469
return ret; // TODO: Is this a read? then continue..
465470
ret.emplace_back(tok2, value);
@@ -468,7 +473,7 @@ static std::list<std::pair<const Token *, MathLib::bigint>> getUnsafeFunction(co
468473
return ret;
469474
}
470475

471-
std::list<CTU::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer &tokenizer, const Settings &settings, bool (*isUnsafeUsage)(const Settings &settings, const Token *argtok, MathLib::bigint *value))
476+
std::list<CTU::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer &tokenizer, const Settings &settings, bool (*isUnsafeUsage)(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *value))
472477
{
473478
std::list<CTU::FileInfo::UnsafeUsage> unsafeUsage;
474479

@@ -482,9 +487,9 @@ std::list<CTU::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer &token
482487

483488
// "Unsafe" functions unconditionally reads data before it is written..
484489
for (int argnr = 0; argnr < function->argCount(); ++argnr) {
485-
for (const std::pair<const Token *, MathLib::bigint> &v : getUnsafeFunction(settings, &scope, argnr, isUnsafeUsage)) {
490+
for (const std::pair<const Token *, CTU::FileInfo::Value> &v : getUnsafeFunction(settings, &scope, argnr, isUnsafeUsage)) {
486491
const Token *tok = v.first;
487-
const MathLib::bigint val = v.second;
492+
const MathLib::bigint val = v.second.value;
488493
unsafeUsage.emplace_back(CTU::getFunctionId(tokenizer, function), argnr+1, tok->str(), CTU::FileInfo::Location(tokenizer,tok), val);
489494
}
490495
}
@@ -518,9 +523,11 @@ static bool findPath(const std::string &callId,
518523
if (functionCall) {
519524
if (!warning && functionCall->warning)
520525
continue;
526+
if (!warning && functionCall->callArgValue.unknownFunctionReturn != ValueFlow::Value::UnknownFunctionReturn::no)
527+
continue;
521528
switch (invalidValue) {
522529
case CTU::FileInfo::InvalidValueType::null:
523-
if (functionCall->callValueType != ValueFlow::Value::ValueType::INT || functionCall->callArgValue != 0)
530+
if (functionCall->callValueType != ValueFlow::Value::ValueType::INT || functionCall->callArgValue.value != 0)
524531
continue;
525532
break;
526533
case CTU::FileInfo::InvalidValueType::uninit:
@@ -530,7 +537,7 @@ static bool findPath(const std::string &callId,
530537
case CTU::FileInfo::InvalidValueType::bufferOverflow:
531538
if (functionCall->callValueType != ValueFlow::Value::ValueType::BUFFER_SIZE)
532539
continue;
533-
if (unsafeValue < 0 || (unsafeValue >= functionCall->callArgValue && functionCall->callArgValue >= 0))
540+
if (unsafeValue < 0 || (unsafeValue >= functionCall->callArgValue.value && functionCall->callArgValue.value >= 0))
534541
break;
535542
continue;
536543
}
@@ -557,14 +564,21 @@ std::list<ErrorMessage::FileLocation> CTU::FileInfo::getErrorPath(InvalidValueTy
557564
const char info[],
558565
const FunctionCall ** const functionCallPtr,
559566
bool warning,
560-
int maxCtuDepth)
567+
int maxCtuDepth,
568+
ValueFlow::Value::UnknownFunctionReturn *unknownFunctionReturn)
561569
{
562-
std::list<ErrorMessage::FileLocation> locationList;
563-
564570
const CTU::FileInfo::CallBase *path[10] = {nullptr};
565571

566572
if (!findPath(unsafeUsage.myId, unsafeUsage.myArgNr, unsafeUsage.value, invalidValue, callsMap, path, 0, warning, maxCtuDepth))
567-
return locationList;
573+
return {};
574+
575+
if (unknownFunctionReturn && path[0]) {
576+
const auto* functionCall = dynamic_cast<const CTU::FileInfo::FunctionCall *>(path[0]);
577+
if (functionCall)
578+
*unknownFunctionReturn = functionCall->callArgValue.unknownFunctionReturn;
579+
}
580+
581+
std::list<ErrorMessage::FileLocation> locationList;
568582

569583
const std::string value1 = (invalidValue == InvalidValueType::null) ? "null" : "uninitialized";
570584

0 commit comments

Comments
 (0)