Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for deviations on next line and multiple lines #807

Merged
merged 23 commits into from
Mar 12, 2025

Conversation

lcartey
Copy link
Collaborator

@lcartey lcartey commented Dec 3, 2024

Description

This PR proposes an extension of our deviations mechanism to support easier suppression of results at scale - including applying deviations to a block of code, to code on the next line and to code in macros.

We introduce a C/C++ attribute format - [[codeql::<standard>_deviation(<code-identifier>)]] - which applies the deviation distinguished by the <code-identifier> to the program element, and those nested within it. This attribute can be applied to:

  • Statements
  • Variables
  • Functions

For example, [[codeql::autosar_deviation("a0-4-2")]] would apply the deviation specified using the code identifier a0-4-2, related to the autosar Coding Standard.

Custom attributes are not universally supported - for example, they were only introduced in C23 - so we also provide an attribute-like syntax to be used in comment markers:

  • codeql::<standard>_deviation(<code-identifier>) - the deviation applies to results on the current line (duplication of existing non-annotated support, included for consistency).
  • codeql::<standard>_deviation_next_line(<code-identifier>) - this deviation applies to results on the next line.
  • codeql::<standard>_deviation_begin(<code-identifier>) - marks the beginning of a range of lines where the deviation applies.
  • codeql::<standard>_deviation_end(<code-identifier>) - marks the end of a range of lines where the deviation applies.

This would address #326.

Change request type

  • Release or process automation (GitHub workflows, internal scripts)
  • Internal documentation
  • External documentation
  • Query files (.ql, .qll, .qls or unit tests)
  • External scripts (analysis report or other code shipped as part of a release)

Rules with added or modified queries

  • No rules added
  • Queries have been added for the following rules:
    • rule number here
  • Queries have been modified for the following rules:
    • rule number here

Release change checklist

A change note (development_handbook.md#change-notes) is required for any pull request which modifies:

  • The structure or layout of the release artifacts.
  • The evaluation performance (memory, execution time) of an existing query.
  • The results of an existing query in any circumstance.

If you are only adding new rule queries, a change note is not required.

Author: Is a change note required?

  • Yes
  • No

🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.

  • Confirmed

Reviewer: Confirm that either a change note is not required or the change note is required and has been added.

  • Confirmed

Query development review checklist

For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:

Author

  • Have all the relevant rule package description files been checked in?
  • Have you verified that the metadata properties of each new query is set appropriately?
  • Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
  • Are the alert messages properly formatted and consistent with the style guide?
  • Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
    As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
  • Does the query have an appropriate level of in-query comments/documentation?
  • Have you considered/identified possible edge cases?
  • Does the query not reinvent features in the standard library?
  • Can the query be simplified further (not golfed!)

Reviewer

  • Have all the relevant rule package description files been checked in?
  • Have you verified that the metadata properties of each new query is set appropriately?
  • Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
  • Are the alert messages properly formatted and consistent with the style guide?
  • Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
    As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
  • Does the query have an appropriate level of in-query comments/documentation?
  • Have you considered/identified possible edge cases?
  • Does the query not reinvent features in the standard library?
  • Can the query be simplified further (not golfed!)

lgtm-style suppressions are no longer supported by CodeQL and
Code Scanning.
This new library supports deviating on the next line, or on ranges,
in addition to deviating on the current line.
This ties in the code-identifier deviation support to the deviations
and exclusions libraries.
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.
Attributes are inherited from their parent.

Includes support for features which are not currently enabled,
due to lack of support in CodeQL itself.
Copy link
Contributor

@MichaelRFairhurst MichaelRFairhurst left a comment

Choose a reason for hiding this comment

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

Fun PR to read through, very cool to see the stack as a solution for implementing pairings in an immutable context like CodeQL! Some thoughts on it since its a cool weedy coding task!

Copy link
Contributor

@MichaelRFairhurst MichaelRFairhurst left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM! Just one question left.

Deviation code identifier markers can have multiple records.
@lcartey
Copy link
Collaborator Author

lcartey commented Feb 18, 2025

/test-performance

Copy link

🏁 Beep Boop! Performance testing for this PR has been initiated. Please check back later for results. Note that the query package generation step must complete before testing will start so it might be a minute.

💡 If you do not hear back from me please check my status! I will report even if I fail!

🏁 Beep Boop! Performance testing complete! See below for performance of the last 3 runs vs your PR. Times are based on predicate performance. You can find full graphs and stats in the PR that was created for this test in the release engineering repo.


Release                            : v2.40.0
Platform                           : x86-linux
Language                           : c
Total_Serialized_Execution_Time_Ms : 2324585
Mean_Predicate_Execution_Time_Ms   : 49.33329796264856
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 609.8238396111128
Total_Serialized_Execution_Time_s  : 2324.585
Mean_Query_Execution_Time_s        : 0.0493332979626485
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 130.0
Number_of_Predicates               : 47120

Release                            : v2.40.0
Platform                           : x86-linux
Language                           : cpp
Total_Serialized_Execution_Time_Ms : 3401112
Mean_Predicate_Execution_Time_Ms   : 71.22300169622851
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 833.5216392962146
Total_Serialized_Execution_Time_s  : 3401.112
Mean_Query_Execution_Time_s        : 0.0712230016962285
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 164.0
Number_of_Predicates               : 47753

Release                            : v2.40.0
Platform                           : x86-windows
Language                           : c
Total_Serialized_Execution_Time_Ms : 2750095
Mean_Predicate_Execution_Time_Ms   : 58.13416902718471
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 625.59540936855
Total_Serialized_Execution_Time_s  : 2750.095
Mean_Query_Execution_Time_s        : 0.0581341690271847
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 157.0
Number_of_Predicates               : 47306

Release                            : v2.40.0
Platform                           : x86-windows
Language                           : cpp
Total_Serialized_Execution_Time_Ms : 3458422
Mean_Predicate_Execution_Time_Ms   : 71.6088702998178
Median_Predicate_Execution_Time_Ms : 1.0
Standard_Deviation_Ms              : 715.7345866890012
Total_Serialized_Execution_Time_s  : 3458.422
Mean_Query_Execution_Time_s        : 0.0716088702998178
Median_Predicate_Execution_Time_s  : 0.001
Percentile95_Ms                    : 185.0
Number_of_Predicates               : 48296

Release                            : v2.41.0
Platform                           : x86-linux
Language                           : c
Total_Serialized_Execution_Time_Ms : 2294636
Mean_Predicate_Execution_Time_Ms   : 48.58120381936358
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 604.830695062583
Total_Serialized_Execution_Time_s  : 2294.636
Mean_Query_Execution_Time_s        : 0.0485812038193635
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 125.0
Number_of_Predicates               : 47233

Release                            : v2.41.0
Platform                           : x86-linux
Language                           : cpp
Total_Serialized_Execution_Time_Ms : 3386576
Mean_Predicate_Execution_Time_Ms   : 71.21537620389452
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 819.0328105072083
Total_Serialized_Execution_Time_s  : 3386.576
Mean_Query_Execution_Time_s        : 0.0712153762038945
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 158.0
Number_of_Predicates               : 47554

Release                            : v2.41.0
Platform                           : x86-windows
Language                           : c
Total_Serialized_Execution_Time_Ms : 2793839
Mean_Predicate_Execution_Time_Ms   : 59.077604618214885
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 617.7343684735914
Total_Serialized_Execution_Time_s  : 2793.839
Mean_Query_Execution_Time_s        : 0.0590776046182148
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 156.0
Number_of_Predicates               : 47291

Release                            : v2.41.0
Platform                           : x86-windows
Language                           : cpp
Total_Serialized_Execution_Time_Ms : 3242287
Mean_Predicate_Execution_Time_Ms   : 67.19905075752865
Median_Predicate_Execution_Time_Ms : 1.0
Standard_Deviation_Ms              : 670.443137227368
Total_Serialized_Execution_Time_s  : 3242.287
Mean_Query_Execution_Time_s        : 0.0671990507575286
Median_Predicate_Execution_Time_s  : 0.001
Percentile95_Ms                    : 184.0
Number_of_Predicates               : 48249

Release                            : 807
Platform                           : x86-linux
Language                           : cpp
Total_Serialized_Execution_Time_Ms : 3080795
Mean_Predicate_Execution_Time_Ms   : 63.72520426104044
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 838.3441207331775
Total_Serialized_Execution_Time_s  : 3080.795
Mean_Query_Execution_Time_s        : 0.0637252042610404
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 160.0
Number_of_Predicates               : 48345

Release                            : 807
Platform                           : x86-linux
Language                           : c
Total_Serialized_Execution_Time_Ms : 2592925
Mean_Predicate_Execution_Time_Ms   : 51.10118050491713
Median_Predicate_Execution_Time_Ms : 0.0
Standard_Deviation_Ms              : 781.4418270988062
Total_Serialized_Execution_Time_s  : 2592.925
Mean_Query_Execution_Time_s        : 0.0511011805049171
Median_Predicate_Execution_Time_s  : 0.0
Percentile95_Ms                    : 122.0
Number_of_Predicates               : 50741

🏁 Below are the slowest predicates for the last 2 releases vs this PR.


Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : c
Suite             : misra-default
Predicate         : _ExternalFlow::elementSpecMatchesSignature/6#c67d3446_Function::Function.getClassAndName/1#27b7404e___#shared
Execution_Time_Ms : 44405

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : c
Suite             : misra-default
Predicate         : QualifiedName::getUserTypeNameWithoutArgs/1#8cfc98e9
Execution_Time_Ms : 29847

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : OutOfBounds::OOB::libraryFunctionNameParamTable/5#79217c12
Execution_Time_Ms : 33073

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : c
Suite             : misra-default
Predicate         : _Macro::Macro.getName/0#dispred#e28b3699_Preprocessor::PreprocessorBranchDirective#bcd2bde4_Preproce__#antijoin_rhs
Execution_Time_Ms : 63221

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : OutOfBounds::OOB::libraryFunctionNameParamTableSimpleString/5#6de8614f#cpe#1236
Execution_Time_Ms : 39638

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : cpp
Suite             : autosar-default
Predicate         : _comments#shared
Execution_Time_Ms : 26504

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : cpp
Suite             : autosar-default
Predicate         : __Class::Class.accessOfBaseMemberMulti/2#6d0f61c0#ffbf_specifiers#max_range__Class::Class.accessOfBa__#join_rhs
Execution_Time_Ms : 33628

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : cpp
Suite             : autosar-default
Predicate         : HardwareOrProtocolInterface::HardwareOrProtocolInterfaceClass#class#7f026171
Execution_Time_Ms : 32209

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : cpp
Suite             : autosar-default
Predicate         : _ExternalFlow::elementSpecMatchesSignature/6#c67d3446_Function::Function.getClassAndName/1#27b7404e___#shared
Execution_Time_Ms : 67471

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : c
Suite             : misra-default
Predicate         : OutOfBounds::OOB::libraryFunctionNameParamTableSimpleString/5#6de8614f#cpe#1236
Execution_Time_Ms : 39301

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : c
Suite             : misra-default
Predicate         : OutOfBounds::OOB::libraryFunctionNameParamTable/5#79217c12
Execution_Time_Ms : 30349

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : _Macro::Macro.getName/0#dispred#e28b3699_Preprocessor::PreprocessorBranchDirective#bcd2bde4_Preproce__#antijoin_rhs
Execution_Time_Ms : 66713

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : @element#b
Execution_Time_Ms : 37358

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : _ExternalFlow::elementSpecMatchesSignature/6#c67d3446_Function::Function.getClassAndName/1#27b7404e___#shared
Execution_Time_Ms : 47157

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : _Call::FunctionCall#2b14a517_Call::FunctionCall.getTarget/0#dispred#935da4c5_Function::Function.getP__#antijoin_rhs
Execution_Time_Ms : 27555

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : FunctionEquivalence::typeSig/1#194ac728
Execution_Time_Ms : 48401

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : _comments#shared
Execution_Time_Ms : 38869

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : OrderingPredicateMustBeStrictlyWeak::UserDefinedStrictlyWeakOrderingComparator#6c60f803
Execution_Time_Ms : 37907

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : _Call::Call.getAnArgument/0#dispred#0c956949_10#join_rhs_Call::FunctionCall#2b14a517_Element::Elemen__#shared
Execution_Time_Ms : 34634

Release           : v2.41.0
Run               : 2025-01-27_15-31-47
Platform          : x86-windows
Language          : cpp
Suite             : autosar-default
Predicate         : FunctionEquivalence::typeSig/1#194ac728
Execution_Time_Ms : 53166

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : _comments#shared
Execution_Time_Ms : 45242

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : UnusedVariables::maybeACompileTimeTemplateArgument/1#9ea118f0
Execution_Time_Ms : 69124

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : Function::Function.getParameter/1#dispred#200dcf26#fbf
Execution_Time_Ms : 76438

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : _Macro::Macro.getName/0#dispred#e28b3699_Preprocessor::PreprocessorBranchDirective#bcd2bde4_Preproce__#antijoin_rhs
Execution_Time_Ms : 59978

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : ResourceLeakAnalysis::ResourceLeakConfigSig[MutexObjectsNotAlwaysUnlocked::MutexLeakConfig]::getAnAlias/1#e76446be
Execution_Time_Ms : 104697

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : OutOfBounds::OOB::libraryFunctionNameParamTableSimpleString/5#6de8614f#cpe#1236
Execution_Time_Ms : 39521

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : _ExternalFlow::elementSpecMatchesSignature/6#c67d3446_Function::Function.getClassAndName/1#27b7404e___#shared
Execution_Time_Ms : 39656

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : cpp
Suite             : autosar-default
Predicate         : _Declaration::Declaration.getADeclarationEntry/0#dispred#c5d61b67_Declaration::DeclarationEntry.isDe__#antijoin_rhs
Execution_Time_Ms : 48111

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : OutOfBounds::OOB::libraryFunctionNameParamTable/5#79217c12
Execution_Time_Ms : 43210

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : _ExternalFlow::elementSpecMatchesSignature/6#c67d3446_Function::Function.getClassAndName/1#27b7404e___#shared
Execution_Time_Ms : 59198

@fjatWbyT
Copy link
Contributor


Release                            : v2.40.0
Platform                           : x86-linux
Language                           : c
[ snip ]
Mean_Predicate_Execution_Time_Ms   : 49.33329796264856
[ snip ]
Mean_Query_Execution_Time_s        : 0.0493332979626485
[ snip ]

While going through the first table, I think that Mean_Query_Execution_Time_s is Mean_Predicate_Execution_Time_s.

The salient result I noticed in 807 is in the second table:

Release           : 807
Run               : 2025-02-18_23-18-05
Platform          : x86-linux
Language          : c
Suite             : misra-default
Predicate         : ResourceLeakAnalysis::ResourceLeakConfigSig[MutexObjectsNotAlwaysUnlocked::MutexLeakConfig]::getAnAlias/1#e76446be
Execution_Time_Ms : 104697

Copy link
Contributor

@MichaelRFairhurst MichaelRFairhurst left a comment

Choose a reason for hiding this comment

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

LGTM!

@MichaelRFairhurst MichaelRFairhurst added this pull request to the merge queue Mar 12, 2025
@MichaelRFairhurst
Copy link
Contributor

@fjatWbyT yes, good catch. That was introduced past the last release (by me!) and was fixed. Overall, esp. with that fixed, everything here looks great to me.

Merged via the queue into main with commit afbfbec Mar 12, 2025
25 checks passed
@MichaelRFairhurst MichaelRFairhurst deleted the lcartey/extend-deviations branch March 12, 2025 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants