diff --git a/c/misra/src/rules/RULE-20-8/ControllingExpressionIfDirective.ql b/c/misra/src/rules/RULE-20-8/ControllingExpressionIfDirective.ql index 72495b5d5b..cd55e03ee0 100644 --- a/c/misra/src/rules/RULE-20-8/ControllingExpressionIfDirective.ql +++ b/c/misra/src/rules/RULE-20-8/ControllingExpressionIfDirective.ql @@ -14,24 +14,19 @@ import cpp import codingstandards.c.misra +import codingstandards.cpp.PreprocessorDirective /* A controlling expression is evaluated if it is not excluded (guarded by another controlling expression that is not taken). This translates to it either being taken or not taken. */ predicate isEvaluated(PreprocessorBranch b) { b.wasTaken() or b.wasNotTaken() } -class IfOrElifPreprocessorBranch extends PreprocessorBranch { - IfOrElifPreprocessorBranch() { - this instanceof PreprocessorIf or this instanceof PreprocessorElif - } -} - /** * Looks like it contains a single macro, which may be undefined */ -class SimpleMacroPreprocessorBranch extends IfOrElifPreprocessorBranch { +class SimpleMacroPreprocessorBranch extends PreprocessorIfOrElif { SimpleMacroPreprocessorBranch() { this.getHead().regexpMatch("[a-zA-Z_][a-zA-Z0-9_]+") } } -class SimpleNumericPreprocessorBranch extends IfOrElifPreprocessorBranch { +class SimpleNumericPreprocessorBranch extends PreprocessorIfOrElif { SimpleNumericPreprocessorBranch() { this.getHead().regexpMatch("[0-9]+") } } @@ -39,7 +34,7 @@ class ZeroOrOnePreprocessorBranch extends SimpleNumericPreprocessorBranch { ZeroOrOnePreprocessorBranch() { this.getHead().regexpMatch("[0|1]") } } -predicate containsOnlySafeOperators(IfOrElifPreprocessorBranch b) { +predicate containsOnlySafeOperators(PreprocessorIfOrElif b) { containsOnlyDefinedOperator(b) or //logic: comparison operators eval last, so they make it safe? @@ -47,7 +42,7 @@ predicate containsOnlySafeOperators(IfOrElifPreprocessorBranch b) { } //all defined operators is definitely safe -predicate containsOnlyDefinedOperator(IfOrElifPreprocessorBranch b) { +predicate containsOnlyDefinedOperator(PreprocessorIfOrElif b) { forall(string portion | portion = b.getHead() @@ -65,7 +60,7 @@ class BinaryValuedMacro extends Macro { BinaryValuedMacro() { this.getBody().regexpMatch("\\(?(0|1)\\)?") } } -from IfOrElifPreprocessorBranch b, string msg +from PreprocessorIfOrElif b, string msg where not isExcluded(b, Preprocessor3Package::controllingExpressionIfDirectiveQuery()) and isEvaluated(b) and diff --git a/change_notes/2023-12-06-m16-1-1-perf.md b/change_notes/2023-12-06-m16-1-1-perf.md new file mode 100644 index 0000000000..9603f84e4f --- /dev/null +++ b/change_notes/2023-12-06-m16-1-1-perf.md @@ -0,0 +1,4 @@ + - `M16-1-1` - `DefinedPreProcessorOperatorGeneratedFromExpansionFound.ql`: + - Optimize query to improve performance + - Improve detection of macros whose body contains the `defined` operator after the start of the macro (e.g. `#define X Y || defined(Z)`). + - Enable exclusions to be applied for this rule. \ No newline at end of file diff --git a/cpp/autosar/src/rules/M16-1-1/DefinedMacro.qll b/cpp/autosar/src/rules/M16-1-1/DefinedMacro.qll index 6fed938b38..91d6f614a0 100644 --- a/cpp/autosar/src/rules/M16-1-1/DefinedMacro.qll +++ b/cpp/autosar/src/rules/M16-1-1/DefinedMacro.qll @@ -2,27 +2,27 @@ import cpp import codingstandards.cpp.autosar /** - * A helper class describing macros wrapping defined operator + * A helper class describing macros wrapping the defined operator */ -class DefinedMacro extends Macro { - DefinedMacro() { - this.getBody().regexpMatch("defined\\s*\\(.*") +class MacroUsesDefined extends Macro { + MacroUsesDefined() { + // Uses `defined` directly + exists(this.getBody().regexpFind("\\bdefined\\b", _, _)) or - this.getBody().regexpMatch("defined[\\s]+|defined$") + // Uses a macro that uses `defined` (directly or indirectly) + exists(MacroUsesDefined dm | exists(this.getBody().regexpFind(dm.getRegexForMatch(), _, _))) } - Macro getAUse() { - result = this or - anyAliasing(result, this) + /** + * Gets a regex for matching uses of this macro. + */ + string getRegexForMatch() { + exists(string arguments | + // If there are arguments + if getHead() = getName() then arguments = "" else arguments = "\\s*\\(" + | + // Use word boundary matching to find identifiers that match + result = "\\b" + getName() + "\\b" + arguments + ) } } - -predicate directAlias(Macro alias, Macro aliased) { - not alias.getBody() = alias.getBody().replaceAll(aliased.getHead(), "") -} - -predicate anyAliasing(Macro alias, Macro inQ) { - directAlias(alias, inQ) - or - exists(Macro intermediate | anyAliasing(intermediate, inQ) and anyAliasing(alias, intermediate)) -} diff --git a/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.ql b/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.ql index fc86674d9d..761ef27ebb 100644 --- a/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.ql +++ b/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.ql @@ -14,18 +14,12 @@ import cpp import codingstandards.cpp.autosar +import codingstandards.cpp.PreprocessorDirective import DefinedMacro -from DefinedMacro m, PreprocessorBranch e +from PreprocessorIfOrElif e, MacroUsesDefined m where - ( - e instanceof PreprocessorIf or - e instanceof PreprocessorElif - ) and - ( - e.getHead().regexpMatch(m.getAUse().getHead() + "\\s*\\(.*") - or - e.getHead().regexpMatch(m.getAUse().getHead().replaceAll("(", "\\(").replaceAll(")", "\\)")) - ) and - not isExcluded(e) + not isExcluded(e, MacrosPackage::definedPreProcessorOperatorInOneOfTheTwoStandardFormsQuery()) and + // A`#if` or `#elif` which uses a macro which uses `defined` + exists(e.getHead().regexpFind(m.getRegexForMatch(), _, _)) select e, "The macro $@ expands to 'defined'", m, m.getName() diff --git a/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.ql b/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.ql index f5dcb2f5dc..2a53875067 100644 --- a/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.ql +++ b/cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.ql @@ -15,6 +15,7 @@ import cpp import codingstandards.cpp.autosar +import codingstandards.cpp.PreprocessorDirective //get what comes after each 'defined' used with or without parenth string matchesDefinedOperator(PreprocessorBranch e) { @@ -34,12 +35,8 @@ string matchesDefinedOperator(PreprocessorBranch e) { ) } -from PreprocessorBranch e, string arg +from PreprocessorIfOrElif e, string arg where - ( - e instanceof PreprocessorIf or - e instanceof PreprocessorElif - ) and arg = matchesDefinedOperator(e) and not arg.regexpMatch("^\\w*$") and not isExcluded(e, MacrosPackage::definedPreProcessorOperatorInOneOfTheTwoStandardFormsQuery()) diff --git a/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.expected b/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.expected index 594463345d..9f07d10900 100644 --- a/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.expected +++ b/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.expected @@ -1,2 +1,4 @@ -| test.cpp:22:1:22:18 | #if DBLWRAPUSES(X) | The macro $@ expands to 'defined' | test.cpp:18:1:18:22 | #define BADDEF defined | BADDEF | +| test.cpp:22:1:22:18 | #if DBLWRAPUSES(X) | The macro $@ expands to 'defined' | test.cpp:21:1:21:24 | #define DBLWRAPUSES USES | DBLWRAPUSES | | test.cpp:26:1:26:16 | #if BADDEFTWO(X) | The macro $@ expands to 'defined' | test.cpp:25:1:25:31 | #define BADDEFTWO(X) defined(X) | BADDEFTWO | +| test.cpp:29:1:29:16 | #if BADDEFTWO(Y) | The macro $@ expands to 'defined' | test.cpp:25:1:25:31 | #define BADDEFTWO(X) defined(X) | BADDEFTWO | +| test.cpp:42:1:42:11 | #if WRAPPER | The macro $@ expands to 'defined' | test.cpp:40:1:40:35 | #define WRAPPER X < Y \|\| defined(z) | WRAPPER | diff --git a/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.expected b/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.expected index 73df12d247..69cdd9e644 100644 --- a/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.expected +++ b/cpp/autosar/test/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.expected @@ -2,4 +2,4 @@ | test.cpp:9:1:9:19 | #elif defined X < Y | Use of defined with non-standard form: X < Y. | | test.cpp:13:1:13:18 | #if defined(X > Y) | Use of defined with non-standard form: X > Y. | | test.cpp:14:1:14:20 | #elif defined(X < Y) | Use of defined with non-standard form: X < Y. | -| test.cpp:34:1:34:47 | #if defined(X) \|\| defined _Y_ + X && defined(Y) | Use of defined with non-standard form: _Y_ + X. | +| test.cpp:37:1:37:47 | #if defined(X) \|\| defined _Y_ + X && defined(Y) | Use of defined with non-standard form: _Y_ + X. | diff --git a/cpp/autosar/test/rules/M16-1-1/test.cpp b/cpp/autosar/test/rules/M16-1-1/test.cpp index fa1087f431..c7e9f91fdd 100644 --- a/cpp/autosar/test/rules/M16-1-1/test.cpp +++ b/cpp/autosar/test/rules/M16-1-1/test.cpp @@ -26,10 +26,18 @@ #if BADDEFTWO(X) // NON_COMPLIANT #endif +#if BADDEFTWO(Y) // NON_COMPLIANT +#endif + // clang-format off #if defined (X) || (defined(_Y_)) // COMPLIANT // clang-format on #endif #if defined(X) || defined _Y_ + X && defined(Y) // NON_COMPLIANT +#endif + +#define WRAPPER X < Y || defined(z) + +#if WRAPPER // NON_COMPLIANT #endif \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/PreprocessorDirective.qll b/cpp/common/src/codingstandards/cpp/PreprocessorDirective.qll index 4194ad65e3..fe619e5317 100644 --- a/cpp/common/src/codingstandards/cpp/PreprocessorDirective.qll +++ b/cpp/common/src/codingstandards/cpp/PreprocessorDirective.qll @@ -30,3 +30,13 @@ PreprocessorDirective isLocatedInAMacroInvocation(MacroInvocation m) { result = p ) } + +/** + * An `if` or `elif` preprocessor branch. + */ +class PreprocessorIfOrElif extends PreprocessorBranch { + PreprocessorIfOrElif() { + this instanceof PreprocessorIf or + this instanceof PreprocessorElif + } +} diff --git a/cpp/common/src/codingstandards/cpp/rules/undefinedmacroidentifiers/UndefinedMacroIdentifiers.qll b/cpp/common/src/codingstandards/cpp/rules/undefinedmacroidentifiers/UndefinedMacroIdentifiers.qll index a7e52406a8..d5cafe7416 100644 --- a/cpp/common/src/codingstandards/cpp/rules/undefinedmacroidentifiers/UndefinedMacroIdentifiers.qll +++ b/cpp/common/src/codingstandards/cpp/rules/undefinedmacroidentifiers/UndefinedMacroIdentifiers.qll @@ -1,5 +1,6 @@ import cpp import codingstandards.cpp.Exclusions +import codingstandards.cpp.PreprocessorDirective abstract class UndefinedMacroIdentifiersSharedQuery extends Query { } @@ -76,17 +77,10 @@ string getAnIfDefdMacroIdentifier(PreprocessorBranch pb) { ) } -class IfAndElifs extends PreprocessorBranch { - IfAndElifs() { - this instanceof PreprocessorIf or - this instanceof PreprocessorElif - } -} - -class BadIfAndElifs extends IfAndElifs { +class UndefinedIdIfOrElif extends PreprocessorIfOrElif { string undefinedMacroIdentifier; - BadIfAndElifs() { + UndefinedIdIfOrElif() { exists(string defRM | defRM = this.getHead() @@ -113,7 +107,7 @@ class BadIfAndElifs extends IfAndElifs { string getAnUndefinedMacroIdentifier() { result = undefinedMacroIdentifier } } -query predicate problems(BadIfAndElifs b, string message) { +query predicate problems(UndefinedIdIfOrElif b, string message) { not isExcluded(b, getQuery()) and message = "#if/#elif that uses a macro identifier " + b.getAnUndefinedMacroIdentifier() +