Skip to content

Commit 1668b0b

Browse files
francois-berderfrancois-berder
authored
Fix #11311 Do not search for null pointer in dead code (#6442)
1 parent d19d8e8 commit 1668b0b

6 files changed

Lines changed: 124 additions & 36 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ $(libcppdir)/checkleakautovar.o: lib/checkleakautovar.cpp lib/addoninfo.h lib/as
533533
$(libcppdir)/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkmemoryleak.h lib/color.h lib/config.h lib/errorlogger.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
534534
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkmemoryleak.cpp
535535

536-
$(libcppdir)/checknullpointer.o: lib/checknullpointer.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.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
536+
$(libcppdir)/checknullpointer.o: lib/checknullpointer.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/findtoken.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
537537
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp
538538

539539
$(libcppdir)/checkother.o: lib/checkother.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkother.h lib/config.h lib/errortypes.h lib/fwdanalysis.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

‎lib/checknullpointer.cpp‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "ctu.h"
2525
#include "errorlogger.h"
2626
#include "errortypes.h"
27+
#include "findtoken.h"
2728
#include "library.h"
2829
#include "mathlib.h"
2930
#include "settings.h"
@@ -279,47 +280,54 @@ static bool isNullablePointer(const Token* tok)
279280
return false;
280281
}
281282

282-
void CheckNullPointer::nullPointerByDeRefAndChec()
283+
void CheckNullPointer::nullPointerByDeRefAndCheck()
283284
{
284285
const bool printInconclusive = (mSettings->certainty.isEnabled(Certainty::inconclusive));
285286

286-
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
287-
if (isUnevaluated(tok)) {
288-
tok = tok->linkAt(1);
289-
continue;
290-
}
287+
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
288+
for (const Scope * scope : symbolDatabase->functionScopes) {
289+
auto pred = [printInconclusive](const Token* tok) -> bool {
290+
if (!tok)
291+
return false;
291292

292-
if (Token::Match(tok, "%num%|%char%|%str%"))
293-
continue;
293+
if (Token::Match(tok, "%num%|%char%|%str%"))
294+
return false;
294295

295-
if (!isNullablePointer(tok) ||
296-
(tok->str() == "." && isNullablePointer(tok->astOperand2()) && tok->astOperand2()->getValue(0))) // avoid duplicate warning
297-
continue;
296+
if (!isNullablePointer(tok) ||
297+
(tok->str() == "." && isNullablePointer(tok->astOperand2()) && tok->astOperand2()->getValue(0))) // avoid duplicate warning
298+
return false;
298299

299-
// Can pointer be NULL?
300-
const ValueFlow::Value *value = tok->getValue(0);
301-
if (!value)
302-
continue;
300+
// Can pointer be NULL?
301+
const ValueFlow::Value *value = tok->getValue(0);
302+
if (!value)
303+
return false;
303304

304-
if (!printInconclusive && value->isInconclusive())
305-
continue;
305+
if (!printInconclusive && value->isInconclusive())
306+
return false;
306307

307-
// Pointer dereference.
308-
bool unknown = false;
309-
if (!isPointerDeRef(tok,unknown)) {
310-
if (unknown)
311-
nullPointerError(tok, tok->expressionString(), value, true);
312-
continue;
313-
}
308+
return true;
309+
};
310+
std::vector<const Token *> tokens = findTokensSkipDeadAndUnevaluatedCode(mSettings->library, scope->bodyStart, scope->bodyEnd, pred);
311+
for (const Token *tok : tokens) {
312+
const ValueFlow::Value *value = tok->getValue(0);
313+
314+
// Pointer dereference.
315+
bool unknown = false;
316+
if (!isPointerDeRef(tok, unknown)) {
317+
if (unknown)
318+
nullPointerError(tok, tok->expressionString(), value, true);
319+
continue;
320+
}
314321

315-
nullPointerError(tok, tok->expressionString(), value, value->isInconclusive());
322+
nullPointerError(tok, tok->expressionString(), value, value->isInconclusive());
323+
}
316324
}
317325
}
318326

319327
void CheckNullPointer::nullPointer()
320328
{
321329
logChecker("CheckNullPointer::nullPointer");
322-
nullPointerByDeRefAndChec();
330+
nullPointerByDeRefAndCheck();
323331
}
324332

325333
namespace {

‎lib/checknullpointer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class CPPCHECKLIB CheckNullPointer : public Check {
137137
* @brief Does one part of the check for nullPointer().
138138
* Dereferencing a pointer and then checking if it's NULL..
139139
*/
140-
void nullPointerByDeRefAndChec();
140+
void nullPointerByDeRefAndCheck();
141141

142142
/** undefined null pointer arithmetic */
143143
void arithmetic();

‎lib/findtoken.h‎

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
8484
const Token* end,
8585
const Predicate& pred,
8686
Found found,
87-
const Evaluate& evaluate)
87+
const Evaluate& evaluate,
88+
bool skipUnevaluated)
8889
{
8990
for (T* tok = start; precedes(tok, end); tok = tok->next()) {
9091
if (pred(tok)) {
@@ -98,7 +99,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
9899
auto result = evaluate(condTok);
99100
if (result.empty())
100101
continue;
101-
if (findTokensSkipDeadCodeImpl(library, tok->next(), tok->linkAt(1), pred, found, evaluate))
102+
if (findTokensSkipDeadCodeImpl(library, tok->next(), tok->linkAt(1), pred, found, evaluate, skipUnevaluated))
102103
return true;
103104
T* thenStart = tok->linkAt(1)->next();
104105
T* elseStart = nullptr;
@@ -108,7 +109,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
108109
int r = result.front();
109110
if (r == 0) {
110111
if (elseStart) {
111-
if (findTokensSkipDeadCodeImpl(library, elseStart, elseStart->link(), pred, found, evaluate))
112+
if (findTokensSkipDeadCodeImpl(library, elseStart, elseStart->link(), pred, found, evaluate, skipUnevaluated))
112113
return true;
113114
if (isReturnScope(elseStart->link(), library))
114115
return true;
@@ -117,7 +118,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
117118
tok = thenStart->link();
118119
}
119120
} else {
120-
if (findTokensSkipDeadCodeImpl(library, thenStart, thenStart->link(), pred, found, evaluate))
121+
if (findTokensSkipDeadCodeImpl(library, thenStart, thenStart->link(), pred, found, evaluate, skipUnevaluated))
121122
return true;
122123
if (isReturnScope(thenStart->link(), library))
123124
return true;
@@ -137,7 +138,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
137138
if (!cond) {
138139
next = colon;
139140
} else {
140-
if (findTokensSkipDeadCodeImpl(library, tok->astParent()->next(), colon, pred, found, evaluate))
141+
if (findTokensSkipDeadCodeImpl(library, tok->astParent()->next(), colon, pred, found, evaluate, skipUnevaluated))
141142
return true;
142143
next = nextAfterAstRightmostLeaf(colon);
143144
}
@@ -164,6 +165,12 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
164165
else
165166
tok = afterCapture;
166167
}
168+
if (skipUnevaluated && isUnevaluated(tok)) {
169+
T *next = tok->linkAt(1);
170+
if (!next)
171+
continue;
172+
tok = next;
173+
}
167174
}
168175
return false;
169176
}
@@ -185,7 +192,8 @@ std::vector<T*> findTokensSkipDeadCode(const Library& library,
185192
result.push_back(tok);
186193
return false;
187194
},
188-
evaluate);
195+
evaluate,
196+
false);
189197
return result;
190198
}
191199

@@ -195,6 +203,35 @@ std::vector<T*> findTokensSkipDeadCode(const Library& library, T* start, const T
195203
return findTokensSkipDeadCode(library, start, end, pred, &evaluateKnownValues);
196204
}
197205

206+
template<class T, class Predicate, class Evaluate, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
207+
std::vector<T*> findTokensSkipDeadAndUnevaluatedCode(const Library& library,
208+
T* start,
209+
const Token* end,
210+
const Predicate& pred,
211+
const Evaluate& evaluate)
212+
{
213+
std::vector<T*> result;
214+
(void)findTokensSkipDeadCodeImpl(
215+
library,
216+
start,
217+
end,
218+
pred,
219+
[&](T* tok) {
220+
result.push_back(tok);
221+
return false;
222+
},
223+
evaluate,
224+
true);
225+
return result;
226+
}
227+
228+
template<class T, class Predicate, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
229+
std::vector<T*> findTokensSkipDeadAndUnevaluatedCode(const Library& library, T* start, const Token* end, const Predicate& pred)
230+
{
231+
return findTokensSkipDeadAndUnevaluatedCode(library, start, end, pred, &evaluateKnownValues);
232+
}
233+
234+
198235
template<class T, class Predicate, class Evaluate, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
199236
T* findTokenSkipDeadCode(const Library& library, T* start, const Token* end, const Predicate& pred, const Evaluate& evaluate)
200237
{
@@ -208,7 +245,8 @@ T* findTokenSkipDeadCode(const Library& library, T* start, const Token* end, con
208245
result = tok;
209246
return true;
210247
},
211-
evaluate);
248+
evaluate,
249+
false);
212250
return result;
213251
}
214252

‎oss-fuzz/Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ $(libcppdir)/checkleakautovar.o: ../lib/checkleakautovar.cpp ../lib/addoninfo.h
212212
$(libcppdir)/checkmemoryleak.o: ../lib/checkmemoryleak.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkmemoryleak.h ../lib/color.h ../lib/config.h ../lib/errorlogger.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
213213
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkmemoryleak.cpp
214214

215-
$(libcppdir)/checknullpointer.o: ../lib/checknullpointer.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checknullpointer.h ../lib/color.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.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
215+
$(libcppdir)/checknullpointer.o: ../lib/checknullpointer.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checknullpointer.h ../lib/color.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/findtoken.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
216216
$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp
217217

218218
$(libcppdir)/checkother.o: ../lib/checkother.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkother.h ../lib/config.h ../lib/errortypes.h ../lib/fwdanalysis.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/testnullpointer.cpp‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ class TestNullPointer : public TestFixture {
158158
TEST_CASE(nullpointer_in_typeid);
159159
TEST_CASE(nullpointer_in_alignof); // #11401
160160
TEST_CASE(nullpointer_in_for_loop);
161+
TEST_CASE(nullpointerDeadCode); // #11311
161162
TEST_CASE(nullpointerDelete);
162163
TEST_CASE(nullpointerSubFunction);
163164
TEST_CASE(nullpointerExit);
@@ -3657,6 +3658,47 @@ class TestNullPointer : public TestFixture {
36573658
ASSERT_EQUALS("", errout_str());
36583659
}
36593660

3661+
void nullpointerDeadCode() {
3662+
// Ticket #11311
3663+
check ("void f() {\n"
3664+
" if (0)\n"
3665+
" *(int *)0 = 1;\n"
3666+
" else\n"
3667+
" ;\n"
3668+
"}\n");
3669+
ASSERT_EQUALS("", errout_str());
3670+
3671+
check ("void f() {\n"
3672+
" if (0)\n"
3673+
" *(int *)0 = 1;\n"
3674+
" else {\n"
3675+
" if (0)\n"
3676+
" *(int *)0 = 2;\n"
3677+
" else\n"
3678+
" ;\n"
3679+
" }\n"
3680+
"}\n");
3681+
ASSERT_EQUALS("", errout_str());
3682+
3683+
check ("void f() {\n"
3684+
" while(0)\n"
3685+
" *(int*)0 = 1;\n"
3686+
" do {\n"
3687+
" } while(0);\n"
3688+
"}\n");
3689+
ASSERT_EQUALS("", errout_str());
3690+
3691+
check ("int f() {\n"
3692+
" return 0 ? *(int*)0 = 1 : 1;\n"
3693+
"}\n");
3694+
ASSERT_EQUALS("", errout_str());
3695+
3696+
check ("int f() {\n"
3697+
" return 1 ? 1 : *(int*)0 = 1;\n"
3698+
"}\n");
3699+
ASSERT_EQUALS("", errout_str());
3700+
}
3701+
36603702
void nullpointerDelete() {
36613703
check("void f() {\n"
36623704
" K *k = getK();\n"

0 commit comments

Comments
 (0)