Skip to content

M16-1-1: Optimize query and improve detection of nested uses of defined #471

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.

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

Merged
merged 7 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions change_notes/2023-12-06-m16-1-1-perf.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
* `M16-1-1`
- 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.
36 changes: 18 additions & 18 deletions cpp/autosar/src/rules/M16-1-1/DefinedMacro.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,19 @@ import cpp
import codingstandards.cpp.autosar
import DefinedMacro

from DefinedMacro m, PreprocessorBranch e
/**
* An `if` or `elif` preprocessor branch.
*/
class PreprocessorIfOrElif extends PreprocessorBranch {
PreprocessorIfOrElif() {
this instanceof PreprocessorIf or
this instanceof PreprocessorElif
}
}

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()
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
8 changes: 8 additions & 0 deletions cpp/autosar/test/rules/M16-1-1/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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