From f3a6c3f4e53fbc80870d848da14c7f9b50d2a6c7 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Mon, 2 Dec 2024 14:33:04 +0000 Subject: [PATCH 01/22] Deviations: remove no longer required query lgtm-style suppressions are no longer supported by CodeQL and Code Scanning. --- .../deviations/DeviationsSuppression.qhelp | 12 -- .../cpp/deviations/DeviationsSuppression.ql | 120 ------------------ 2 files changed, 132 deletions(-) delete mode 100644 cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.qhelp delete mode 100644 cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.ql diff --git a/cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.qhelp b/cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.qhelp deleted file mode 100644 index 0bf3a3a71b..0000000000 --- a/cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.qhelp +++ /dev/null @@ -1,12 +0,0 @@ - - - -

This query generates suppression information for rules that have an associated deviation record.

-
- -
  • - MISRA Compliance 2020 document: - Chapter 4.2 (page 12) - Deviations. -
  • -
    -
    \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.ql b/cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.ql deleted file mode 100644 index 9035b7d288..0000000000 --- a/cpp/common/src/codingstandards/cpp/deviations/DeviationsSuppression.ql +++ /dev/null @@ -1,120 +0,0 @@ -/** - * @name Deviation suppression - * @description Generates information about files and locations where certain alerts should be considered suppressed by deviations. - * @kind alert-suppression - * @id cpp/coding-standards/deviation-suppression - */ - -import cpp -import Deviations - -/** Holds if `lineNumber` is an indexed line number in file `f`. */ -private predicate isLineNumber(File f, int lineNumber) { - exists(Location l | l.getFile() = f | - l.getStartLine() = lineNumber - or - l.getEndLine() = lineNumber - ) -} - -/** Gets the last line number in `f`. */ -private int getLastLineNumber(File f) { result = max(int lineNumber | isLineNumber(f, lineNumber)) } - -/** Gets the last column number on the last line of `f`. */ -int getLastColumnNumber(File f) { - result = - max(Location l | - l.getFile() = f and - l.getEndLine() = getLastLineNumber(f) - | - l.getEndColumn() - ) -} - -newtype TDeviationScope = - TDeviationRecordFileScope(DeviationRecord dr, File file) { - exists(string deviationPath | - dr.isDeviated(_, deviationPath) and - file.getRelativePath().prefix(deviationPath.length()) = deviationPath - ) - } or - TDeviationRecordCommentScope(DeviationRecord dr, Comment c) { c = dr.getACodeIdentifierComment() } - -/** A deviation scope. */ -class DeviationScope extends TDeviationScope { - /** Gets the location at which this deviation was defined. */ - abstract Locatable getDeviationDefinitionLocation(); - - /** Gets the Query being deviated. */ - abstract Query getQuery(); - - abstract string toString(); - - abstract predicate hasLocationInfo( - string filepath, int startline, int startcolumn, int endline, int endcolumn - ); -} - -/** A deviation scope derived from a "path" entry in a `DeviationRecord`. */ -class DeviationRecordFileScope extends DeviationScope, TDeviationRecordFileScope { - private DeviationRecord getDeviationRecord() { this = TDeviationRecordFileScope(result, _) } - - override Locatable getDeviationDefinitionLocation() { result = getDeviationRecord() } - - private File getFile() { this = TDeviationRecordFileScope(_, result) } - - override Query getQuery() { result = getDeviationRecord().getQuery() } - - override predicate hasLocationInfo( - string filepath, int startline, int startcolumn, int endline, int endcolumn - ) { - // In an ideal world, we would produce a URL here that informed the AlertSuppression code that - // the whole file was suppressed. However, experimentation suggestions the alert suppression - // code only works with locations with lines and columns, so we generate a location that covers - // the whole "indexed" file, by finding the location indexed in the database with the latest - // line and column number. - exists(File f | f = getFile() | - f.getLocation().hasLocationInfo(filepath, _, _, _, _) and - startline = 1 and - startcolumn = 1 and - endline = getLastLineNumber(f) and - endcolumn = getLastColumnNumber(f) - ) - } - - override string toString() { - result = "Deviation of " + getDeviationRecord().getQuery() + " for " + getFile() + "." - } -} - -/** - * A deviation scope derived from a comment corresponding to a "code-identifier" entry for a - * `DeviationRecord`. - */ -class DeviationRecordCommentScope extends DeviationScope, TDeviationRecordCommentScope { - private DeviationRecord getDeviationRecord() { this = TDeviationRecordCommentScope(result, _) } - - private Comment getComment() { this = TDeviationRecordCommentScope(_, result) } - - override Locatable getDeviationDefinitionLocation() { result = getDeviationRecord() } - - override Query getQuery() { result = getDeviationRecord().getQuery() } - - override predicate hasLocationInfo( - string filepath, int startline, int startcolumn, int endline, int endcolumn - ) { - getComment().getLocation().hasLocationInfo(filepath, startline, _, endline, endcolumn) and - startcolumn = 1 - } - - override string toString() { - result = - "Deviation of " + getDeviationRecord().getQuery() + " for comment " + getComment() + "." - } -} - -from DeviationScope deviationScope -select deviationScope.getDeviationDefinitionLocation(), // suppression comment - "// lgtm[" + deviationScope.getQuery().getQueryId() + "]", // text of suppression comment (excluding delimiters) - "lgtm[" + deviationScope.getQuery().getQueryId() + "]", // text of suppression annotation - deviationScope // scope of suppression From 7b2a2e0e183931cc12c2138f792ecfb045f74513 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 3 Dec 2024 12:02:37 +0000 Subject: [PATCH 02/22] Add library for more complex code identifier deviations This new library supports deviating on the next line, or on ranges, in addition to deviating on the current line. --- .../deviations/CodeIdentifierDeviation.qll | 242 ++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll new file mode 100644 index 0000000000..e6220711a9 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -0,0 +1,242 @@ +/** + * A module for identifying comment markers in code that trigger deviations. + * + * Each comment marker consists of a `code-identifier` with some optional annotations. A deviation will be applied to + * some range of lines in the file containing the comment based on the annotation. The supported marker annotation + * formats are: + * - `` - the deviation applies to results on the current line. + * - `DEVIATION()` - same as above. + * - `DEVIATION_NEXT_LINE()` - this deviation applies to results on the next line. + * - `DEVIATION_BEGIN()` - marks the beginning of a range of lines where the deviation applies. + * - `DEVIATION_END()` - marks the end of a range of lines where the deviation applies. + * + * The valid `code-identifier`s are specified in deviation records, which also specify the query whose results are + * suppressed by the deviation. + * + * For begin/end, we maintain a stack of begin markers. When we encounter an end marker, we pop the stack to determine + * the range of that begin/end marker. If the stack is empty, the end marker is considered unmatched and invalid. If + * the stack is non-empty at the end of the file, all the begin markers are considered unmatched and invalid. + * + * Begin/end markers are not valid across include boundaries, as the stack is not maintained across files. + */ + +import cpp +import Deviations + +/** + * Holds if the given comment contains the code identifier. + */ +bindingset[codeIdentifier] +private predicate commentMatches(Comment comment, string codeIdentifier) { + exists(string text | + comment instanceof CppStyleComment and + // strip the beginning slashes + text = comment.getContents().suffix(2).trim() + or + comment instanceof CStyleComment and + // strip both the beginning /* and the end */ the comment + exists(string text0 | + text0 = comment.getContents().suffix(2) and + text = text0.prefix(text0.length() - 2).trim() + ) and + // The /* */ comment must be a single-line comment + not text.matches("%\n%") + | + // Code identifier appears at the start of the comment (modulo whitespace) + text.prefix(codeIdentifier.length()) = codeIdentifier + or + // Code identifier appears at the end of the comment (modulo whitespace) + text.suffix(text.length() - codeIdentifier.length()) = codeIdentifier + ) +} + +/** + * A deviation marker in the code. + */ +abstract class DeviationMarker extends Comment { + DeviationRecord record; + + /** + * Gets the deviation record associated with this deviation marker. + */ + DeviationRecord getRecord() { result = record } +} + +/** + * A deviation marker for a deviation that applies to the current line. + */ +class DeviationEndOfLineMarker extends DeviationMarker { + DeviationEndOfLineMarker() { + commentMatches(this, "DEVIATION(" + record.getCodeIdentifier() + ")") + } +} + +/** + * A deviation marker for a deviation that applies to the next line. + */ +class DeviationNextLineMarker extends DeviationMarker { + DeviationNextLineMarker() { + commentMatches(this, "DEVIATION_NEXT_LINE(" + record.getCodeIdentifier() + ")") + } +} + +/** + * A deviation marker for a deviation that applies to a range of lines + */ +abstract class DeviationRangeMarker extends DeviationMarker { } + +/** + * A deviation marker for a deviation that begins on this line. + */ +class DeviationBegin extends DeviationRangeMarker { + DeviationBegin() { commentMatches(this, "DEVIATION_BEGIN(" + record.getCodeIdentifier() + ")") } +} + +/** + * A deviation marker for a deviation that ends on this line. + */ +class DeviationEnd extends DeviationRangeMarker { + DeviationEnd() { commentMatches(this, "DEVIATION_END(" + record.getCodeIdentifier() + ")") } +} + +private predicate hasDeviationCommentFileOrdering( + DeviationRecord record, DeviationRangeMarker comment, File file, int index +) { + comment = + rank[index](DeviationRangeMarker c | + c.getRecord() = record and + file = c.getFile() + | + c order by c.getLocation().getStartLine(), c.getLocation().getStartColumn() + ) +} + +private predicate mkBeginStack(DeviationRecord record, File file, BeginStack stack, int index) { + // Stack is empty at the start + index = 0 and + stack = TEmptyBeginStack() and + exists(DeviationRangeMarker marker | + marker.getRecord() = record and marker.getLocation().getFile() = file + ) + or + // Next token is begin, so push it to the stack + exists(DeviationBegin begin, BeginStack prev | + record = begin.getRecord() and + hasDeviationCommentFileOrdering(record, begin, file, index) and + mkBeginStack(record, file, prev, index - 1) and + stack = TConsBeginStack(begin, prev) + ) + or + // Next token is end + exists(DeviationEnd end, BeginStack prevStack | + record = end.getRecord() and + hasDeviationCommentFileOrdering(record, end, file, index) and + mkBeginStack(record, file, prevStack, index - 1) + | + // There is, so pop the most recent begin off the stack + prevStack = TConsBeginStack(_, stack) + or + // Error, no begin on the stack, ignore and continue + prevStack = TEmptyBeginStack() and + stack = TEmptyBeginStack() + ) +} + +newtype TBeginStack = + TConsBeginStack(DeviationBegin begin, TBeginStack prev) { + exists(File file, int index | + hasDeviationCommentFileOrdering(begin.getRecord(), begin, file, index) and + mkBeginStack(begin.getRecord(), file, prev, index - 1) + ) + } or + TEmptyBeginStack() + +private class BeginStack extends TBeginStack { + string toString() { + exists(DeviationBegin begin, BeginStack prev | this = TConsBeginStack(begin, prev) | + result = "(" + begin + ", " + prev.toString() + ")" + ) + or + this = TEmptyBeginStack() and + result = "()" + } +} + +private predicate isDeviationRangePaired( + DeviationRecord record, DeviationBegin begin, DeviationEnd end +) { + exists(File file, int index | + record = end.getRecord() and + hasDeviationCommentFileOrdering(record, end, file, index) and + mkBeginStack(record, file, TConsBeginStack(begin, _), index - 1) + ) +} + +newtype TCodeIndentifierDeviation = + TSingleLineDeviation(DeviationRecord record, Comment comment, string filepath, int suppressedLine) { + ( + commentMatches(comment, record.getCodeIdentifier()) or + comment.(DeviationEndOfLineMarker).getRecord() = record + ) and + comment.getLocation().hasLocationInfo(filepath, suppressedLine, _, _, _) + or + comment.(DeviationNextLineMarker).getRecord() = record and + comment.getLocation().hasLocationInfo(filepath, suppressedLine - 1, _, _, _) + } or + TMultiLineDeviation( + DeviationRecord record, DeviationBegin beginComment, DeviationEnd endComment, string filepath, + int suppressedStartLine, int suppressedEndLine + ) { + isDeviationRangePaired(record, beginComment, endComment) and + beginComment.getLocation().hasLocationInfo(filepath, suppressedStartLine, _, _, _) and + endComment.getLocation().hasLocationInfo(filepath, suppressedEndLine, _, _, _) + } + +class CodeIdentifierDeviation extends TCodeIndentifierDeviation { + /** The deviation record associated with the deviation comment. */ + DeviationRecord getDeviationRecord() { + this = TSingleLineDeviation(result, _, _, _) + or + this = TMultiLineDeviation(result, _, _, _, _, _) + } + + /** + * Holds if the given element is matched by this code identifier deviation. + */ + bindingset[e] + pragma[inline_late] + predicate isElementMatching(Element e) { + exists(string filepath, int elementLocationStart | + e.getLocation().hasLocationInfo(filepath, elementLocationStart, _, _, _) + | + exists(int suppressedLine | + this = TSingleLineDeviation(_, _, filepath, suppressedLine) and + suppressedLine = elementLocationStart + ) + or + exists(int suppressedStartLine, int suppressedEndLine | + this = TMultiLineDeviation(_, _, _, filepath, suppressedStartLine, suppressedEndLine) and + suppressedStartLine < elementLocationStart and + suppressedEndLine > elementLocationStart + ) + ) + } + + string toString() { + exists(string filepath | + exists(int suppressedLine | + this = TSingleLineDeviation(_, _, filepath, suppressedLine) and + result = + "Deviation record " + getDeviationRecord() + " applied to " + filepath + " Line " + + suppressedLine + ) + or + exists(int suppressedStartLine, int suppressedEndLine | + this = TMultiLineDeviation(_, _, _, filepath, suppressedStartLine, suppressedEndLine) and + result = + "Deviation record " + getDeviationRecord() + " applied to " + filepath + " Line" + + suppressedStartLine + ":" + suppressedEndLine + ) + ) + } +} From f1730722ed98a0c7df7b27b755a43d27beb97302 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 3 Dec 2024 12:04:31 +0000 Subject: [PATCH 03/22] Deviations: Integrate extended code-identifier deviations This ties in the code-identifier deviation support to the deviations and exclusions libraries. --- .../src/codingstandards/cpp/Exclusions.qll | 21 +++++--------- .../cpp/deviations/Deviations.qll | 29 ++----------------- 2 files changed, 11 insertions(+), 39 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Exclusions.qll b/cpp/common/src/codingstandards/cpp/Exclusions.qll index b718f6535d..e6a477b220 100644 --- a/cpp/common/src/codingstandards/cpp/Exclusions.qll +++ b/cpp/common/src/codingstandards/cpp/Exclusions.qll @@ -35,19 +35,14 @@ predicate isExcluded(Element e, Query query, string reason) { ) and reason = "Query has an associated deviation record for the element's file." or - // The element is on the same line as a suppression comment - exists(Comment c | - c = dr.getACodeIdentifierComment() and - query = dr.getQuery() - | - exists(string filepath, int endLine | - // Comment occurs on the same line as the end line of the element - e.getLocation().hasLocationInfo(filepath, _, _, endLine, _) and - c.getLocation().hasLocationInfo(filepath, endLine, _, _, _) - ) - ) and - reason = - "Query has an associated deviation record with a code identifier that is applied to the element." + // The element is annotated by a code identifier that deviates this rule + exists(CodeIdentifierDeviation deviationInCode | + dr.getQuery() = query and + deviationInCode = dr.getACodeIdentifierDeviation() and + deviationInCode.isElementMatching(e) and + reason = + "Query has an associated deviation record with a code identifier that is applied to the element." + ) ) or // The effective category of the query is 'Disapplied'. diff --git a/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll b/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll index 4dfadd12eb..2388f95a37 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll @@ -8,6 +8,7 @@ import cpp import semmle.code.cpp.XML import codingstandards.cpp.exclusions.RuleMetadata import codingstandards.cpp.Config +import CodeIdentifierDeviation predicate applyDeviationsAtQueryLevel() { not exists(CodingStandardsReportDeviatedAlerts reportDeviatedResults | @@ -219,32 +220,8 @@ class DeviationRecord extends XmlElement { else result = getADeviationPermit().getCodeIdentifier() } - /** Gets a comment which starts or ends with the code identifier comment. */ - Comment getACodeIdentifierComment() { - exists(string text | - ( - result instanceof CppStyleComment and - // strip the beginning slashes - text = result.getContents().suffix(2).trim() - or - result instanceof CStyleComment and - // strip both the beginning /* and the end */ the comment - exists(string text0 | - text0 = result.getContents().suffix(2) and - text = text0.prefix(text0.length() - 2).trim() - ) and - // The /* */ comment must be a single-line comment - not text.matches("%\n%") - ) and - ( - // Code identifier appears at the start of the comment (modulo whitespace) - text.prefix(getCodeIdentifier().length()) = getCodeIdentifier() - or - // Code identifier appears at the end of the comment (modulo whitespace) - text.suffix(text.length() - getCodeIdentifier().length()) = getCodeIdentifier() - ) - ) - } + /** Gets a code identifier deviation in code which starts or ends with the code identifier comment. */ + CodeIdentifierDeviation getACodeIdentifierDeviation() { this = result.getDeviationRecord() } /** Gets the `rule-id` specified for this record, if any. */ private string getRawRuleId() { result = getAChild("rule-id").getTextValue() } From e36828d9236f6c8c1fbfb631dc639aa6724f92cd Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 3 Dec 2024 12:05:28 +0000 Subject: [PATCH 04/22] Deviations: Add test cases for new code-identifier deviations --- .../TypeLongDoubleUsed.expected | 4 +++ .../deviations/deviations_basic_test/main.cpp | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected index 7b78d54892..a4e045edcf 100644 --- a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected +++ b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected @@ -1 +1,5 @@ | main.cpp:13:15:13:16 | d1 | Use of long double type. | +| main.cpp:18:15:18:16 | d4 | Use of long double type. | +| main.cpp:21:15:21:16 | d6 | Use of long double type. | +| main.cpp:30:15:30:17 | d10 | Use of long double type. | +| main.cpp:38:15:38:17 | d14 | Use of long double type. | diff --git a/cpp/common/test/deviations/deviations_basic_test/main.cpp b/cpp/common/test/deviations/deviations_basic_test/main.cpp index 0b302ea1f2..53258f00fd 100644 --- a/cpp/common/test/deviations/deviations_basic_test/main.cpp +++ b/cpp/common/test/deviations/deviations_basic_test/main.cpp @@ -12,5 +12,30 @@ int main(int argc, char **argv) { getX(); // NON_COMPLIANT long double d1; // NON_COMPLIANT (A0-4-2) long double d2; // a-0-4-2-deviation COMPLIANT[DEVIATED] + + long double d3; // DEVIATION(a-0-4-2-deviation) COMPLIANT[DEVIATED] + + long double d4; // NON_COMPLIANT (A0-4-2) + // DEVIATION_NEXT_LINE(a-0-4-2-deviation) + long double d5; // COMPLIANT[DEVIATED] + long double d6; // NON_COMPLIANT (A0-4-2) + + // DEVIATION_BEGIN(a-0-4-2-deviation) + long double d7; // COMPLIANT[DEVIATED] + getX(); // NON_COMPLIANT (A0-1-2) + long double d8; // COMPLIANT[DEVIATED] + getX(); // NON_COMPLIANT (A0-1-2) + long double d9; // COMPLIANT[DEVIATED] + // DEVIATION_END(a-0-4-2-deviation) + long double d10; // NON_COMPLIANT (A0-4-2) + // DEVIATION_BEGIN(a-0-4-2-deviation) + long double d11; // COMPLIANT[DEVIATED] + getX(); // NON_COMPLIANT (A0-1-2) + long double d12; // COMPLIANT[DEVIATED] + getX(); // NON_COMPLIANT (A0-1-2) + long double d13; // COMPLIANT[DEVIATED] + // DEVIATION_END(a-0-4-2-deviation) + long double d14; // NON_COMPLIANT (A0-4-2) + getX(); // NON_COMPLIANT (A0-1-2) return 0; } \ No newline at end of file From e0779350a71ca7cbcafaa483017ff686aba7724e Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 3 Dec 2024 12:06:53 +0000 Subject: [PATCH 05/22] Update user manual for new code identifier deviations --- docs/user_manual.md | 49 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/docs/user_manual.md b/docs/user_manual.md index 4c020dc73b..7ad4dc4208 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -29,7 +29,8 @@ | 0.21.0 | 2024-05-01 | Luke Cartey | Add MISRA C++ 2023 as under development, and clarify MISRA C 2012 coverage. | | 0.22.0 | 2024-10-02 | Luke Cartey | Add MISRA C 2023 as under development, and clarify MISRA C 2012 coverage. | | 0.23.0 | 2024-10-21 | Luke Cartey | Add assembly as a hazard. | -| 0.24.0 | 2024-10-22 | Luke Cartey | Add CodeQL packs as a usable output, update release artifacts list. | +| 0.24.0 | 2024-10-22 | Luke Cartey | Add CodeQL packs as a usable output, update release artifacts list. | +| 0.25.0 | 2024-12-03 | Luke Cartey | Discuss support for new deviation code identifier formats | ## Release information @@ -405,7 +406,7 @@ The example describes three ways of scoping a deviation: 1. The deviation for `A18-1-1` applies to any source file in the same or a child directory of the directory containing the example `coding-standards.yml`. 2. The deviation for `A18-5-1` applies to any source file in the directory `foo/bar` or a child directory of `foo/bar` relative to the directory containing the `coding-standards.yml`. -3. The deviation for `A0-4-2` applies to any source element that has a comment residing on **the same line** containing the identifier specified in `code-identifier`. +3. The deviation for `A0-4-2` applies to any source element that marked by a comment containing the identifier specified in `code-identifier`. The different acceptable formats are discussed in the next section. The activation of the deviation mechanism requires an extra step in the database creation process. This extra step is the invocation of the Python script `path/to/codeql-coding-standards/scripts/configuration/process_coding_standards_config.py` that is part of the coding standards code scanning pack. @@ -420,6 +421,50 @@ The `process_coding_standards_config.py` has a dependency on the package `pyyaml `pip3 install -r path/to/codeql-coding-standards/scripts/configuration/requirements.txt` +##### Deviation code identifiers + +A code identifier specified in a deviation record can be applied to certain results in the code by adding a comment marker consisting of a `code-identifier` with some optional annotations. The supported marker annotation formats are: + + - `` - the deviation applies to results on the current line. + - `DEVIATION()` - the deviation applies to results on the current line. + - `DEVIATION_NEXT_LINE()` - this deviation applies to results on the next line. + - `DEVIATION_BEGIN()` - marks the beginning of a range of lines where the deviation applies. + - `DEVIATION_END()` - marks the end of a range of lines where the deviation applies. + +Here are some examples, using the deviation record with the `a-0-4-2-deviation` code-identifier specified above: +```cpp + long double x1; // NON_COMPLIANT + + long double x2; // a-0-4-2-deviation - COMPLIANT + long double x3; // COMPLIANT - a-0-4-2-deviation + + long double x4; // DEVIATION(a-0-4-2-deviation) - COMPLIANT + long double x5; // COMPLIANT - DEVIATION(a-0-4-2-deviation) + + // DEVIATION_NEXT_LINE(a-0-4-2-deviation) + long double x6; // COMPLIANT + + // DEVIATION_BEGIN(a-0-4-2-deviation) + long double x7; // COMPLIANT + // DEVIATION_END(a-0-4-2-deviation) +``` + +`DEVIATION_END` markers will pair with the closest unmatched `DEVIATION_BEGIN` for the same `code-identifier`. Consider this example: +```cpp +1 | // DEVIATION_BEGIN(a-0-4-2-deviation) +2 | +3 | // DEVIATION_BEGIN(a-0-4-2-deviation) +4 | +5 | // DEVIATION_END(a-0-4-2-deviation) +6 | +7 | // DEVIATION_END(a-0-4-2-deviation) +``` +Here, Line 1 will pair with Line 7, and Line 3 will pair with Line 8. + +A `DEVIATION_END` without a matching `DEVIATION_BEGIN`, or `DEVIATION_BEGIN` without a matching `DEVIATION_END` is invalid and will be ignored. + +`DEVIATION_BEGIN` and `DEVIATION_END` markers only apply within a single file. Markers cannot be paired across files, and deviations do not apply to included files. + ##### Deviation permit The current implementation supports _deviation permits_ as described in the [MISRA Compliance:2020](https://www.misra.org.uk/app/uploads/2021/06/MISRA-Compliance-2020.pdf) section _4.3 Deviation permits_. From 6e68fb86452eb386a972aae9c81c14e4f85347b1 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Thu, 23 Jan 2025 23:08:40 +0000 Subject: [PATCH 06/22] Deviations: Support an attribute like comment syntax --- .../deviations/CodeIdentifierDeviation.qll | 39 +++++++++++-------- .../deviations/deviations_basic_test/main.cpp | 14 +++---- docs/user_manual.md | 28 ++++++------- 3 files changed, 43 insertions(+), 38 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index e6220711a9..94587dca34 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -5,10 +5,10 @@ * some range of lines in the file containing the comment based on the annotation. The supported marker annotation * formats are: * - `` - the deviation applies to results on the current line. - * - `DEVIATION()` - same as above. - * - `DEVIATION_NEXT_LINE()` - this deviation applies to results on the next line. - * - `DEVIATION_BEGIN()` - marks the beginning of a range of lines where the deviation applies. - * - `DEVIATION_END()` - marks the end of a range of lines where the deviation applies. + * - `[[codingstandards::deviation()]]` - same as above. + * - `[[codingstandards::deviation_next_line()]]` - this deviation applies to results on the next line. + * - `[[codingstandards::deviation_begin()]]` - marks the beginning of a range of lines where the deviation applies. + * - `[[codingstandards::deviation_end()]]` - marks the end of a range of lines where the deviation applies. * * The valid `code-identifier`s are specified in deviation records, which also specify the query whose results are * suppressed by the deviation. @@ -53,7 +53,7 @@ private predicate commentMatches(Comment comment, string codeIdentifier) { /** * A deviation marker in the code. */ -abstract class DeviationMarker extends Comment { +abstract class CommentDeviationMarker extends Comment { DeviationRecord record; /** @@ -65,45 +65,50 @@ abstract class DeviationMarker extends Comment { /** * A deviation marker for a deviation that applies to the current line. */ -class DeviationEndOfLineMarker extends DeviationMarker { +class DeviationEndOfLineMarker extends CommentDeviationMarker { DeviationEndOfLineMarker() { - commentMatches(this, "DEVIATION(" + record.getCodeIdentifier() + ")") + commentMatches(this, "[[codingstandards::deviation(" + record.getCodeIdentifier() + ")]]") } } /** * A deviation marker for a deviation that applies to the next line. */ -class DeviationNextLineMarker extends DeviationMarker { +class DeviationNextLineMarker extends CommentDeviationMarker { DeviationNextLineMarker() { - commentMatches(this, "DEVIATION_NEXT_LINE(" + record.getCodeIdentifier() + ")") + commentMatches(this, + "[[codingstandards::deviation_next_line(" + record.getCodeIdentifier() + ")]]") } } /** * A deviation marker for a deviation that applies to a range of lines */ -abstract class DeviationRangeMarker extends DeviationMarker { } +abstract class CommentDeviationRangeMarker extends CommentDeviationMarker { } /** * A deviation marker for a deviation that begins on this line. */ -class DeviationBegin extends DeviationRangeMarker { - DeviationBegin() { commentMatches(this, "DEVIATION_BEGIN(" + record.getCodeIdentifier() + ")") } +class DeviationBegin extends CommentDeviationRangeMarker { + DeviationBegin() { + commentMatches(this, "[[codingstandards::deviation_begin(" + record.getCodeIdentifier() + ")]]") + } } /** * A deviation marker for a deviation that ends on this line. */ -class DeviationEnd extends DeviationRangeMarker { - DeviationEnd() { commentMatches(this, "DEVIATION_END(" + record.getCodeIdentifier() + ")") } +class DeviationEnd extends CommentDeviationRangeMarker { + DeviationEnd() { + commentMatches(this, "[[codingstandards::deviation_end(" + record.getCodeIdentifier() + ")]]") + } } private predicate hasDeviationCommentFileOrdering( - DeviationRecord record, DeviationRangeMarker comment, File file, int index + DeviationRecord record, CommentDeviationRangeMarker comment, File file, int index ) { comment = - rank[index](DeviationRangeMarker c | + rank[index](CommentDeviationRangeMarker c | c.getRecord() = record and file = c.getFile() | @@ -115,7 +120,7 @@ private predicate mkBeginStack(DeviationRecord record, File file, BeginStack sta // Stack is empty at the start index = 0 and stack = TEmptyBeginStack() and - exists(DeviationRangeMarker marker | + exists(CommentDeviationRangeMarker marker | marker.getRecord() = record and marker.getLocation().getFile() = file ) or diff --git a/cpp/common/test/deviations/deviations_basic_test/main.cpp b/cpp/common/test/deviations/deviations_basic_test/main.cpp index 53258f00fd..e1faaec68c 100644 --- a/cpp/common/test/deviations/deviations_basic_test/main.cpp +++ b/cpp/common/test/deviations/deviations_basic_test/main.cpp @@ -13,28 +13,28 @@ int main(int argc, char **argv) { long double d1; // NON_COMPLIANT (A0-4-2) long double d2; // a-0-4-2-deviation COMPLIANT[DEVIATED] - long double d3; // DEVIATION(a-0-4-2-deviation) COMPLIANT[DEVIATED] - + long double d3; // [[codingstandards::deviation(a-0-4-2-deviation)]] + // COMPLIANT[DEVIATED] long double d4; // NON_COMPLIANT (A0-4-2) - // DEVIATION_NEXT_LINE(a-0-4-2-deviation) + // [[codingstandards::deviation_next_line(a-0-4-2-deviation)]] long double d5; // COMPLIANT[DEVIATED] long double d6; // NON_COMPLIANT (A0-4-2) - // DEVIATION_BEGIN(a-0-4-2-deviation) + // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] long double d7; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d8; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d9; // COMPLIANT[DEVIATED] - // DEVIATION_END(a-0-4-2-deviation) + // [[codingstandards::deviation_end(a-0-4-2-deviation)]] long double d10; // NON_COMPLIANT (A0-4-2) - // DEVIATION_BEGIN(a-0-4-2-deviation) + // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] long double d11; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d12; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d13; // COMPLIANT[DEVIATED] - // DEVIATION_END(a-0-4-2-deviation) + // [[codingstandards::deviation_end(a-0-4-2-deviation)]] long double d14; // NON_COMPLIANT (A0-4-2) getX(); // NON_COMPLIANT (A0-1-2) return 0; diff --git a/docs/user_manual.md b/docs/user_manual.md index 7ad4dc4208..a936118758 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -426,8 +426,8 @@ The `process_coding_standards_config.py` has a dependency on the package `pyyaml A code identifier specified in a deviation record can be applied to certain results in the code by adding a comment marker consisting of a `code-identifier` with some optional annotations. The supported marker annotation formats are: - `` - the deviation applies to results on the current line. - - `DEVIATION()` - the deviation applies to results on the current line. - - `DEVIATION_NEXT_LINE()` - this deviation applies to results on the next line. + - `codingstandards::deviation()` - the deviation applies to results on the current line. + - `codingstandards::deviation_next_line()` - this deviation applies to results on the next line. - `DEVIATION_BEGIN()` - marks the beginning of a range of lines where the deviation applies. - `DEVIATION_END()` - marks the end of a range of lines where the deviation applies. @@ -438,32 +438,32 @@ Here are some examples, using the deviation record with the `a-0-4-2-deviation` long double x2; // a-0-4-2-deviation - COMPLIANT long double x3; // COMPLIANT - a-0-4-2-deviation - long double x4; // DEVIATION(a-0-4-2-deviation) - COMPLIANT - long double x5; // COMPLIANT - DEVIATION(a-0-4-2-deviation) + long double x4; // [[codingstandards::deviation(a-0-4-2-deviation)]] - COMPLIANT + long double x5; // COMPLIANT - [[codingstandards::deviation(a-0-4-2-deviation)]] - // DEVIATION_NEXT_LINE(a-0-4-2-deviation) + // [[codingstandards::deviation_next_line(a-0-4-2-deviation)]] long double x6; // COMPLIANT - // DEVIATION_BEGIN(a-0-4-2-deviation) + // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] long double x7; // COMPLIANT - // DEVIATION_END(a-0-4-2-deviation) + // [[codingstandards::deviation_end(a-0-4-2-deviation)]] ``` -`DEVIATION_END` markers will pair with the closest unmatched `DEVIATION_BEGIN` for the same `code-identifier`. Consider this example: +`codingstandards::deviation_end` markers will pair with the closest unmatched `codingstandards::deviation_begin` for the same `code-identifier`. Consider this example: ```cpp -1 | // DEVIATION_BEGIN(a-0-4-2-deviation) +1 | // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] 2 | -3 | // DEVIATION_BEGIN(a-0-4-2-deviation) +3 | // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] 4 | -5 | // DEVIATION_END(a-0-4-2-deviation) +5 | // [[codingstandards::deviation_end(a-0-4-2-deviation)]] 6 | -7 | // DEVIATION_END(a-0-4-2-deviation) +7 | // [[codingstandards::deviation_end(a-0-4-2-deviation)]] ``` Here, Line 1 will pair with Line 7, and Line 3 will pair with Line 8. -A `DEVIATION_END` without a matching `DEVIATION_BEGIN`, or `DEVIATION_BEGIN` without a matching `DEVIATION_END` is invalid and will be ignored. +A `codingstandards::deviation_end` without a matching `codingstandards::deviation_begin`, or `codingstandards::deviation_begin` without a matching `codingstandards::deviation_end` is invalid and will be ignored. -`DEVIATION_BEGIN` and `DEVIATION_END` markers only apply within a single file. Markers cannot be paired across files, and deviations do not apply to included files. +`codingstandards::deviation_begin` and `codingstandards::deviation_end` markers only apply within a single file. Markers cannot be paired across files, and deviations do not apply to included files. ##### Deviation permit From 1a2454165d16e8babd686b19727c6c7b6f77674b Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 28 Jan 2025 13:17:02 +0000 Subject: [PATCH 07/22] Deviations: Support C/C++ attributes This commit adds support for C/C++ attributes to specify deviations with code identifiers in the code. Attributes are inherited from parents, and support multiple code identifiers in a single definition. --- .../deviations/CodeIdentifierDeviation.qll | 49 +++++++++++++++++++ .../TypeLongDoubleUsed.expected | 4 ++ .../UnusedReturnValue.expected | 12 +++++ .../attribute_syntax.cpp | 44 +++++++++++++++++ docs/user_manual.md | 47 ++++++++++++++++-- 5 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index 94587dca34..bd7100021a 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -177,6 +177,40 @@ private predicate isDeviationRangePaired( ) } +/** + * A standard attribute that either deviates a result. + */ +class DeviationAttribute extends StdAttribute { + DeviationRecord record; + + DeviationAttribute() { + this.hasQualifiedName("codingstandards", "deviation") and + // Support multiple argument deviations + "\"" + record.getCodeIdentifier() + "\"" = this.getAnArgument().getValueText() + } + + DeviationRecord getDeviationRecord() { result = record } + + pragma[nomagic] + Element getASuppressedElement() { + result.(Type).getAnAttribute() = this + or + result.(Stmt).getAnAttribute() = this + or + result.(Variable).getAnAttribute() = this + or + result.(Function).getAnAttribute() = this + or + result.(Expr).getEnclosingStmt() = this.getASuppressedElement() + or + result.(Stmt).getParentStmt() = this.getASuppressedElement() + or + result.(Stmt).getEnclosingFunction() = this.getASuppressedElement() + or + result.(LocalVariable) = this.getASuppressedElement().(DeclStmt).getADeclaration() + } +} + newtype TCodeIndentifierDeviation = TSingleLineDeviation(DeviationRecord record, Comment comment, string filepath, int suppressedLine) { ( @@ -195,6 +229,9 @@ newtype TCodeIndentifierDeviation = isDeviationRangePaired(record, beginComment, endComment) and beginComment.getLocation().hasLocationInfo(filepath, suppressedStartLine, _, _, _) and endComment.getLocation().hasLocationInfo(filepath, suppressedEndLine, _, _, _) + } or + TCodeIdentifierDeviation(DeviationRecord record, DeviationAttribute attribute) { + attribute.getDeviationRecord() = record } class CodeIdentifierDeviation extends TCodeIndentifierDeviation { @@ -203,6 +240,8 @@ class CodeIdentifierDeviation extends TCodeIndentifierDeviation { this = TSingleLineDeviation(result, _, _, _) or this = TMultiLineDeviation(result, _, _, _, _, _) + or + this = TCodeIdentifierDeviation(result, _) } /** @@ -225,6 +264,11 @@ class CodeIdentifierDeviation extends TCodeIndentifierDeviation { suppressedEndLine > elementLocationStart ) ) + or + exists(DeviationAttribute attribute | + this = TCodeIdentifierDeviation(_, attribute) and + attribute.getASuppressedElement() = e + ) } string toString() { @@ -243,5 +287,10 @@ class CodeIdentifierDeviation extends TCodeIndentifierDeviation { suppressedStartLine + ":" + suppressedEndLine ) ) + or + exists(DeviationAttribute attribute | + this = TCodeIdentifierDeviation(_, attribute) and + result = "Deviation record " + getDeviationRecord() + " applied to " + attribute + ) } } diff --git a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected index a4e045edcf..99b3c89bfb 100644 --- a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected +++ b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected @@ -1,3 +1,7 @@ +| attribute_syntax.cpp:6:15:6:17 | dd1 | Use of long double type. | +| attribute_syntax.cpp:22:15:22:17 | d10 | Use of long double type. | +| attribute_syntax.cpp:30:15:30:17 | d14 | Use of long double type. | +| attribute_syntax.cpp:34:20:34:22 | d16 | Use of long double type. | | main.cpp:13:15:13:16 | d1 | Use of long double type. | | main.cpp:18:15:18:16 | d4 | Use of long double type. | | main.cpp:21:15:21:16 | d6 | Use of long double type. | diff --git a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected index 7cc5d2e1ab..7538df2195 100644 --- a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected +++ b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected @@ -1,3 +1,15 @@ +| attribute_syntax.cpp:5:3:5:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:17:5:17:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:19:5:19:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:25:5:25:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:27:5:27:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:31:3:31:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:42:3:42:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | main.cpp:12:3:12:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | +| main.cpp:25:3:25:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | +| main.cpp:27:3:27:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | +| main.cpp:33:3:33:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | +| main.cpp:35:3:35:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | +| main.cpp:39:3:39:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | | nested/nested3/test3.h:5:3:5:7 | call to getZ3 | Return value from call to $@ is unused. | nested/nested3/test3.h:1:5:1:9 | getZ3 | getZ3 | | nested/test.h:5:3:5:6 | call to getY | Return value from call to $@ is unused. | nested/test.h:1:5:1:8 | getY | getY | diff --git a/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp new file mode 100644 index 0000000000..97b4ba987d --- /dev/null +++ b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp @@ -0,0 +1,44 @@ +int getZ() { return 5; } + +int alt() { + int x = 0; // COMPLIANT[DEVIATED] + getZ(); // NON_COMPLIANT + long double dd1; // NON_COMPLIANT (A0-4-2) + + long double [[codingstandards::deviation( + "a-0-4-2-deviation")]] dd3; // COMPLIANT[DEVIATED] + long double [[codingstandards::deviation("a")]] dd3a; // NON_COMPLIAT + + [[codingstandards::deviation( + "a-0-4-2-deviation")]] long double dd4; // COMPLIANT[DEVIATED] + + [[codingstandards::deviation("a-0-4-2-deviation")]] { + long double d7; // COMPLIANT[DEVIATED] + getZ(); // NON_COMPLIANT (A0-1-2) + long double d8; // COMPLIANT[DEVIATED] + getZ(); // NON_COMPLIANT (A0-1-2) + long double d9; // COMPLIANT[DEVIATED] + } + long double d10; // NON_COMPLIANT (A0-4-2) + [[codingstandards::deviation("a-0-4-2-deviation")]] { + long double d11; // COMPLIANT[DEVIATED] + getZ(); // NON_COMPLIANT (A0-1-2) + long double d12; // COMPLIANT[DEVIATED] + getZ(); // NON_COMPLIANT (A0-1-2) + long double d13; // COMPLIANT[DEVIATED] + } + long double d14; // NON_COMPLIANT (A0-4-2) + getZ(); // NON_COMPLIANT (A0-1-2) + [[codingstandards::deviation("a-0-4-2-deviation")]] + for (long double d15 = 0.0; true;) {} // COMPLIANT[DEVIATED] + for (long double d16 = 0.0; true;) { // NON_COMPLIANT (A0-4-2) + } + return 0; +} + +[[codingstandards::deviation("a-0-4-2-deviation")]] +int alt2() { + int x = 0; // COMPLIANT[DEVIATED] + getZ(); // NON_COMPLIANT + long double dd1; // COMPLIANT[DEVIATED] +} \ No newline at end of file diff --git a/docs/user_manual.md b/docs/user_manual.md index a936118758..5d2236ed10 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -421,15 +421,50 @@ The `process_coding_standards_config.py` has a dependency on the package `pyyaml `pip3 install -r path/to/codeql-coding-standards/scripts/configuration/requirements.txt` -##### Deviation code identifiers +##### Deviation code identifier attributes -A code identifier specified in a deviation record can be applied to certain results in the code by adding a comment marker consisting of a `code-identifier` with some optional annotations. The supported marker annotation formats are: +A code identifier specified in a deviation record can be applied to certain results in the code by adding a C or C++ attribute of the following format: + +``` +[[codingstandards::deviation("code-identifier")]] +``` + +This attribute may be added to the following program elements: + * Functions + * Statements + * Variables + * Type declarations + +Deviation attributes are inherited from parents in the code structure. For example, a deviation attribute applied to a function will apply the deviation to all code within the function. Note: deviations are not inherited by lambda expressions. + +Multiple code identifiers may be passed in a single attribute to apply multiple deviations, for example: + +``` +[[codingstandards::deviation("code-identifier-1", "code-identifier-2")]] +``` + +Note - considation should be taken to ensure the use of custom attributes for deviations is compatible with your chosen language version, compiler, compiler configuration and coding standard. + +**Use of attributes in C Coding Standards**: The C Standard introduces attributes in C23, however some compilers support attributes as a language extension in prior versions. You should: + * Confirm that your compiler supports attributes for your chosen compiler configuration, if necessary as a language extension. + * Confirm that unknown attributes are ignored by the compiler. + * For MISRA C, add a project deviation against "Rule 1.2: Language extensions should not be used", if attribute support is a language extension in your language version. + +**Use of attributes in C++ Coding Standards**: The C++ Standard supports attributes in C++14, however the handling of unknown attributes is implementation defined. From C++17 onwards, unknown attributes are mandated to be ignored. Unknown attributes will usually raise an "unknown attribute" warning. You should: + * If using C++14, confirm that your compiler ignores unknown attributes. + * If using AUTOSAR and a compiler which produces warnings on unknown attributes, the compiler warning should be disabled (as per `A1-1-2: A warning level of the compilation process shall be set in compliance with project policies`), to ensure compliance with `A1-4-3: All code should compiler free of compiler warnings`. + +If you cannot satisfy these condition, please use the deviation code identifier comment format instead. + +##### Deviation code identifier comments + +As an alternative to attributes, a code identifier specified in a deviation record can be applied to certain results in the code by adding a comment marker consisting of a `code-identifier` with some optional annotations. The supported marker annotation formats are: - `` - the deviation applies to results on the current line. - `codingstandards::deviation()` - the deviation applies to results on the current line. - `codingstandards::deviation_next_line()` - this deviation applies to results on the next line. - - `DEVIATION_BEGIN()` - marks the beginning of a range of lines where the deviation applies. - - `DEVIATION_END()` - marks the end of a range of lines where the deviation applies. + - `codingstandards::deviation_begin()` - marks the beginning of a range of lines where the deviation applies. + - `codingstandards::deviation_end()` - marks the end of a range of lines where the deviation applies. Here are some examples, using the deviation record with the `a-0-4-2-deviation` code-identifier specified above: ```cpp @@ -465,7 +500,9 @@ A `codingstandards::deviation_end` without a matching `codingstandards::deviatio `codingstandards::deviation_begin` and `codingstandards::deviation_end` markers only apply within a single file. Markers cannot be paired across files, and deviations do not apply to included files. -##### Deviation permit +Note: deviation markers cannot be applied to the body of a macro. Please apply the deviation to macro expansion, or use the attribute deviation format. + +##### Deviation permits The current implementation supports _deviation permits_ as described in the [MISRA Compliance:2020](https://www.misra.org.uk/app/uploads/2021/06/MISRA-Compliance-2020.pdf) section _4.3 Deviation permits_. From 42836526bb9953e0fb539f805988823612bc608d Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Mon, 10 Feb 2025 16:22:57 +0000 Subject: [PATCH 08/22] Deviations: Switch to new deviations format --- .../deviations/CodeIdentifierDeviation.qll | 37 ++++++++++++------ .../TypeLongDoubleUsed.expected | 6 +-- .../UnusedReturnValue.expected | 12 +++--- .../attribute_syntax.cpp | 17 ++++---- .../deviations/deviations_basic_test/main.cpp | 12 +++--- docs/user_manual.md | 39 ++++++++++--------- 6 files changed, 71 insertions(+), 52 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index bd7100021a..731a04cfc7 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -1,14 +1,24 @@ /** - * A module for identifying comment markers in code that trigger deviations. + * A module for identifying in code markers in code that trigger deviations. * - * Each comment marker consists of a `code-identifier` with some optional annotations. A deviation will be applied to + * This module supports two different code identifier markers: + * - A C/C++ attribute based syntax + * - A comment-based format + * + * The C/C++ attribute based syntax uses the following format: + * ``` + * [[codeql::_deviation("code-identifier")]] + * ``` + * The deviation will be applied to the selected program element, and any syntactically nested children of that program element. + * + * For the comment format the marker consists of a `code-identifier` with some optional annotations. A deviation will be applied to * some range of lines in the file containing the comment based on the annotation. The supported marker annotation * formats are: * - `` - the deviation applies to results on the current line. - * - `[[codingstandards::deviation()]]` - same as above. - * - `[[codingstandards::deviation_next_line()]]` - this deviation applies to results on the next line. - * - `[[codingstandards::deviation_begin()]]` - marks the beginning of a range of lines where the deviation applies. - * - `[[codingstandards::deviation_end()]]` - marks the end of a range of lines where the deviation applies. + * - `codeql::_deviation()` - same as above. + * - `codeql::_deviation_next_line()` - this deviation applies to results on the next line. + * - `codeql::_deviation_begin()` - marks the beginning of a range of lines where the deviation applies. + * - `codeql::_deviation_end()` - marks the end of a range of lines where the deviation applies. * * The valid `code-identifier`s are specified in deviation records, which also specify the query whose results are * suppressed by the deviation. @@ -23,6 +33,8 @@ import cpp import Deviations +string supportedStandard() { result = ["misra", "autosar", "cert"] } + /** * Holds if the given comment contains the code identifier. */ @@ -67,7 +79,8 @@ abstract class CommentDeviationMarker extends Comment { */ class DeviationEndOfLineMarker extends CommentDeviationMarker { DeviationEndOfLineMarker() { - commentMatches(this, "[[codingstandards::deviation(" + record.getCodeIdentifier() + ")]]") + commentMatches(this, + "codeql::" + supportedStandard() + "_deviation(" + record.getCodeIdentifier() + ")") } } @@ -77,7 +90,7 @@ class DeviationEndOfLineMarker extends CommentDeviationMarker { class DeviationNextLineMarker extends CommentDeviationMarker { DeviationNextLineMarker() { commentMatches(this, - "[[codingstandards::deviation_next_line(" + record.getCodeIdentifier() + ")]]") + "codeql::" + supportedStandard() + "_deviation_next_line(" + record.getCodeIdentifier() + ")") } } @@ -91,7 +104,8 @@ abstract class CommentDeviationRangeMarker extends CommentDeviationMarker { } */ class DeviationBegin extends CommentDeviationRangeMarker { DeviationBegin() { - commentMatches(this, "[[codingstandards::deviation_begin(" + record.getCodeIdentifier() + ")]]") + commentMatches(this, + "codeql::" + supportedStandard() + "_deviation_begin(" + record.getCodeIdentifier() + ")") } } @@ -100,7 +114,8 @@ class DeviationBegin extends CommentDeviationRangeMarker { */ class DeviationEnd extends CommentDeviationRangeMarker { DeviationEnd() { - commentMatches(this, "[[codingstandards::deviation_end(" + record.getCodeIdentifier() + ")]]") + commentMatches(this, + "codeql::" + supportedStandard() + "_deviation_end(" + record.getCodeIdentifier() + ")") } } @@ -184,7 +199,7 @@ class DeviationAttribute extends StdAttribute { DeviationRecord record; DeviationAttribute() { - this.hasQualifiedName("codingstandards", "deviation") and + this.hasQualifiedName("codeql", supportedStandard() + "_deviation") and // Support multiple argument deviations "\"" + record.getCodeIdentifier() + "\"" = this.getAnArgument().getValueText() } diff --git a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected index 99b3c89bfb..1786c4ce9e 100644 --- a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected +++ b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected @@ -1,7 +1,7 @@ | attribute_syntax.cpp:6:15:6:17 | dd1 | Use of long double type. | -| attribute_syntax.cpp:22:15:22:17 | d10 | Use of long double type. | -| attribute_syntax.cpp:30:15:30:17 | d14 | Use of long double type. | -| attribute_syntax.cpp:34:20:34:22 | d16 | Use of long double type. | +| attribute_syntax.cpp:21:15:21:17 | d10 | Use of long double type. | +| attribute_syntax.cpp:29:15:29:17 | d14 | Use of long double type. | +| attribute_syntax.cpp:33:20:33:22 | d16 | Use of long double type. | | main.cpp:13:15:13:16 | d1 | Use of long double type. | | main.cpp:18:15:18:16 | d4 | Use of long double type. | | main.cpp:21:15:21:16 | d6 | Use of long double type. | diff --git a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected index 7538df2195..8b258328ab 100644 --- a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected +++ b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected @@ -1,10 +1,10 @@ | attribute_syntax.cpp:5:3:5:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | -| attribute_syntax.cpp:17:5:17:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | -| attribute_syntax.cpp:19:5:19:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | -| attribute_syntax.cpp:25:5:25:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | -| attribute_syntax.cpp:27:5:27:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | -| attribute_syntax.cpp:31:3:31:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | -| attribute_syntax.cpp:42:3:42:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:16:5:16:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:18:5:18:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:24:5:24:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:26:5:26:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:30:3:30:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:41:3:41:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | main.cpp:12:3:12:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | | main.cpp:25:3:25:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | | main.cpp:27:3:27:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | diff --git a/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp index 97b4ba987d..30acac3bfb 100644 --- a/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp +++ b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp @@ -5,14 +5,13 @@ int alt() { getZ(); // NON_COMPLIANT long double dd1; // NON_COMPLIANT (A0-4-2) - long double [[codingstandards::deviation( - "a-0-4-2-deviation")]] dd3; // COMPLIANT[DEVIATED] - long double [[codingstandards::deviation("a")]] dd3a; // NON_COMPLIAT + long double [[codeql::autosar_deviation( + "a-0-4-2-deviation")]] dd3; // COMPLIANT[DEVIATED] - [[codingstandards::deviation( + [[codeql::autosar_deviation( "a-0-4-2-deviation")]] long double dd4; // COMPLIANT[DEVIATED] - [[codingstandards::deviation("a-0-4-2-deviation")]] { + [[codeql::autosar_deviation("a-0-4-2-deviation")]] { long double d7; // COMPLIANT[DEVIATED] getZ(); // NON_COMPLIANT (A0-1-2) long double d8; // COMPLIANT[DEVIATED] @@ -20,7 +19,7 @@ int alt() { long double d9; // COMPLIANT[DEVIATED] } long double d10; // NON_COMPLIANT (A0-4-2) - [[codingstandards::deviation("a-0-4-2-deviation")]] { + [[codeql::autosar_deviation("a-0-4-2-deviation")]] { long double d11; // COMPLIANT[DEVIATED] getZ(); // NON_COMPLIANT (A0-1-2) long double d12; // COMPLIANT[DEVIATED] @@ -29,16 +28,18 @@ int alt() { } long double d14; // NON_COMPLIANT (A0-4-2) getZ(); // NON_COMPLIANT (A0-1-2) - [[codingstandards::deviation("a-0-4-2-deviation")]] + [[codeql::autosar_deviation("a-0-4-2-deviation")]] for (long double d15 = 0.0; true;) {} // COMPLIANT[DEVIATED] for (long double d16 = 0.0; true;) { // NON_COMPLIANT (A0-4-2) } return 0; } -[[codingstandards::deviation("a-0-4-2-deviation")]] +[[codeql::autosar_deviation("a-0-4-2-deviation")]] int alt2() { int x = 0; // COMPLIANT[DEVIATED] getZ(); // NON_COMPLIANT long double dd1; // COMPLIANT[DEVIATED] + [[codeql::autosar_deviation( + "a-0-4-2-deviation")]] long double dd2; // COMPLIANT[DEVIATED] } \ No newline at end of file diff --git a/cpp/common/test/deviations/deviations_basic_test/main.cpp b/cpp/common/test/deviations/deviations_basic_test/main.cpp index e1faaec68c..aa389ed0ad 100644 --- a/cpp/common/test/deviations/deviations_basic_test/main.cpp +++ b/cpp/common/test/deviations/deviations_basic_test/main.cpp @@ -13,28 +13,28 @@ int main(int argc, char **argv) { long double d1; // NON_COMPLIANT (A0-4-2) long double d2; // a-0-4-2-deviation COMPLIANT[DEVIATED] - long double d3; // [[codingstandards::deviation(a-0-4-2-deviation)]] + long double d3; // codeql::autosar_deviation(a-0-4-2-deviation) // COMPLIANT[DEVIATED] long double d4; // NON_COMPLIANT (A0-4-2) - // [[codingstandards::deviation_next_line(a-0-4-2-deviation)]] + // codeql::autosar_deviation_next_line(a-0-4-2-deviation) long double d5; // COMPLIANT[DEVIATED] long double d6; // NON_COMPLIANT (A0-4-2) - // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] + // codeql::autosar_deviation_begin(a-0-4-2-deviation) long double d7; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d8; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d9; // COMPLIANT[DEVIATED] - // [[codingstandards::deviation_end(a-0-4-2-deviation)]] + // codeql::autosar_deviation_end(a-0-4-2-deviation) long double d10; // NON_COMPLIANT (A0-4-2) - // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] + // codeql::autosar_deviation_begin(a-0-4-2-deviation) long double d11; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d12; // COMPLIANT[DEVIATED] getX(); // NON_COMPLIANT (A0-1-2) long double d13; // COMPLIANT[DEVIATED] - // [[codingstandards::deviation_end(a-0-4-2-deviation)]] + // codeql::autosar_deviation_end(a-0-4-2-deviation) long double d14; // NON_COMPLIANT (A0-4-2) getX(); // NON_COMPLIANT (A0-1-2) return 0; diff --git a/docs/user_manual.md b/docs/user_manual.md index 5d2236ed10..34c675c4bc 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -426,10 +426,13 @@ The `process_coding_standards_config.py` has a dependency on the package `pyyaml A code identifier specified in a deviation record can be applied to certain results in the code by adding a C or C++ attribute of the following format: ``` -[[codingstandards::deviation("code-identifier")]] +[[codeql::_deviation("code-identifier")]] ``` +For example `[[codeql::misra_deviation("a1-2-4")]]` would apply a deviation of a rule in a MISRA standard, using the code identifier `a1-2-4`. The supported standard names are `misra`, `autosar` and `cert`. + This attribute may be added to the following program elements: + * Functions * Statements * Variables @@ -440,7 +443,7 @@ Deviation attributes are inherited from parents in the code structure. For examp Multiple code identifiers may be passed in a single attribute to apply multiple deviations, for example: ``` -[[codingstandards::deviation("code-identifier-1", "code-identifier-2")]] +[[codeql::misra_deviation("code-identifier-1", "code-identifier-2")]] ``` Note - considation should be taken to ensure the use of custom attributes for deviations is compatible with your chosen language version, compiler, compiler configuration and coding standard. @@ -461,10 +464,10 @@ If you cannot satisfy these condition, please use the deviation code identifier As an alternative to attributes, a code identifier specified in a deviation record can be applied to certain results in the code by adding a comment marker consisting of a `code-identifier` with some optional annotations. The supported marker annotation formats are: - `` - the deviation applies to results on the current line. - - `codingstandards::deviation()` - the deviation applies to results on the current line. - - `codingstandards::deviation_next_line()` - this deviation applies to results on the next line. - - `codingstandards::deviation_begin()` - marks the beginning of a range of lines where the deviation applies. - - `codingstandards::deviation_end()` - marks the end of a range of lines where the deviation applies. + - `codeql::_deviation()` - the deviation applies to results on the current line. + - `codeql::_deviation_next_line()` - this deviation applies to results on the next line. + - `codeql::_deviation_begin()` - marks the beginning of a range of lines where the deviation applies. + - `codeql::_deviation_end()` - marks the end of a range of lines where the deviation applies. Here are some examples, using the deviation record with the `a-0-4-2-deviation` code-identifier specified above: ```cpp @@ -473,32 +476,32 @@ Here are some examples, using the deviation record with the `a-0-4-2-deviation` long double x2; // a-0-4-2-deviation - COMPLIANT long double x3; // COMPLIANT - a-0-4-2-deviation - long double x4; // [[codingstandards::deviation(a-0-4-2-deviation)]] - COMPLIANT - long double x5; // COMPLIANT - [[codingstandards::deviation(a-0-4-2-deviation)]] + long double x4; // codeql::_deviation(a-0-4-2-deviation) - COMPLIANT + long double x5; // COMPLIANT - codeql::_deviation(a-0-4-2-deviation) - // [[codingstandards::deviation_next_line(a-0-4-2-deviation)]] + // codeql::_deviation_next_line(a-0-4-2-deviation) long double x6; // COMPLIANT - // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] + // codeql::_deviation_begin(a-0-4-2-deviation) long double x7; // COMPLIANT - // [[codingstandards::deviation_end(a-0-4-2-deviation)]] + // codeql::_deviation_end(a-0-4-2-deviation) ``` -`codingstandards::deviation_end` markers will pair with the closest unmatched `codingstandards::deviation_begin` for the same `code-identifier`. Consider this example: +`codeql::_deviation_end` markers will pair with the closest unmatched `codeql::_deviation_begin` for the same `code-identifier`. Consider this example: ```cpp -1 | // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] +1 | // codeql::_deviation_begin(a-0-4-2-deviation) 2 | -3 | // [[codingstandards::deviation_begin(a-0-4-2-deviation)]] +3 | // codeql::_deviation_begin(a-0-4-2-deviation) 4 | -5 | // [[codingstandards::deviation_end(a-0-4-2-deviation)]] +5 | // codeql::_deviation_end(a-0-4-2-deviation) 6 | -7 | // [[codingstandards::deviation_end(a-0-4-2-deviation)]] +7 | // codeql::_deviation_end(a-0-4-2-deviation) ``` Here, Line 1 will pair with Line 7, and Line 3 will pair with Line 8. -A `codingstandards::deviation_end` without a matching `codingstandards::deviation_begin`, or `codingstandards::deviation_begin` without a matching `codingstandards::deviation_end` is invalid and will be ignored. +A `codeql::_deviation_end` without a matching `codeql::_deviation_begin`, or `codeql::_deviation_begin` without a matching `codeql::_deviation_end` is invalid and will be ignored. -`codingstandards::deviation_begin` and `codingstandards::deviation_end` markers only apply within a single file. Markers cannot be paired across files, and deviations do not apply to included files. +`codeql::_deviation_begin` and `ccodeql::_deviation_end` markers only apply within a single file. Markers cannot be paired across files, and deviations do not apply to included files. Note: deviation markers cannot be applied to the body of a macro. Please apply the deviation to macro expansion, or use the attribute deviation format. From c65f635fc0740f45987f58a02dae5583b6bef2d3 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Mon, 10 Feb 2025 18:23:59 +0000 Subject: [PATCH 09/22] Remove deviation suppression query tests --- .../DeviationsSuppression.expected | 7 ------- .../deviations_report_deviated/DeviationsSuppression.qlref | 1 - 2 files changed, 8 deletions(-) delete mode 100644 cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.expected delete mode 100644 cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.qlref diff --git a/cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.expected b/cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.expected deleted file mode 100644 index 50ceb35b9d..0000000000 --- a/cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.expected +++ /dev/null @@ -1,7 +0,0 @@ -| file://:0:0:0:0 | (no string representation) | // lgtm[cpp/autosar/type-long-double-used] | lgtm[cpp/autosar/type-long-double-used] | main.cpp:12:1:12:58 | Deviation of cpp/autosar/type-long-double-used for comment // a-0-4-2-deviation COMPLIANT[DEVIATED]. | -| file://:0:0:0:0 | (no string representation) | // lgtm[cpp/autosar/unused-return-value] | lgtm[cpp/autosar/unused-return-value] | nested/nested2/test2.h:1:1:6:1 | Deviation of cpp/autosar/unused-return-value for nested/nested2/test2.h. | -| file://:0:0:0:0 | (no string representation) | // lgtm[cpp/autosar/useless-assignment] | lgtm[cpp/autosar/useless-assignment] | coding-standards.xml:1:1:17:19 | Deviation of cpp/autosar/useless-assignment for coding-standards.xml. | -| file://:0:0:0:0 | (no string representation) | // lgtm[cpp/autosar/useless-assignment] | lgtm[cpp/autosar/useless-assignment] | main.cpp:1:1:14:1 | Deviation of cpp/autosar/useless-assignment for main.cpp. | -| file://:0:0:0:0 | (no string representation) | // lgtm[cpp/autosar/useless-assignment] | lgtm[cpp/autosar/useless-assignment] | nested/coding-standards.xml:1:1:13:19 | Deviation of cpp/autosar/useless-assignment for nested/coding-standards.xml. | -| file://:0:0:0:0 | (no string representation) | // lgtm[cpp/autosar/useless-assignment] | lgtm[cpp/autosar/useless-assignment] | nested/nested2/test2.h:1:1:6:1 | Deviation of cpp/autosar/useless-assignment for nested/nested2/test2.h. | -| file://:0:0:0:0 | (no string representation) | // lgtm[cpp/autosar/useless-assignment] | lgtm[cpp/autosar/useless-assignment] | nested/test.h:1:1:6:1 | Deviation of cpp/autosar/useless-assignment for nested/test.h. | diff --git a/cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.qlref b/cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.qlref deleted file mode 100644 index 6268ee7342..0000000000 --- a/cpp/common/test/deviations/deviations_report_deviated/DeviationsSuppression.qlref +++ /dev/null @@ -1 +0,0 @@ -codingstandards/cpp/deviations/DeviationsSuppression.ql \ No newline at end of file From f56fa0f05bac0c78f1816a6cee4948eeea5b2bb5 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 12 Feb 2025 23:48:55 +0000 Subject: [PATCH 10/22] Detect invalid deviation markers --- .../deviations/CodeIdentifierDeviation.qll | 29 +++++++++++++-- .../InvalidDeviationCodeIdentifier.md | 19 ++++++++++ .../InvalidDeviationCodeIdentifier.ql | 35 +++++++++++++++++++ .../InvalidDeviationCodeIdentifier.expected | 10 ++++++ .../InvalidDeviationCodeIdentifier.qlref | 1 + .../invalid_deviations/coding-standards.xml | 5 +++ .../invalid_deviations/coding-standards.yml | 3 ++ .../deviations/invalid_deviations/dummy.cpp | 1 - .../invalidcodeidentifiers.cpp | 22 ++++++++++++ 9 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md create mode 100644 cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql create mode 100644 cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected create mode 100644 cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.qlref delete mode 100644 cpp/common/test/deviations/invalid_deviations/dummy.cpp create mode 100644 cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index 731a04cfc7..8c55cc9428 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -74,6 +74,16 @@ abstract class CommentDeviationMarker extends Comment { DeviationRecord getRecord() { result = record } } +/** + * A deviation marker in a comment that is not a valid deviation marker. + */ +class InvalidCommentDeviationMarker extends Comment { + InvalidCommentDeviationMarker() { + not this instanceof CommentDeviationMarker and + commentMatches(this, "codeql::" + supportedStandard() + "_deviation") + } +} + /** * A deviation marker for a deviation that applies to the current line. */ @@ -182,9 +192,7 @@ private class BeginStack extends TBeginStack { } } -private predicate isDeviationRangePaired( - DeviationRecord record, DeviationBegin begin, DeviationEnd end -) { +predicate isDeviationRangePaired(DeviationRecord record, DeviationBegin begin, DeviationEnd end) { exists(File file, int index | record = end.getRecord() and hasDeviationCommentFileOrdering(record, end, file, index) and @@ -226,6 +234,21 @@ class DeviationAttribute extends StdAttribute { } } +/** + * A deviation attribute that is not associated with any deviation record. + */ +class InvalidDeviationAttribute extends StdAttribute { + string unknownCodeIdentifier; + + InvalidDeviationAttribute() { + this.hasQualifiedName("codeql", supportedStandard() + "_deviation") and + "\"" + unknownCodeIdentifier + "\"" = this.getAnArgument().getValueText() and + not exists(DeviationRecord record | record.getCodeIdentifier() = unknownCodeIdentifier) + } + + string getAnUnknownCodeIdentifier() { result = unknownCodeIdentifier } +} + newtype TCodeIndentifierDeviation = TSingleLineDeviation(DeviationRecord record, Comment comment, string filepath, int suppressedLine) { ( diff --git a/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md new file mode 100644 index 0000000000..9c128bab2d --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md @@ -0,0 +1,19 @@ +# Invalid deviation dode identifier + +## Overview + +Invalid deviation markers in code have no effect on the results but may indicate confusion over which results will be suppressed. + +Deviation code markers are used to suppress CodeQL Coding Standards results, following the process specified in the "MISRA Compliance 2020" document. There are a range of different deviation markers, with specific syntactic requirements. If those syntactic requirements are not met, the marker is invalid and will not be applied, which is likely contrary to developer expectations. + +## Recommendation + +Ensure the following requirements are met: + + * All `codeql::_deviation_begin(..)` markers are paired with a matching `codeql::_deviation_end(..)` marker. + * All instances of `codeql::_deviation` in comments are correctly formatted comment markers, and reference a `code-identifier`s that is specified in a deviation record included in the analysis. + * All deviation attributes reference `code-identifier`s that are specified in a deviation record included in the analysis. + +## References + +* [MISRA Compliance 2020 document - Chapter 4.2 (page 12) - Deviations](https://www.misra.org.uk/app/uploads/2021/06/MISRA-Compliance-2020.pdf) \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql new file mode 100644 index 0000000000..03e1ffc2b0 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql @@ -0,0 +1,35 @@ +/** + * @id cpp/coding-standards/invalid-deviation-code-identifiers + * @name Invalid deviation code identifiers + * @description Deviation code identifiers must be valid. + * @kind problem + * @problem.severity error + */ + +import cpp +import CodeIdentifierDeviation + +predicate deviationCodeIdentifierError(Element e, string message) { + exists(DeviationEnd end | + e = end and + not isDeviationRangePaired(_, _, end) and + message = "Deviation end block is unmatched." + ) + or + exists(InvalidDeviationAttribute b | + e = b and + message = + "Deviation attribute references unknown code identifier " + b.getAnUnknownCodeIdentifier() + + "." + ) + or + exists(InvalidCommentDeviationMarker m | + e = m and + message = + "Deviation marker does not match an expected format, or references an unknown code identifier." + ) +} + +from Element e, string message +where deviationCodeIdentifierError(e, message) +select e, message diff --git a/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected new file mode 100644 index 0000000000..eb81658007 --- /dev/null +++ b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected @@ -0,0 +1,10 @@ +| invalidcodeidentifiers.cpp:1:1:1:45 | // codeql::misra_deviation(x) - invalid, no x | Deviation marker does not match an expected format, or references an unknown code identifier. | +| invalidcodeidentifiers.cpp:2:1:2:47 | // codeql::autosar_deviation(x) - invalid, no x | Deviation marker does not match an expected format, or references an unknown code identifier. | +| invalidcodeidentifiers.cpp:3:1:3:44 | // codeql::cert_deviation(x) - invalid, no x | Deviation marker does not match an expected format, or references an unknown code identifier. | +| invalidcodeidentifiers.cpp:4:1:4:71 | // codeql::misra_deviation_next(a-0-4-2-deviation) - invalid, next_line | Deviation marker does not match an expected format, or references an unknown code identifier. | +| invalidcodeidentifiers.cpp:5:1:5:73 | // codeql::autosar_deviation_next(a-0-4-2-deviation) - invalid, next_line | Deviation marker does not match an expected format, or references an unknown code identifier. | +| invalidcodeidentifiers.cpp:6:1:6:70 | // codeql::cert_deviation_next(a-0-4-2-deviation) - invalid, next_line | Deviation marker does not match an expected format, or references an unknown code identifier. | +| invalidcodeidentifiers.cpp:14:1:14:74 | // codeql::misra_deviation_end(a-0-4-2-deviation) - invalid, unmatched end | Deviation end block is unmatched. | +| invalidcodeidentifiers.cpp:15:1:15:76 | // codeql::autosar_deviation_end(a-0-4-2-deviation) - invalid, unmatched end | Deviation end block is unmatched. | +| invalidcodeidentifiers.cpp:16:1:16:73 | // codeql::cert_deviation_end(a-0-4-2-deviation) - invalid, unmatched end | Deviation end block is unmatched. | +| invalidcodeidentifiers.cpp:18:3:18:25 | misra_deviation | Deviation attribute references unknown code identifier x. | diff --git a/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.qlref b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.qlref new file mode 100644 index 0000000000..c70989966f --- /dev/null +++ b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.qlref @@ -0,0 +1 @@ +codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql \ No newline at end of file diff --git a/cpp/common/test/deviations/invalid_deviations/coding-standards.xml b/cpp/common/test/deviations/invalid_deviations/coding-standards.xml index 179227a13d..2b5d798b35 100644 --- a/cpp/common/test/deviations/invalid_deviations/coding-standards.xml +++ b/cpp/common/test/deviations/invalid_deviations/coding-standards.xml @@ -86,6 +86,11 @@ RULE-13-6 c/misra/sizeof-operand-with-side-effect + + A0-4-2 + long double is required for interaction with third-party libraries. + a-0-4-2-deviation + diff --git a/cpp/common/test/deviations/invalid_deviations/coding-standards.yml b/cpp/common/test/deviations/invalid_deviations/coding-standards.yml index 7b12c7a8c2..679ef8a31e 100644 --- a/cpp/common/test/deviations/invalid_deviations/coding-standards.yml +++ b/cpp/common/test/deviations/invalid_deviations/coding-standards.yml @@ -46,6 +46,9 @@ deviations: - permit-id: DP2 - rule-id: RULE-13-6 query-id: c/misra/sizeof-operand-with-side-effect + - rule-id: A0-4-2 + justification: long double is required for interaction with third-party libraries. + code-identifier: a-0-4-2-deviation deviation-permits: - permit-id: DP1 justification: foo bar baz diff --git a/cpp/common/test/deviations/invalid_deviations/dummy.cpp b/cpp/common/test/deviations/invalid_deviations/dummy.cpp deleted file mode 100644 index 4a3cb36e40..0000000000 --- a/cpp/common/test/deviations/invalid_deviations/dummy.cpp +++ /dev/null @@ -1 +0,0 @@ -// Deliberately blank \ No newline at end of file diff --git a/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp b/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp new file mode 100644 index 0000000000..714c83d264 --- /dev/null +++ b/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp @@ -0,0 +1,22 @@ +// codeql::misra_deviation(x) - invalid, no x +// codeql::autosar_deviation(x) - invalid, no x +// codeql::cert_deviation(x) - invalid, no x +// codeql::misra_deviation_next(a-0-4-2-deviation) - invalid, next_line +// codeql::autosar_deviation_next(a-0-4-2-deviation) - invalid, next_line +// codeql::cert_deviation_next(a-0-4-2-deviation) - invalid, next_line + +// codeql::misra_deviation_begin(a-0-4-2-deviation) +// codeql::autosar_deviation_begin(a-0-4-2-deviation) +// codeql::cert_deviation_begin(a-0-4-2-deviation) +// codeql::misra_deviation_end(a-0-4-2-deviation) +// codeql::autosar_deviation_end(a-0-4-2-deviation) +// codeql::cert_deviation_end(a-0-4-2-deviation) +// codeql::misra_deviation_end(a-0-4-2-deviation) - invalid, unmatched end +// codeql::autosar_deviation_end(a-0-4-2-deviation) - invalid, unmatched end +// codeql::cert_deviation_end(a-0-4-2-deviation) - invalid, unmatched end + +[[codeql::misra_deviation("x")]] // invalid +void test() {} + +[[codeql::autosar_deviation("a-0-4-2-deviation")]] +void test2() {} From 5a6d7ca84f2dd1f00be7380d1800c095bc4a7439 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Thu, 13 Feb 2025 11:24:29 +0000 Subject: [PATCH 11/22] Deviations: Support attribute inheritence Attributes are inherited from their parent. Includes support for features which are not currently enabled, due to lack of support in CodeQL itself. --- .../deviations/CodeIdentifierDeviation.qll | 15 +++++++ .../TypeLongDoubleUsed.expected | 3 ++ .../UnusedReturnValue.expected | 2 + .../attribute_syntax.cpp | 39 +++++++++++++++++-- docs/user_manual.md | 5 +-- 5 files changed, 57 insertions(+), 7 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index 8c55cc9428..2a2d0eec15 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -231,6 +231,21 @@ class DeviationAttribute extends StdAttribute { result.(Stmt).getEnclosingFunction() = this.getASuppressedElement() or result.(LocalVariable) = this.getASuppressedElement().(DeclStmt).getADeclaration() + or + result.(Function).getDeclaringType() = this.getASuppressedElement() + or + result.(Variable).getDeclaringType() = this.getASuppressedElement() + or + exists(LambdaExpression expr | + expr = this.getASuppressedElement() and + result = expr.getLambdaFunction() + ) + or + exists(Function f | + f = this.getASuppressedElement() and + // A suppression on the function should apply to the noexcept expression + result = f.getADeclarationEntry().getNoExceptExpr() + ) } } diff --git a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected index 1786c4ce9e..172b623195 100644 --- a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected +++ b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected @@ -2,6 +2,9 @@ | attribute_syntax.cpp:21:15:21:17 | d10 | Use of long double type. | | attribute_syntax.cpp:29:15:29:17 | d14 | Use of long double type. | | attribute_syntax.cpp:33:20:33:22 | d16 | Use of long double type. | +| attribute_syntax.cpp:55:15:55:16 | d1 | Use of long double type. | +| attribute_syntax.cpp:57:17:57:18 | d2 | Use of long double type. | +| attribute_syntax.cpp:60:17:60:18 | d3 | Use of long double type. | | main.cpp:13:15:13:16 | d1 | Use of long double type. | | main.cpp:18:15:18:16 | d4 | Use of long double type. | | main.cpp:21:15:21:16 | d6 | Use of long double type. | diff --git a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected index 8b258328ab..120337ffdc 100644 --- a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected +++ b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected @@ -5,6 +5,8 @@ | attribute_syntax.cpp:26:5:26:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | attribute_syntax.cpp:30:3:30:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | attribute_syntax.cpp:41:3:41:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:49:5:49:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:61:5:61:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | main.cpp:12:3:12:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | | main.cpp:25:3:25:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | | main.cpp:27:3:27:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX | diff --git a/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp index 30acac3bfb..12a21e9673 100644 --- a/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp +++ b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp @@ -36,10 +36,41 @@ int alt() { } [[codeql::autosar_deviation("a-0-4-2-deviation")]] -int alt2() { +int test_function_deviation() { int x = 0; // COMPLIANT[DEVIATED] getZ(); // NON_COMPLIANT long double dd1; // COMPLIANT[DEVIATED] - [[codeql::autosar_deviation( - "a-0-4-2-deviation")]] long double dd2; // COMPLIANT[DEVIATED] -} \ No newline at end of file +} + +[[codeql::autosar_deviation("a-0-4-2-deviation")]] +void test_lambdas() { + auto l = []() { + long double d4; // COMPLIANT[DEVIATED] + getZ(); // NON_COMPLIANT + }; +} + +// Attributes are not supported on a class level at the moment +[[codeql::autosar_deviation("a-0-4-2-deviation")]] class ClassA { + long double d1; // COMPLIANT[DEVIATED - false positive] + class ClassNested { + long double d2; // COMPLIANT[DEVIATED - false positive] + }; + void test() { + long double d3; // COMPLIANT[DEVIATED - false positive] + getZ(); // NON_COMPLIANT + } +}; + +// static_assert, templates, noexcept, multiple declarations + +// Namespaces not currently supported by attributes +// [[codeql::autosar_deviation("a-0-4-2-deviation")]] namespace NS { +// long double d1; // COMPLIANT[DEVIATED] +// class ClassA { +// long double d1; // COMPLIANT[DEVIATED] +// }; +// void test() { +// long double d1; // COMPLIANT[DEVIATED] +// } +// } \ No newline at end of file diff --git a/docs/user_manual.md b/docs/user_manual.md index 9b58764eb8..a9b119f0da 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -446,9 +446,8 @@ This attribute may be added to the following program elements: * Functions * Statements * Variables - * Type declarations -Deviation attributes are inherited from parents in the code structure. For example, a deviation attribute applied to a function will apply the deviation to all code within the function. Note: deviations are not inherited by lambda expressions. +Deviation attributes are inherited from parents in the code structure. For example, a deviation attribute applied to a function will apply the deviation to all code within the function. Multiple code identifiers may be passed in a single attribute to apply multiple deviations, for example: @@ -507,7 +506,7 @@ Here are some examples, using the deviation record with the `a-0-4-2-deviation` 6 | 7 | // codeql::_deviation_end(a-0-4-2-deviation) ``` -Here, Line 1 will pair with Line 7, and Line 3 will pair with Line 8. +Here, Line 1 will pair with Line 7, and Line 3 will pair with Line 5. A `codeql::_deviation_end` without a matching `codeql::_deviation_begin`, or `codeql::_deviation_begin` without a matching `codeql::_deviation_end` is invalid and will be ignored. From e38e416dc60edcd4093146984c777f58fdfa7ffe Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Thu, 13 Feb 2025 11:30:50 +0000 Subject: [PATCH 12/22] Reformatting test file --- .../deviations_basic_test/attribute_syntax.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp index 12a21e9673..e363de55af 100644 --- a/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp +++ b/cpp/common/test/deviations/deviations_basic_test/attribute_syntax.cpp @@ -28,22 +28,22 @@ int alt() { } long double d14; // NON_COMPLIANT (A0-4-2) getZ(); // NON_COMPLIANT (A0-1-2) - [[codeql::autosar_deviation("a-0-4-2-deviation")]] - for (long double d15 = 0.0; true;) {} // COMPLIANT[DEVIATED] - for (long double d16 = 0.0; true;) { // NON_COMPLIANT (A0-4-2) + [[codeql::autosar_deviation("a-0-4-2-deviation")]] for (long double d15 = 0.0; + true;) { + } // COMPLIANT[DEVIATED] + for (long double d16 = 0.0; true;) { // NON_COMPLIANT (A0-4-2) } return 0; } -[[codeql::autosar_deviation("a-0-4-2-deviation")]] -int test_function_deviation() { +[[codeql::autosar_deviation("a-0-4-2-deviation")]] int +test_function_deviation() { int x = 0; // COMPLIANT[DEVIATED] getZ(); // NON_COMPLIANT long double dd1; // COMPLIANT[DEVIATED] } -[[codeql::autosar_deviation("a-0-4-2-deviation")]] -void test_lambdas() { +[[codeql::autosar_deviation("a-0-4-2-deviation")]] void test_lambdas() { auto l = []() { long double d4; // COMPLIANT[DEVIATED] getZ(); // NON_COMPLIANT From 9f35565b83aae672819b16a30b709de7a11de89d Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Thu, 13 Feb 2025 11:37:36 +0000 Subject: [PATCH 13/22] Add change note, update manual. --- change_notes/2025-02-13-deviations.md | 13 +++++++++++++ docs/user_manual.md | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 change_notes/2025-02-13-deviations.md diff --git a/change_notes/2025-02-13-deviations.md b/change_notes/2025-02-13-deviations.md new file mode 100644 index 0000000000..fb01cdf596 --- /dev/null +++ b/change_notes/2025-02-13-deviations.md @@ -0,0 +1,13 @@ + - A new in code deviation format has been introduced, using the C/C++ attribute syntax: + ``` + [[codeql::_deviation("")]] + ``` + This can be applied to functions, statements and variables to apply a deviation from the Coding Standards configuration file. The user manual has been updated to describe the new format. + - For those codebases that cannot use standard attributes, we have also introduced a comment based syntax + ``` + // codeql::_deviation() + // codeql::_deviation_next_line() + // codeql::_deviation_begin() + // codeql::_deviation_end() + ``` + Further information is available in the user manual. \ No newline at end of file diff --git a/docs/user_manual.md b/docs/user_manual.md index a9b119f0da..8bbb900682 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -439,7 +439,7 @@ A code identifier specified in a deviation record can be applied to certain resu [[codeql::_deviation("code-identifier")]] ``` -For example `[[codeql::misra_deviation("a1-2-4")]]` would apply a deviation of a rule in a MISRA standard, using the code identifier `a1-2-4`. The supported standard names are `misra`, `autosar` and `cert`. +For example `[[codeql::autosar_deviation("a1-2-4")]]` would apply a deviation of a rule in the AUTOSAR standard, using the code identifier `a1-2-4`. The supported standard names are `misra`, `autosar` and `cert`. This attribute may be added to the following program elements: From d2f8670f4e2ae6d1e625a19dd867b3f2be876765 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Thu, 13 Feb 2025 11:40:56 +0000 Subject: [PATCH 14/22] Reformat file --- .../deviations/invalid_deviations/invalidcodeidentifiers.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp b/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp index 714c83d264..07a12eb713 100644 --- a/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp +++ b/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp @@ -18,5 +18,4 @@ [[codeql::misra_deviation("x")]] // invalid void test() {} -[[codeql::autosar_deviation("a-0-4-2-deviation")]] -void test2() {} +[[codeql::autosar_deviation("a-0-4-2-deviation")]] void test2() {} From 0de32d39b093d59e6f3c857cd39cdb5cd91224bd Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Thu, 13 Feb 2025 15:15:35 +0000 Subject: [PATCH 15/22] Update expected results --- .../invalid_deviations/InvalidDeviationPermits.expected | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/common/test/deviations/invalid_deviations/InvalidDeviationPermits.expected b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationPermits.expected index 609d517c05..4378fdf11d 100644 --- a/cpp/common/test/deviations/invalid_deviations/InvalidDeviationPermits.expected +++ b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationPermits.expected @@ -1,2 +1,2 @@ -| coding-standards.xml:100:7:103:33 | deviation-permits-entry | coding-standards.xml: Deviation permit does not specify a permit identifier. | -| coding-standards.xml:104:7:107:33 | deviation-permits-entry | coding-standards.xml: Deviation permit specifies unknown property `invalid-property`. | +| coding-standards.xml:105:7:108:33 | deviation-permits-entry | coding-standards.xml: Deviation permit does not specify a permit identifier. | +| coding-standards.xml:109:7:112:33 | deviation-permits-entry | coding-standards.xml: Deviation permit specifies unknown property `invalid-property`. | From 029537df335fc3cd8dda1ec7a1d123a4f036114e Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 18 Feb 2025 13:36:22 +0000 Subject: [PATCH 16/22] Fix typos --- .../InvalidDeviationCodeIdentifier.md | 2 +- docs/user_manual.md | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md index 9c128bab2d..364e1ae915 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md +++ b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.md @@ -1,4 +1,4 @@ -# Invalid deviation dode identifier +# Invalid deviation code identifier ## Overview diff --git a/docs/user_manual.md b/docs/user_manual.md index 8bbb900682..fb36d30fb7 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -485,26 +485,26 @@ Here are some examples, using the deviation record with the `a-0-4-2-deviation` long double x2; // a-0-4-2-deviation - COMPLIANT long double x3; // COMPLIANT - a-0-4-2-deviation - long double x4; // codeql::_deviation(a-0-4-2-deviation) - COMPLIANT - long double x5; // COMPLIANT - codeql::_deviation(a-0-4-2-deviation) + long double x4; // codeql::autosar_deviation(a-0-4-2-deviation) - COMPLIANT + long double x5; // COMPLIANT - codeql::autosar_deviation(a-0-4-2-deviation) - // codeql::_deviation_next_line(a-0-4-2-deviation) + // codeql::autosar_deviation_next_line(a-0-4-2-deviation) long double x6; // COMPLIANT - // codeql::_deviation_begin(a-0-4-2-deviation) + // codeql::autosar_deviation_begin(a-0-4-2-deviation) long double x7; // COMPLIANT - // codeql::_deviation_end(a-0-4-2-deviation) + // codeql::autosar_deviation_end(a-0-4-2-deviation) ``` `codeql::_deviation_end` markers will pair with the closest unmatched `codeql::_deviation_begin` for the same `code-identifier`. Consider this example: ```cpp -1 | // codeql::_deviation_begin(a-0-4-2-deviation) +1 | // codeql::autosar_deviation_begin(a-0-4-2-deviation) 2 | -3 | // codeql::_deviation_begin(a-0-4-2-deviation) +3 | // codeql::autosar_deviation_begin(a-0-4-2-deviation) 4 | -5 | // codeql::_deviation_end(a-0-4-2-deviation) +5 | // codeql::autosar_deviation_end(a-0-4-2-deviation) 6 | -7 | // codeql::_deviation_end(a-0-4-2-deviation) +7 | // codeql::autosar_deviation_end(a-0-4-2-deviation) ``` Here, Line 1 will pair with Line 7, and Line 3 will pair with Line 5. From 3f1997b6ce3bac8e2c42d0d5f93f8f12430ed5e3 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 18 Feb 2025 13:36:40 +0000 Subject: [PATCH 17/22] Deviations: Highlight invalid starts --- .../cpp/deviations/InvalidDeviationCodeIdentifier.ql | 6 ++++++ .../InvalidDeviationCodeIdentifier.expected | 5 ++++- .../invalid_deviations/invalidcodeidentifiers.cpp | 3 +++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql index 03e1ffc2b0..87dafbba13 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql +++ b/cpp/common/src/codingstandards/cpp/deviations/InvalidDeviationCodeIdentifier.ql @@ -16,6 +16,12 @@ predicate deviationCodeIdentifierError(Element e, string message) { message = "Deviation end block is unmatched." ) or + exists(DeviationBegin begin | + e = begin and + not isDeviationRangePaired(_, begin, _) and + message = "Deviation start block is unmatched." + ) + or exists(InvalidDeviationAttribute b | e = b and message = diff --git a/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected index eb81658007..1d7153bafd 100644 --- a/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected +++ b/cpp/common/test/deviations/invalid_deviations/InvalidDeviationCodeIdentifier.expected @@ -7,4 +7,7 @@ | invalidcodeidentifiers.cpp:14:1:14:74 | // codeql::misra_deviation_end(a-0-4-2-deviation) - invalid, unmatched end | Deviation end block is unmatched. | | invalidcodeidentifiers.cpp:15:1:15:76 | // codeql::autosar_deviation_end(a-0-4-2-deviation) - invalid, unmatched end | Deviation end block is unmatched. | | invalidcodeidentifiers.cpp:16:1:16:73 | // codeql::cert_deviation_end(a-0-4-2-deviation) - invalid, unmatched end | Deviation end block is unmatched. | -| invalidcodeidentifiers.cpp:18:3:18:25 | misra_deviation | Deviation attribute references unknown code identifier x. | +| invalidcodeidentifiers.cpp:17:1:17:78 | // codeql::misra_deviation_begin(a-0-4-2-deviation) - invalid, unmatched begin | Deviation start block is unmatched. | +| invalidcodeidentifiers.cpp:18:1:18:80 | // codeql::autosar_deviation_begin(a-0-4-2-deviation) - invalid, unmatched begin | Deviation start block is unmatched. | +| invalidcodeidentifiers.cpp:19:1:19:77 | // codeql::cert_deviation_begin(a-0-4-2-deviation) - invalid, unmatched begin | Deviation start block is unmatched. | +| invalidcodeidentifiers.cpp:21:3:21:25 | misra_deviation | Deviation attribute references unknown code identifier x. | diff --git a/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp b/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp index 07a12eb713..a4da098dcb 100644 --- a/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp +++ b/cpp/common/test/deviations/invalid_deviations/invalidcodeidentifiers.cpp @@ -14,6 +14,9 @@ // codeql::misra_deviation_end(a-0-4-2-deviation) - invalid, unmatched end // codeql::autosar_deviation_end(a-0-4-2-deviation) - invalid, unmatched end // codeql::cert_deviation_end(a-0-4-2-deviation) - invalid, unmatched end +// codeql::misra_deviation_begin(a-0-4-2-deviation) - invalid, unmatched begin +// codeql::autosar_deviation_begin(a-0-4-2-deviation) - invalid, unmatched begin +// codeql::cert_deviation_begin(a-0-4-2-deviation) - invalid, unmatched begin [[codeql::misra_deviation("x")]] // invalid void test() {} From 665a7d16c8c0f2339075c3097b40405bb393fc97 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 18 Feb 2025 16:50:33 +0000 Subject: [PATCH 18/22] Improve documentation --- .../codingstandards/cpp/deviations/CodeIdentifierDeviation.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index 2a2d0eec15..ab121f6095 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -145,6 +145,8 @@ private predicate mkBeginStack(DeviationRecord record, File file, BeginStack sta // Stack is empty at the start index = 0 and stack = TEmptyBeginStack() and + // Only initialize when there is at least one such comment marker for this file and record + // pairing exists(CommentDeviationRangeMarker marker | marker.getRecord() = record and marker.getLocation().getFile() = file ) From 8437c7cc76d011455580810501ccd7de2ce26190 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 18 Feb 2025 17:34:51 +0000 Subject: [PATCH 19/22] Deviation: Refactor begin/end for clarity. --- .../deviations/CodeIdentifierDeviation.qll | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index ab121f6095..9a694ccc8f 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -141,10 +141,14 @@ private predicate hasDeviationCommentFileOrdering( ) } -private predicate mkBeginStack(DeviationRecord record, File file, BeginStack stack, int index) { +/** + * Calculate the stack of deviation begin markers related to the given deviation record, in the given file, + * at the given `markerRecordFileIndex` into the list of deviation markers for that record in that file. + */ +private BeginStack calculateBeginStack(DeviationRecord record, File file, int markerRecordFileIndex) { // Stack is empty at the start - index = 0 and - stack = TEmptyBeginStack() and + markerRecordFileIndex = 0 and + result = TEmptyBeginStack() and // Only initialize when there is at least one such comment marker for this file and record // pairing exists(CommentDeviationRangeMarker marker | @@ -154,23 +158,23 @@ private predicate mkBeginStack(DeviationRecord record, File file, BeginStack sta // Next token is begin, so push it to the stack exists(DeviationBegin begin, BeginStack prev | record = begin.getRecord() and - hasDeviationCommentFileOrdering(record, begin, file, index) and - mkBeginStack(record, file, prev, index - 1) and - stack = TConsBeginStack(begin, prev) + hasDeviationCommentFileOrdering(record, begin, file, markerRecordFileIndex) and + prev = calculateBeginStack(record, file, markerRecordFileIndex - 1) and + result = TConsBeginStack(begin, prev) ) or // Next token is end exists(DeviationEnd end, BeginStack prevStack | record = end.getRecord() and - hasDeviationCommentFileOrdering(record, end, file, index) and - mkBeginStack(record, file, prevStack, index - 1) + hasDeviationCommentFileOrdering(record, end, file, markerRecordFileIndex) and + prevStack = calculateBeginStack(record, file, markerRecordFileIndex - 1) | // There is, so pop the most recent begin off the stack - prevStack = TConsBeginStack(_, stack) + prevStack = TConsBeginStack(_, result) or - // Error, no begin on the stack, ignore and continue + // Error, no begin on the stack, ignore the end and continue prevStack = TEmptyBeginStack() and - stack = TEmptyBeginStack() + result = TEmptyBeginStack() ) } @@ -178,12 +182,18 @@ newtype TBeginStack = TConsBeginStack(DeviationBegin begin, TBeginStack prev) { exists(File file, int index | hasDeviationCommentFileOrdering(begin.getRecord(), begin, file, index) and - mkBeginStack(begin.getRecord(), file, prev, index - 1) + prev = calculateBeginStack(begin.getRecord(), file, index - 1) ) } or TEmptyBeginStack() +/** + * A stack of begin markers that occur in the same file, referring to the same record. + */ private class BeginStack extends TBeginStack { + /** Gets the top begin marker on the stack. */ + DeviationBegin peek() { this = TConsBeginStack(result, _) } + string toString() { exists(DeviationBegin begin, BeginStack prev | this = TConsBeginStack(begin, prev) | result = "(" + begin + ", " + prev.toString() + ")" @@ -198,7 +208,7 @@ predicate isDeviationRangePaired(DeviationRecord record, DeviationBegin begin, D exists(File file, int index | record = end.getRecord() and hasDeviationCommentFileOrdering(record, end, file, index) and - mkBeginStack(record, file, TConsBeginStack(begin, _), index - 1) + begin = calculateBeginStack(record, file, index - 1).peek() ) } From 20669c7cc43e2c5fb2b6ab6cb2e76a6196ad2102 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 18 Feb 2025 17:35:08 +0000 Subject: [PATCH 20/22] Deviation: Clarification in user manual. --- docs/user_manual.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user_manual.md b/docs/user_manual.md index fb36d30fb7..a04ba0814e 100644 --- a/docs/user_manual.md +++ b/docs/user_manual.md @@ -512,7 +512,7 @@ A `codeql::_deviation_end` without a matching `codeql::_devi `codeql::_deviation_begin` and `ccodeql::_deviation_end` markers only apply within a single file. Markers cannot be paired across files, and deviations do not apply to included files. -Note: deviation markers cannot be applied to the body of a macro. Please apply the deviation to macro expansion, or use the attribute deviation format. +Note: deviation comment markers cannot be applied to the body of a macro. Please apply the deviation to macro expansion, or use the attribute deviation format. ##### Deviation permits From 9e35e593bd116a22ae8a9f1e2de2fc41a75c9072 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 18 Feb 2025 23:07:52 +0000 Subject: [PATCH 21/22] Deviations: use getADeviationRecord Deviation code identifier markers can have multiple records. --- .../cpp/deviations/CodeIdentifierDeviation.qll | 12 ++++++------ .../codingstandards/cpp/deviations/Deviations.qll | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll index 9a694ccc8f..310a2b678b 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll @@ -224,7 +224,7 @@ class DeviationAttribute extends StdAttribute { "\"" + record.getCodeIdentifier() + "\"" = this.getAnArgument().getValueText() } - DeviationRecord getDeviationRecord() { result = record } + DeviationRecord getADeviationRecord() { result = record } pragma[nomagic] Element getASuppressedElement() { @@ -296,12 +296,12 @@ newtype TCodeIndentifierDeviation = endComment.getLocation().hasLocationInfo(filepath, suppressedEndLine, _, _, _) } or TCodeIdentifierDeviation(DeviationRecord record, DeviationAttribute attribute) { - attribute.getDeviationRecord() = record + attribute.getADeviationRecord() = record } class CodeIdentifierDeviation extends TCodeIndentifierDeviation { /** The deviation record associated with the deviation comment. */ - DeviationRecord getDeviationRecord() { + DeviationRecord getADeviationRecord() { this = TSingleLineDeviation(result, _, _, _) or this = TMultiLineDeviation(result, _, _, _, _, _) @@ -341,21 +341,21 @@ class CodeIdentifierDeviation extends TCodeIndentifierDeviation { exists(int suppressedLine | this = TSingleLineDeviation(_, _, filepath, suppressedLine) and result = - "Deviation record " + getDeviationRecord() + " applied to " + filepath + " Line " + + "Deviation record " + getADeviationRecord() + " applied to " + filepath + " Line " + suppressedLine ) or exists(int suppressedStartLine, int suppressedEndLine | this = TMultiLineDeviation(_, _, _, filepath, suppressedStartLine, suppressedEndLine) and result = - "Deviation record " + getDeviationRecord() + " applied to " + filepath + " Line" + + "Deviation record " + getADeviationRecord() + " applied to " + filepath + " Line" + suppressedStartLine + ":" + suppressedEndLine ) ) or exists(DeviationAttribute attribute | this = TCodeIdentifierDeviation(_, attribute) and - result = "Deviation record " + getDeviationRecord() + " applied to " + attribute + result = "Deviation record " + getADeviationRecord() + " applied to " + attribute ) } } diff --git a/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll b/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll index 434d6988e9..e8c030cdd4 100644 --- a/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll +++ b/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll @@ -221,7 +221,7 @@ class DeviationRecord extends XmlElement { } /** Gets a code identifier deviation in code which starts or ends with the code identifier comment. */ - CodeIdentifierDeviation getACodeIdentifierDeviation() { this = result.getDeviationRecord() } + CodeIdentifierDeviation getACodeIdentifierDeviation() { this = result.getADeviationRecord() } /** Gets the `rule-id` specified for this record, if any. */ private string getRawRuleId() { result = getAChild("rule-id").getTextValue() } From b273d0f66fbf252dcc5ab8bb865285fd79bdd3ed Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 18 Feb 2025 23:12:11 +0000 Subject: [PATCH 22/22] Deviations: Update expected tests --- .../deviations_basic_test/TypeLongDoubleUsed.expected | 2 +- .../deviations/deviations_basic_test/UnusedReturnValue.expected | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected index 172b623195..f2cfd03dc6 100644 --- a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected +++ b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected @@ -1,7 +1,7 @@ | attribute_syntax.cpp:6:15:6:17 | dd1 | Use of long double type. | | attribute_syntax.cpp:21:15:21:17 | d10 | Use of long double type. | | attribute_syntax.cpp:29:15:29:17 | d14 | Use of long double type. | -| attribute_syntax.cpp:33:20:33:22 | d16 | Use of long double type. | +| attribute_syntax.cpp:34:20:34:22 | d16 | Use of long double type. | | attribute_syntax.cpp:55:15:55:16 | d1 | Use of long double type. | | attribute_syntax.cpp:57:17:57:18 | d2 | Use of long double type. | | attribute_syntax.cpp:60:17:60:18 | d3 | Use of long double type. | diff --git a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected index 120337ffdc..fc7af4b197 100644 --- a/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected +++ b/cpp/common/test/deviations/deviations_basic_test/UnusedReturnValue.expected @@ -4,7 +4,7 @@ | attribute_syntax.cpp:24:5:24:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | attribute_syntax.cpp:26:5:26:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | attribute_syntax.cpp:30:3:30:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | -| attribute_syntax.cpp:41:3:41:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | +| attribute_syntax.cpp:42:3:42:6 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | attribute_syntax.cpp:49:5:49:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | attribute_syntax.cpp:61:5:61:8 | call to getZ | Return value from call to $@ is unused. | attribute_syntax.cpp:1:5:1:8 | getZ | getZ | | main.cpp:12:3:12:6 | call to getX | Return value from call to $@ is unused. | main.cpp:8:5:8:8 | getX | getX |