From bd9f5231b83ee54a0956e2660687976c91931270 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sun, 10 Apr 2022 22:47:27 +0200 Subject: [PATCH] Fix #10179 FP divideSizeof with dereferenced pointer-to-pointer (#3786) --- lib/checksizeof.cpp | 29 +++++++++++++++++++++-------- test/testsizeof.cpp | 33 ++++++++++++++++++++++++++++++++- tools/donate_cpu_lib.py | 22 +++++++++++++++++----- tools/test-my-pr.py | 1 + 4 files changed, 71 insertions(+), 14 deletions(-) diff --git a/lib/checksizeof.cpp b/lib/checksizeof.cpp index 71b89d502..ed9e8099f 100644 --- a/lib/checksizeof.cpp +++ b/lib/checksizeof.cpp @@ -373,16 +373,29 @@ void CheckSizeof::suspiciousSizeofCalculation() if (!mSettings->severity.isEnabled(Severity::warning) || !mSettings->certainty.isEnabled(Certainty::inconclusive)) return; - // TODO: Use AST here. This should be possible as soon as sizeof without brackets is correctly parsed for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { if (Token::simpleMatch(tok, "sizeof (")) { - const Token* const end = tok->linkAt(1); - const Variable* var = end->previous()->variable(); - if (end->strAt(-1) == "*" || (var && var->isPointer() && !var->isArray())) { - if (end->strAt(1) == "/") - divideSizeofError(tok); - } else if (Token::simpleMatch(end, ") * sizeof") && end->next()->astOperand1() == tok->next()) - multiplySizeofError(tok); + const Token* lPar = tok->astParent(); + if (lPar && lPar->str() == "(") { + const Token* const rPar = lPar->link(); + const Token* varTok = rPar->previous(); + while (varTok && varTok->str() == "]") + varTok = varTok->link()->previous(); + if (lPar->astParent() && lPar->astParent()->str() == "/") { + const Variable* var = varTok ? varTok->variable() : nullptr; + if (var && var->isPointer() && !var->isArray() && !(lPar->astOperand2() && lPar->astOperand2()->str() == "*")) + divideSizeofError(tok); + else if (varTok && varTok->str() == "*") { + const Token* arrTok = lPar->astParent()->astOperand1(); + arrTok = arrTok ? arrTok->next() : nullptr; + var = arrTok ? arrTok->variable() : nullptr; + if (var && var->isPointer() && !var->isArray()) + divideSizeofError(tok); + } + } + else if (Token::simpleMatch(rPar, ") * sizeof") && rPar->next()->astOperand1() == tok->next()) + multiplySizeofError(tok); + } } } } diff --git a/test/testsizeof.cpp b/test/testsizeof.cpp index a4aa81c89..4f4602439 100644 --- a/test/testsizeof.cpp +++ b/test/testsizeof.cpp @@ -427,6 +427,35 @@ private: " return (end - source) / sizeof(encode_block_type) * sizeof(encode_block_type);\n" "}"); ASSERT_EQUALS("", errout.str()); + + check("struct S { T* t; };\n" // #10179 + "int f(S* s) {\n" + " return g(sizeof(*s->t) / 4);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " const char* a[N];\n" + " for (int i = 0; i < (int)(sizeof(a) / sizeof(char*)); i++) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(const S& s) {\n" + " int** p;\n" + " return sizeof(p[0]) / 4;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Division of result of sizeof() on pointer type.\n", errout.str()); + + check("struct S {\n" + " unsigned char* s;\n" + "};\n" + "struct T {\n" + " S s[38];\n" + "};\n" + "void f(T* t) {\n" + " for (size_t i = 0; i < sizeof(t->s) / sizeof(t->s[0]); i++) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void checkPointerSizeof() { @@ -722,7 +751,9 @@ private: check("void foo(memoryMapEntry_t* entry, memoryMapEntry_t* memoryMapEnd) {\n" " memmove(entry, entry + 1, (memoryMapEnd - entry) / sizeof(entry));\n" "}"); - 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()); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Division of result of sizeof() on pointer type.\n" + "[test.cpp:2]: (warning) Division by result of sizeof(). memmove() expects a size in bytes, did you intend to multiply instead?\n", + errout.str()); check("Foo* allocFoo(int num) {\n" " return malloc(num / sizeof(Foo));\n" diff --git a/tools/donate_cpu_lib.py b/tools/donate_cpu_lib.py index 246f4ecbb..e364e4a16 100644 --- a/tools/donate_cpu_lib.py +++ b/tools/donate_cpu_lib.py @@ -125,8 +125,13 @@ def compile_version(cppcheck_path, jobs): def compile_cppcheck(cppcheck_path, jobs): print('Compiling {}'.format(os.path.basename(cppcheck_path))) try: - subprocess.check_call(['make', jobs, 'MATCHCOMPILER=yes', 'CXXFLAGS=-O2 -g -w'], cwd=cppcheck_path) - subprocess.check_call([os.path.join(cppcheck_path, 'cppcheck'), '--version'], cwd=cppcheck_path) + os.chdir(cppcheck_path) + if sys.platform == 'win32': + subprocess.call(['MSBuild.exe', cppcheck_path + '/cppcheck.sln', '/property:Configuration=Release', '/property:Platform=x64']) + subprocess.call([cppcheck_path + '/bin/cppcheck.exe', '--version']) + else: + subprocess.check_call(['make', jobs, 'MATCHCOMPILER=yes', 'CXXFLAGS=-O2 -g -w'], cwd=cppcheck_path) + subprocess.check_call([os.path.join(cppcheck_path, 'cppcheck'), '--version'], cwd=cppcheck_path) except: return False return True @@ -279,7 +284,10 @@ def __run_command(cmd, print_cmd=True): print(cmd) start_time = time.time() comm = None - p = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, stderr=subprocess.PIPE, preexec_fn=os.setsid) + if sys.platform == 'win32': + p = subprocess.Popen(shlex.split(cmd, comments=False, posix=False), stdout=subprocess.PIPE, stderr=subprocess.PIPE) + else: + p = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, stderr=subprocess.PIPE, preexec_fn=os.setsid) try: comm = p.communicate(timeout=CPPCHECK_TIMEOUT) return_code = p.returncode @@ -323,8 +331,12 @@ def scan_package(work_path, cppcheck_path, jobs, libraries, capture_callstack=Tr options = libs + ' --showtime=top5 --check-library --inconclusive --enable=style,information --template=daca2' options += ' -D__GNUC__ --platform=unix64' options += ' -rp={}'.format(dir_to_scan) - cppcheck_cmd = os.path.join(cppcheck_path, 'cppcheck') + ' ' + options - cmd = 'nice ' + cppcheck_cmd + ' ' + jobs + ' ' + dir_to_scan + if sys.platform == 'win32': + cppcheck_cmd = cppcheck_path + '/bin/cppcheck.exe ' + options + cmd = cppcheck_cmd + ' ' + jobs + ' ' + dir_to_scan + else: + cppcheck_cmd = os.path.join(cppcheck_path, 'cppcheck') + ' ' + options + cmd = 'nice ' + cppcheck_cmd + ' ' + jobs + ' ' + dir_to_scan returncode, stdout, stderr, elapsed_time = __run_command(cmd) # collect messages diff --git a/tools/test-my-pr.py b/tools/test-my-pr.py index f85d2356f..dca0f062f 100755 --- a/tools/test-my-pr.py +++ b/tools/test-my-pr.py @@ -3,6 +3,7 @@ # Run this script from your branch with proposed Cppcheck patch to verify your # patch against current main. It will compare output of testing a bunch of # opensource packages +# If running on Windows, make sure that git.exe, wget.exe, and MSBuild.exe are available in PATH import donate_cpu_lib as lib import argparse