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
4 changes: 4 additions & 0 deletions lib/checkcondition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,10 @@ void CheckConditionImpl::checkIncorrectLogicOperator()
if (isfloat && (op1 == "==" || op1 == "!=" || op2 == "==" || op2 == "!="))
continue;

// no double value to compare (1e400L would become infinity)
if (isfloat && (MathLib::isLongDoubleOutOfDoubleRange(value1) || MathLib::isLongDoubleOutOfDoubleRange(value2)))
continue;

// the expr are not the token of the value but they provide better context
const double d1 = isfloat ? MathLib::toDoubleNumber(value1) : 0;
const double d2 = isfloat ? MathLib::toDoubleNumber(value2) : 0;
Expand Down
18 changes: 16 additions & 2 deletions lib/mathlib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,15 @@ double MathLib::toDoubleNumber(const std::string &str, const Token * const tok)
std::istringstream istr(str);
istr.imbue(std::locale::classic());
double ret;
if (!(istr >> ret))
throw InternalError(tok, "Internal Error. MathLib::toDoubleNumber: conversion failed: " + str);
if (!(istr >> ret)) {
// Out of the range of double, such as 1e10000 (glibc's HUGE_VAL for
// compilers other than GCC): the stream stores the largest finite
// value and fails, a compiler makes it infinity.
if (!isFloat(str) || std::fabs(ret) < std::numeric_limits<double>::max())
throw InternalError(tok, "Internal Error. MathLib::toDoubleNumber: conversion failed: " + str);
ret = std::copysign(std::numeric_limits<double>::infinity(), ret);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI review

For a literal with an l/L suffix, inf is not the value the compiler uses: 1e400L and 1e500L are different finite long double values. ValueFlow makes this a known value (vf_common.cpp calls toDoubleNumber() for each float literal and calls setKnown()), so different literals now compare equal and checkers give false positives:

long double r1(void) { return 1e400L / 1e500L; }            // duplicateExpression
int r2(long double x) { return x > 1e400L && x < 1e500L; } // incorrectLogicOperator
int r4(void) { return 1e400L < 1e500L; }                   // knownConditionTrueFalse: always false

Infinity is fine for literals without a suffix or with f. For long double literals out of the range of double, it is better to give the token no known value, for example by checking for the L suffix or std::isinf in vf_common.cpp, than to give it inf. Please also add a regression test for this case, for example in TestCondition or TestOther.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AI review

For a literal with an l/L suffix, inf is not the value the compiler uses: 1e400L and 1e500L are different finite long double values. ValueFlow makes this a known value (vf_common.cpp calls toDoubleNumber() for each float literal and calls setKnown()), so different literals now compare equal and checkers give false positives:

long double r1(void) { return 1e400L / 1e500L; }            // duplicateExpression
int r2(long double x) { return x > 1e400L && x < 1e500L; } // incorrectLogicOperator
int r4(void) { return 1e400L < 1e500L; }                   // knownConditionTrueFalse: always false

Infinity is fine for literals without a suffix or with f. For long double literals out of the range of double, it is better to give the token no known value, for example by checking for the L suffix or std::isinf in vf_common.cpp, than to give it inf. Please also add a regression test for this case, for example in TestCondition or TestOther.

Thanks, you're right: inf is only correct without a suffix or with f. Fixed in 8e0d456: MathLib::isLongDoubleOutOfDoubleRange() detects L literals that don't fit a double; ValueFlow gives them no value, and CheckCondition (which converts the literals itself for incorrectLogicOperator) skips them. Your examples are added as regression tests (TestCondition::incorrectLogicOperator18, TestOther::duplicateExpression23); they failed before this commit.

istr.clear();
}
std::string s;
if (istr >> s) {
if (isDecimalFloat(str))
Expand Down Expand Up @@ -584,6 +591,13 @@ bool MathLib::isFloat(const std::string &str)
return isDecimalFloat(str) || isFloatHex(str);
}

bool MathLib::isLongDoubleOutOfDoubleRange(const std::string &str)
{
if (str.empty() || (str.back() != 'l' && str.back() != 'L') || !isFloat(str))
return false;
return std::isinf(toDoubleNumber(str));
}

bool MathLib::isDecimalFloat(const std::string &str)
{
if (str.empty())
Expand Down
2 changes: 2 additions & 0 deletions lib/mathlib.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ class CPPCHECKLIB MathLib {

static bool isInt(const std::string & str);
static bool isFloat(const std::string &str);
/** @brief a long double literal out of the range of double, such as 1e400L, which toDoubleNumber() makes infinity */
static bool isLongDoubleOutOfDoubleRange(const std::string &str);
static bool isDecimalFloat(const std::string &str);
static bool isNegative(const std::string &str);
static bool isPositive(const std::string &str);
Expand Down
16 changes: 10 additions & 6 deletions lib/vf_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,16 @@ namespace ValueFlow
// Bad character literal
}
} else if (tok->isNumber() && MathLib::isFloat(tok->str())) {
Value value;
value.valueType = Value::ValueType::FLOAT;
value.floatValue = MathLib::toDoubleNumber(tok);
if (!tok->isTemplateArg())
value.setKnown();
setTokenValue(tok, std::move(value), settings);
// A long double literal out of the range of double (1e400L) has no
// double value: infinity would make different literals compare equal.
if (!MathLib::isLongDoubleOutOfDoubleRange(tok->str())) {
Value value;
value.valueType = Value::ValueType::FLOAT;
value.floatValue = MathLib::toDoubleNumber(tok);
if (!tok->isTemplateArg())
value.setKnown();
setTokenValue(tok, std::move(value), settings);
}
} else if (tok->enumerator() && tok->enumerator()->value_known) {
Value value(tok->enumerator()->value);
if (!tok->isTemplateArg())
Expand Down
12 changes: 12 additions & 0 deletions test/testcondition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class TestCondition : public TestFixture {
TEST_CASE(incorrectLogicOperator15);
TEST_CASE(incorrectLogicOperator16); // #10070
TEST_CASE(incorrectLogicOperator17);
TEST_CASE(incorrectLogicOperator18); // #12282
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
TEST_CASE(incorrectLogicOp_condSwapping);
TEST_CASE(testBug5895);
Expand Down Expand Up @@ -1758,6 +1759,17 @@ class TestCondition : public TestFixture {
ASSERT_EQUALS("", errout_str());
}

void incorrectLogicOperator18() { // #12282
// long double literals out of the range of double are different values
check("int f(long double x) {\n"
" return x > 1e400L && x < 1e500L;\n"
"}\n"
"int g() {\n"
" return 1e400L < 1e500L;\n"
"}\n");
ASSERT_EQUALS("", errout_str());
}

void incorrectLogicOperator17() { // #12471
check("struct R {\n"
" void set() { i = 1; }\n"
Expand Down
19 changes: 19 additions & 0 deletions test/testmathlib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "token.h"
#include "tokenlist.h"

#include <cmath>
#include <limits>
#include <memory>
#include <string>
Expand All @@ -44,6 +45,7 @@ class TestMathLib : public TestFixture {
TEST_CASE(isnegative);
TEST_CASE(ispositive);
TEST_CASE(isFloat);
TEST_CASE(isLongDoubleOutOfDoubleRange);
TEST_CASE(isDecimalFloat);
TEST_CASE(isGreater);
TEST_CASE(isGreaterEqual);
Expand Down Expand Up @@ -598,6 +600,16 @@ class TestMathLib : public TestFixture {
// TODO: test with 128-bit values
}

void isLongDoubleOutOfDoubleRange() const {
ASSERT_EQUALS(true, MathLib::isLongDoubleOutOfDoubleRange("1e400L"));
ASSERT_EQUALS(true, MathLib::isLongDoubleOutOfDoubleRange("1.0e4931l"));
ASSERT_EQUALS(false, MathLib::isLongDoubleOutOfDoubleRange("1e300L"));
ASSERT_EQUALS(false, MathLib::isLongDoubleOutOfDoubleRange("1e10000"));
ASSERT_EQUALS(false, MathLib::isLongDoubleOutOfDoubleRange("1e10000f"));
ASSERT_EQUALS(false, MathLib::isLongDoubleOutOfDoubleRange("10L"));
ASSERT_EQUALS(false, MathLib::isLongDoubleOutOfDoubleRange(""));
}

void toDoubleNumber() const {
// float values
ASSERT_EQUALS_DOUBLE(1.0, MathLib::toDoubleNumber("1."), 0.001);
Expand Down Expand Up @@ -677,6 +689,13 @@ class TestMathLib : public TestFixture {
ASSERT_EQUALS_DOUBLE(static_cast<double>('\200'), MathLib::toDoubleNumber("'\\200'"), 0.000001);
ASSERT_EQUALS_DOUBLE(static_cast<double>(L'A'), MathLib::toDoubleNumber("L'A'"), 0.000001);

// out of the range of double, like glibc's HUGE_VAL for compilers other than GCC
ASSERT_EQUALS(true, std::isinf(MathLib::toDoubleNumber("1e10000")));
ASSERT_EQUALS(true, std::isinf(MathLib::toDoubleNumber("1e10000f")));
ASSERT_EQUALS(true, std::isinf(MathLib::toDoubleNumber("1e10000L")));
ASSERT_EQUALS(true, std::isinf(MathLib::toDoubleNumber("1.5E+400")));
ASSERT_EQUALS(true, std::isinf(MathLib::toDoubleNumber("1.0e4931L"))); // #12282

ASSERT_THROW_INTERNAL_EQUALS(MathLib::toDoubleNumber("invalid"), INTERNAL, "Internal Error. MathLib::toDoubleNumber: conversion failed: invalid");

// AppleClang before 18 reports a different error
Expand Down
12 changes: 12 additions & 0 deletions test/testother.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ class TestOther : public TestFixture {
TEST_CASE(duplicateExpression20);
TEST_CASE(duplicateExpression21);
TEST_CASE(duplicateExpression22);
TEST_CASE(duplicateExpression23); // #12282
TEST_CASE(duplicateExpressionLoop);
TEST_CASE(duplicateValueTernary);
TEST_CASE(duplicateValueTernarySizeof); // #13773
Expand Down Expand Up @@ -8498,6 +8499,17 @@ class TestOther : public TestFixture {
ASSERT_EQUALS("", errout_str());
}

void duplicateExpression23() { // #12282
// long double literals out of the range of double are different values
check("long double f() {\n"
" return 1e400L / 1e500L;\n"
"}\n"
"int g(long double x) {\n"
" return x == 1e400L || x == 1e500L;\n"
"}\n");
ASSERT_EQUALS("", errout_str());
}

void duplicateExpression22() {
check("int f() {\n" // #14913
" return 0x1 | (0x2 | 0x4) | 0x1;\n"
Expand Down
Loading