Skip to content

Support floats in valid config #1297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danmar merged 7 commits into from
Jul 15, 2018
Merged

Support floats in valid config #1297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d05e224
Refactor loadLibErrors
rikardfalkeborn Jul 5, 2018
1c9f46c
Add tests for invalid ranges
rikardfalkeborn Jul 3, 2018
ff46ff1
Handle empty valid field
rikardfalkeborn Jul 6, 2018
09055eb
Add support for valid for floating point arguments
rikardfalkeborn Jul 7, 2018
34e5ba4
Add support for floats in configuration valid range
rikardfalkeborn Jul 6, 2018
30dea89
Move check to within if-clause
rikardfalkeborn Jul 6, 2018
e6383b9
Move asin{,f,l} and acos{,f,l} input checks to config file
rikardfalkeborn Jul 12, 2018
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
2 changes: 1 addition & 1 deletion cfg/cppcheck-cfg.rng
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@
<element name="not-uninit"><empty/></element>
<element name="valid">
<data type="string">
<param name="pattern">(-?[0-9]*[,:])*([-]?[0-9]+)?</param>
<param name="pattern">(-?[0-9]*(\.[0-9]+)?[,:])*([-]?[0-9]+(\.[0-9]+)?)?</param>
</data>
</element>
<element name="minsize">
Expand Down
6 changes: 6 additions & 0 deletions cfg/std.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
<leak-ignore/>
<arg nr="1">
<not-uninit/>
<valid>-1.0:1.0</valid>
</arg>
</function>
<!-- float acosf(float x); -->
Expand All @@ -119,6 +120,7 @@
<leak-ignore/>
<arg nr="1">
<not-uninit/>
<valid>-1.0:1.0</valid>
</arg>
</function>
<!-- long double acosl(long double x); -->
Expand All @@ -130,6 +132,7 @@
<leak-ignore/>
<arg nr="1">
<not-uninit/>
<valid>-1.0:1.0</valid>
</arg>
</function>
<!-- double acosh(double x); -->
Expand Down Expand Up @@ -348,6 +351,7 @@
<noreturn>false</noreturn>
<leak-ignore/>
<arg nr="1">
<valid>-1.0:1.0</valid>
<not-uninit/>
</arg>
</function>
Expand All @@ -359,6 +363,7 @@
<noreturn>false</noreturn>
<leak-ignore/>
<arg nr="1">
<valid>-1.0:1.0</valid>
<not-uninit/>
</arg>
</function>
Expand All @@ -370,6 +375,7 @@
<noreturn>false</noreturn>
<leak-ignore/>
<arg nr="1">
<valid>-1.0:1.0</valid>
<not-uninit/>
</arg>
</function>
Expand Down
12 changes: 4 additions & 8 deletions lib/checkfunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include <cmath>
#include <cstddef>
#include <iomanip>
#include <ostream>
#include <vector>

Expand Down Expand Up @@ -119,9 +120,9 @@ void CheckFunctions::invalidFunctionUsage()
invalidFunctionArgBoolError(argtok, functionToken->str(), argnr);

// Are the values 0 and 1 valid?
else if (!mSettings->library.isargvalid(functionToken, argnr, 0))
else if (!mSettings->library.isargvalid(functionToken, argnr, static_cast<MathLib::bigint>(0)))
invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings->library.validarg(functionToken, argnr));
else if (!mSettings->library.isargvalid(functionToken, argnr, 1))
else if (!mSettings->library.isargvalid(functionToken, argnr, static_cast<MathLib::bigint>(1)))
invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings->library.validarg(functionToken, argnr));
}
}
Expand All @@ -139,7 +140,7 @@ void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string
else
errmsg << "Invalid $symbol() argument nr " << argnr << '.';
if (invalidValue)
errmsg << " The value is " << invalidValue->intvalue << " but the valid values are '" << validstr << "'.";
errmsg << " The value is " << std::setprecision(10) << (invalidValue->isIntValue() ? invalidValue->intvalue : invalidValue->floatValue) << " but the valid values are '" << validstr << "'.";
else
errmsg << " The value is 0 or 1 (boolean) but the valid values are '" << validstr << "'.";
if (invalidValue)
Expand Down Expand Up @@ -241,11 +242,6 @@ void CheckFunctions::checkMathFunctions()
}
}

// acos( x ), asin( x ) where x is defined for interval [-1,+1], but not beyond
else if (Token::Match(tok, "acos|acosl|acosf|asin|asinf|asinl ( %num% )")) {
if (std::fabs(MathLib::toDoubleNumber(tok->strAt(2))) > 1.0)
mathfunctionCallWarning(tok);
}
// sqrt( x ): if x is negative the result is undefined
else if (Token::Match(tok, "sqrt|sqrtf|sqrtl ( %num% )")) {
if (MathLib::isNegative(tok->strAt(2)))
Expand Down
65 changes: 52 additions & 13 deletions lib/library.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ static std::vector<std::string> getnames(const char *names)
return ret;
}

static void gettokenlistfromvalid(const std::string& valid, TokenList& tokenList)
{
std::istringstream istr(valid + ',');
tokenList.createTokens(istr);
for (Token *tok = tokenList.front(); tok; tok = tok->next()) {
if (Token::Match(tok,"- %num%")) {
tok->str("-" + tok->strAt(1));
tok->deleteNext();
}
}
}

Library::Library() : mAllocId(0)
{
}
Expand Down Expand Up @@ -578,19 +590,33 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
const char *p = argnode->GetText();
bool error = false;
bool range = false;
bool has_dot = false;

if (!p)
return Error(BAD_ATTRIBUTE_VALUE, "\"\"");

error = *p == '.';
for (; *p; p++) {
if (std::isdigit(*p))
error |= (*(p+1) == '-');
else if (*p == ':')
error |= range;
else if (*p == ':') {
error |= range | (*(p+1) == '.');
range = true;
has_dot = false;
}
else if (*p == '-')
error |= (!std::isdigit(*(p+1)));
else if (*p == ',')
else if (*p == ',') {
range = false;
error |= *(p+1) == '.';
has_dot = false;
}
else if (*p == '.') {
error |= has_dot | (!std::isdigit(*(p+1)));
has_dot = true;
}
else
error = true;

range |= (*p == ':');
}
if (error)
return Error(BAD_ATTRIBUTE_VALUE, argnode->GetText());
Expand Down Expand Up @@ -705,15 +731,10 @@ bool Library::isargvalid(const Token *ftok, int argnr, const MathLib::bigint arg
const ArgumentChecks *ac = getarg(ftok, argnr);
if (!ac || ac->valid.empty())
return true;
else if (ac->valid.find('.') != std::string::npos)
return isargvalid(ftok, argnr, static_cast<double>(argvalue));
TokenList tokenList(nullptr);
std::istringstream istr(ac->valid + ',');
tokenList.createTokens(istr);
for (Token *tok = tokenList.front(); tok; tok = tok->next()) {
if (Token::Match(tok,"- %num%")) {
tok->str("-" + tok->strAt(1));
tok->deleteNext();
}
}
gettokenlistfromvalid(ac->valid, tokenList);
for (const Token *tok = tokenList.front(); tok; tok = tok->next()) {
if (tok->isNumber() && argvalue == MathLib::toLongNumber(tok->str()))
return true;
Expand All @@ -727,6 +748,24 @@ bool Library::isargvalid(const Token *ftok, int argnr, const MathLib::bigint arg
return false;
}

bool Library::isargvalid(const Token *ftok, int argnr, double argvalue) const
Copy link
Copy Markdown
Collaborator

@danmar danmar Jul 10, 2018

Choose a reason for hiding this comment

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

hmm.. we now have copy/paste I wonder if we can avoid it without using double for the integers. one thing that can be done is creating a utility function that creates the token list.

Copy link
Copy Markdown
Contributor Author

@rikardfalkeborn rikardfalkeborn Jul 10, 2018

Choose a reason for hiding this comment

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

I extracted the token list creation to a separate function. There's still a lot of copy/paste but I couldn't figure out a good way to remove it.

{
const ArgumentChecks *ac = getarg(ftok, argnr);
if (!ac || ac->valid.empty())
return true;
TokenList tokenList(nullptr);
gettokenlistfromvalid(ac->valid, tokenList);
for (const Token *tok = tokenList.front(); tok; tok = tok->next()) {
if (Token::Match(tok, "%num% : %num%") && argvalue >= MathLib::toDoubleNumber(tok->str()) && argvalue <= MathLib::toDoubleNumber(tok->strAt(2)))
return true;
if (Token::Match(tok, "%num% : ,") && argvalue >= MathLib::toDoubleNumber(tok->str()))
return true;
if ((!tok->previous() || tok->previous()->str() == ",") && Token::Match(tok,": %num%") && argvalue <= MathLib::toDoubleNumber(tok->strAt(1)))
return true;
}
return false;
}

std::string Library::getFunctionName(const Token *ftok, bool *error) const
{
if (!ftok) {
Expand Down
1 change: 1 addition & 0 deletions lib/library.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ class CPPCHECKLIB Library {
}

bool isargvalid(const Token *ftok, int argnr, const MathLib::bigint argvalue) const;
bool isargvalid(const Token *ftok, int argnr, double argvalue) const;

const std::string& validarg(const Token *ftok, int argnr) const {
const ArgumentChecks *arg = getarg(ftok, argnr);
Expand Down
3 changes: 2 additions & 1 deletion lib/token.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,8 @@ const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, unsigned int
const ValueFlow::Value *ret = nullptr;
std::list<ValueFlow::Value>::const_iterator it;
for (it = mValues->begin(); it != mValues->end(); ++it) {
if (it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) {
if ((it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) ||
(it->isFloatValue() && !settings->library.isargvalid(ftok, argnr, it->floatValue))) {
if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive()))
ret = &(*it);
if (!ret->isInconclusive() && !ret->condition)
Expand Down
3 changes: 2 additions & 1 deletion man/manual.docbook
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,8 @@ Checking range.c...
-10:20 =&gt; all values between -10 and 20 are valid
:0 =&gt; all values that are less or equal to 0 are valid
0: =&gt; all values that are greater or equal to 0 are valid
0,2:32 =&gt; the value 0 and all values between 2 and 32 are valid </programlisting>
0,2:32 =&gt; the value 0 and all values between 2 and 32 are valid
-1.5:5.6 =&gt; all values between -1.5 and 5.6 are valid </programlisting>
</section>

<section>
Expand Down
24 changes: 12 additions & 12 deletions test/testfunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,19 +604,19 @@ class TestFunctions : public TestFixture {
" std::cout << acosf(1.1) << std::endl;\n"
" std::cout << acosl(1.1) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value 1.1 to acos() leads to implementation-defined result.\n"
"[test.cpp:4]: (warning) Passing value 1.1 to acosf() leads to implementation-defined result.\n"
"[test.cpp:5]: (warning) Passing value 1.1 to acosl() leads to implementation-defined result.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Invalid acos() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:4]: (error) Invalid acosf() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:5]: (error) Invalid acosl() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n", errout.str());

check("void foo()\n"
"{\n"
" std::cout << acos(-1.1) << std::endl;\n"
" std::cout << acosf(-1.1) << std::endl;\n"
" std::cout << acosl(-1.1) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value -1.1 to acos() leads to implementation-defined result.\n"
"[test.cpp:4]: (warning) Passing value -1.1 to acosf() leads to implementation-defined result.\n"
"[test.cpp:5]: (warning) Passing value -1.1 to acosl() leads to implementation-defined result.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Invalid acos() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:4]: (error) Invalid acosf() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:5]: (error) Invalid acosl() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n", errout.str());
}

void mathfunctionCall_asin() {
Expand Down Expand Up @@ -665,19 +665,19 @@ class TestFunctions : public TestFixture {
" std::cout << asinf(1.1) << std::endl;\n"
" std::cout << asinl(1.1) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value 1.1 to asin() leads to implementation-defined result.\n"
"[test.cpp:4]: (warning) Passing value 1.1 to asinf() leads to implementation-defined result.\n"
"[test.cpp:5]: (warning) Passing value 1.1 to asinl() leads to implementation-defined result.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Invalid asin() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:4]: (error) Invalid asinf() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:5]: (error) Invalid asinl() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n", errout.str());

check("void foo()\n"
"{\n"
" std::cout << asin(-1.1) << std::endl;\n"
" std::cout << asinf(-1.1) << std::endl;\n"
" std::cout << asinl(-1.1) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value -1.1 to asin() leads to implementation-defined result.\n"
"[test.cpp:4]: (warning) Passing value -1.1 to asinf() leads to implementation-defined result.\n"
"[test.cpp:5]: (warning) Passing value -1.1 to asinl() leads to implementation-defined result.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Invalid asin() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:4]: (error) Invalid asinf() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n"
"[test.cpp:5]: (error) Invalid asinl() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n", errout.str());
}

void mathfunctionCall_pow() {
Expand Down
Loading