Skip to content

Commit bd9f523

Browse files
chrchr-githubchrchr-github
authored
Fix #10179 FP divideSizeof with dereferenced pointer-to-pointer (cppcheck-opensource#3786)
1 parent aa7a1f2 commit bd9f523

4 files changed

Lines changed: 71 additions & 14 deletions

File tree

‎lib/checksizeof.cpp‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -373,16 +373,29 @@ void CheckSizeof::suspiciousSizeofCalculation()
373373
if (!mSettings->severity.isEnabled(Severity::warning) || !mSettings->certainty.isEnabled(Certainty::inconclusive))
374374
return;
375375

376-
// TODO: Use AST here. This should be possible as soon as sizeof without brackets is correctly parsed
377376
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
378377
if (Token::simpleMatch(tok, "sizeof (")) {
379-
const Token* const end = tok->linkAt(1);
380-
const Variable* var = end->previous()->variable();
381-
if (end->strAt(-1) == "*" || (var && var->isPointer() && !var->isArray())) {
382-
if (end->strAt(1) == "/")
383-
divideSizeofError(tok);
384-
} else if (Token::simpleMatch(end, ") * sizeof") && end->next()->astOperand1() == tok->next())
385-
multiplySizeofError(tok);
378+
const Token* lPar = tok->astParent();
379+
if (lPar && lPar->str() == "(") {
380+
const Token* const rPar = lPar->link();
381+
const Token* varTok = rPar->previous();
382+
while (varTok && varTok->str() == "]")
383+
varTok = varTok->link()->previous();
384+
if (lPar->astParent() && lPar->astParent()->str() == "/") {
385+
const Variable* var = varTok ? varTok->variable() : nullptr;
386+
if (var && var->isPointer() && !var->isArray() && !(lPar->astOperand2() && lPar->astOperand2()->str() == "*"))
387+
divideSizeofError(tok);
388+
else if (varTok && varTok->str() == "*") {
389+
const Token* arrTok = lPar->astParent()->astOperand1();
390+
arrTok = arrTok ? arrTok->next() : nullptr;
391+
var = arrTok ? arrTok->variable() : nullptr;
392+
if (var && var->isPointer() && !var->isArray())
393+
divideSizeofError(tok);
394+
}
395+
}
396+
else if (Token::simpleMatch(rPar, ") * sizeof") && rPar->next()->astOperand1() == tok->next())
397+
multiplySizeofError(tok);
398+
}
386399
}
387400
}
388401
}

‎test/testsizeof.cpp‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,35 @@ class TestSizeof : public TestFixture {
427427
" return (end - source) / sizeof(encode_block_type) * sizeof(encode_block_type);\n"
428428
"}");
429429
ASSERT_EQUALS("", errout.str());
430+
431+
check("struct S { T* t; };\n" // #10179
432+
"int f(S* s) {\n"
433+
" return g(sizeof(*s->t) / 4);\n"
434+
"}\n");
435+
ASSERT_EQUALS("", errout.str());
436+
437+
check("void f() {\n"
438+
" const char* a[N];\n"
439+
" for (int i = 0; i < (int)(sizeof(a) / sizeof(char*)); i++) {}\n"
440+
"}\n");
441+
ASSERT_EQUALS("", errout.str());
442+
443+
check("int f(const S& s) {\n"
444+
" int** p;\n"
445+
" return sizeof(p[0]) / 4;\n"
446+
"}\n");
447+
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Division of result of sizeof() on pointer type.\n", errout.str());
448+
449+
check("struct S {\n"
450+
" unsigned char* s;\n"
451+
"};\n"
452+
"struct T {\n"
453+
" S s[38];\n"
454+
"};\n"
455+
"void f(T* t) {\n"
456+
" for (size_t i = 0; i < sizeof(t->s) / sizeof(t->s[0]); i++) {}\n"
457+
"}\n");
458+
ASSERT_EQUALS("", errout.str());
430459
}
431460

432461
void checkPointerSizeof() {
@@ -722,7 +751,9 @@ class TestSizeof : public TestFixture {
722751
check("void foo(memoryMapEntry_t* entry, memoryMapEntry_t* memoryMapEnd) {\n"
723752
" memmove(entry, entry + 1, (memoryMapEnd - entry) / sizeof(entry));\n"
724753
"}");
725-
ASSERT_EQUALS("[test.cpp:2]: (warning) Division by result of sizeof(). memmove() expects a size in bytes, did you intend to multiply instead?\n", errout.str());
754+
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Division of result of sizeof() on pointer type.\n"
755+
"[test.cpp:2]: (warning) Division by result of sizeof(). memmove() expects a size in bytes, did you intend to multiply instead?\n",
756+
errout.str());
726757

727758
check("Foo* allocFoo(int num) {\n"
728759
" return malloc(num / sizeof(Foo));\n"

‎tools/donate_cpu_lib.py‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,13 @@ def compile_version(cppcheck_path, jobs):
125125
def compile_cppcheck(cppcheck_path, jobs):
126126
print('Compiling {}'.format(os.path.basename(cppcheck_path)))
127127
try:
128-
subprocess.check_call(['make', jobs, 'MATCHCOMPILER=yes', 'CXXFLAGS=-O2 -g -w'], cwd=cppcheck_path)
129-
subprocess.check_call([os.path.join(cppcheck_path, 'cppcheck'), '--version'], cwd=cppcheck_path)
128+
os.chdir(cppcheck_path)
129+
if sys.platform == 'win32':
130+
subprocess.call(['MSBuild.exe', cppcheck_path + '/cppcheck.sln', '/property:Configuration=Release', '/property:Platform=x64'])
131+
subprocess.call([cppcheck_path + '/bin/cppcheck.exe', '--version'])
132+
else:
133+
subprocess.check_call(['make', jobs, 'MATCHCOMPILER=yes', 'CXXFLAGS=-O2 -g -w'], cwd=cppcheck_path)
134+
subprocess.check_call([os.path.join(cppcheck_path, 'cppcheck'), '--version'], cwd=cppcheck_path)
130135
except:
131136
return False
132137
return True
@@ -279,7 +284,10 @@ def __run_command(cmd, print_cmd=True):
279284
print(cmd)
280285
start_time = time.time()
281286
comm = None
282-
p = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, stderr=subprocess.PIPE, preexec_fn=os.setsid)
287+
if sys.platform == 'win32':
288+
p = subprocess.Popen(shlex.split(cmd, comments=False, posix=False), stdout=subprocess.PIPE, stderr=subprocess.PIPE)
289+
else:
290+
p = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, stderr=subprocess.PIPE, preexec_fn=os.setsid)
283291
try:
284292
comm = p.communicate(timeout=CPPCHECK_TIMEOUT)
285293
return_code = p.returncode
@@ -323,8 +331,12 @@ def scan_package(work_path, cppcheck_path, jobs, libraries, capture_callstack=Tr
323331
options = libs + ' --showtime=top5 --check-library --inconclusive --enable=style,information --template=daca2'
324332
options += ' -D__GNUC__ --platform=unix64'
325333
options += ' -rp={}'.format(dir_to_scan)
326-
cppcheck_cmd = os.path.join(cppcheck_path, 'cppcheck') + ' ' + options
327-
cmd = 'nice ' + cppcheck_cmd + ' ' + jobs + ' ' + dir_to_scan
334+
if sys.platform == 'win32':
335+
cppcheck_cmd = cppcheck_path + '/bin/cppcheck.exe ' + options
336+
cmd = cppcheck_cmd + ' ' + jobs + ' ' + dir_to_scan
337+
else:
338+
cppcheck_cmd = os.path.join(cppcheck_path, 'cppcheck') + ' ' + options
339+
cmd = 'nice ' + cppcheck_cmd + ' ' + jobs + ' ' + dir_to_scan
328340
returncode, stdout, stderr, elapsed_time = __run_command(cmd)
329341

330342
# collect messages

‎tools/test-my-pr.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# Run this script from your branch with proposed Cppcheck patch to verify your
44
# patch against current main. It will compare output of testing a bunch of
55
# opensource packages
6+
# If running on Windows, make sure that git.exe, wget.exe, and MSBuild.exe are available in PATH
67

78
import donate_cpu_lib as lib
89
import argparse

0 commit comments

Comments
 (0)