From 5e59652fd387476ac813911145a7f5391e86bcdd Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Wed, 3 Jan 2024 11:50:28 +0100 Subject: [PATCH 1/3] Add tests for #1644, #3929, #6109 (#5821) --- test/testbufferoverrun.cpp | 13 +++++++++++++ test/testnullpointer.cpp | 9 +++++++++ test/teststring.cpp | 8 ++++++++ 3 files changed, 30 insertions(+) diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 4069a164afc..c655c3d8f7a 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -166,6 +166,7 @@ class TestBufferOverrun : public TestFixture { TEST_CASE(array_index_72); // #11784 TEST_CASE(array_index_73); // #11530 TEST_CASE(array_index_74); // #11088 + TEST_CASE(array_index_75); TEST_CASE(array_index_multidim); TEST_CASE(array_index_switch_in_for); TEST_CASE(array_index_for_in_for); // FP: #2634 @@ -1976,6 +1977,18 @@ class TestBufferOverrun : public TestFixture { ASSERT_EQUALS("", errout.str()); } + // #1644 + void array_index_75() + { + check("void f() {\n" + " char buf[10];\n" + " int i = 10;\n" + " while (i >= 3)\n" + " buf[i--] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Array 'buf[10]' accessed at index 10, which is out of bounds.\n", errout.str()); + } + void array_index_multidim() { check("void f()\n" "{\n" diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index f984b5aace2..217e724057c 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2752,6 +2752,15 @@ class TestNullPointer : public TestFixture { " return 0;\n" "}"); ASSERT_EQUALS("[test.cpp:7]: (error) Null pointer dereference: myNull\n", errout.str()); + + check("struct T { bool g() const; };\n" + "void f(T* p) {\n" + " if (!p)\n" + " return;\n" + " while (p->g())\n" + " p = nullptr;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (warning) Possible null pointer dereference: p\n", errout.str()); } void nullpointer94() // #11040 diff --git a/test/teststring.cpp b/test/teststring.cpp index 8d443e215b4..b4d8d815646 100644 --- a/test/teststring.cpp +++ b/test/teststring.cpp @@ -798,6 +798,14 @@ class TestString : public TestFixture { " ASSERT((void*)(\"def\") == 0);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("class C {\n" // #6109 + " void check(const char code[], bool validate = true, const char* filename = \"test.cpp\");\n" + " void f() {\n" + " check(\"class A;\", \"test.C\");\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:4]: (warning) Conversion of string literal \"test.C\" to bool always evaluates to true.\n", errout.str()); } void deadStrcmp() { From 8261ded47531490266f69ae7d29a769178385683 Mon Sep 17 00:00:00 2001 From: thingsconnected <45596685+thingsconnected@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:00:47 +0100 Subject: [PATCH 2/3] addons/namingng.py: Improve output and unit test. (#5820) For naming issues reported, column was always set to `0`, which is now fixed. Global variable naming errors were reported as "Public member" issues, which is also fixed. The unit test now covers namespaces, class names, public and private member variables. --- addons/cppcheckdata.py | 4 +-- addons/namingng.py | 52 +++++++++++++++--------------- test/cli/test-other.py | 72 +++++++++++++++++++++++++++++++++--------- 3 files changed, 86 insertions(+), 42 deletions(-) diff --git a/addons/cppcheckdata.py b/addons/cppcheckdata.py index ee669b0e420..2fad2c7cfaf 100755 --- a/addons/cppcheckdata.py +++ b/addons/cppcheckdata.py @@ -1619,11 +1619,11 @@ def is_suppressed(location, message, errorId): return True return False -def reportError(location, severity, message, addon, errorId, extra=''): +def reportError(location, severity, message, addon, errorId, extra='', columnOverride=None): if '--cli' in sys.argv: msg = { 'file': location.file, 'linenr': location.linenr, - 'column': location.column, + 'column': location.column if columnOverride is None else columnOverride, 'severity': severity, 'message': message, 'addon': addon, diff --git a/addons/namingng.py b/addons/namingng.py index fb852c1740f..ad922c87079 100755 --- a/addons/namingng.py +++ b/addons/namingng.py @@ -41,14 +41,14 @@ # Auxiliary class class DataStruct: - def __init__(self, file, linenr, string): + def __init__(self, file, linenr, string, column=0): self.file = file self.linenr = linenr self.str = string - self.column = 0 + self.column = column -def reportNamingError(location,message,errorId='namingConvention',severity='style',extra=''): - cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra) +def reportNamingError(location,message,errorId='namingConvention',severity='style',extra='',column=None): + cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra,columnOverride=column) def configError(error,fatal=True): print('config error: %s'%error) @@ -167,8 +167,9 @@ def check_include_guard_name(conf_include_guard,cfg,directive): if len(parts) != 2: msg = 'syntax error' reportNamingError(directive,msg,'syntax') - return None + return None,None guard_name = parts[1] + guard_column = 1+directive.str.find(guard_name) filename = directive.file if conf_include_guard.get('input','path') == 'basename': @@ -188,9 +189,9 @@ def check_include_guard_name(conf_include_guard,cfg,directive): expect_guard_name = conf_include_guard.get('prefix','') + barename + conf_include_guard.get('suffix','') if expect_guard_name != guard_name: msg = 'include guard naming violation; %s != %s'%(guard_name,expect_guard_name) - reportNamingError(directive,msg,'includeGuardName') + reportNamingError(directive,msg,'includeGuardName',column=guard_column) - return guard_name + return guard_name,guard_column def check_include_guard(conf_include_guard,cfg,unguarded_include_files): # Scan for '#ifndef FILE_H' as the first directive, in the first N lines. @@ -200,11 +201,11 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files): # - test whether include guards are in place max_linenr = conf_include_guard.get('max_linenr', 5) - def report(directive,msg,errorId): - reportNamingError(directive,msg,errorId) + def report(directive,msg,errorId,column=0): + reportNamingError(directive,msg,errorId,column=column) - def report_pending_ifndef(directive): - report(directive,'include guard #ifndef is not followed by #define','includeGuardIncomplete') + def report_pending_ifndef(directive,column): + report(directive,'include guard #ifndef is not followed by #define','includeGuardIncomplete',column=column) last_fn = None pending_ifndef = None @@ -212,7 +213,7 @@ def report_pending_ifndef(directive): for directive in cfg.directives: if last_fn != directive.file: if pending_ifndef: - report_pending_ifndef(pending_ifndef) + report_pending_ifndef(pending_ifndef,guard_column) pending_ifndef = None last_fn = directive.file phase = 0 @@ -237,7 +238,7 @@ def report_pending_ifndef(directive): report(directive,'first preprocessor directive should be include guard #ifndef','includeGuardMissing') phase = -1 continue - guard_name = check_include_guard_name(conf_include_guard,cfg,directive) + guard_name,guard_column = check_include_guard_name(conf_include_guard,cfg,directive) if guard_name == None: phase = -1 continue @@ -256,13 +257,13 @@ def report_pending_ifndef(directive): phase = -1 continue if guard_name != parts[1]: - report(directive,'include guard does not guard; %s != %s'%(guard_name,parts[1]),'includeGuardAwayFromDuty',severity='warning') + report(directive,'include guard does not guard; %s != %s'%(guard_name,parts[1]),'includeGuardAwayFromDuty',severity='warning',column=guard_column) unguarded_include_files.remove(directive.file) phase = -1 if pending_ifndef: - report_pending_ifndef(pending_ifndef) + report_pending_ifndef(pending_ifndef,guard_column) def process(dumpfiles, configfile, debugprint=False): conf = loadConfig(configfile) @@ -294,7 +295,7 @@ def process(dumpfiles, configfile, debugprint=False): if conf.namespace: for tk in data.rawTokens: if tk.str == 'namespace': - mockToken = DataStruct(tk.next.file, tk.next.linenr, tk.next.str) + mockToken = DataStruct(tk.next.file, tk.next.linenr, tk.next.str, tk.next.column) msgType = 'Namespace' for exp in conf.namespace: evalExpr(conf.namespace, exp, mockToken, msgType) @@ -333,9 +334,10 @@ def process(dumpfiles, configfile, debugprint=False): reportNamingError(var.typeStartToken, 'Variable ' + var.nameToken.str + - ' violates naming convention') + ' violates naming convention', + column=var.nameToken.column) - mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) + mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column) msgType = 'Variable' for exp in conf.variable: evalExpr(conf.variable, exp, mockToken, msgType) @@ -346,7 +348,7 @@ def process(dumpfiles, configfile, debugprint=False): for var in cfg.variables: if (var.access is None) or var.access != 'Private': continue - mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) + mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column) msgType = 'Private member variable' for exp in conf.private_member: evalExpr(conf.private_member, exp, mockToken, msgType) @@ -356,7 +358,7 @@ def process(dumpfiles, configfile, debugprint=False): for var in cfg.variables: if (var.access is None) or var.access != 'Public': continue - mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) + mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column) msgType = 'Public member variable' for exp in conf.public_member: evalExpr(conf.public_member, exp, mockToken, msgType) @@ -366,8 +368,8 @@ def process(dumpfiles, configfile, debugprint=False): for var in cfg.variables: if (var.access is None) or var.access != 'Global': continue - mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) - msgType = 'Public member variable' + mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column) + msgType = 'Global variable' for exp in conf.global_variable: evalExpr(conf.global_variable, exp, mockToken, msgType) @@ -387,8 +389,8 @@ def process(dumpfiles, configfile, debugprint=False): if retval and retval in conf.function_prefixes: if not token.function.name.startswith(conf.function_prefixes[retval]): - reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention') - mockToken = DataStruct(token.file, token.linenr, token.function.name) + reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention', column=token.column) + mockToken = DataStruct(token.file, token.linenr, token.function.name, token.column) msgType = 'Function' for exp in conf.function_name: evalExpr(conf.function_name, exp, mockToken, msgType) @@ -398,7 +400,7 @@ def process(dumpfiles, configfile, debugprint=False): for fnc in cfg.functions: # Check if it is Constructor/Destructor if fnc.type == 'Constructor' or fnc.type == 'Destructor': - mockToken = DataStruct(fnc.tokenDef.file, fnc.tokenDef.linenr, fnc.name) + mockToken = DataStruct(fnc.tokenDef.file, fnc.tokenDef.linenr, fnc.name, fnc.tokenDef.column) msgType = 'Class ' + fnc.type for exp in conf.class_name: evalExpr(conf.class_name, exp, mockToken, msgType) diff --git a/test/cli/test-other.py b/test/cli/test-other.py index 7a2e8177235..1d67a634ecb 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -343,7 +343,11 @@ def test_addon_namingng(tmpdir): "RE_FILE": [ "[^/]*[a-z][a-z0-9_]*[a-z0-9]\\.c\\Z" ], + "RE_CLASS_NAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], + "RE_NAMESPACE": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], "RE_VARNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], + "RE_PUBLIC_MEMBER_VARIABLE": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], + "RE_PRIVATE_MEMBER_VARIABLE": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], "RE_GLOBAL_VARNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], "RE_FUNCTIONNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], "include_guard": { @@ -384,11 +388,11 @@ def test_addon_namingng(tmpdir): #endif """.format(test_unguarded_include_file)) - test_file_basename = 'test_.c' + test_file_basename = 'test_.cpp' test_file = os.path.join(tmpdir, test_file_basename) with open(test_file, 'wt') as f: f.write(""" -#include "{}" +#include "%s" void invalid_function_(); void _invalid_function(); @@ -403,7 +407,17 @@ def test_addon_namingng(tmpdir): int _invalid_global; static int _invalid_static_global; - """.format(test_include_file_basename)) + +class _clz { +public: + _clz() : _invalid_public(0), _invalid_private(0) { } + int _invalid_public; +private: + char _invalid_private; +}; + +namespace _invalid_namespace { } + """%(test_include_file_basename)) args = ['--addon='+addon_file, '--verbose', '--enable=all', test_file] @@ -417,38 +431,66 @@ def test_addon_namingng(tmpdir): 'Includes:', 'Platform:native' ] - lines = [line for line in stderr.splitlines() if line.strip() != '^' and line != ''] + lines = [line for line in stderr.splitlines() if line != ''] expect = [ '{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_include_file,test_include_file_basename), - '{}:2:0: style: include guard naming violation; TEST_H != _TEST_H [namingng-includeGuardName]'.format(test_include_file), + '^', + '{}:2:9: style: include guard naming violation; TEST_H != _TEST_H [namingng-includeGuardName]'.format(test_include_file), '#ifndef TEST_H', - '{}:5:0: style: Function InvalidFunction violates naming convention [namingng-namingConvention]'.format(test_include_file), + ' ^', + '{}:5:6: style: Function InvalidFunction violates naming convention [namingng-namingConvention]'.format(test_include_file), 'void InvalidFunction();', - '{}:6:0: style: Public member variable _invalid_extern_global violates naming convention [namingng-namingConvention]'.format(test_include_file), + ' ^', + '{}:6:12: style: Global variable _invalid_extern_global violates naming convention [namingng-namingConvention]'.format(test_include_file), 'extern int _invalid_extern_global;', + ' ^', '{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_unguarded_include_file,test_unguarded_include_file_basename), + '^', '{}:0:0: style: Missing include guard [namingng-includeGuardMissing]'.format(test_unguarded_include_file), - '{}:2:0: style: Function InvalidFunctionUnguarded violates naming convention [namingng-namingConvention]'.format(test_unguarded_include_file), + '^', + '{}:2:6: style: Function InvalidFunctionUnguarded violates naming convention [namingng-namingConvention]'.format(test_unguarded_include_file), 'void InvalidFunctionUnguarded();', + ' ^', '{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_file,test_file_basename), - '{}:7:0: style: Variable _invalid_arg violates naming convention [namingng-namingConvention]'.format(test_file), + '^', + '{}:7:26: style: Variable _invalid_arg violates naming convention [namingng-namingConvention]'.format(test_file), 'void valid_function2(int _invalid_arg);', - '{}:8:0: style: Variable invalid_arg_ violates naming convention [namingng-namingConvention]'.format(test_file), + ' ^', + '{}:8:26: style: Variable invalid_arg_ violates naming convention [namingng-namingConvention]'.format(test_file), 'void valid_function3(int invalid_arg_);', - '{}:10:22: style: Variable invalid_arg32 violates naming convention [namingng-namingConvention]'.format(test_file), + ' ^', + '{}:10:31: style: Variable invalid_arg32 violates naming convention [namingng-namingConvention]'.format(test_file), 'void valid_function5(uint32_t invalid_arg32);', - '{}:4:0: style: Function invalid_function_ violates naming convention [namingng-namingConvention]'.format(test_file), + ' ^', + '{}:4:6: style: Function invalid_function_ violates naming convention [namingng-namingConvention]'.format(test_file), 'void invalid_function_();', - '{}:5:0: style: Function _invalid_function violates naming convention [namingng-namingConvention]'.format(test_file), + ' ^', + '{}:5:6: style: Function _invalid_function violates naming convention [namingng-namingConvention]'.format(test_file), 'void _invalid_function();', + ' ^', '{}:12:10: style: Function invalid_function7 violates naming convention [namingng-namingConvention]'.format(test_file), 'uint16_t invalid_function7(int valid_arg);', - '{}:15:0: style: Public member variable _invalid_global violates naming convention [namingng-namingConvention]'.format(test_file), + ' ^', + '{}:15:5: style: Global variable _invalid_global violates naming convention [namingng-namingConvention]'.format(test_file), 'int _invalid_global;', - '{}:16:0: style: Public member variable _invalid_static_global violates naming convention [namingng-namingConvention]'.format(test_file), + ' ^', + '{}:16:12: style: Global variable _invalid_static_global violates naming convention [namingng-namingConvention]'.format(test_file), 'static int _invalid_static_global;', + ' ^', + '{}:20:5: style: Class Constructor _clz violates naming convention [namingng-namingConvention]'.format(test_file), + ' _clz() : _invalid_public(0), _invalid_private(0) { }', + ' ^', + '{}:21:9: style: Public member variable _invalid_public violates naming convention [namingng-namingConvention]'.format(test_file), + ' int _invalid_public;', + ' ^', + '{}:23:10: style: Private member variable _invalid_private violates naming convention [namingng-namingConvention]'.format(test_file), + ' char _invalid_private;', + ' ^', + '{}:26:11: style: Namespace _invalid_namespace violates naming convention [namingng-namingConvention]'.format(test_file), + 'namespace _invalid_namespace { }', + ' ^', ] # test sorted lines; the order of messages may vary and is not of importance lines.sort() From 481d4578ab008e0eaf57b2cd3b51a03faa439807 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Thu, 4 Jan 2024 11:02:59 +0100 Subject: [PATCH 3/3] Fix #12301 FP doubleFree with GTK functions (#5823) --- lib/checkleakautovar.cpp | 6 +++--- lib/checkleakautovar.h | 2 +- test/cfg/gtk.c | 6 ++++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index 86f1ab8164b..138916c11c3 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -815,7 +815,7 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken, } -const Token * CheckLeakAutoVar::checkTokenInsideExpression(const Token * const tok, VarInfo &varInfo) +const Token * CheckLeakAutoVar::checkTokenInsideExpression(const Token * const tok, VarInfo &varInfo, bool inFuncCall) { // Deallocation and then dereferencing pointer.. if (tok->varId() > 0) { @@ -862,7 +862,7 @@ const Token * CheckLeakAutoVar::checkTokenInsideExpression(const Token * const t } // check for function call - const Token * const openingPar = isFunctionCall(tok); + const Token * const openingPar = inFuncCall ? nullptr : isFunctionCall(tok); if (openingPar) { const Library::AllocFunc* allocFunc = mSettings->library.getDeallocFuncInfo(tok); VarInfo::AllocInfo alloc(allocFunc ? allocFunc->groupId : 0, VarInfo::DEALLOC, tok); @@ -1045,7 +1045,7 @@ void CheckLeakAutoVar::functionCall(const Token *tokName, const Token *tokOpenin const VarInfo::AllocInfo sp_allocation(sp_af ? sp_af->groupId : (arrayDelete ? NEW_ARRAY : NEW), VarInfo::OWNED, allocTok); changeAllocStatus(varInfo, sp_allocation, vtok, vtok); } else { - checkTokenInsideExpression(arg, varInfo); + checkTokenInsideExpression(arg, varInfo, /*inFuncCall*/ isLeakIgnore); } // TODO: check each token in argument expression (could contain multiple variables) argNr++; diff --git a/lib/checkleakautovar.h b/lib/checkleakautovar.h index 23af88ff11c..b91f6b5cc3f 100644 --- a/lib/checkleakautovar.h +++ b/lib/checkleakautovar.h @@ -135,7 +135,7 @@ class CPPCHECKLIB CheckLeakAutoVar : public Check { * @param varInfo Variable info * @return next token to process (if no other checks needed for this token). NULL if other checks could be performed. */ - const Token * checkTokenInsideExpression(const Token * const tok, VarInfo &varInfo); + const Token * checkTokenInsideExpression(const Token * const tok, VarInfo &varInfo, bool inFuncCall = false); /** parse function call */ void functionCall(const Token *tokName, const Token *tokOpeningPar, VarInfo &varInfo, const VarInfo::AllocInfo& allocation, const Library::AllocFunc* af); diff --git a/test/cfg/gtk.c b/test/cfg/gtk.c index b7e4f21485d..4557173b9d2 100644 --- a/test/cfg/gtk.c +++ b/test/cfg/gtk.c @@ -425,3 +425,9 @@ void g_abort_test() //cppcheck-suppress unreachableCode printf("Never reached"); } + +gchar* g_strchug_string_free_test(GString* t) // #12301 +{ + gchar* p = g_strchug(g_string_free(t, FALSE)); + return p; +}