Skip to content

Commit faf8047

Browse files
chrchr-githubchrchr-github
authored
Fix FP truncLongCastReturn on Windows (#5262)
1 parent de0fdc8 commit faf8047

File tree

5 files changed

+91
-46
lines changed

5 files changed

+91
-46
lines changed

‎lib/checktype.cpp‎

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,26 @@ void CheckType::signConversionError(const Token *tok, const ValueFlow::Value *ne
289289
//---------------------------------------------------------------------------
290290
// Checking for long cast of int result const long x = var1 * var2;
291291
//---------------------------------------------------------------------------
292+
static bool checkTypeCombination(const ValueType& src, const ValueType& tgt, const Settings* settings)
293+
{
294+
static const std::pair<ValueType::Type, ValueType::Type> typeCombinations[] = {
295+
{ ValueType::Type::INT, ValueType::Type::LONG },
296+
{ ValueType::Type::INT, ValueType::Type::LONGLONG },
297+
{ ValueType::Type::LONG, ValueType::Type::LONGLONG },
298+
{ ValueType::Type::FLOAT, ValueType::Type::DOUBLE },
299+
{ ValueType::Type::FLOAT, ValueType::Type::LONGDOUBLE },
300+
{ ValueType::Type::DOUBLE, ValueType::Type::LONGDOUBLE },
301+
};
302+
303+
const std::size_t sizeSrc = ValueFlow::getSizeOf(src, settings);
304+
const std::size_t sizeTgt = ValueFlow::getSizeOf(tgt, settings);
305+
if (!(sizeSrc > 0 && sizeTgt > 0 && sizeSrc < sizeTgt))
306+
return false;
307+
308+
return std::any_of(std::begin(typeCombinations), std::end(typeCombinations), [&](const std::pair<ValueType::Type, ValueType::Type>& p) {
309+
return src.type == p.first && tgt.type == p.second;
310+
});
311+
}
292312

293313
void CheckType::checkLongCast()
294314
{
@@ -311,16 +331,15 @@ void CheckType::checkLongCast()
311331

312332
if (!lhstype || !rhstype)
313333
continue;
334+
if (!checkTypeCombination(*rhstype, *lhstype, mSettings))
335+
continue;
314336

315337
// assign int result to long/longlong const nonpointer?
316-
if (rhstype->type == ValueType::Type::INT &&
317-
rhstype->pointer == 0U &&
338+
if (rhstype->pointer == 0U &&
318339
rhstype->originalTypeName.empty() &&
319-
(lhstype->type == ValueType::Type::LONG || lhstype->type == ValueType::Type::LONGLONG) &&
320340
lhstype->pointer == 0U &&
321-
lhstype->constness == 1U &&
322341
lhstype->originalTypeName.empty())
323-
longCastAssignError(tok);
342+
longCastAssignError(tok, rhstype, lhstype);
324343
}
325344

326345
// Return..
@@ -329,24 +348,19 @@ void CheckType::checkLongCast()
329348

330349
// function must return long data
331350
const Token * def = scope->classDef;
332-
bool islong = false;
333-
while (Token::Match(def, "%type%|::")) {
334-
if (def->str() == "long" && def->originalName().empty()) {
335-
islong = true;
336-
break;
337-
}
338-
def = def->previous();
339-
}
340-
if (!islong)
351+
if (!Token::Match(def, "%name% (") || !def->next()->valueType())
341352
continue;
353+
const ValueType* retVt = def->next()->valueType();
342354

343355
// return statements
344356
const Token *ret = nullptr;
345357
for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
346358
if (tok->str() == "return") {
347359
if (Token::Match(tok->astOperand1(), "<<|*")) {
348360
const ValueType *type = tok->astOperand1()->valueType();
349-
if (type && type->type == ValueType::Type::INT && type->pointer == 0U && type->originalTypeName.empty())
361+
if (type && checkTypeCombination(*type, *retVt, mSettings) &&
362+
type->pointer == 0U &&
363+
type->originalTypeName.empty())
350364
ret = tok;
351365
}
352366
// All return statements must have problem otherwise no warning
@@ -358,26 +372,41 @@ void CheckType::checkLongCast()
358372
}
359373

360374
if (ret)
361-
longCastReturnError(ret);
375+
longCastReturnError(ret, ret->astOperand1()->valueType(), retVt);
362376
}
363377
}
364378

365-
void CheckType::longCastAssignError(const Token *tok)
379+
static void makeBaseTypeString(std::string& typeStr)
380+
{
381+
const auto pos = typeStr.find("signed");
382+
if (pos != std::string::npos)
383+
typeStr.erase(typeStr.begin(), typeStr.begin() + pos + 6 + 1);
384+
}
385+
386+
void CheckType::longCastAssignError(const Token *tok, const ValueType* src, const ValueType* tgt)
366387
{
388+
std::string srcStr = src ? src->str() : "int";
389+
makeBaseTypeString(srcStr);
390+
std::string tgtStr = tgt ? tgt->str() : "long";
391+
makeBaseTypeString(tgtStr);
367392
reportError(tok,
368393
Severity::style,
369394
"truncLongCastAssignment",
370-
"int result is assigned to long variable. If the variable is long to avoid loss of information, then you have loss of information.\n"
371-
"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);
395+
srcStr + " result is assigned to " + tgtStr + " variable. If the variable is " + tgtStr + " to avoid loss of information, then you have loss of information.\n" +
396+
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);
372397
}
373398

374-
void CheckType::longCastReturnError(const Token *tok)
399+
void CheckType::longCastReturnError(const Token *tok, const ValueType* src, const ValueType* tgt)
375400
{
401+
std::string srcStr = src ? src->str() : "int";
402+
makeBaseTypeString(srcStr);
403+
std::string tgtStr = tgt ? tgt->str() : "long";
404+
makeBaseTypeString(tgtStr);
376405
reportError(tok,
377406
Severity::style,
378407
"truncLongCastReturn",
379-
"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"
380-
"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);
408+
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" +
409+
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);
381410
}
382411

383412
//---------------------------------------------------------------------------

‎lib/checktype.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ class CPPCHECKLIB CheckType : public Check {
8484
void tooBigSignedBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits);
8585
void integerOverflowError(const Token *tok, const ValueFlow::Value &value);
8686
void signConversionError(const Token *tok, const ValueFlow::Value *negativeValue, const bool constvalue);
87-
void longCastAssignError(const Token *tok);
88-
void longCastReturnError(const Token *tok);
87+
void longCastAssignError(const Token *tok, const ValueType* src = nullptr, const ValueType* tgt = nullptr);
88+
void longCastReturnError(const Token *tok, const ValueType* src = nullptr, const ValueType* tgt = nullptr);
8989
void floatToIntegerOverflowError(const Token *tok, const ValueFlow::Value &value);
9090

9191
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override {

‎lib/tokenize.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3428,8 +3428,10 @@ void Tokenizer::fillTypeSizes()
34283428
mTypeSize["short"] = mSettings->platform.sizeof_short;
34293429
mTypeSize["int"] = mSettings->platform.sizeof_int;
34303430
mTypeSize["long"] = mSettings->platform.sizeof_long;
3431+
mTypeSize["long long"] = mSettings->platform.sizeof_long_long;
34313432
mTypeSize["float"] = mSettings->platform.sizeof_float;
34323433
mTypeSize["double"] = mSettings->platform.sizeof_double;
3434+
mTypeSize["long double"] = mSettings->platform.sizeof_long_double;
34333435
mTypeSize["wchar_t"] = mSettings->platform.sizeof_wchar_t;
34343436
mTypeSize["size_t"] = mSettings->platform.sizeof_size_t;
34353437
mTypeSize["*"] = mSettings->platform.sizeof_pointer;

‎lib/valueflow.cpp‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,29 +1091,15 @@ static void setTokenValueCast(Token *parent, const ValueType &valueType, const V
10911091
static nonneg int getSizeOfType(const Token *typeTok, const Settings *settings)
10921092
{
10931093
const ValueType &valueType = ValueType::parseDecl(typeTok, *settings, true); // TODO: set isCpp
1094-
if (valueType.pointer > 0)
1095-
return settings->platform.sizeof_pointer;
1096-
if (valueType.type == ValueType::Type::BOOL || valueType.type == ValueType::Type::CHAR)
1097-
return 1;
1098-
if (valueType.type == ValueType::Type::SHORT)
1099-
return settings->platform.sizeof_short;
1100-
if (valueType.type == ValueType::Type::INT)
1101-
return settings->platform.sizeof_int;
1102-
if (valueType.type == ValueType::Type::LONG)
1103-
return settings->platform.sizeof_long;
1104-
if (valueType.type == ValueType::Type::LONGLONG)
1105-
return settings->platform.sizeof_long_long;
1106-
if (valueType.type == ValueType::Type::WCHAR_T)
1107-
return settings->platform.sizeof_wchar_t;
11081094

1109-
return 0;
1095+
return ValueFlow::getSizeOf(valueType, settings);
11101096
}
11111097

11121098
size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings *settings)
11131099
{
11141100
if (vt.pointer)
11151101
return settings->platform.sizeof_pointer;
1116-
if (vt.type == ValueType::Type::CHAR)
1102+
if (vt.type == ValueType::Type::BOOL || vt.type == ValueType::Type::CHAR)
11171103
return 1;
11181104
if (vt.type == ValueType::Type::SHORT)
11191105
return settings->platform.sizeof_short;

‎test/testtype.cpp‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,19 @@ class TestType : public TestFixture {
325325

326326
void longCastAssign() {
327327
const Settings settings = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Unix64).build();
328+
const Settings settingsWin = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Win64).build();
329+
330+
const char code[] = "long f(int x, int y) {\n"
331+
" const long ret = x * y;\n"
332+
" return ret;\n"
333+
"}\n";
334+
check(code, settings);
335+
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());
336+
check(code, settingsWin);
337+
ASSERT_EQUALS("", errout.str());
328338

329339
check("long f(int x, int y) {\n"
330-
" const long ret = x * y;\n"
340+
" long ret = x * y;\n"
331341
" return ret;\n"
332342
"}\n", settings);
333343
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 @@ class TestType : public TestFixture {
351361
" return ret;\n"
352362
"}\n", settings);
353363
ASSERT_EQUALS("", errout.str());
364+
365+
check("double g(float f) {\n"
366+
" return f * f;\n"
367+
"}\n", settings);
368+
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",
369+
errout.str());
354370
}
355371

356372
void longCastReturn() {
357-
const Settings settings = settingsBuilder().severity(Severity::style).build();
373+
const Settings settings = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Unix64).build();
374+
const Settings settingsWin = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Win64).build();
358375

359-
check("long f(int x, int y) {\n"
360-
" return x * y;\n"
361-
"}\n", settings);
376+
const char code[] = "long f(int x, int y) {\n"
377+
" return x * y;\n"
378+
"}\n";
379+
check(code, settings);
362380
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());
381+
check(code, settingsWin);
382+
ASSERT_EQUALS("", errout.str());
383+
384+
const char code2[] = "long long f(int x, int y) {\n"
385+
" return x * y;\n"
386+
"}\n";
387+
check(code2, settings);
388+
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());
389+
check(code2, settingsWin);
390+
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());
363391

364392
// typedef
365393
check("size_t f(int x, int y) {\n"
366394
" return x * y;\n"
367395
"}\n", settings);
368-
ASSERT_EQUALS("", errout.str());
396+
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());
369397
}
370398

371399
// This function ensure that test works with different compilers. Floats can

0 commit comments

Comments
 (0)