diff --git a/cpp/common/src/codingstandards/cpp/Exclusions.qll b/cpp/common/src/codingstandards/cpp/Exclusions.qll index b718f6535..e6a477b22 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/CodeIdentifierDeviation.qll b/cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll new file mode 100644 index 000000000..e6220711a --- /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 + ) + ) + } +} diff --git a/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll b/cpp/common/src/codingstandards/cpp/deviations/Deviations.qll index 4dfadd12e..2388f95a3 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() } 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 0bf3a3a71..000000000 --- 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 9035b7d28..000000000 --- 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 diff --git a/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected b/cpp/common/test/deviations/deviations_basic_test/TypeLongDoubleUsed.expected index 7b78d5489..a4e045edc 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 0b302ea1f..53258f00f 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 diff --git a/docs/user_manual.md b/docs/user_manual.md index 4c020dc73..7ad4dc420 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_.