Skip to content

Commit b775714

Browse files
committed
Moved several hardcoded function names in format string checking into libraries (std.cfg and windows.cfg).
Added support for loading a library in test suite.
1 parent 21d317b commit b775714

File tree

8 files changed

+117
-51
lines changed

8 files changed

+117
-51
lines changed

cfg/std.cfg

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,5 +55,20 @@
5555
<function name="wcstoll"> <leak-ignore/> <arg nr="3"><valid>0,2-36</valid></arg> </function>
5656
<function name="wcstoul"> <leak-ignore/> <arg nr="3"><valid>0,2-36</valid></arg> </function>
5757
<function name="wcstoull"> <leak-ignore/> <arg nr="3"><valid>0,2-36</valid></arg> </function>
58+
59+
<function name="printf"> <noreturn>false</noreturn> <formatstr/> <arg nr="1"><formatstr/></arg> </function>
60+
<function name="wprintf"> <noreturn>false</noreturn> <formatstr/> <arg nr="1"><formatstr/></arg> </function>
61+
<function name="sprintf"> <noreturn>false</noreturn> <formatstr/> <arg nr="2"><formatstr/></arg> </function>
62+
<function name="fprintf"> <noreturn>false</noreturn> <formatstr/> <arg nr="2"><formatstr/></arg> </function>
63+
<function name="fwprintf"> <noreturn>false</noreturn> <formatstr/> <arg nr="2"><formatstr/></arg> </function>
64+
<function name="snprintf"> <noreturn>false</noreturn> <formatstr/> <arg nr="3"><formatstr/></arg> </function>
65+
<function name="fnprintf"> <noreturn>false</noreturn> <formatstr/> <arg nr="3"><formatstr/></arg> </function>
66+
67+
<function name="scanf"> <noreturn>false</noreturn> <formatstr scan="true"/> <arg nr="1"><formatstr/></arg> </function>
68+
<function name="wscanf"> <noreturn>false</noreturn> <formatstr scan="true"/> <arg nr="1"><formatstr/></arg> </function>
69+
<function name="sscanf"> <noreturn>false</noreturn> <formatstr scan="true"/> <arg nr="2"><formatstr/></arg> </function>
70+
<function name="fscanf"> <noreturn>false</noreturn> <formatstr scan="true"/> <arg nr="2"><formatstr/></arg> </function>
71+
<function name="fwscanf"> <noreturn>false</noreturn> <formatstr scan="true"/> <arg nr="2"><formatstr/></arg> </function>
72+
<function name="swscanf"> <noreturn>false</noreturn> <formatstr scan="true"/> <arg nr="2"><formatstr/></arg> </function>
5873
</def>
5974

cfg/windows.cfg

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,18 @@
99
<noreturn>false</noreturn>
1010
</function>
1111

12+
<function name="printf_s"> <noreturn>false</noreturn> <formatstr secure="true"/> <arg nr="1"><formatstr/></arg> </function>
13+
<function name="wprintf_s"> <noreturn>false</noreturn> <formatstr secure="true"/> <arg nr="1"><formatstr/></arg> </function>
14+
<function name="fprintf_s"> <noreturn>false</noreturn> <formatstr secure="true"/> <arg nr="2"><formatstr/></arg> </function>
15+
<function name="fwprintf_s"> <noreturn>false</noreturn> <formatstr secure="true"/> <arg nr="2"><formatstr/></arg> </function>
16+
<function name="_snprintf"> <noreturn>false</noreturn> <formatstr secure="true"/> <arg nr="3"><formatstr/></arg> </function>
17+
<function name="_snwprintf"> <noreturn>false</noreturn> <formatstr secure="true"/> <arg nr="3"><formatstr/></arg> </function>
18+
19+
<function name="scanf_s"> <noreturn>false</noreturn> <formatstr scan="true" secure="true"/> <arg nr="1"><formatstr/></arg> </function>
20+
<function name="wscanf_s"> <noreturn>false</noreturn> <formatstr scan="true" secure="true"/> <arg nr="1"><formatstr/></arg> </function>
21+
<function name="sscanf_s"> <noreturn>false</noreturn> <formatstr scan="true" secure="true"/> <arg nr="2"><formatstr/></arg> </function>
22+
<function name="fscanf_s"> <noreturn>false</noreturn> <formatstr scan="true" secure="true"/> <arg nr="2"><formatstr/></arg> </function>
23+
<function name="fwscanf_s"> <noreturn>false</noreturn> <formatstr scan="true" secure="true"/> <arg nr="2"><formatstr/></arg> </function>
24+
<function name="swscanf_s"> <noreturn>false</noreturn> <formatstr scan="true" secure="true"/> <arg nr="2"><formatstr/></arg> </function>
25+
1226
</def>

lib/checkio.cpp

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -457,43 +457,38 @@ void CheckIO::checkWrongPrintfScanfArguments()
457457
const Token* formatStringTok = 0; // Points to format string token
458458
std::string formatString;
459459

460-
if (Token::Match(tok->next(), "( %any%")) {
461-
const Token *arg = tok->tokAt(2);
462-
int argnr = 1;
463-
while (arg) {
464-
if (Token::Match(arg, "%str% [,)]") && _settings->library.isargformatstr(tok->str(),argnr)) {
465-
formatStringTok = arg;
466-
if (arg->strAt(1) == ",")
467-
argListTok = arg->tokAt(2);
468-
else
469-
argListTok = 0;
460+
bool scan = false;
461+
bool scanf_s = false;
462+
int formatStringArgNo = -1;
463+
464+
if (Token::Match(tok->next(), "( %any%") && _settings->library.formatstr_function(tok->str())) {
465+
const std::map<int, Library::ArgumentChecks>& argumentChecks = _settings->library.argumentChecks.at(tok->str());
466+
for (std::map<int, Library::ArgumentChecks>::const_iterator i = argumentChecks.cbegin(); i != argumentChecks.cend(); ++i) {
467+
if (i->second.formatstr) {
468+
formatStringArgNo = i->first - 1;
470469
break;
471470
}
472-
473-
arg = arg->nextArgument();
474-
argnr++;
475471
}
472+
473+
scan = _settings->library.formatstr_scan(tok->str());
474+
scanf_s = _settings->library.formatstr_secure(tok->str());
476475
}
477476

478-
if (formatStringTok) {
479-
/* formatstring found in library */
480-
} else if (Token::Match(tok, "printf|scanf|wprintf|wscanf (") ||
481-
(windows && (Token::Match(tok, "printf_s|wprintf_s|scanf_s|wscanf_s (") ||
482-
(Token::Match(tok, "Format|AppendFormat (") &&
483-
Token::Match(tok->tokAt(-2), "%var% .") && tok->tokAt(-2)->variable() &&
484-
tok->tokAt(-2)->variable()->typeStartToken()->str() == "CString")))) {
477+
if (formatStringArgNo >= 0) {
478+
// formatstring found in library. Find format string and first argument belonging to format string.
479+
if (!findFormat(formatStringArgNo, tok->tokAt(2), &formatStringTok, &argListTok))
480+
continue;
481+
} else if (windows && Token::Match(tok, "Format|AppendFormat (") &&
482+
Token::Match(tok->tokAt(-2), "%var% .") && tok->tokAt(-2)->variable() &&
483+
tok->tokAt(-2)->variable()->typeStartToken()->str() == "CString") {
485484
// Find second parameter and format string
486485
if (!findFormat(0, tok->tokAt(2), &formatStringTok, &argListTok))
487486
continue;
488-
} else if (Token::Match(tok, "sprintf|fprintf|sscanf|fscanf|swscanf|fwprintf|fwscanf ( %any%") ||
489-
(Token::simpleMatch(tok, "swprintf (") && Token::Match(tok->tokAt(2)->nextArgument(), "%str%")) ||
490-
(windows && Token::Match(tok, "sscanf_s|swscanf_s|fscanf_s|fwscanf_s|fprintf_s|fwprintf_s ( %any%"))) {
487+
} else if (Token::simpleMatch(tok, "swprintf (") && Token::Match(tok->tokAt(2)->nextArgument(), "%str%")) {
491488
// Find third parameter and format string
492489
if (!findFormat(1, tok->tokAt(2), &formatStringTok, &argListTok))
493490
continue;
494-
} else if (Token::Match(tok, "snprintf|fnprintf (") ||
495-
(windows && Token::Match(tok, "_snprintf|_snwprintf (")) ||
496-
(Token::simpleMatch(tok, "swprintf (") && !Token::Match(tok->tokAt(2)->nextArgument(), "%str%"))) {
491+
} else if (Token::simpleMatch(tok, "swprintf (") && !Token::Match(tok->tokAt(2)->nextArgument(), "%str%")) {
497492
// Find forth parameter and format string
498493
if (!findFormat(2, tok->tokAt(2), &formatStringTok, &argListTok))
499494
continue;
@@ -529,8 +524,6 @@ void CheckIO::checkWrongPrintfScanfArguments()
529524
continue;
530525

531526
// Count format string parameters..
532-
bool scanf_s = windows ? Token::Match(tok, "scanf_s|wscanf_s|sscanf_s|swscanf_s|fscanf_s|fwscanf_s") : false;
533-
bool scan = Token::Match(tok, "sscanf|fscanf|scanf|swscanf|fwscanf|wscanf") || scanf_s;
534527
unsigned int numFormat = 0;
535528
unsigned int numSecure = 0;
536529
bool percent = false;

lib/library.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,10 @@ bool Library::load(const tinyxml2::XMLDocument &doc)
186186
argumentChecks[name][nr].valid = valid;
187187
} else if (strcmp(functionnode->Name(), "ignorefunction") == 0) {
188188
_ignorefunction.insert(name);
189+
} else if (strcmp(functionnode->Name(), "formatstr") == 0) {
190+
const tinyxml2::XMLAttribute* scan = functionnode->FindAttribute("scan");
191+
const tinyxml2::XMLAttribute* secure = functionnode->FindAttribute("secure");
192+
_formatstr[name] = std::make_pair(scan && scan->BoolValue(), secure && secure->BoolValue());
189193
} else
190194
return false;
191195
}

lib/library.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,18 @@ class CPPCHECKLIB Library {
8484
return ((id > 0) && ((id & 1) == 1));
8585
}
8686

87+
bool formatstr_function(const std::string& funcname) const {
88+
return _formatstr.find(funcname) != _formatstr.cend();
89+
}
90+
91+
bool formatstr_scan(const std::string& funcname) const {
92+
return _formatstr.at(funcname).first;
93+
}
94+
95+
bool formatstr_secure(const std::string& funcname) const {
96+
return _formatstr.at(funcname).second;
97+
}
98+
8799
std::set<std::string> use;
88100
std::set<std::string> leakignore;
89101

@@ -325,7 +337,8 @@ class CPPCHECKLIB Library {
325337
std::map<std::string, CodeBlock> _executableblocks; // keywords for blocks of executable code
326338
std::map<std::string, ExportedFunctions> _exporters; // keywords that export variables/functions to libraries (meta-code/macros)
327339
std::map<std::string, std::set<std::string> > _importers; // keywords that import variables/functions
328-
std::map<std::string,std::map<std::string,int> > _reflection; // invocation of reflection
340+
std::map<std::string, std::map<std::string,int> > _reflection; // invocation of reflection
341+
std::map<std::string, std::pair<bool, bool> > _formatstr; // Parameters for format string checking
329342

330343

331344
const ArgumentChecks * getarg(const std::string &functionName, int argnr) const {

test/testio.cpp

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,9 @@ class TestIO : public TestFixture {
5353
TEST_CASE(testMicrosoftCStringFormatArguments); // ticket #4920
5454
TEST_CASE(testMicrosoftSecurePrintfArgument);
5555
TEST_CASE(testMicrosoftSecureScanfArgument);
56-
57-
TEST_CASE(testlibrarycfg); // library configuration
5856
}
5957

60-
void check(const char code[], bool inconclusive = false, bool portability = false, Settings::PlatformType platform = Settings::Unspecified, Library *lib = NULL) {
58+
void check(const char code[], bool inconclusive = false, bool portability = false, Settings::PlatformType platform = Settings::Unspecified) {
6159
// Clear the error buffer..
6260
errout.str("");
6361

@@ -69,8 +67,7 @@ class TestIO : public TestFixture {
6967
settings.inconclusive = inconclusive;
7068
settings.platform(platform);
7169

72-
if (lib)
73-
settings.library = *lib;
70+
settings.library = _lib;
7471

7572
// Tokenize..
7673
Tokenizer tokenizer(&settings, this);
@@ -437,6 +434,8 @@ class TestIO : public TestFixture {
437434

438435

439436
void testScanf1() {
437+
LOAD_LIB("std.cfg");
438+
440439
check("void foo() {\n"
441440
" int a, b;\n"
442441
" FILE *file = fopen(\"test\", \"r\");\n"
@@ -455,6 +454,8 @@ class TestIO : public TestFixture {
455454
}
456455

457456
void testScanf2() {
457+
LOAD_LIB("std.cfg");
458+
458459
check("void foo() {\n"
459460
" scanf(\"%5s\", bar);\n" // Width specifier given
460461
" scanf(\"%5[^~]\", bar);\n" // Width specifier given
@@ -481,6 +482,7 @@ class TestIO : public TestFixture {
481482
}
482483

483484
void testScanf4() { // ticket #2553
485+
LOAD_LIB("std.cfg");
484486

485487
check("void f()\n"
486488
"{\n"
@@ -494,6 +496,8 @@ class TestIO : public TestFixture {
494496

495497

496498
void testScanfArgument() {
499+
LOAD_LIB("std.cfg");
500+
497501
check("void foo() {\n"
498502
" scanf(\"%1d\", &foo);\n"
499503
" sscanf(bar, \"%1d\", &foo);\n"
@@ -1322,6 +1326,7 @@ class TestIO : public TestFixture {
13221326
}
13231327

13241328
void testPrintfArgument() {
1329+
LOAD_LIB("std.cfg");
13251330
check("void foo() {\n"
13261331
" printf(\"%u\");\n"
13271332
" printf(\"%u%s\", 123);\n"
@@ -2107,6 +2112,8 @@ class TestIO : public TestFixture {
21072112
}
21082113

21092114
void testPosixPrintfScanfParameterPosition() { // #4900 - No support for parameters in format strings
2115+
LOAD_LIB("std.cfg");
2116+
21102117
check("void foo() {"
21112118
" int bar;"
21122119
" printf(\"%1$d\", 1);"
@@ -2131,6 +2138,9 @@ class TestIO : public TestFixture {
21312138

21322139

21332140
void testMicrosoftPrintfArgument() {
2141+
LOAD_LIB("std.cfg");
2142+
LOAD_LIB("windows.cfg");
2143+
21342144
check("void foo() {\n"
21352145
" size_t s;\n"
21362146
" ptrdiff_t p;\n"
@@ -2220,6 +2230,9 @@ class TestIO : public TestFixture {
22202230
}
22212231

22222232
void testMicrosoftScanfArgument() {
2233+
LOAD_LIB("std.cfg");
2234+
LOAD_LIB("windows.cfg");
2235+
22232236
check("void foo() {\n"
22242237
" size_t s;\n"
22252238
" ptrdiff_t p;\n"
@@ -2324,6 +2337,9 @@ class TestIO : public TestFixture {
23242337
}
23252338

23262339
void testMicrosoftSecurePrintfArgument() {
2340+
LOAD_LIB("std.cfg");
2341+
LOAD_LIB("windows.cfg");
2342+
23272343
check("void foo() {\n"
23282344
" int i;\n"
23292345
" unsigned int u;\n"
@@ -2514,6 +2530,8 @@ class TestIO : public TestFixture {
25142530
}
25152531

25162532
void testMicrosoftSecureScanfArgument() {
2533+
LOAD_LIB("windows.cfg");
2534+
25172535
check("void foo() {\n"
25182536
" int i;\n"
25192537
" unsigned int u;\n"
@@ -2644,22 +2662,6 @@ class TestIO : public TestFixture {
26442662
"}\n", false, false, Settings::Win32W);
26452663
ASSERT_EQUALS("", errout.str());
26462664
}
2647-
2648-
void testlibrarycfg() {
2649-
const char code[] = "void f() {\n"
2650-
" format(\"%s\");\n"
2651-
"}";
2652-
2653-
// no error if configuration for 'format' is not provided
2654-
check(code);
2655-
ASSERT_EQUALS("", errout.str());
2656-
2657-
// error if configuration for 'format' is provided
2658-
Library lib;
2659-
lib.argumentChecks["format"][1].formatstr = true;
2660-
check(code, false, false, Settings::Unspecified, &lib);
2661-
ASSERT_EQUALS("[test.cpp:2]: (error) format format string requires 1 parameter but only 0 are given.\n", errout.str());
2662-
}
26632665
};
26642666

26652667
REGISTER_TEST(TestIO)

test/testsuite.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ unsigned int TestFixture::countTests;
6262
std::size_t TestFixture::fails_counter = 0;
6363
std::size_t TestFixture::todos_counter = 0;
6464
std::size_t TestFixture::succeeded_todos_counter = 0;
65+
std::set<std::string> TestFixture::missingLibs;
6566

6667
TestFixture::TestFixture(const std::string &_name)
6768
:classname(_name)
@@ -206,6 +207,11 @@ void TestFixture::assertThrowFail(const char *filename, unsigned int linenr) con
206207
}
207208
}
208209

210+
void TestFixture::complainMissingLib(const char* libname) const
211+
{
212+
missingLibs.insert(libname);
213+
}
214+
209215
void TestFixture::run(const std::string &str)
210216
{
211217
testToRun = str;
@@ -261,6 +267,13 @@ std::size_t TestFixture::runTests(const options& args)
261267

262268
std::cerr << "Tests failed: " << fails_counter << std::endl << std::endl;
263269
std::cerr << errmsg.str();
270+
271+
if (!missingLibs.empty()) {
272+
std::cerr << "Missing libraries: ";
273+
for (std::set<std::string>::const_iterator i = missingLibs.cbegin(); i != missingLibs.cend(); ++i)
274+
std::cerr << *i << " ";
275+
std::cerr << std::endl << std::endl;
276+
}
264277
std::cerr.flush();
265278
return fails_counter;
266279
}

test/testsuite.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
#define testsuiteH
2222

2323
#include <sstream>
24+
#include <set>
2425
#include "errorlogger.h"
2526
#include "redirect.h"
27+
#include "library.h"
2628

2729
class options;
2830

@@ -33,8 +35,10 @@ class TestFixture : public ErrorLogger {
3335
static std::size_t fails_counter;
3436
static std::size_t todos_counter;
3537
static std::size_t succeeded_todos_counter;
38+
static std::set<std::string> missingLibs;
3639

3740
protected:
41+
Library _lib;
3842
std::string classname;
3943
std::string testToRun;
4044
bool gcc_style_errors;
@@ -57,6 +61,7 @@ class TestFixture : public ErrorLogger {
5761
void todoAssertEquals(const char *filename, unsigned int linenr, long long wanted,
5862
long long current, long long actual) const;
5963
void assertThrowFail(const char *filename, unsigned int linenr) const;
64+
void complainMissingLib(const char* libname) const;
6065
void processOptions(const options& args);
6166
public:
6267
virtual void reportOut(const std::string &outmsg);
@@ -70,7 +75,7 @@ class TestFixture : public ErrorLogger {
7075
static std::size_t runTests(const options& args);
7176
};
7277

73-
#define TEST_CASE( NAME ) if ( runTest(#NAME) ) { currentTest = classname + "::" + #NAME; if (quiet_tests) { REDIRECT; NAME(); } else { NAME ();} }
78+
#define TEST_CASE( NAME ) if ( runTest(#NAME) ) { _lib = Library(); currentTest = classname + "::" + #NAME; if (quiet_tests) { REDIRECT; NAME(); } else { NAME ();} }
7479
#define ASSERT( CONDITION ) assert_(__FILE__, __LINE__, CONDITION)
7580
#define ASSERT_EQUALS( EXPECTED , ACTUAL ) assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL)
7681
#define ASSERT_EQUALS_DOUBLE( EXPECTED , ACTUAL ) assertEqualsDouble(__FILE__, __LINE__, EXPECTED, ACTUAL)
@@ -79,4 +84,11 @@ class TestFixture : public ErrorLogger {
7984
#define TODO_ASSERT_EQUALS( WANTED , CURRENT , ACTUAL ) todoAssertEquals(__FILE__, __LINE__, WANTED, CURRENT, ACTUAL)
8085
#define REGISTER_TEST( CLASSNAME ) namespace { CLASSNAME instance; }
8186

87+
#ifdef _WIN32
88+
#define REQUIRE_LIB( NAME ) { if (!_lib.load("./testrunner", "../cfg/" NAME) && !_lib.load("./testrunner", "cfg/" NAME)) { complainMissingLib(NAME); return; } }
89+
#else
90+
#define REQUIRE_LIB( NAME ) { if (!_lib.load("./testrunner", "cfg/" NAME)) { complainMissingLib(NAME); return; } }
91+
#endif
92+
#define LOAD_LIB( NAME ) { REQUIRE_LIB(NAME); }
93+
8294
#endif

0 commit comments

Comments
 (0)