Fix FP truncLongCastReturn on Windows (#5262)

This commit is contained in:
chrchr-github 2023-08-02 12:27:29 +02:00 committed by GitHub
parent de0fdc85a2
commit faf8047050
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 91 additions and 46 deletions

View File

@ -289,6 +289,26 @@ void CheckType::signConversionError(const Token *tok, const ValueFlow::Value *ne
//---------------------------------------------------------------------------
// Checking for long cast of int result const long x = var1 * var2;
//---------------------------------------------------------------------------
static bool checkTypeCombination(const ValueType& src, const ValueType& tgt, const Settings* settings)
{
static const std::pair<ValueType::Type, ValueType::Type> typeCombinations[] = {
{ ValueType::Type::INT, ValueType::Type::LONG },
{ ValueType::Type::INT, ValueType::Type::LONGLONG },
{ ValueType::Type::LONG, ValueType::Type::LONGLONG },
{ ValueType::Type::FLOAT, ValueType::Type::DOUBLE },
{ ValueType::Type::FLOAT, ValueType::Type::LONGDOUBLE },
{ ValueType::Type::DOUBLE, ValueType::Type::LONGDOUBLE },
};
const std::size_t sizeSrc = ValueFlow::getSizeOf(src, settings);
const std::size_t sizeTgt = ValueFlow::getSizeOf(tgt, settings);
if (!(sizeSrc > 0 && sizeTgt > 0 && sizeSrc < sizeTgt))
return false;
return std::any_of(std::begin(typeCombinations), std::end(typeCombinations), [&](const std::pair<ValueType::Type, ValueType::Type>& p) {
return src.type == p.first && tgt.type == p.second;
});
}
void CheckType::checkLongCast()
{
@ -311,16 +331,15 @@ void CheckType::checkLongCast()
if (!lhstype || !rhstype)
continue;
if (!checkTypeCombination(*rhstype, *lhstype, mSettings))
continue;
// assign int result to long/longlong const nonpointer?
if (rhstype->type == ValueType::Type::INT &&
rhstype->pointer == 0U &&
if (rhstype->pointer == 0U &&
rhstype->originalTypeName.empty() &&
(lhstype->type == ValueType::Type::LONG || lhstype->type == ValueType::Type::LONGLONG) &&
lhstype->pointer == 0U &&
lhstype->constness == 1U &&
lhstype->originalTypeName.empty())
longCastAssignError(tok);
longCastAssignError(tok, rhstype, lhstype);
}
// Return..
@ -329,16 +348,9 @@ void CheckType::checkLongCast()
// function must return long data
const Token * def = scope->classDef;
bool islong = false;
while (Token::Match(def, "%type%|::")) {
if (def->str() == "long" && def->originalName().empty()) {
islong = true;
break;
}
def = def->previous();
}
if (!islong)
if (!Token::Match(def, "%name% (") || !def->next()->valueType())
continue;
const ValueType* retVt = def->next()->valueType();
// return statements
const Token *ret = nullptr;
@ -346,7 +358,9 @@ void CheckType::checkLongCast()
if (tok->str() == "return") {
if (Token::Match(tok->astOperand1(), "<<|*")) {
const ValueType *type = tok->astOperand1()->valueType();
if (type && type->type == ValueType::Type::INT && type->pointer == 0U && type->originalTypeName.empty())
if (type && checkTypeCombination(*type, *retVt, mSettings) &&
type->pointer == 0U &&
type->originalTypeName.empty())
ret = tok;
}
// All return statements must have problem otherwise no warning
@ -358,26 +372,41 @@ void CheckType::checkLongCast()
}
if (ret)
longCastReturnError(ret);
longCastReturnError(ret, ret->astOperand1()->valueType(), retVt);
}
}
void CheckType::longCastAssignError(const Token *tok)
static void makeBaseTypeString(std::string& typeStr)
{
const auto pos = typeStr.find("signed");
if (pos != std::string::npos)
typeStr.erase(typeStr.begin(), typeStr.begin() + pos + 6 + 1);
}
void CheckType::longCastAssignError(const Token *tok, const ValueType* src, const ValueType* tgt)
{
std::string srcStr = src ? src->str() : "int";
makeBaseTypeString(srcStr);
std::string tgtStr = tgt ? tgt->str() : "long";
makeBaseTypeString(tgtStr);
reportError(tok,
Severity::style,
"truncLongCastAssignment",
"int result is assigned to long variable. If the variable is long to avoid loss of information, then you have loss of information.\n"
"int result is assigned to long variable. If the variable is long to avoid loss of information, then there is loss of information. To avoid loss of information you must cast a calculation operand to long, for example 'l = a * b;' => 'l = (long)a * b;'.", CWE197, Certainty::normal);
srcStr + " result is assigned to " + tgtStr + " variable. If the variable is " + tgtStr + " to avoid loss of information, then you have loss of information.\n" +
srcStr + " result is assigned to " + tgtStr + " variable. If the variable is " + tgtStr + " to avoid loss of information, then there is loss of information. To avoid loss of information you must cast a calculation operand to " + tgtStr + ", for example 'l = a * b;' => 'l = (" + tgtStr + ")a * b;'.", CWE197, Certainty::normal);
}
void CheckType::longCastReturnError(const Token *tok)
void CheckType::longCastReturnError(const Token *tok, const ValueType* src, const ValueType* tgt)
{
std::string srcStr = src ? src->str() : "int";
makeBaseTypeString(srcStr);
std::string tgtStr = tgt ? tgt->str() : "long";
makeBaseTypeString(tgtStr);
reportError(tok,
Severity::style,
"truncLongCastReturn",
"int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information.\n"
"int result is returned as long value. If the return value is long to avoid loss of information, then there is loss of information. To avoid loss of information you must cast a calculation operand to long, for example 'return a*b;' => 'return (long)a*b'.", CWE197, Certainty::normal);
srcStr +" result is returned as " + tgtStr + " value. If the return value is " + tgtStr + " to avoid loss of information, then you have loss of information.\n" +
srcStr +" result is returned as " + tgtStr + " value. If the return value is " + tgtStr + " to avoid loss of information, then there is loss of information. To avoid loss of information you must cast a calculation operand to long, for example 'return a*b;' => 'return (long)a*b'.", CWE197, Certainty::normal);
}
//---------------------------------------------------------------------------

View File

@ -84,8 +84,8 @@ private:
void tooBigSignedBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits);
void integerOverflowError(const Token *tok, const ValueFlow::Value &value);
void signConversionError(const Token *tok, const ValueFlow::Value *negativeValue, const bool constvalue);
void longCastAssignError(const Token *tok);
void longCastReturnError(const Token *tok);
void longCastAssignError(const Token *tok, const ValueType* src = nullptr, const ValueType* tgt = nullptr);
void longCastReturnError(const Token *tok, const ValueType* src = nullptr, const ValueType* tgt = nullptr);
void floatToIntegerOverflowError(const Token *tok, const ValueFlow::Value &value);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override {

View File

@ -3428,8 +3428,10 @@ void Tokenizer::fillTypeSizes()
mTypeSize["short"] = mSettings->platform.sizeof_short;
mTypeSize["int"] = mSettings->platform.sizeof_int;
mTypeSize["long"] = mSettings->platform.sizeof_long;
mTypeSize["long long"] = mSettings->platform.sizeof_long_long;
mTypeSize["float"] = mSettings->platform.sizeof_float;
mTypeSize["double"] = mSettings->platform.sizeof_double;
mTypeSize["long double"] = mSettings->platform.sizeof_long_double;
mTypeSize["wchar_t"] = mSettings->platform.sizeof_wchar_t;
mTypeSize["size_t"] = mSettings->platform.sizeof_size_t;
mTypeSize["*"] = mSettings->platform.sizeof_pointer;

View File

@ -1091,29 +1091,15 @@ static void setTokenValueCast(Token *parent, const ValueType &valueType, const V
static nonneg int getSizeOfType(const Token *typeTok, const Settings *settings)
{
const ValueType &valueType = ValueType::parseDecl(typeTok, *settings, true); // TODO: set isCpp
if (valueType.pointer > 0)
return settings->platform.sizeof_pointer;
if (valueType.type == ValueType::Type::BOOL || valueType.type == ValueType::Type::CHAR)
return 1;
if (valueType.type == ValueType::Type::SHORT)
return settings->platform.sizeof_short;
if (valueType.type == ValueType::Type::INT)
return settings->platform.sizeof_int;
if (valueType.type == ValueType::Type::LONG)
return settings->platform.sizeof_long;
if (valueType.type == ValueType::Type::LONGLONG)
return settings->platform.sizeof_long_long;
if (valueType.type == ValueType::Type::WCHAR_T)
return settings->platform.sizeof_wchar_t;
return 0;
return ValueFlow::getSizeOf(valueType, settings);
}
size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings *settings)
{
if (vt.pointer)
return settings->platform.sizeof_pointer;
if (vt.type == ValueType::Type::CHAR)
if (vt.type == ValueType::Type::BOOL || vt.type == ValueType::Type::CHAR)
return 1;
if (vt.type == ValueType::Type::SHORT)
return settings->platform.sizeof_short;

View File

@ -325,9 +325,19 @@ private:
void longCastAssign() {
const Settings settings = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Unix64).build();
const Settings settingsWin = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Win64).build();
const char code[] = "long f(int x, int y) {\n"
" const long ret = x * y;\n"
" return ret;\n"
"}\n";
check(code, settings);
ASSERT_EQUALS("[test.cpp:2]: (style) int result is assigned to long variable. If the variable is long to avoid loss of information, then you have loss of information.\n", errout.str());
check(code, settingsWin);
ASSERT_EQUALS("", errout.str());
check("long f(int x, int y) {\n"
" const long ret = x * y;\n"
" long ret = x * y;\n"
" return ret;\n"
"}\n", settings);
ASSERT_EQUALS("[test.cpp:2]: (style) int result is assigned to long variable. If the variable is long to avoid loss of information, then you have loss of information.\n", errout.str());
@ -351,21 +361,39 @@ private:
" return ret;\n"
"}\n", settings);
ASSERT_EQUALS("", errout.str());
check("double g(float f) {\n"
" return f * f;\n"
"}\n", settings);
ASSERT_EQUALS("[test.cpp:2]: (style) float result is returned as double value. If the return value is double to avoid loss of information, then you have loss of information.\n",
errout.str());
}
void longCastReturn() {
const Settings settings = settingsBuilder().severity(Severity::style).build();
const Settings settings = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Unix64).build();
const Settings settingsWin = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Win64).build();
check("long f(int x, int y) {\n"
" return x * y;\n"
"}\n", settings);
const char code[] = "long f(int x, int y) {\n"
" return x * y;\n"
"}\n";
check(code, settings);
ASSERT_EQUALS("[test.cpp:2]: (style) int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information.\n", errout.str());
check(code, settingsWin);
ASSERT_EQUALS("", errout.str());
const char code2[] = "long long f(int x, int y) {\n"
" return x * y;\n"
"}\n";
check(code2, settings);
ASSERT_EQUALS("[test.cpp:2]: (style) int result is returned as long long value. If the return value is long long to avoid loss of information, then you have loss of information.\n", errout.str());
check(code2, settingsWin);
ASSERT_EQUALS("[test.cpp:2]: (style) int result is returned as long long value. If the return value is long long to avoid loss of information, then you have loss of information.\n", errout.str());
// typedef
check("size_t f(int x, int y) {\n"
" return x * y;\n"
"}\n", settings);
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information.\n", errout.str());
}
// This function ensure that test works with different compilers. Floats can