Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #12079: Misra: calling unknown function in condition => false pos… #5568

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Fix #12079: Misra: calling unknown function in condition => false pos… #5568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 52 additions & 7 deletions addons/misra.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2805,7 +2805,9 @@ def misra_14_4(self, data):
continue
if not token.astOperand1 or not (token.astOperand1.str in ['if', 'while']):
continue
if not isBoolExpression(token.astOperand2):
if isBoolExpression(token.astOperand2):
continue
if token.astOperand2.valueType:
self.reportError(token, 14, 4)

def misra_15_1(self, data):
Expand Down Expand Up @@ -3185,6 +3187,39 @@ def misra_17_3(self, cfg):
for w in cfg.clang_warnings:
if w['message'].endswith('[-Wimplicit-function-declaration]'):
self.reportError(cppcheckdata.Location(w), 17, 3)
for token in cfg.tokenlist:
if token.str not in ["while", "if"]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is misra-17.3 only checked inside if/while condition ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was probably to avoid some false positives. But it's not a good heuristic.

We don't assume that cppcheck users provide the include paths for standard headers. And the misra.py addon does not load std.cfg file.

Copy link
Contributor

@dzid26 dzid26 Jun 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I noticed misra addon treated __typeof__ extension as a function and it was only reported when inside if( ) condition.

Loading Gnu.cfg library also was not helping as it misra addon also doesn't know typeof() .

continue
if token.next.str != "(":
continue
tok = token.next
end_token = token.next.link
while tok != end_token:
if tok.isName and tok.function is None and tok.valueType is None and tok.next.str == "(" and \
tok.next.valueType is None and not isKeyword(tok.str) and not isStdLibId(tok.str):
self.reportError(tok, 17, 3)
break
tok = tok.next

def misra_config(self, data):
for token in data.tokenlist:
if token.str not in ["while", "if"]:
continue
if token.next.str != "(":
continue
tok = token.next
while tok != token.next.link:
if tok.str == "(" and tok.isCast:
tok = tok.link
continue
if not tok.isName or tok.function or tok.variable or tok.varId or tok.valueType \
or tok.next.str == "(" or tok.str in ["EOF"] \
or isKeyword(tok.str) or isStdLibId(tok.str):
tok = tok.next
continue
errmsg = tok.str + " Variable is unknown"
self.reportError(token, 0, 0, "config")
break

def misra_17_6(self, rawTokens):
for token in rawTokens:
Expand Down Expand Up @@ -4093,21 +4128,30 @@ def setSuppressionList(self, suppressionlist):

self.addSuppressedRule(ruleNum)

def reportError(self, location, num1, num2):
ruleNum = num1 * 100 + num2
def reportError(self, location, num1, num2, other_id = None):
if not other_id:
ruleNum = num1 * 100 + num2
else:
ruleNum = other_id

if self.isRuleGloballySuppressed(ruleNum):
return

if self.settings.verify:
self.verify_actual.append('%s:%d %d.%d' % (location.file, location.linenr, num1, num2))
if not other_id:
self.verify_actual.append('%s:%d %d.%d' % (location.file, location.linenr, num1, num2))
else:
self.verify_actual.append('%s:%d %s' % (location.file, location.linenr, other_id))
elif self.isRuleSuppressed(location.file, location.linenr, ruleNum):
# Error is suppressed. Ignore
self.suppressionStats.setdefault(ruleNum, 0)
self.suppressionStats[ruleNum] += 1
return
else:
errorId = 'c2012-' + str(num1) + '.' + str(num2)
if not other_id:
errorId = 'c2012-' + str(num1) + '.' + str(num2)
else:
errorId = 'c2012-' + other_id
misra_severity = 'Undefined'
cppcheck_severity = 'style'
if ruleNum in self.ruleTexts:
Expand Down Expand Up @@ -4293,7 +4337,7 @@ def fillVerifyExpected(verify_expected, tok):
rule_re = re.compile(r'[0-9]+\.[0-9]+')
if tok.str.startswith('//') and 'TODO' not in tok.str:
for word in tok.str[2:].split(' '):
if rule_re.match(word):
if rule_re.match(word) or word == "config":
verify_expected.append('%s:%d %s' % (tok.file, tok.linenr, word))

data = cppcheckdata.parsedump(dumpfile)
Expand Down Expand Up @@ -4431,6 +4475,7 @@ def fillVerifyExpected(verify_expected, tok):
self.executeCheck(1701, self.misra_17_1, cfg)
self.executeCheck(1702, self.misra_17_2, cfg)
self.executeCheck(1703, self.misra_17_3, cfg)
self.misra_config(cfg)
if cfgNumber == 0:
self.executeCheck(1706, self.misra_17_6, data.rawTokens)
self.executeCheck(1707, self.misra_17_7, cfg)
Expand Down Expand Up @@ -4751,7 +4796,7 @@ def main():
for misra_id in ids:
rules_violated[misra_id] = rules_violated.get(misra_id, 0) + 1
print("MISRA rules violated:")
convert = lambda text: int(text) if text.isdigit() else text
convert = lambda text: int(text) if text.isdigit() else 0
misra_sort = lambda key: [convert(c) for c in re.split(r'[\.-]([0-9]*)', key)]
for misra_id in sorted(rules_violated.keys(), key=misra_sort):
res = re.match(r'misra-c2012-([0-9]+)\\.([0-9]+)', misra_id)
Expand Down
4 changes: 2 additions & 2 deletions addons/test/misra/misra-suppressions1-test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ static void foo(void)
{
for(i = 0; i < 10; i++)
{
if(misra_5_2_func3()) //14.4
if(misra_5_2_func3()) //17.3
{
int misra_5_2_var_hides_var_1____31x;
int misra_5_2_var_hides_var_1____31y;//5.2
}
}
} while(misra_5_2_func2()); //14.4
} while(misra_5_2_func2()); //17.3
}
}
}
47 changes: 33 additions & 14 deletions addons/test/misra/misra-test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ typedef unsigned int u32;
typedef signed int s32;
typedef unsigned long long u64;

static void misra_1_2(void)
static void misra_1_2(bool expr)
{
(void)(condition ? : 0); // 1.2
a = 1 + ({if (!expr) {code;} 1;}); // 1.2
Expand All @@ -67,7 +67,7 @@ static void misra_1_2(void)
static _Atomic int misra_1_4_var; // 1.4
static _Noreturn void misra_1_4_func(void) // 1.4
{
if (0 != _Generic(misra_1_4_var)) {} // 1.4
if (0 != _Generic(misra_1_4_var)) {} // 1.4 17.3
printf_s("hello"); // 1.4
}

Expand Down Expand Up @@ -160,13 +160,13 @@ static void foo(void)
{
for(i = 0; i < 10; i++)
{
if(misra_5_2_func3()) //14.4
if(misra_5_2_func3()) //17.3
{
int misra_5_2_var_hides_var_1____31x;
int misra_5_2_var_hides_var_1____31y;//5.2
}
}
} while(misra_5_2_func2()); //14.4
} while(misra_5_2_func2()); //17.3
}
break;
}
Expand Down Expand Up @@ -252,11 +252,11 @@ static void misra_5_5_func1(void)
{
do
{
if(misra_5_5_func3()) //14.4
if(misra_5_5_func3()) //17.3
{
int misra_5_5_hides_macro________31y; //5.5
}
} while(misra_5_5_func2()); //14.4
} while(misra_5_5_func2()); //17.3
}
break;
}
Expand Down Expand Up @@ -1143,12 +1143,15 @@ static s13_4_t s13_4 =
.string = STRING_DEF_13_4 // no-warning
};

static void misra_13_4(void) {
static void misra_13_4(int x, int z) {
int y;
if (x != (y = z)) {} // 13.4
else {}
}

static void misra_13_5(void) {
int x = 0;
int y = 0;
if (x && (y++ < 123)){} // 13.5
if (x || ((y += 19) > 33)){} // 13.5
if (x || ((y = 25) > 33)){} // 13.5 13.4
Expand Down Expand Up @@ -1297,13 +1300,16 @@ struct {
unsigned int y:1;
} r14_4_struct; // 8.4
static void misra_14_4(bool b) {
if (x+4){} // 14.4
if (x+4){} //config
else {}

if (b) {}
else {}

if (r14_4_struct.x) {}

// #12079
if (z) {} //config
}

static void misra_15_1(void) {
Expand All @@ -1316,7 +1322,9 @@ static void misra_15_2(void) {
goto label; // 15.2 15.1
}

static void misra_15_3(void) {
static void misra_15_3(int a) {
int x = 0;
int y;
if (x!=0) {
goto L1; // 15.3 15.1
if (y!=0) {
Expand Down Expand Up @@ -1431,14 +1439,14 @@ static void misra_15_4(void) {
}
}

static int misra_15_5(void) {
static int misra_15_5(int x) {
if (x!=0) {
return 1; // 15.5
} else {}
return 2;
}

static void misra_15_6(void) {
static void misra_15_6(int x) {
if (x!=0); // 15.6
else{}

Expand Down Expand Up @@ -1470,7 +1478,7 @@ static void misra_15_6_fp(void)
#if defined(M_20_9) && M_20_9 > 1 // no-warning (#10380)
#endif

static void misra_15_7(void) {
static void misra_15_7(int x, int a, int b) {
uint32_t var = 0;
uint32_t var2 = 0;

Expand Down Expand Up @@ -1506,7 +1514,7 @@ static void misra_16_1(int32_t i) {
}
}

static void misra_16_2(void) {
static void misra_16_2(int y) {
switch (x) {
default:
break;
Expand All @@ -1518,7 +1526,8 @@ static void misra_16_2(void) {
}
}

static void misra_16_3(void) {
static void misra_16_3(int b) {
int a;
switch (x) {
case 1:
case 2:
Expand Down Expand Up @@ -1690,6 +1699,16 @@ static void misra_17_2_5(void) {
misra_17_2_1(); // no-warning
}

bool (*dostuff)(); //8.2 8.4
static void misra_17_3(void) {
if (dostuff()) {}
}

static void misra_config(const char* str) {
if (strlen(str) > 3){} //10.4
if (sizeof(int) > 1){} //10.4
}

static void misra_17_6(int x[static 20]) {(void)x;} // 17.6

static int calculation(int x) { return x + 1; }
Expand Down
2 changes: 1 addition & 1 deletion addons/test/misra/suppressions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
misra-c2012-21.6:*/misra-suppressions1-test.c:7
misra-c2012-14.4
misra-c2012-17.3
misra-c2012-5.2
misra-c2012-8.4:*/misra-suppressions1-test.c
misra-c2012-16.4:*/misra-suppressions1-test.c
Expand Down
Loading