Skip to content

Commit 491ee57

Browse files
rikardfalkeborndanmar
rikardfalkeborn
authored and
danmar
committed
Support floats in valid config (danmar#1297)
* Add tests for invalid ranges * Refactor loadLibErrors This reduces the amount of code slightly and will simplify adding more tests. * Handle empty valid field Before this change, the sequence <valid></valid> in a config file would result in a segmentation fault. Now an empty field results in the error message: cppcheck: Failed to load library configuration file 'mycfg.cfg'. Bad attribute value '""' * Add support for valid for floating point arguments Previously, it was not possible to add valid ranges to floating point arguments since it only handled integers. This made ranges not work well for floating point arguments since arguments were cast to integers before the ranges were handled. Fix this by using doubles instead of integers if the argument is a float. Add some tests for this and make sure errors are printed with enough precision (somewhat arbitrarily chosen). Note that it is still only possible to add integer ranges (i.e. -1:1). * Add support for floats in configuration valid range Now that it is possible to handle decimal arguments, there is no reason to not allow non-integer ranges. Take care to not allow broken configurations. * Move check to within if-clause * Move asin{,f,l} and acos{,f,l} input checks to config file
1 parent b05ae44 commit 491ee57

File tree

9 files changed

+290
-106
lines changed

9 files changed

+290
-106
lines changed

‎cfg/cppcheck-cfg.rng‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@
154154
<element name="not-uninit"><empty/></element>
155155
<element name="valid">
156156
<data type="string">
157-
<param name="pattern">(-?[0-9]*[,:])*([-]?[0-9]+)?</param>
157+
<param name="pattern">(-?[0-9]*(\.[0-9]+)?[,:])*([-]?[0-9]+(\.[0-9]+)?)?</param>
158158
</data>
159159
</element>
160160
<element name="minsize">

‎cfg/std.cfg‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
<leak-ignore/>
109109
<arg nr="1">
110110
<not-uninit/>
111+
<valid>-1.0:1.0</valid>
111112
</arg>
112113
</function>
113114
<!-- float acosf(float x); -->
@@ -119,6 +120,7 @@
119120
<leak-ignore/>
120121
<arg nr="1">
121122
<not-uninit/>
123+
<valid>-1.0:1.0</valid>
122124
</arg>
123125
</function>
124126
<!-- long double acosl(long double x); -->
@@ -130,6 +132,7 @@
130132
<leak-ignore/>
131133
<arg nr="1">
132134
<not-uninit/>
135+
<valid>-1.0:1.0</valid>
133136
</arg>
134137
</function>
135138
<!-- double acosh(double x); -->
@@ -348,6 +351,7 @@
348351
<noreturn>false</noreturn>
349352
<leak-ignore/>
350353
<arg nr="1">
354+
<valid>-1.0:1.0</valid>
351355
<not-uninit/>
352356
</arg>
353357
</function>
@@ -359,6 +363,7 @@
359363
<noreturn>false</noreturn>
360364
<leak-ignore/>
361365
<arg nr="1">
366+
<valid>-1.0:1.0</valid>
362367
<not-uninit/>
363368
</arg>
364369
</function>
@@ -370,6 +375,7 @@
370375
<noreturn>false</noreturn>
371376
<leak-ignore/>
372377
<arg nr="1">
378+
<valid>-1.0:1.0</valid>
373379
<not-uninit/>
374380
</arg>
375381
</function>

‎lib/checkfunctions.cpp‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
#include <cmath>
3434
#include <cstddef>
35+
#include <iomanip>
3536
#include <ostream>
3637
#include <vector>
3738

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

121122
// Are the values 0 and 1 valid?
122-
else if (!mSettings->library.isargvalid(functionToken, argnr, 0))
123+
else if (!mSettings->library.isargvalid(functionToken, argnr, static_cast<MathLib::bigint>(0)))
123124
invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings->library.validarg(functionToken, argnr));
124-
else if (!mSettings->library.isargvalid(functionToken, argnr, 1))
125+
else if (!mSettings->library.isargvalid(functionToken, argnr, static_cast<MathLib::bigint>(1)))
125126
invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings->library.validarg(functionToken, argnr));
126127
}
127128
}
@@ -139,7 +140,7 @@ void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string
139140
else
140141
errmsg << "Invalid $symbol() argument nr " << argnr << '.';
141142
if (invalidValue)
142-
errmsg << " The value is " << invalidValue->intvalue << " but the valid values are '" << validstr << "'.";
143+
errmsg << " The value is " << std::setprecision(10) << (invalidValue->isIntValue() ? invalidValue->intvalue : invalidValue->floatValue) << " but the valid values are '" << validstr << "'.";
143144
else
144145
errmsg << " The value is 0 or 1 (boolean) but the valid values are '" << validstr << "'.";
145146
if (invalidValue)
@@ -241,11 +242,6 @@ void CheckFunctions::checkMathFunctions()
241242
}
242243
}
243244

244-
// acos( x ), asin( x ) where x is defined for interval [-1,+1], but not beyond
245-
else if (Token::Match(tok, "acos|acosl|acosf|asin|asinf|asinl ( %num% )")) {
246-
if (std::fabs(MathLib::toDoubleNumber(tok->strAt(2))) > 1.0)
247-
mathfunctionCallWarning(tok);
248-
}
249245
// sqrt( x ): if x is negative the result is undefined
250246
else if (Token::Match(tok, "sqrt|sqrtf|sqrtl ( %num% )")) {
251247
if (MathLib::isNegative(tok->strAt(2)))

‎lib/library.cpp‎

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,18 @@ static std::vector<std::string> getnames(const char *names)
4343
return ret;
4444
}
4545

46+
static void gettokenlistfromvalid(const std::string& valid, TokenList& tokenList)
47+
{
48+
std::istringstream istr(valid + ',');
49+
tokenList.createTokens(istr);
50+
for (Token *tok = tokenList.front(); tok; tok = tok->next()) {
51+
if (Token::Match(tok,"- %num%")) {
52+
tok->str("-" + tok->strAt(1));
53+
tok->deleteNext();
54+
}
55+
}
56+
}
57+
4658
Library::Library() : mAllocId(0)
4759
{
4860
}
@@ -578,19 +590,33 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
578590
const char *p = argnode->GetText();
579591
bool error = false;
580592
bool range = false;
593+
bool has_dot = false;
594+
595+
if (!p)
596+
return Error(BAD_ATTRIBUTE_VALUE, "\"\"");
597+
598+
error = *p == '.';
581599
for (; *p; p++) {
582600
if (std::isdigit(*p))
583601
error |= (*(p+1) == '-');
584-
else if (*p == ':')
585-
error |= range;
602+
else if (*p == ':') {
603+
error |= range | (*(p+1) == '.');
604+
range = true;
605+
has_dot = false;
606+
}
586607
else if (*p == '-')
587608
error |= (!std::isdigit(*(p+1)));
588-
else if (*p == ',')
609+
else if (*p == ',') {
589610
range = false;
611+
error |= *(p+1) == '.';
612+
has_dot = false;
613+
}
614+
else if (*p == '.') {
615+
error |= has_dot | (!std::isdigit(*(p+1)));
616+
has_dot = true;
617+
}
590618
else
591619
error = true;
592-
593-
range |= (*p == ':');
594620
}
595621
if (error)
596622
return Error(BAD_ATTRIBUTE_VALUE, argnode->GetText());
@@ -705,15 +731,10 @@ bool Library::isargvalid(const Token *ftok, int argnr, const MathLib::bigint arg
705731
const ArgumentChecks *ac = getarg(ftok, argnr);
706732
if (!ac || ac->valid.empty())
707733
return true;
734+
else if (ac->valid.find('.') != std::string::npos)
735+
return isargvalid(ftok, argnr, static_cast<double>(argvalue));
708736
TokenList tokenList(nullptr);
709-
std::istringstream istr(ac->valid + ',');
710-
tokenList.createTokens(istr);
711-
for (Token *tok = tokenList.front(); tok; tok = tok->next()) {
712-
if (Token::Match(tok,"- %num%")) {
713-
tok->str("-" + tok->strAt(1));
714-
tok->deleteNext();
715-
}
716-
}
737+
gettokenlistfromvalid(ac->valid, tokenList);
717738
for (const Token *tok = tokenList.front(); tok; tok = tok->next()) {
718739
if (tok->isNumber() && argvalue == MathLib::toLongNumber(tok->str()))
719740
return true;
@@ -727,6 +748,24 @@ bool Library::isargvalid(const Token *ftok, int argnr, const MathLib::bigint arg
727748
return false;
728749
}
729750

751+
bool Library::isargvalid(const Token *ftok, int argnr, double argvalue) const
752+
{
753+
const ArgumentChecks *ac = getarg(ftok, argnr);
754+
if (!ac || ac->valid.empty())
755+
return true;
756+
TokenList tokenList(nullptr);
757+
gettokenlistfromvalid(ac->valid, tokenList);
758+
for (const Token *tok = tokenList.front(); tok; tok = tok->next()) {
759+
if (Token::Match(tok, "%num% : %num%") && argvalue >= MathLib::toDoubleNumber(tok->str()) && argvalue <= MathLib::toDoubleNumber(tok->strAt(2)))
760+
return true;
761+
if (Token::Match(tok, "%num% : ,") && argvalue >= MathLib::toDoubleNumber(tok->str()))
762+
return true;
763+
if ((!tok->previous() || tok->previous()->str() == ",") && Token::Match(tok,": %num%") && argvalue <= MathLib::toDoubleNumber(tok->strAt(1)))
764+
return true;
765+
}
766+
return false;
767+
}
768+
730769
std::string Library::getFunctionName(const Token *ftok, bool *error) const
731770
{
732771
if (!ftok) {

‎lib/library.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ class CPPCHECKLIB Library {
301301
}
302302

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

305306
const std::string& validarg(const Token *ftok, int argnr) const {
306307
const ArgumentChecks *arg = getarg(ftok, argnr);

‎lib/token.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1497,7 +1497,8 @@ const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, unsigned int
14971497
const ValueFlow::Value *ret = nullptr;
14981498
std::list<ValueFlow::Value>::const_iterator it;
14991499
for (it = mValues->begin(); it != mValues->end(); ++it) {
1500-
if (it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) {
1500+
if ((it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) ||
1501+
(it->isFloatValue() && !settings->library.isargvalid(ftok, argnr, it->floatValue))) {
15011502
if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive()))
15021503
ret = &(*it);
15031504
if (!ret->isInconclusive() && !ret->condition)

‎man/manual.docbook‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1498,7 +1498,8 @@ Checking range.c...
14981498
-10:20 =&gt; all values between -10 and 20 are valid
14991499
:0 =&gt; all values that are less or equal to 0 are valid
15001500
0: =&gt; all values that are greater or equal to 0 are valid
1501-
0,2:32 =&gt; the value 0 and all values between 2 and 32 are valid </programlisting>
1501+
0,2:32 =&gt; the value 0 and all values between 2 and 32 are valid
1502+
-1.5:5.6 =&gt; all values between -1.5 and 5.6 are valid </programlisting>
15021503
</section>
15031504

15041505
<section>

‎test/testfunctions.cpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -604,19 +604,19 @@ class TestFunctions : public TestFixture {
604604
" std::cout << acosf(1.1) << std::endl;\n"
605605
" std::cout << acosl(1.1) << std::endl;\n"
606606
"}");
607-
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value 1.1 to acos() leads to implementation-defined result.\n"
608-
"[test.cpp:4]: (warning) Passing value 1.1 to acosf() leads to implementation-defined result.\n"
609-
"[test.cpp:5]: (warning) Passing value 1.1 to acosl() leads to implementation-defined result.\n", errout.str());
607+
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"
608+
"[test.cpp:4]: (error) Invalid acosf() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n"
609+
"[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());
610610

611611
check("void foo()\n"
612612
"{\n"
613613
" std::cout << acos(-1.1) << std::endl;\n"
614614
" std::cout << acosf(-1.1) << std::endl;\n"
615615
" std::cout << acosl(-1.1) << std::endl;\n"
616616
"}");
617-
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value -1.1 to acos() leads to implementation-defined result.\n"
618-
"[test.cpp:4]: (warning) Passing value -1.1 to acosf() leads to implementation-defined result.\n"
619-
"[test.cpp:5]: (warning) Passing value -1.1 to acosl() leads to implementation-defined result.\n", errout.str());
617+
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"
618+
"[test.cpp:4]: (error) Invalid acosf() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n"
619+
"[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());
620620
}
621621

622622
void mathfunctionCall_asin() {
@@ -665,19 +665,19 @@ class TestFunctions : public TestFixture {
665665
" std::cout << asinf(1.1) << std::endl;\n"
666666
" std::cout << asinl(1.1) << std::endl;\n"
667667
"}");
668-
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value 1.1 to asin() leads to implementation-defined result.\n"
669-
"[test.cpp:4]: (warning) Passing value 1.1 to asinf() leads to implementation-defined result.\n"
670-
"[test.cpp:5]: (warning) Passing value 1.1 to asinl() leads to implementation-defined result.\n", errout.str());
668+
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"
669+
"[test.cpp:4]: (error) Invalid asinf() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n"
670+
"[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());
671671

672672
check("void foo()\n"
673673
"{\n"
674674
" std::cout << asin(-1.1) << std::endl;\n"
675675
" std::cout << asinf(-1.1) << std::endl;\n"
676676
" std::cout << asinl(-1.1) << std::endl;\n"
677677
"}");
678-
ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value -1.1 to asin() leads to implementation-defined result.\n"
679-
"[test.cpp:4]: (warning) Passing value -1.1 to asinf() leads to implementation-defined result.\n"
680-
"[test.cpp:5]: (warning) Passing value -1.1 to asinl() leads to implementation-defined result.\n", errout.str());
678+
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"
679+
"[test.cpp:4]: (error) Invalid asinf() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n"
680+
"[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());
681681
}
682682

683683
void mathfunctionCall_pow() {

0 commit comments

Comments
 (0)