Skip to content

Commit 33ad30f

Browse files
pfultz2pfultz2
authored
Fix 10617, 9824: conditions in expanded macro (#3578)
1 parent 085d25f commit 33ad30f

4 files changed

Lines changed: 78 additions & 5 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ test/testbool.o: test/testbool.cpp lib/astutils.h lib/check.h lib/checkbool.h li
621621
test/testboost.o: test/testboost.cpp lib/astutils.h lib/check.h lib/checkboost.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h
622622
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testboost.o test/testboost.cpp
623623

624-
test/testbufferoverrun.o: test/testbufferoverrun.cpp externals/tinyxml2/tinyxml2.h lib/astutils.h lib/check.h lib/checkbufferoverrun.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h
624+
test/testbufferoverrun.o: test/testbufferoverrun.cpp externals/simplecpp/simplecpp.h externals/tinyxml2/tinyxml2.h lib/astutils.h lib/check.h lib/checkbufferoverrun.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h
625625
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o test/testbufferoverrun.o test/testbufferoverrun.cpp
626626

627627
test/testbughuntingchecks.o: test/testbughuntingchecks.cpp lib/astutils.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/exprengine.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h test/testsuite.h

‎lib/valueflow.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5224,8 +5224,12 @@ struct ConditionHandler {
52245224
if (!top)
52255225
return;
52265226

5227-
if (top->previous()->isExpandedMacro())
5228-
return;
5227+
if (top->previous()->isExpandedMacro()) {
5228+
for (std::list<ValueFlow::Value>* values : {&thenValues, &elseValues}) {
5229+
for (ValueFlow::Value& v : *values)
5230+
v.macro = true;
5231+
}
5232+
}
52295233

52305234
if (!Token::Match(top->previous(), "if|while|for ("))
52315235
return;
@@ -7545,6 +7549,7 @@ ValueFlow::Value::Value(const Token* c, long long val, Bound b)
75457549
varId(0),
75467550
safe(false),
75477551
conditional(false),
7552+
macro(false),
75487553
defaultArg(false),
75497554
indirect(0),
75507555
path(0),

‎lib/valueflow.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ namespace ValueFlow {
9898
varId(0U),
9999
safe(false),
100100
conditional(false),
101+
macro(false),
101102
defaultArg(false),
102103
indirect(0),
103104
path(0),
@@ -346,6 +347,9 @@ namespace ValueFlow {
346347
/** Conditional value */
347348
bool conditional;
348349

350+
/** Value is is from an expanded macro */
351+
bool macro;
352+
349353
/** Is this value passed as default parameter to the function? */
350354
bool defaultArg;
351355

‎test/testbufferoverrun.cpp‎

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@
2222
#include "config.h"
2323
#include "ctu.h"
2424
#include "library.h"
25+
#include "preprocessor.h"
2526
#include "settings.h"
2627
#include "testsuite.h"
2728
#include "tokenize.h"
2829

29-
#include <tinyxml2.h>
3030
#include <list>
31+
#include <simplecpp.h>
3132
#include <string>
32-
33+
#include <tinyxml2.h>
3334

3435
class TestBufferOverrun : public TestFixture {
3536
public:
@@ -67,6 +68,45 @@ class TestBufferOverrun : public TestFixture {
6768
checkBufferOverrun.runChecks(&tokenizer, &settings, this);
6869
}
6970

71+
void checkP(const char code[], const char* filename = "test.cpp")
72+
{
73+
// Clear the error buffer..
74+
errout.str("");
75+
76+
Settings* settings = &settings0;
77+
settings->severity.enable(Severity::style);
78+
settings->severity.enable(Severity::warning);
79+
settings->severity.enable(Severity::portability);
80+
settings->severity.enable(Severity::performance);
81+
settings->standards.c = Standards::CLatest;
82+
settings->standards.cpp = Standards::CPPLatest;
83+
settings->certainty.enable(Certainty::inconclusive);
84+
settings->certainty.disable(Certainty::experimental);
85+
86+
// Raw tokens..
87+
std::vector<std::string> files(1, filename);
88+
std::istringstream istr(code);
89+
const simplecpp::TokenList tokens1(istr, files, files[0]);
90+
91+
// Preprocess..
92+
simplecpp::TokenList tokens2(files);
93+
std::map<std::string, simplecpp::TokenList*> filedata;
94+
simplecpp::preprocess(tokens2, tokens1, files, filedata, simplecpp::DUI());
95+
96+
Preprocessor preprocessor(*settings, nullptr);
97+
preprocessor.setDirectives(tokens1);
98+
99+
// Tokenizer..
100+
Tokenizer tokenizer(settings, this);
101+
tokenizer.createTokens(std::move(tokens2));
102+
tokenizer.simplifyTokens1("");
103+
tokenizer.setPreprocessor(&preprocessor);
104+
105+
// Check for buffer overruns..
106+
CheckBufferOverrun checkBufferOverrun(&tokenizer, settings, this);
107+
checkBufferOverrun.runChecks(&tokenizer, settings, this);
108+
}
109+
70110
void run() OVERRIDE {
71111
LOAD_LIB_2(settings0.library, "std.cfg");
72112

@@ -136,6 +176,7 @@ class TestBufferOverrun : public TestFixture {
136176
TEST_CASE(array_index_57); // #10023
137177
TEST_CASE(array_index_58); // #7524
138178
TEST_CASE(array_index_59); // #10413
179+
TEST_CASE(array_index_60); // #10617, #9824
139180
TEST_CASE(array_index_multidim);
140181
TEST_CASE(array_index_switch_in_for);
141182
TEST_CASE(array_index_for_in_for); // FP: #2634
@@ -1665,6 +1706,29 @@ class TestBufferOverrun : public TestFixture {
16651706
ASSERT_EQUALS("", errout.str());
16661707
}
16671708

1709+
void array_index_60()
1710+
{
1711+
checkP("#define CKR(B) if (!(B)) { return -1; }\n"
1712+
"int f(int i) {\n"
1713+
" const int A[3] = {};\n"
1714+
" CKR(i < 3);\n"
1715+
" if (i > 0)\n"
1716+
" i = A[i];\n"
1717+
" return i;\n"
1718+
"}\n");
1719+
ASSERT_EQUALS("", errout.str());
1720+
1721+
checkP("#define ASSERT(expression, action) if (expression) {action;}\n"
1722+
"int array[5];\n"
1723+
"void func (int index) {\n"
1724+
" ASSERT(index > 5, return);\n"
1725+
" array[index]++;\n"
1726+
"}\n");
1727+
ASSERT_EQUALS(
1728+
"[test.cpp:4] -> [test.cpp:5]: (warning) Either the condition 'index>5' is redundant or the array 'array[5]' is accessed at index 5, which is out of bounds.\n",
1729+
errout.str());
1730+
}
1731+
16681732
void array_index_multidim() {
16691733
check("void f()\n"
16701734
"{\n"

0 commit comments

Comments
 (0)