Skip to content

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
43 changes: 43 additions & 0 deletions cpp/common/src/codingstandards/cpp/Macro.qll
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,49 @@ class FunctionLikeMacro extends Macro {
exists(this.getBody().regexpFind("\\#?\\b" + parameter + "\\b", _, result))
)
}

/**
* Holds if the parameter is used in a way that may make it vulnerable to precedence issues.
*
* Typically, parameters are wrapped in parentheses to protect them from precedence issues, but
* that is not always possible.
*/
predicate parameterPrecedenceUnprotected(int index) {
// Check if the parameter is used in a way that requires parentheses
exists(string parameter | parameter = getParameter(index) |
// Finds any occurence of the parameter that is not preceded by, or followed by, either a
// parenthesis or the '#' token operator.
//
// Note the following cases:
// - "(x + 1)" is preceded by a parenthesis, but not followed by one, so SHOULD be matched.
// - "x # 1" is followed by "#" (though not preceded by #) and SHOULD be matched.
// - "(1 + x)" is followed by a parenthesis, but not preceded by one, so SHOULD be matched.
// - "1 # x" is preceded by "#" (though not followed by #) and SHOULD NOT be matched.
//
// So the regex is structured as follows:
// - paramMatch: Matches the parameter at a word boundary, with optional whitespace
// - notHashed: Finds parameters not used with a leading # operator.
// - The final regex finds cases of `notHashed` that are not preceded by a parenthesis,
// and cases of `notHashed` that are not followed by a parenthesis.
//
// Therefore, a parameter with parenthesis on both sides is not matched, a parameter with
// parenthesis missing on one or both sides is only matched if there is no leading or trailing
// ## operator.
exists(string noBeforeParen, string noAfterParen, string paramMatch, string notHashed |
// Not preceded by a parenthesis
noBeforeParen = "(?<!\\(\\s*)" and
// Not followed by a parenthesis
noAfterParen = "(?!\\s*\\))" and
// Parameter at word boundary in optional whitespace
paramMatch = "\\s*\\b" + parameter + "\\b\\s*" and
// A parameter is ##'d if it is preceded or followed by the # operator.
notHashed = "(?<!#)" + paramMatch and
// Parameter is used without a leading or trailing parenthesis, and without #.
getBody()
.regexpMatch(".*(" + noBeforeParen + notHashed + "|" + notHashed + noAfterParen + ").*")
)
)
}
}

newtype TMacroOperator =
Expand Down
2 changes: 1 addition & 1 deletion cpp/common/src/codingstandards/cpp/MatchingParenthesis.qll
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ module MatchingParenthesis<InputString Input> {
occurrence = prevOccurrence + 1
) else (
token = TNotParen() and
exists(inputStr.regexpFind("\\(|\\)", prevOccurrence + 1, endPos)) and
exists(inputStr.regexpFind("\\(|\\)|$", prevOccurrence + 1, endPos)) and
Copy link
Preview

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

Including '$' in the regex "\\(|\\)|$" allows matching an empty string at end‐of‐input, which can lead to zero‐length matches and infinite loops. Consider matching only parentheses here and handling end‐of‐input separately.

Suggested change
exists(inputStr.regexpFind("\\(|\\)|$", prevOccurrence + 1, endPos)) and
exists(inputStr.regexpFind("\\(|\\)", prevOccurrence + 1, endPos)) and
(endPos < inputStr.length() or endPos = inputStr.length()) and

Copilot uses AI. Check for mistakes.

occurrence = prevOccurrence
)
)
Expand Down
78 changes: 78 additions & 0 deletions cpp/common/src/codingstandards/cpp/exclusions/cpp/Preprocessor.qll
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()))
}
}
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()))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import Operators
import OrderOfEvaluation
import OutOfBounds
import Pointers
import Preprocessor
import Preprocessor2
import Representation
import Scope
import SideEffects1
Expand Down Expand Up @@ -94,6 +96,8 @@ newtype TCPPQuery =
TOrderOfEvaluationPackageQuery(OrderOfEvaluationQuery q) or
TOutOfBoundsPackageQuery(OutOfBoundsQuery q) or
TPointersPackageQuery(PointersQuery q) or
TPreprocessorPackageQuery(PreprocessorQuery q) or
TPreprocessor2PackageQuery(Preprocessor2Query q) or
TRepresentationPackageQuery(RepresentationQuery q) or
TScopePackageQuery(ScopeQuery q) or
TSideEffects1PackageQuery(SideEffects1Query q) or
Expand Down Expand Up @@ -148,6 +152,8 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
isOrderOfEvaluationQueryMetadata(query, queryId, ruleId, category) or
isOutOfBoundsQueryMetadata(query, queryId, ruleId, category) or
isPointersQueryMetadata(query, queryId, ruleId, category) or
isPreprocessorQueryMetadata(query, queryId, ruleId, category) or
isPreprocessor2QueryMetadata(query, queryId, ruleId, category) or
isRepresentationQueryMetadata(query, queryId, ruleId, category) or
isScopeQueryMetadata(query, queryId, ruleId, category) or
isSideEffects1QueryMetadata(query, queryId, ruleId, category) or
Expand Down
105 changes: 105 additions & 0 deletions cpp/common/src/codingstandards/cpp/util/CondensedList.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
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 }
}
}
21 changes: 21 additions & 0 deletions cpp/common/src/codingstandards/cpp/util/Pair.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
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() }
}
}
}
Loading
Loading