-
Notifications
You must be signed in to change notification settings - Fork 68
Implement MISRA-C++23 Preprocesser package rules 19-0-4, 19-1-1, and 19-2-1 #893
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
Open
MichaelRFairhurst
wants to merge
7
commits into
main
Choose a base branch
from
michaelrfairhurst/implement-package-preprocessor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+651
−3
Open
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9830cc0
Implement MISRA-C++23 Preprocesser package rules 19-0-4, 19-1-1, and …
MichaelRFairhurst 2095d1d
Fix Preprocessor.json, for defined in if directive
MichaelRFairhurst ef9d96b
Fix package errors
MichaelRFairhurst a49c0d1
s/maintanability/maintainability
MichaelRFairhurst ea173e5
Regenerate query metadata
MichaelRFairhurst a31e047
Format additional files
MichaelRFairhurst b47ec45
Merge branch 'main' into michaelrfairhurst/implement-package-preproce…
lcartey 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
78 changes: 78 additions & 0 deletions
78
cpp/common/src/codingstandards/cpp/exclusions/cpp/Preprocessor.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,78 @@ | ||
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ | ||
import cpp | ||
import RuleMetadata | ||
import codingstandards.cpp.exclusions.RuleMetadata | ||
|
||
newtype PreprocessorQuery = | ||
TUndefOfMacroNotDefinedInFileQuery() or | ||
TInvalidTokenInDefinedOperatorQuery() or | ||
TDefinedOperatorExpandedInIfDirectiveQuery() or | ||
TNoValidIfdefGuardInHeaderQuery() | ||
|
||
predicate isPreprocessorQueryMetadata(Query query, string queryId, string ruleId, string category) { | ||
query = | ||
// `Query` instance for the `undefOfMacroNotDefinedInFile` query | ||
PreprocessorPackage::undefOfMacroNotDefinedInFileQuery() and | ||
queryId = | ||
// `@id` for the `undefOfMacroNotDefinedInFile` query | ||
"cpp/misra/undef-of-macro-not-defined-in-file" and | ||
ruleId = "RULE-19-0-4" and | ||
category = "advisory" | ||
or | ||
query = | ||
// `Query` instance for the `invalidTokenInDefinedOperator` query | ||
PreprocessorPackage::invalidTokenInDefinedOperatorQuery() and | ||
queryId = | ||
// `@id` for the `invalidTokenInDefinedOperator` query | ||
"cpp/misra/invalid-token-in-defined-operator" and | ||
ruleId = "RULE-19-1-1" and | ||
category = "required" | ||
or | ||
query = | ||
// `Query` instance for the `definedOperatorExpandedInIfDirective` query | ||
PreprocessorPackage::definedOperatorExpandedInIfDirectiveQuery() and | ||
queryId = | ||
// `@id` for the `definedOperatorExpandedInIfDirective` query | ||
"cpp/misra/defined-operator-expanded-in-if-directive" and | ||
ruleId = "RULE-19-1-1" and | ||
category = "required" | ||
or | ||
query = | ||
// `Query` instance for the `noValidIfdefGuardInHeader` query | ||
PreprocessorPackage::noValidIfdefGuardInHeaderQuery() and | ||
queryId = | ||
// `@id` for the `noValidIfdefGuardInHeader` query | ||
"cpp/misra/no-valid-ifdef-guard-in-header" and | ||
ruleId = "RULE-19-2-1" and | ||
category = "required" | ||
} | ||
|
||
module PreprocessorPackage { | ||
Query undefOfMacroNotDefinedInFileQuery() { | ||
//autogenerate `Query` type | ||
result = | ||
// `Query` type for `undefOfMacroNotDefinedInFile` query | ||
TQueryCPP(TPreprocessorPackageQuery(TUndefOfMacroNotDefinedInFileQuery())) | ||
} | ||
|
||
Query invalidTokenInDefinedOperatorQuery() { | ||
//autogenerate `Query` type | ||
result = | ||
// `Query` type for `invalidTokenInDefinedOperator` query | ||
TQueryCPP(TPreprocessorPackageQuery(TInvalidTokenInDefinedOperatorQuery())) | ||
} | ||
|
||
Query definedOperatorExpandedInIfDirectiveQuery() { | ||
//autogenerate `Query` type | ||
result = | ||
// `Query` type for `definedOperatorExpandedInIfDirective` query | ||
TQueryCPP(TPreprocessorPackageQuery(TDefinedOperatorExpandedInIfDirectiveQuery())) | ||
} | ||
|
||
Query noValidIfdefGuardInHeaderQuery() { | ||
//autogenerate `Query` type | ||
result = | ||
// `Query` type for `noValidIfdefGuardInHeader` query | ||
TQueryCPP(TPreprocessorPackageQuery(TNoValidIfdefGuardInHeaderQuery())) | ||
} | ||
} |
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
107 changes: 107 additions & 0 deletions
107
cpp/common/src/codingstandards/cpp/util/CondensedList.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,107 @@ | ||
private import codeql.util.DenseRank | ||
|
||
/** | ||
* Describes how to construct a condensed list from sparse but orderable data, and how that data | ||
* should be connected, with one such list per specified division. | ||
*/ | ||
signature module CondensedListSig { | ||
/** | ||
* The division specifies which items are connected into lists, with one list per division. | ||
* | ||
* For instance, if connecting variables defined in a file, the division will be the file. | ||
*/ | ||
class Division; | ||
|
||
/** | ||
* The class of the items to be condensed into lists. | ||
* | ||
* For instance, when connecting variables defined in a file, the items are the variables. | ||
*/ | ||
class Item { | ||
string toString(); | ||
} | ||
|
||
/** | ||
* The index specifies the order of the items in the condensed list, and may be sparse (have | ||
* gaps). | ||
* | ||
* For instance, if connecting variables defined in a file, the index will be the line number of | ||
* the variable in the file. | ||
* | ||
* The sparse index (which may have gaps) is used to determine the ordering of the items in the | ||
* condensed list. Once the condensed list is created, the items in the list will automatically be | ||
* assigned a dense index (which has no gaps). | ||
* | ||
* There must be no duplicate indices for the same division for correctness. | ||
*/ | ||
int getSparseIndex(Division d, Item l); | ||
} | ||
|
||
/** | ||
* A module to take orderable data (which may not be continuous) and condense it into one or more | ||
* dense lists, with one such list per specified division. | ||
* | ||
* To instantiate this module, you need to provide a `CondensedListSig` module that | ||
* specifies the spare index and division of the items to be connected. | ||
* | ||
* For instance, to create a condensed list of variables defined in every file, you can | ||
* create a `CondensedListSig` module that specifies the file as the division and | ||
* the line number as the sparse index. | ||
* | ||
* ```ql | ||
* module ConfigFileListConfig { | ||
* class Division = File; | ||
* class Item = Variable; | ||
* int getSparseIndex(File file, Variable var) { | ||
* file = var.getLocation().getFile() and | ||
* var.getLocation().getStartLine() | ||
* } | ||
* } | ||
* | ||
* import Condense<ConfigFileListConfig> | ||
* | ||
* from Condense::ListEntry l | ||
* select l, l.getItem(), l.getDenseIndex(), l.getNext(), l.getPrev(), | ||
* ``` | ||
*/ | ||
module Condense<CondensedListSig Config> { | ||
newtype TList = | ||
THead(Config::Item l, Config::Division t) { denseRank(t, l) = 1 } or | ||
TCons(ListEntry prev, Config::Item l) { prev.getDenseIndex() = denseRank(prev.getDivision(), l) - 1 } | ||
|
||
private module DenseRankConfig implements DenseRankInputSig2 { | ||
class Ranked = Config::Item; | ||
|
||
class C = Config::Division; | ||
|
||
predicate getRank = Config::getSparseIndex/2; | ||
} | ||
|
||
private import DenseRank2<DenseRankConfig> | ||
|
||
class ListEntry extends TList { | ||
Config::Division getDivision() { | ||
this = THead(_, result) | ||
or | ||
exists(ListEntry prev | this = TCons(prev, _) and result = prev.getDivision()) | ||
} | ||
|
||
string toString() { | ||
result = getItem().toString() + " [index " + getDenseIndex() + "]" | ||
} | ||
|
||
Config::Item getItem() { | ||
this = THead(result, _) | ||
or | ||
this = TCons(_, result) | ||
} | ||
|
||
int getDenseIndex() { | ||
result = denseRank(getDivision(), getItem()) | ||
} | ||
|
||
ListEntry getPrev() { this = TCons(result, _) } | ||
|
||
ListEntry getNext() { result.getPrev() = this } | ||
} | ||
} |
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,29 @@ | ||
bindingset[this] | ||
signature class ItemSig { | ||
bindingset[this] | ||
string toString(); | ||
} | ||
|
||
module Pair<ItemSig A, ItemSig B> { | ||
signature predicate pred(A a, B b); | ||
|
||
module Where<pred/2 ctor> { | ||
private newtype TAll = TSome(A a, B b) { | ||
ctor(a, b) | ||
} | ||
|
||
class Pair extends TAll { | ||
A getFirst() { | ||
this = TSome(result, _) | ||
} | ||
|
||
B getSecond() { | ||
this = TSome(_, result) | ||
} | ||
|
||
string toString() { | ||
result = getFirst().toString() + ", " + getSecond().toString() | ||
} | ||
} | ||
} | ||
} |
66 changes: 66 additions & 0 deletions
66
cpp/misra/src/rules/RULE-19-0-4/UndefOfMacroNotDefinedInFile.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,66 @@ | ||
/** | ||
* @id cpp/misra/undef-of-macro-not-defined-in-file | ||
* @name RULE-19-0-4: #undef should only be used for macros defined previously in the same file | ||
* @description Using #undef to undefine a macro that is not defined in the same file can lead to | ||
* confusion. | ||
* @kind problem | ||
* @precision very-high | ||
* @problem.severity warning | ||
* @tags external/misra/id/rule-19-0-4 | ||
* scope/single-translation-unit | ||
* readability | ||
* maintanability | ||
* external/misra/enforcement/decidable | ||
* external/misra/obligation/advisory | ||
*/ | ||
|
||
import cpp | ||
import codingstandards.cpp.misra | ||
import codingstandards.cpp.util.CondensedList | ||
import codingstandards.cpp.util.Pair | ||
|
||
class DefOrUndef extends PreprocessorDirective { | ||
string name; | ||
|
||
DefOrUndef() { | ||
name = this.(PreprocessorUndef).getName() or | ||
name = this.(Macro).getName() | ||
} | ||
|
||
string getName() { result = name } | ||
} | ||
|
||
predicate relevantNameAndFile(string name, File file) { | ||
exists(DefOrUndef m | | ||
m.getName() = name and | ||
m.getFile() = file | ||
) | ||
} | ||
|
||
class StringFilePair = Pair<string, File>::Where<relevantNameAndFile/2>::Pair; | ||
|
||
module DefUndefListConfig implements CondensedListSig { | ||
class Division = StringFilePair; | ||
|
||
class Item = DefOrUndef; | ||
|
||
int getSparseIndex(StringFilePair division, DefOrUndef directive) { | ||
directive.getName() = division.getFirst() and | ||
directive.getFile() = division.getSecond() and | ||
result = directive.getLocation().getStartLine() | ||
} | ||
} | ||
|
||
class ListEntry = Condense<DefUndefListConfig>::ListEntry; | ||
|
||
from PreprocessorUndef undef, ListEntry defUndefListEntry | ||
where | ||
not isExcluded(undef, PreprocessorPackage::undefOfMacroNotDefinedInFileQuery()) and | ||
// There exists a def or undef for a given name and file, and it is an #undef | ||
undef = defUndefListEntry.getItem() and | ||
// Exclude cases where the previous def or undef with the same name in the same file is a #define | ||
not exists(ListEntry prev | | ||
prev = defUndefListEntry.getPrev() and | ||
prev.getItem() instanceof Macro | ||
) | ||
select undef, "Undef of name '" + undef.getName() + "' not defined in the same file." |
27 changes: 27 additions & 0 deletions
27
cpp/misra/src/rules/RULE-19-1-1/DefinedOperatorExpandedInIfDirective.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,27 @@ | ||
/** | ||
* @id cpp/misra/defined-operator-expanded-in-if-directive | ||
* @name RULE-19-1-1: The defined preprocessor operator shall be used appropriately | ||
* @description Macro expansions that produce the token 'defined' inside of an if directive result | ||
* in undefined behavior. | ||
* @kind problem | ||
* @precision very-high | ||
* @problem.severity error | ||
* @tags external/misra/id/rule-19-1-1 | ||
* scope/single-translation-unit | ||
* correctness | ||
* maintainability | ||
* external/misra/enforcement/decidable | ||
* external/misra/obligation/required | ||
*/ | ||
|
||
import cpp | ||
import codingstandards.cpp.misra | ||
|
||
from PreprocessorIf ifDirective, MacroInvocation mi | ||
where | ||
not isExcluded(ifDirective, PreprocessorPackage::definedOperatorExpandedInIfDirectiveQuery()) and | ||
ifDirective.getLocation().subsumes(mi.getLocation()) and | ||
mi.getMacro().getBody().regexpMatch(".*defined.*") | ||
select ifDirective, | ||
"If directive contains macro expansion including the token 'defined' from macro $@, which results in undefined behavior.", | ||
mi.getMacro(), mi.getMacroName() |
45 changes: 45 additions & 0 deletions
45
cpp/misra/src/rules/RULE-19-1-1/InvalidTokenInDefinedOperator.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,45 @@ | ||
/** | ||
* @id cpp/misra/invalid-token-in-defined-operator | ||
* @name RULE-19-1-1: The defined preprocessor operator shall be used appropriately | ||
* @description Using the defined operator without an immediately following optionally parenthesized | ||
* identifier results in undefined behavior. | ||
* @kind problem | ||
* @precision very-high | ||
* @problem.severity error | ||
* @tags external/misra/id/rule-19-1-1 | ||
* scope/single-translation-unit | ||
* correctness | ||
* maintainability | ||
* external/misra/enforcement/decidable | ||
* external/misra/obligation/required | ||
*/ | ||
|
||
import cpp | ||
import codingstandards.cpp.misra | ||
|
||
string idRegex() { | ||
result = "[a-zA-Z_]([a-zA-Z_0-9]*)" | ||
} | ||
|
||
bindingset[body] | ||
predicate hasInvalidDefinedOperator(string body) { | ||
body.regexpMatch( | ||
// Contains text "defined" at a word break | ||
".*\\bdefined" + | ||
// Negative zero width lookahead: | ||
"(?!(" + | ||
// (group) optional whitespace followed by a valid identifier | ||
"(\\s*" + idRegex() + ")" + | ||
// or | ||
"|" + | ||
// (group) optional whitespace followed by parenthesis and valid identifier | ||
"(\\s*\\(\\s*" + idRegex() + "\\s*\\))" + | ||
// End negative zero width lookahead, match remaining text | ||
")).*") | ||
} | ||
|
||
from PreprocessorIf ifDirective | ||
where | ||
not isExcluded(ifDirective, PreprocessorPackage::invalidTokenInDefinedOperatorQuery()) and | ||
hasInvalidDefinedOperator(ifDirective.getHead()) | ||
select ifDirective, "Invalid use of defined operator in if directive." |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: I would recognise this as a sparse list or sparse array - I don't think I've seen the term condensed list before.