Skip to content

Commit 1f0ab48

Browse files
chrchr-githubchrchr-github
authored
Partial fix for #12468 Library functions not handled in assertWithSideEffect check (#6364)
1 parent e2fa3cb commit 1f0ab48

9 files changed

Lines changed: 52 additions & 5 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ $(libcppdir)/check.o: lib/check.cpp lib/addoninfo.h lib/check.h lib/color.h lib/
484484
$(libcppdir)/check64bit.o: lib/check64bit.cpp lib/addoninfo.h lib/check.h lib/check64bit.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h
485485
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/check64bit.cpp
486486

487-
$(libcppdir)/checkassert.o: lib/checkassert.cpp lib/addoninfo.h lib/check.h lib/checkassert.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h
487+
$(libcppdir)/checkassert.o: lib/checkassert.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkassert.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h
488488
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkassert.cpp
489489

490490
$(libcppdir)/checkautovariables.o: lib/checkautovariables.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkautovariables.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h

‎cfg/gtk.cfg‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4112,6 +4112,7 @@
41124112
<!-- int g_strcmp0 (const char *str1, const char *str2); -->
41134113
<function name="g_strcmp0">
41144114
<leak-ignore/>
4115+
<pure/>
41154116
<noreturn>false</noreturn>
41164117
<returnValue type="int"/>
41174118
<use-retval/>

‎lib/checkassert.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
#include "checkassert.h"
2424

25+
#include "astutils.h"
2526
#include "errortypes.h"
2627
#include "settings.h"
2728
#include "symboldatabase.h"
@@ -57,8 +58,20 @@ void CheckAssert::assertWithSideEffects()
5758

5859
checkVariableAssignment(tmp, tok->scope());
5960

60-
if (tmp->tokType() != Token::eFunction)
61+
if (tmp->tokType() != Token::eFunction) {
62+
if (const Library::Function* f = mSettings->library.getFunction(tmp)) {
63+
if (f->isconst || f->ispure)
64+
continue;
65+
if (Library::getContainerYield(tmp->next()) != Library::Container::Yield::NO_YIELD) // bailout, assume read access
66+
continue;
67+
if (std::any_of(f->argumentChecks.begin(), f->argumentChecks.end(), [](const std::pair<int, Library::ArgumentChecks>& ac) {
68+
return ac.second.iteratorInfo.container > 0; // bailout, takes iterators -> assume read access
69+
}))
70+
continue;
71+
sideEffectInAssertError(tmp, mSettings->library.getFunctionName(tmp));
72+
}
6173
continue;
74+
}
6275

6376
const Function* f = tmp->function();
6477
const Scope* scope = f->functionScope;

‎lib/library.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,6 +1266,22 @@ bool Library::isContainerYield(const Token * const cond, Library::Container::Yie
12661266
return false;
12671267
}
12681268

1269+
Library::Container::Yield Library::getContainerYield(const Token* const cond)
1270+
{
1271+
if (Token::simpleMatch(cond, "(")) {
1272+
const Token* tok = cond->astOperand1();
1273+
if (tok && tok->str() == ".") {
1274+
if (tok->astOperand1() && tok->astOperand1()->valueType()) {
1275+
if (const Library::Container *container = tok->astOperand1()->valueType()->container) {
1276+
if (tok->astOperand2())
1277+
return container->getYield(tok->astOperand2()->str());
1278+
}
1279+
}
1280+
}
1281+
}
1282+
return Library::Container::Yield::NO_YIELD;
1283+
}
1284+
12691285
// returns true if ftok is not a library function
12701286
bool Library::isNotLibraryFunction(const Token *ftok) const
12711287
{

‎lib/library.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,9 @@ class CPPCHECKLIB Library {
413413
const Token* getContainerFromYield(const Token* tok, Container::Yield yield) const;
414414
const Token* getContainerFromAction(const Token* tok, Container::Action action) const;
415415

416+
static bool isContainerYield(const Token* const cond, Library::Container::Yield y, const std::string& fallback = emptyString);
417+
static Library::Container::Yield getContainerYield(const Token* const cond);
418+
416419
bool isreflection(const std::string &token) const {
417420
return mReflection.find(token) != mReflection.end();
418421
}
@@ -496,8 +499,6 @@ class CPPCHECKLIB Library {
496499
*/
497500
std::string getFunctionName(const Token *ftok) const;
498501

499-
static bool isContainerYield(const Token * const cond, Library::Container::Yield y, const std::string& fallback=emptyString);
500-
501502
/** Suppress/check a type */
502503
enum class TypeCheck { def,
503504
check,

‎oss-fuzz/Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ $(libcppdir)/check.o: ../lib/check.cpp ../lib/addoninfo.h ../lib/check.h ../lib/
163163
$(libcppdir)/check64bit.o: ../lib/check64bit.cpp ../lib/addoninfo.h ../lib/check.h ../lib/check64bit.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h
164164
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/check64bit.cpp
165165

166-
$(libcppdir)/checkassert.o: ../lib/checkassert.cpp ../lib/addoninfo.h ../lib/check.h ../lib/checkassert.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h
166+
$(libcppdir)/checkassert.o: ../lib/checkassert.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkassert.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h
167167
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkassert.cpp
168168

169169
$(libcppdir)/checkautovariables.o: ../lib/checkautovariables.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkautovariables.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h

‎test/cfg/gtk.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ void validCode(int argInt, GHashTableIter * hash_table_iter, GHashTable * hash_t
6868
// NULL is handled graciously
6969
char* str = g_strdup(NULL);
7070
if (g_strcmp0(str, NULL) || g_strcmp0(NULL, str))
71+
// cppcheck-suppress valueFlowBailout // TODO: caused by <pure/>?
7172
printf("%s", str);
7273
g_free(str);
7374
}

‎test/cfg/std.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5016,3 +5016,9 @@ void eraseIteratorOutOfBounds_std_deque(std::deque<int>& x) // #8690
50165016
// cppcheck-suppress eraseIteratorOutOfBounds
50175017
x.erase(x.end());
50185018
}
5019+
5020+
void assertWithSideEffect_system()
5021+
{
5022+
// cppcheck-suppress [assertWithSideEffect,checkLibraryNoReturn] // TODO: #8329
5023+
assert(std::system("abc"));
5024+
}

‎test/testassert.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,15 @@ class TestAssert : public TestFixture {
134134
" assert(empty());\n"
135135
"}");
136136
ASSERT_EQUALS("", errout_str());
137+
138+
check("struct S {\n" // #4811
139+
" void f() const;\n"
140+
" bool g(std::ostream& os = std::cerr) const;\n"
141+
"};\n"
142+
"void S::f() const {\n"
143+
" assert(g());\n"
144+
"}\n");
145+
ASSERT_EQUALS("", errout_str());
137146
}
138147

139148
void memberFunctionCallInAssert() {

0 commit comments

Comments
 (0)