-
Notifications
You must be signed in to change notification settings - Fork 76
Implement package preprocessor2 #898
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
+753
−1
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
a653a58
Implement package preprocessor2
MichaelRFairhurst a020c0f
Update query metadata
MichaelRFairhurst 36eee16
Update to use features from codeql-qtil
MichaelRFairhurst 3b3b3be
Merge branch 'michaelrfairhurst/implement-package-preprocessor' into …
MichaelRFairhurst 687783b
Merge branch 'michaelrfairhurst/implement-package-preprocessor' into …
MichaelRFairhurst 8d537d6
Merge remote-tracking branch 'origin/michaelrfairhurst/implement-pack…
MichaelRFairhurst d1940ab
Merge remote-tracking branch 'origin/michaelrfairhurst/implement-pack…
MichaelRFairhurst 466bb1f
Merge remote-tracking branch 'origin/michaelrfairhurst/implement-pack…
MichaelRFairhurst 4e2e128
Merge branch 'main' into implement-package-preprocessor2
MichaelRFairhurst 5b9b524
Update cpp/misra/src/rules/RULE-19-3-4/UnparenthesizedMacroArgument.ql
MichaelRFairhurst 0e7ed11
Update cpp/misra/src/rules/RULE-19-3-4/UnparenthesizedMacroArgument.ql
MichaelRFairhurst 6010732
Update cpp/misra/test/rules/RULE-19-3-4/test.cpp
MichaelRFairhurst 5f10e7c
copilot feedback
MichaelRFairhurst File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
61 changes: 61 additions & 0 deletions
61
cpp/common/src/codingstandards/cpp/exclusions/cpp/Preprocessor2.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| //** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ | ||
| import cpp | ||
| import RuleMetadata | ||
| import codingstandards.cpp.exclusions.RuleMetadata | ||
|
|
||
| newtype Preprocessor2Query = | ||
| TInvalidIncludeDirectiveQuery() or | ||
| TUnparenthesizedMacroArgumentQuery() or | ||
| TDisallowedUseOfPragmaQuery() | ||
|
|
||
| predicate isPreprocessor2QueryMetadata(Query query, string queryId, string ruleId, string category) { | ||
| query = | ||
| // `Query` instance for the `invalidIncludeDirective` query | ||
| Preprocessor2Package::invalidIncludeDirectiveQuery() and | ||
| queryId = | ||
| // `@id` for the `invalidIncludeDirective` query | ||
| "cpp/misra/invalid-include-directive" and | ||
| ruleId = "RULE-19-2-2" and | ||
| category = "required" | ||
| or | ||
| query = | ||
| // `Query` instance for the `unparenthesizedMacroArgument` query | ||
| Preprocessor2Package::unparenthesizedMacroArgumentQuery() and | ||
| queryId = | ||
| // `@id` for the `unparenthesizedMacroArgument` query | ||
| "cpp/misra/unparenthesized-macro-argument" and | ||
| ruleId = "RULE-19-3-4" and | ||
| category = "required" | ||
| or | ||
| query = | ||
| // `Query` instance for the `disallowedUseOfPragma` query | ||
| Preprocessor2Package::disallowedUseOfPragmaQuery() and | ||
| queryId = | ||
| // `@id` for the `disallowedUseOfPragma` query | ||
| "cpp/misra/disallowed-use-of-pragma" and | ||
| ruleId = "RULE-19-6-1" and | ||
| category = "advisory" | ||
| } | ||
|
|
||
| module Preprocessor2Package { | ||
| Query invalidIncludeDirectiveQuery() { | ||
| //autogenerate `Query` type | ||
| result = | ||
| // `Query` type for `invalidIncludeDirective` query | ||
| TQueryCPP(TPreprocessor2PackageQuery(TInvalidIncludeDirectiveQuery())) | ||
| } | ||
|
|
||
| Query unparenthesizedMacroArgumentQuery() { | ||
| //autogenerate `Query` type | ||
| result = | ||
| // `Query` type for `unparenthesizedMacroArgument` query | ||
| TQueryCPP(TPreprocessor2PackageQuery(TUnparenthesizedMacroArgumentQuery())) | ||
| } | ||
|
|
||
| Query disallowedUseOfPragmaQuery() { | ||
| //autogenerate `Query` type | ||
| result = | ||
| // `Query` type for `disallowedUseOfPragma` query | ||
| TQueryCPP(TPreprocessor2PackageQuery(TDisallowedUseOfPragmaQuery())) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
cpp/misra/src/rules/RULE-19-2-2/InvalidIncludeDirective.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /** | ||
| * @id cpp/misra/invalid-include-directive | ||
| * @name RULE-19-2-2: The #include directive shall be followed by either a <filename> or "filename" sequence | ||
| * @description Include directives shall only use the <filename> or "filename" forms. | ||
| * @kind problem | ||
| * @precision very-high | ||
| * @problem.severity warning | ||
| * @tags external/misra/id/rule-19-2-2 | ||
| * scope/single-translation-unit | ||
| * maintainability | ||
| * readability | ||
| * external/misra/enforcement/decidable | ||
| * external/misra/obligation/required | ||
| */ | ||
|
|
||
| import cpp | ||
| import codingstandards.cpp.misra | ||
|
|
||
| from Include include | ||
| where | ||
| not isExcluded(include, Preprocessor2Package::invalidIncludeDirectiveQuery()) and | ||
| // Check for < followed by (not >)+ followed by >, or " followed by (not ")+ followed by ". | ||
| not include.getIncludeText().trim().regexpMatch("^(<[^>]+>|\"[^\"]+\")$") | ||
| select include, "Non-compliant #include directive text '" + include.getHead() + "'." |
185 changes: 185 additions & 0 deletions
185
cpp/misra/src/rules/RULE-19-3-4/UnparenthesizedMacroArgument.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| /** | ||
| * @id cpp/misra/unparenthesized-macro-argument | ||
| * @name RULE-19-3-4: Parentheses shall be used to ensure macro arguments are expanded appropriately | ||
| * @description Expanded macro arguments shall be enclosed in parentheses to ensure the resulting | ||
| * expressions have the expected precedence and order of operations. | ||
| * @kind problem | ||
| * @precision high | ||
| * @problem.severity error | ||
| * @tags external/misra/id/rule-19-3-4 | ||
| * scope/single-translation-unit | ||
| * correctness | ||
| * maintainability | ||
| * external/misra/enforcement/decidable | ||
| * external/misra/obligation/required | ||
| */ | ||
|
|
||
| import cpp | ||
| import codingstandards.cpp.misra | ||
| import codingstandards.cpp.Macro | ||
| import codingstandards.cpp.MatchingParenthesis | ||
|
|
||
| /** | ||
| * This regex is used to find macro arguments that appear to have critical operators in them, before | ||
| * we do the expensive process of parsing them to look for parenthesis. | ||
| */ | ||
| pragma[noinline] | ||
| string criticalOperatorRegex() { | ||
| result = | ||
| ".*(" + | ||
| concat(string op | | ||
| op in [ | ||
mbaluda marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "\\*=?", "/=?", "%=?", "\\+=?", "-=?", "<<?=?", ">>?=?", "==?", "!=", "&&?=?", "\\^/?", | ||
MichaelRFairhurst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "\\|\\|?=?", "\\?" | ||
| ] | ||
| | | ||
| op, "|" | ||
| ) + ").*" | ||
| } | ||
|
|
||
| /** | ||
| * Whether a string appears to contain a critical operator. | ||
| */ | ||
| bindingset[input] | ||
| predicate hasCriticalOperator(string input) { input.regexpMatch(criticalOperatorRegex()) } | ||
|
|
||
| /** | ||
| * A critical operator is an operator with "level" between 13 and 2, according to the MISRA C++ | ||
| * standard. This includes from the "multiplicative" level (13) to the "conditional" level (2). | ||
| */ | ||
| class CriticalOperatorExpr extends Expr { | ||
| string operator; | ||
|
|
||
| CriticalOperatorExpr() { | ||
| operator = this.(BinaryOperation).getOperator() | ||
| or | ||
| this instanceof ConditionalExpr and operator = "?" | ||
| or | ||
| operator = this.(Assignment).getOperator() | ||
| } | ||
|
|
||
| string getOperator() { result = operator } | ||
| } | ||
|
|
||
| /** | ||
| * An invocation of a macro that has a parameter that is not precedence-protected with parentheses, | ||
| * and that produces a critical operator expression. | ||
| * | ||
| * This class is used in two passes. Firstly, with `hasRiskyParameter`, to find the macro parameters | ||
| * that should be parsed for parenthesis. Secondly, with `hasNonCompliantParameter`, to parse the | ||
| * risky parameters and attempt to match the produced AST to an unparenthesized occurrence of that | ||
| * operator in the argument text. | ||
| * | ||
| * For a given macro invocation to be considered risky, it must | ||
| * - The macro must have a parameter that is not precedence-protected with parentheses. | ||
| * - The macro must produce a critical operator expression. | ||
| * - The macro must produce only expressions, statements, or variable declarations with initializers. | ||
| * | ||
| * For a risky macro to be non-compliant, it must hold for some values of the predicate | ||
| * `hasNonCompliantParameter`. | ||
| */ | ||
| class RiskyMacroInvocation extends MacroInvocation { | ||
| FunctionLikeMacro macro; | ||
| string riskyParamName; | ||
| int riskyParamIdx; | ||
|
|
||
| RiskyMacroInvocation() { | ||
| macro = getMacro() and | ||
| // The parameter is not precedence-protected with parentheses in the macro body. | ||
| macro.parameterPrecedenceUnprotected(riskyParamIdx) and | ||
| riskyParamName = macro.getParameter(riskyParamIdx) and | ||
| // This macro invocation produces a critical operator expression. | ||
| getAGeneratedElement() instanceof CriticalOperatorExpr and | ||
| // It seems to generate an expression, statement, or variable declaration with initializer. | ||
| forex(Element e | e = getAGeneratedElement() | | ||
| e instanceof Expr | ||
| or | ||
| e instanceof Stmt | ||
| or | ||
| e.(Variable).getInitializer().getExpr() = getAGeneratedElement() | ||
| or | ||
| e.(VariableDeclarationEntry).getDeclaration().getInitializer().getExpr() = | ||
| getAGeneratedElement() | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * A stage 1 pass used to find macro parameters that are not precedence-protected, and have a | ||
| * critical operator in them, and therefore need to be parsed to check for parenthesis at the | ||
| * macro call-site, which is expensive. | ||
| */ | ||
| predicate hasRiskyParameter(string name, int index, string value) { | ||
| name = riskyParamName and | ||
| index = riskyParamIdx and | ||
| value = getExpandedArgument(riskyParamIdx) and | ||
| hasCriticalOperator(value) | ||
| } | ||
|
|
||
| /** | ||
| * A stage 2 pass that occurs after risky parameters have been parsed, to check for parenthesis at the macro | ||
| * call-site. | ||
| * | ||
| * For a given macro argument to be flagged, it must: | ||
| * - be risky as determined by the characteristic predicate (produce a critical operator and only | ||
| * expressions, statements, etc). | ||
| * - be flagged by stage 1 as a risky parameter (i.e. it must have a critical operator in it and | ||
| * correspond to a macro parameter that is not precedence-protected with parentheses) | ||
| * - there must be a top-level text node that contains the operator in the argument string | ||
| * - the operator cannot be the first character in the string (i.e. it should not look like a | ||
| * unary - or +) | ||
| * - the operator cannot exist inside a generated string literal | ||
| * - the operator should not be found inside a "->", "++", or "--" operator. | ||
| * | ||
| * The results of this predicate should be flagged by the query. | ||
| */ | ||
| predicate hasNonCompliantParameter(string name, int index, string value, string operator) { | ||
| hasRiskyParameter(name, index, value) and | ||
| exists( | ||
| ParsedRoot parsedRoot, ParsedText topLevelText, string text, CriticalOperatorExpr opExpr, | ||
| int opIndex | ||
| | | ||
| parsedRoot.getInputString() = value and | ||
| parsedRoot = topLevelText.getParent() and | ||
| text = topLevelText.getText().trim() and | ||
| opExpr = getAGeneratedElement() and | ||
| operator = opExpr.getOperator() and | ||
| opIndex = text.indexOf(operator) and | ||
| // Ignore "->", "++", and "--" operators. | ||
| not [text.substring(opIndex - 1, opIndex + 1), text.substring(opIndex, opIndex + 2)] = | ||
| ["--", "++", "->"] and | ||
| // Ignore operators inside string literals. | ||
| not exists(Literal l | | ||
| l = getAGeneratedElement() and | ||
| exists(l.getValue().indexOf(operator)) | ||
| ) and | ||
| // A leading operator is probably unary and not a problem. | ||
| (opIndex > 0 or topLevelText.getChildIdx() > 0) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A string class that is used to determine what macro arguments will be parsed. | ||
| * | ||
| * This should be a reasonably small set of strings, as parsing is expensive. | ||
| */ | ||
| class RiskyMacroArgString extends string { | ||
| RiskyMacroArgString() { any(RiskyMacroInvocation mi).hasRiskyParameter(_, _, this) } | ||
| } | ||
|
|
||
| // Import `ParsedRoot` etc for the parsed macro arguments. | ||
| import MatchingParenthesis<RiskyMacroArgString> | ||
|
|
||
| from | ||
| RiskyMacroInvocation mi, FunctionLikeMacro m, string paramName, string criticalOperator, | ||
| int paramIndex, string argumentString | ||
| where | ||
| not isExcluded([m.(Element), mi.(Element)], | ||
| Preprocessor2Package::unparenthesizedMacroArgumentQuery()) and | ||
| mi.getMacro() = m and | ||
| mi.hasNonCompliantParameter(paramName, paramIndex, argumentString, criticalOperator) | ||
| select mi, | ||
| "Macro argument " + paramIndex + " (with expanded value '" + argumentString + "') contains a" + | ||
| " critical operator '" + criticalOperator + | ||
| "' that is not parenthesized, but macro $@ argument '" + paramName + | ||
| "' is not precedence-protected with parenthesis.", m, m.getName() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /** | ||
| * @id cpp/misra/disallowed-use-of-pragma | ||
| * @name RULE-19-6-1: The #pragma directive and the _Pragma operator should not be used | ||
| * @description Preprocessor pragma directives are implementation-defined, and should not be used to | ||
| * maintain code portability. | ||
| * @kind problem | ||
| * @precision very-high | ||
| * @problem.severity warning | ||
| * @tags external/misra/id/rule-19-6-1 | ||
| * scope/single-translation-unit | ||
| * maintainability | ||
| * external/misra/enforcement/decidable | ||
| * external/misra/obligation/advisory | ||
| */ | ||
|
|
||
| import cpp | ||
| import codingstandards.cpp.misra | ||
|
|
||
| from PreprocessorDirective pragma, string kind | ||
| where | ||
| not isExcluded(pragma, Preprocessor2Package::disallowedUseOfPragmaQuery()) and | ||
| ( | ||
| pragma instanceof PreprocessorPragma and | ||
| kind = "#pragma directive '" + pragma.getHead() + "'" | ||
| or | ||
| exists(string headOrBody, string pragmaOperand | | ||
| headOrBody = [pragma.getHead(), pragma.(Macro).getBody()] and | ||
| pragmaOperand = headOrBody.regexpCapture(".*\\b(_Pragma\\b\\s*\\([^\\)]+\\)).*", 1) and | ||
| kind = "_Pragma operator used: '" + pragmaOperand + "'" | ||
| ) | ||
| ) | ||
| select pragma, "Non-compliant " + kind + "." |
2 changes: 2 additions & 0 deletions
2
cpp/misra/test/rules/RULE-19-2-2/InvalidIncludeDirective.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| | test.cpp:6:1:6:20 | #include STRING_PATH | Non-compliant #include directive text 'STRING_PATH'. | | ||
| | test.cpp:10:1:10:16 | #include QSTRING | Non-compliant #include directive text 'QSTRING'. | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| rules/RULE-19-2-2/InvalidIncludeDirective.ql |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.