From 5d3c27b32c7634eacaa7614b3ce63c858e2d46a1 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 27 Feb 2024 15:17:29 -0500 Subject: [PATCH 01/22] A2-10-1: add functions and types to identifier consideration --- change_notes/2024-02-27-identifier-hidden.md | 2 + cpp/common/src/codingstandards/cpp/Scope.qll | 78 ++++++++++--------- .../identifierhidden/IdentifierHidden.qll | 4 +- .../IdentifierHidden.expected | 24 +++--- .../test/rules/identifierhidden/test.cpp | 9 +++ 5 files changed, 68 insertions(+), 49 deletions(-) create mode 100644 change_notes/2024-02-27-identifier-hidden.md diff --git a/change_notes/2024-02-27-identifier-hidden.md b/change_notes/2024-02-27-identifier-hidden.md new file mode 100644 index 000000000..a2cffb9d2 --- /dev/null +++ b/change_notes/2024-02-27-identifier-hidden.md @@ -0,0 +1,2 @@ +- `A2-10-1`, `RULE-5-3` - `IdentifierHiding.ql`, `IdentifierHidingC.ql`: + - Address FN reported in #118. Rule was missing detection of functions and types. \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 4dd727b8d..7659cdb4f 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -57,9 +57,15 @@ private Element getParentScope(Element e) { /** A variable which is defined by the user, rather than being from a third party or compiler generated. */ class UserVariable extends Variable { - UserVariable() { + UserVariable() { this instanceof UserDeclaration } +} + +/** A construct which is defined by the user, rather than being from a third party or compiler generated. */ +class UserDeclaration extends Declaration { + UserDeclaration() { exists(getFile().getRelativePath()) and - not isCompilerGenerated() and + not this.(Variable).isCompilerGenerated() and + not this.(Function).isCompilerGenerated() and not this.(Parameter).getFunction().isCompilerGenerated() and // compiler inferred parameters have name of p#0 not this.(Parameter).getName() = "p#0" @@ -78,7 +84,7 @@ class Scope extends Element { Scope getStrictParent() { result = getParentScope(this) } - Declaration getADeclaration() { getParentScope(result) = this } + UserDeclaration getADeclaration() { getParentScope(result) = this } Expr getAnExpr() { this = getParentScope(result) } @@ -122,30 +128,30 @@ class GeneratedBlockStmt extends BlockStmt { GeneratedBlockStmt() { this.getLocation() instanceof UnknownLocation } } -/** Gets a variable that is in the potential scope of variable `v`. */ -private UserVariable getPotentialScopeOfVariable_candidate(UserVariable v) { +/** Gets a Declaration that is in the potential scope of Declaration `v`. */ +private UserDeclaration getPotentialScopeOfDeclaration_candidate(UserDeclaration v) { exists(Scope s | - result = s.getAVariable() and + result = s.getADeclaration() and ( - // Variable in an ancestor scope, but only if there are less than 100 variables in this scope - v = s.getAnAncestor().getAVariable() and + // Declaration in an ancestor scope, but only if there are less than 100 variables in this scope + v = s.getAnAncestor().getADeclaration() and s.getNumberOfVariables() < 100 or - // In the same scope, but not the same variable, and choose just one to report - v = s.getAVariable() and + // In the same scope, but not the same Declaration, and choose just one to report + v = s.getADeclaration() and not result = v and v.getName() <= result.getName() ) ) } -/** Gets a variable that is in the potential scope of variable `v`. */ -private UserVariable getOuterScopesOfVariable_candidate(UserVariable v) { +/** Gets a Declarationthat is in the potential scope of Declaration `v`. */ +private UserDeclaration getOuterScopesOfDeclaration_candidate(UserDeclaration v) { exists(Scope s | - result = s.getAVariable() and + result = s.getADeclaration() and ( - // Variable in an ancestor scope, but only if there are less than 100 variables in this scope - v = s.getAnAncestor().getAVariable() and + // Declaration in an ancestor scope, but only if there are less than 100 variables in this scope + v = s.getAnAncestor().getADeclaration() and s.getNumberOfVariables() < 100 ) ) @@ -161,20 +167,20 @@ predicate inSameTranslationUnit(File f1, File f2) { } /** - * Gets a user variable which occurs in the "potential scope" of variable `v`. + * Gets a user Declaration which occurs in the "outer scope" of Declaration `v`. */ cached -UserVariable getPotentialScopeOfVariable(UserVariable v) { - result = getPotentialScopeOfVariable_candidate(v) and +UserDeclaration getPotentialScopeOfDeclarationStrict(UserDeclaration v) { + result = getOuterScopesOfDeclaration_candidate(v) and inSameTranslationUnit(v.getFile(), result.getFile()) } /** - * Gets a user variable which occurs in the "outer scope" of variable `v`. + * Gets a user variable which occurs in the "potential scope" of variable `v`. */ cached -UserVariable getPotentialScopeOfVariableStrict(UserVariable v) { - result = getOuterScopesOfVariable_candidate(v) and +UserDeclaration getPotentialScopeOfDeclaration(UserDeclaration v) { + result = getPotentialScopeOfDeclaration_candidate(v) and inSameTranslationUnit(v.getFile(), result.getFile()) } @@ -204,18 +210,9 @@ class TranslationUnit extends SourceFile { } /** Holds if `v2` may hide `v1`. */ -private predicate hides_candidate(UserVariable v1, UserVariable v2) { - not v1 = v2 and - v2 = getPotentialScopeOfVariable(v1) and - v1.getName() = v2.getName() and - // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name. - not (v1.isMember() or v2.isMember()) -} - -/** Holds if `v2` may hide `v1`. */ -private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) { +private predicate hides_candidateStrict(UserDeclaration v1, UserDeclaration v2) { not v1 = v2 and - v2 = getPotentialScopeOfVariableStrict(v1) and + v2 = getPotentialScopeOfDeclarationStrict(v1) and v1.getName() = v2.getName() and // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name. not (v1.isMember() or v2.isMember()) and @@ -239,6 +236,15 @@ private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) { ) } +/** Holds if `v2` may hide `v1`. */ +private predicate hides_candidate(UserDeclaration v1, UserDeclaration v2) { + not v1 = v2 and + v2 = getPotentialScopeOfDeclaration(v1) and + v1.getName() = v2.getName() and + // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name. + not (v1.isMember() or v2.isMember()) +} + /** * Gets the enclosing statement of the given variable, if any. */ @@ -257,20 +263,20 @@ private Stmt getEnclosingStmt(LocalScopeVariable v) { } /** Holds if `v2` hides `v1`. */ -predicate hides(UserVariable v1, UserVariable v2) { +predicate hides(UserDeclaration v1, UserDeclaration v2) { hides_candidate(v1, v2) and // Confirm that there's no closer candidate variable which `v2` hides - not exists(UserVariable mid | + not exists(UserDeclaration mid | hides_candidate(v1, mid) and hides_candidate(mid, v2) ) } /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */ -predicate hidesStrict(UserVariable v1, UserVariable v2) { +predicate hidesStrict(UserDeclaration v1, UserDeclaration v2) { hides_candidateStrict(v1, v2) and // Confirm that there's no closer candidate variable which `v2` hides - not exists(UserVariable mid | + not exists(UserDeclaration mid | hides_candidateStrict(v1, mid) and hides_candidateStrict(mid, v2) ) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index fc0a01cbd..109407c12 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -11,7 +11,7 @@ abstract class IdentifierHiddenSharedQuery extends Query { } Query getQuery() { result instanceof IdentifierHiddenSharedQuery } -query predicate problems(UserVariable v2, string message, UserVariable v1, string varName) { +query predicate problems(UserDeclaration v2, string message, UserDeclaration v1, string varName) { not isExcluded(v1, getQuery()) and not isExcluded(v2, getQuery()) and //ignore template variables for this rule @@ -19,5 +19,5 @@ query predicate problems(UserVariable v2, string message, UserVariable v1, strin not v2 instanceof TemplateVariable and hidesStrict(v1, v2) and varName = v1.getName() and - message = "Variable is hiding variable $@." + message = "Declaration is hiding declaration $@." } diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index 2ea18aa9c..512c38952 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -1,11 +1,13 @@ -| test.cpp:4:5:4:7 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 | -| test.cpp:8:5:8:7 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 | -| test.cpp:11:5:11:7 | id1 | Variable is hiding variable $@. | test.cpp:8:5:8:7 | id1 | id1 | -| test.cpp:20:7:20:9 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 | -| test.cpp:23:13:23:15 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 | -| test.cpp:26:12:26:14 | id1 | Variable is hiding variable $@. | test.cpp:1:5:1:7 | id1 | id1 | -| test.cpp:27:14:27:16 | id1 | Variable is hiding variable $@. | test.cpp:26:12:26:14 | id1 | id1 | -| test.cpp:65:11:65:11 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i | -| test.cpp:67:9:67:9 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i | -| test.cpp:70:12:70:12 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i | -| test.cpp:75:16:75:16 | i | Variable is hiding variable $@. | test.cpp:61:7:61:7 | i | i | +| test.cpp:4:5:4:7 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | +| test.cpp:8:5:8:7 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | +| test.cpp:11:5:11:7 | id1 | Declaration is hiding declaration $@. | test.cpp:8:5:8:7 | id1 | id1 | +| test.cpp:20:7:20:9 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | +| test.cpp:23:13:23:15 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | +| test.cpp:26:12:26:14 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | +| test.cpp:27:14:27:16 | id1 | Declaration is hiding declaration $@. | test.cpp:26:12:26:14 | id1 | id1 | +| test.cpp:65:11:65:11 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | +| test.cpp:67:9:67:9 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | +| test.cpp:70:12:70:12 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | +| test.cpp:75:16:75:16 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | +| test.cpp:81:5:81:5 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a | +| test.cpp:85:13:85:13 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index cdd7137c5..c778f60b8 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -74,4 +74,13 @@ void test_scope_order() { } catch (int i) { // NON_COMPLIANT } +} + +int a; +namespace b { +int a() {} // NON_COMPLIANT +} // namespace b + +namespace b1 { +typedef int a; // NON_COMPLIANT } \ No newline at end of file From a1f4362f53de5fa8d17139b81a6f89664d4accc9 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 27 Feb 2024 16:21:48 -0500 Subject: [PATCH 02/22] Fix predicate name change --- .../DifferentIdentifiersNotTypographicallyUnambiguous.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll b/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll index 87a4580ab..4876ca9a5 100644 --- a/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll +++ b/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll @@ -47,7 +47,7 @@ string step1(string s) { string step2(string s) { s = "m_" and result = "rn" } predicate violation(UserVariable v1, UserVariable v2) { - v2 = getPotentialScopeOfVariable(v1) and + v2 = getPotentialScopeOfDeclaration(v1) and exists(string s1, string s2 | // over-approximate a match, because it is cheaper to compute getCanon(v1) = getCanon(v2) and From 9be5a7b9d67df32d0eeb23a85a4e84da18e58356 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 28 Feb 2024 11:31:11 -0500 Subject: [PATCH 03/22] Identifier Hidden: missed expeted for shared, generalize variable count check --- .../rules/identifierhidden/IdentifierHidden.expected | 10 +++++----- cpp/common/src/codingstandards/cpp/Scope.qll | 8 +++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/c/common/test/rules/identifierhidden/IdentifierHidden.expected b/c/common/test/rules/identifierhidden/IdentifierHidden.expected index 67809ee00..d6f574e31 100644 --- a/c/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/c/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -1,5 +1,5 @@ -| test.c:4:7:4:9 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 | -| test.c:7:13:7:15 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 | -| test.c:10:12:10:14 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 | -| test.c:11:14:11:16 | id1 | Variable is hiding variable $@. | test.c:10:12:10:14 | id1 | id1 | -| test.c:24:24:24:26 | id2 | Variable is hiding variable $@. | test.c:22:5:22:7 | id2 | id2 | +| test.c:4:7:4:9 | id1 | Declaration is hiding declaration $@. | test.c:1:5:1:7 | id1 | id1 | +| test.c:7:13:7:15 | id1 | Declaration is hiding declaration $@. | test.c:1:5:1:7 | id1 | id1 | +| test.c:10:12:10:14 | id1 | Declaration is hiding declaration $@. | test.c:1:5:1:7 | id1 | id1 | +| test.c:11:14:11:16 | id1 | Declaration is hiding declaration $@. | test.c:10:12:10:14 | id1 | id1 | +| test.c:24:24:24:26 | id2 | Declaration is hiding declaration $@. | test.c:22:5:22:7 | id2 | id2 | diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 7659cdb4f..7d755eba8 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -80,6 +80,8 @@ class Scope extends Element { int getNumberOfVariables() { result = count(getAVariable()) } + int getNumberOfDeclarations() { result = count(getADeclaration()) } + Scope getAnAncestor() { result = this.getStrictParent+() } Scope getStrictParent() { result = getParentScope(this) } @@ -133,9 +135,9 @@ private UserDeclaration getPotentialScopeOfDeclaration_candidate(UserDeclaration exists(Scope s | result = s.getADeclaration() and ( - // Declaration in an ancestor scope, but only if there are less than 100 variables in this scope + // Declaration in an ancestor scope, but only if there are less than 100 declarations in this scope v = s.getAnAncestor().getADeclaration() and - s.getNumberOfVariables() < 100 + s.getNumberOfDeclarations() < 100 or // In the same scope, but not the same Declaration, and choose just one to report v = s.getADeclaration() and @@ -152,7 +154,7 @@ private UserDeclaration getOuterScopesOfDeclaration_candidate(UserDeclaration v) ( // Declaration in an ancestor scope, but only if there are less than 100 variables in this scope v = s.getAnAncestor().getADeclaration() and - s.getNumberOfVariables() < 100 + s.getNumberOfDeclarations() < 100 ) ) } From ef110cd652876877b471e346c8184c8540c5f6fd Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Mon, 25 Mar 2024 22:55:48 -0400 Subject: [PATCH 04/22] IdentifierHidden: add missing exception testcases and address nested named namespaces exception --- cpp/common/src/codingstandards/cpp/Scope.qll | 15 +++++++++++++ .../identifierhidden/IdentifierHidden.qll | 1 + .../test/rules/identifierhidden/test.cpp | 21 ++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 7d755eba8..948d96ab8 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -295,3 +295,18 @@ predicate hasClassScope(Declaration decl) { exists(decl.getDeclaringType()) } /** Holds if `decl` has block scope. */ predicate hasBlockScope(Declaration decl) { exists(BlockStmt b | b.getADeclaration() = decl) } + +/** + * identifiers in nested (named/nonglobal) namespaces are exceptions to hiding due to being able access via fully qualified ids + */ +predicate excludedViaNestedNamespaces(UserDeclaration v2, UserDeclaration v1) { + exists(Namespace inner, Namespace outer | + outer.getAChildNamespace+() = inner and + //outer is not global + not outer instanceof GlobalNamespace and + not outer.isAnonymous() and + not inner.isAnonymous() and + v2.getNamespace() = inner and + v1.getNamespace() = outer + ) +} diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 109407c12..3a8acaed3 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -18,6 +18,7 @@ query predicate problems(UserDeclaration v2, string message, UserDeclaration v1, not v1 instanceof TemplateVariable and not v2 instanceof TemplateVariable and hidesStrict(v1, v2) and + not excludedViaNestedNamespaces(v2, v1) and varName = v1.getName() and message = "Declaration is hiding declaration $@." } diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index c778f60b8..a7087a017 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -83,4 +83,23 @@ int a() {} // NON_COMPLIANT namespace b1 { typedef int a; // NON_COMPLIANT -} \ No newline at end of file +} + +namespace ns_exception1_outer { +int a1; // COMPLIANT - exception +namespace ns_exception1_inner { +void a1(); // COMPLIANT - exception +} +} // namespace ns_exception1_outer + +void f4() { + int a1, b; + auto lambda1 = [a1]() { + int b = 10; // COMPLIANT - exception - non captured variable b + }; + + auto lambda2 = [b]() { + int b = 10; // NON_COMPLIANT[FALSE_NEGATIVE] - not an exception - captured + // variable b + }; +} From edf41d57536270d9f47b9f5b6ef9cf218933ddd0 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Mon, 25 Mar 2024 23:02:11 -0400 Subject: [PATCH 05/22] Scope: improve user declaration definition --- cpp/common/src/codingstandards/cpp/Scope.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 948d96ab8..9b986e221 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -67,6 +67,8 @@ class UserDeclaration extends Declaration { not this.(Variable).isCompilerGenerated() and not this.(Function).isCompilerGenerated() and not this.(Parameter).getFunction().isCompilerGenerated() and + // will falsely conflict + not this instanceof ClassTemplateInstantiation and // compiler inferred parameters have name of p#0 not this.(Parameter).getName() = "p#0" } From fff63e60860d765cab26ea74cd4eb0798303f407 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 27 Mar 2024 11:02:03 -0400 Subject: [PATCH 06/22] IdentifierHidden: omit types --- cpp/common/src/codingstandards/cpp/Scope.qll | 1 + .../test/rules/identifierhidden/IdentifierHidden.expected | 2 -- cpp/common/test/rules/identifierhidden/test.cpp | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 9b986e221..ba6c9c0c8 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -71,6 +71,7 @@ class UserDeclaration extends Declaration { not this instanceof ClassTemplateInstantiation and // compiler inferred parameters have name of p#0 not this.(Parameter).getName() = "p#0" + and not this instanceof Type } } diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index 512c38952..4a56ef6f5 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -1,6 +1,5 @@ | test.cpp:4:5:4:7 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | | test.cpp:8:5:8:7 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | -| test.cpp:11:5:11:7 | id1 | Declaration is hiding declaration $@. | test.cpp:8:5:8:7 | id1 | id1 | | test.cpp:20:7:20:9 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | | test.cpp:23:13:23:15 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | | test.cpp:26:12:26:14 | id1 | Declaration is hiding declaration $@. | test.cpp:1:5:1:7 | id1 | id1 | @@ -10,4 +9,3 @@ | test.cpp:70:12:70:12 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | | test.cpp:75:16:75:16 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | | test.cpp:81:5:81:5 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a | -| test.cpp:85:13:85:13 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index a7087a017..d050a35a1 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -82,7 +82,7 @@ int a() {} // NON_COMPLIANT } // namespace b namespace b1 { -typedef int a; // NON_COMPLIANT +typedef int a; // COMPLIANT - do not consider types } namespace ns_exception1_outer { From 20d9dd6ee9572a59e38decd3814280a8ab06651a Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 27 Mar 2024 14:36:51 -0400 Subject: [PATCH 07/22] IdentifierHidden: move type exclusion to rule not userdecl type, and omit intentional overloads --- cpp/common/src/codingstandards/cpp/Scope.qll | 3 ++- .../cpp/rules/identifierhidden/IdentifierHidden.qll | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index ba6c9c0c8..786f0d4e6 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -71,7 +71,6 @@ class UserDeclaration extends Declaration { not this instanceof ClassTemplateInstantiation and // compiler inferred parameters have name of p#0 not this.(Parameter).getName() = "p#0" - and not this instanceof Type } } @@ -275,6 +274,8 @@ predicate hides(UserDeclaration v1, UserDeclaration v2) { hides_candidate(v1, mid) and hides_candidate(mid, v2) ) + //ignore intentional overloads + and not v1.(Function).getAnOverload() = v2 } /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */ diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 3a8acaed3..e3a337f19 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -17,6 +17,9 @@ query predicate problems(UserDeclaration v2, string message, UserDeclaration v1, //ignore template variables for this rule not v1 instanceof TemplateVariable and not v2 instanceof TemplateVariable and + //ignore types for this rule + not v2 instanceof Type and + not v1 instanceof Type and hidesStrict(v1, v2) and not excludedViaNestedNamespaces(v2, v1) and varName = v1.getName() and From 169cbe728cf8cedf14afe2f28f9fde2ed3822268 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 27 Mar 2024 15:28:14 -0400 Subject: [PATCH 08/22] identifierhidden: update testcase for overload omission, update changenote --- change_notes/2024-02-27-identifier-hidden.md | 3 ++- cpp/common/test/rules/identifierhidden/test.cpp | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/change_notes/2024-02-27-identifier-hidden.md b/change_notes/2024-02-27-identifier-hidden.md index a2cffb9d2..d43e1ad9e 100644 --- a/change_notes/2024-02-27-identifier-hidden.md +++ b/change_notes/2024-02-27-identifier-hidden.md @@ -1,2 +1,3 @@ - `A2-10-1`, `RULE-5-3` - `IdentifierHiding.ql`, `IdentifierHidingC.ql`: - - Address FN reported in #118. Rule was missing detection of functions and types. \ No newline at end of file + - Address FN reported in #118. Rule was missing detection of functions. Additionally omitted class template instantiations. + - Fix FP for identifiers in nested namespaces. \ No newline at end of file diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index d050a35a1..831910f3a 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -103,3 +103,6 @@ void f4() { // variable b }; } + +void f5(int i) {} // COMPLIANT - exception - assume purposefully overloaded +void f5(double d) {} // COMPLIANT - exception - assume purposefully overloaded \ No newline at end of file From 63cec5235a325725d396a26080473e0051cdd244 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 27 Mar 2024 15:30:39 -0400 Subject: [PATCH 09/22] IdentifierHidden: reformat --- cpp/common/src/codingstandards/cpp/Scope.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 786f0d4e6..555f4bc01 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -273,9 +273,9 @@ predicate hides(UserDeclaration v1, UserDeclaration v2) { not exists(UserDeclaration mid | hides_candidate(v1, mid) and hides_candidate(mid, v2) - ) + ) and //ignore intentional overloads - and not v1.(Function).getAnOverload() = v2 + not v1.(Function).getAnOverload() = v2 } /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */ From 97ccd29f17a466dd1146fc26ad38b56916083e9e Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 28 Mar 2024 10:42:03 -0400 Subject: [PATCH 10/22] IdentifierHidden: add heuristic for hiding in lambda --- .../identifierhidden/IdentifierHidden.qll | 21 ++++++++++++++++++- .../IdentifierHidden.expected | 1 + .../test/rules/identifierhidden/test.cpp | 2 +- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index e3a337f19..a7719ba81 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -11,6 +11,25 @@ abstract class IdentifierHiddenSharedQuery extends Query { } Query getQuery() { result instanceof IdentifierHiddenSharedQuery } +/** + * There is a lambda that contains a declaration + * that hides something that is captured + * and the lambda exists in the function where this lamda is enclosed + */ +predicate hiddenInLambda(UserDeclaration v2, UserDeclaration v1) { + exists(Scope s, Closure le | + s.getADeclaration() = v2 and + s.getAnAncestor() = le and + le.getEnclosingFunction().getBasicBlock().(Scope) = v1.getParentScope() and + exists(LambdaCapture cap, Variable v | + v.getAnAccess() = cap.getInitializer().(VariableAccess) and + v = v1 and + le.getLambdaExpression().getACapture() = cap + ) and + v2.getName() = v1.getName() + ) +} + query predicate problems(UserDeclaration v2, string message, UserDeclaration v1, string varName) { not isExcluded(v1, getQuery()) and not isExcluded(v2, getQuery()) and @@ -20,7 +39,7 @@ query predicate problems(UserDeclaration v2, string message, UserDeclaration v1, //ignore types for this rule not v2 instanceof Type and not v1 instanceof Type and - hidesStrict(v1, v2) and + (hidesStrict(v1, v2) or hiddenInLambda(v2, v1)) and not excludedViaNestedNamespaces(v2, v1) and varName = v1.getName() and message = "Declaration is hiding declaration $@." diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index 4a56ef6f5..b857842ec 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -9,3 +9,4 @@ | test.cpp:70:12:70:12 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | | test.cpp:75:16:75:16 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | | test.cpp:81:5:81:5 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a | +| test.cpp:102:9:102:9 | b | Declaration is hiding declaration $@. | test.cpp:96:11:96:11 | b | b | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index 831910f3a..e5e75d751 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -99,7 +99,7 @@ void f4() { }; auto lambda2 = [b]() { - int b = 10; // NON_COMPLIANT[FALSE_NEGATIVE] - not an exception - captured + int b = 10; // NON_COMPLIANT - not an exception - captured // variable b }; } From ab57036d13aaa9751f0a7d579da578e9794365e8 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 2 Apr 2024 14:13:55 -0400 Subject: [PATCH 11/22] Update cpp/common/src/codingstandards/cpp/Scope.qll Co-authored-by: Remco Vermeulen --- cpp/common/src/codingstandards/cpp/Scope.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 555f4bc01..5a2d82c09 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -67,7 +67,7 @@ class UserDeclaration extends Declaration { not this.(Variable).isCompilerGenerated() and not this.(Function).isCompilerGenerated() and not this.(Parameter).getFunction().isCompilerGenerated() and - // will falsely conflict + // Class template instantiations are compiler generated instances that share the same parent scope. This will result in a cross-product on class template instantiations because they have the same name and same parent scope. We therefore exclude these from consideration like we do with other compiler generated identifiers of interest. not this instanceof ClassTemplateInstantiation and // compiler inferred parameters have name of p#0 not this.(Parameter).getName() = "p#0" From 3d2e4138d1b3b76cdb7e44b6b2e2b4d1845275cf Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 2 Apr 2024 14:14:24 -0400 Subject: [PATCH 12/22] Update cpp/common/src/codingstandards/cpp/Scope.qll Co-authored-by: Remco Vermeulen --- cpp/common/src/codingstandards/cpp/Scope.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 5a2d82c09..bd3a5051a 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -274,7 +274,7 @@ predicate hides(UserDeclaration v1, UserDeclaration v2) { hides_candidate(v1, mid) and hides_candidate(mid, v2) ) and - //ignore intentional overloads + // Unlike `hidesStrict`, that requires a different scope, `hides` considers declarations in the same scope. This will include function overloads based on their name. To remove overloads from consideration, we exclude them. not v1.(Function).getAnOverload() = v2 } From 5606a6c065c69b3484ed1ea840990ad0e8ca958f Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 3 Apr 2024 19:32:55 -0400 Subject: [PATCH 13/22] IdentifierHidden: improve variable names and docs/overall readability --- cpp/common/src/codingstandards/cpp/Scope.qll | 6 +-- .../identifierhidden/IdentifierHidden.qll | 42 +++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 555f4bc01..bf6e50293 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -303,14 +303,14 @@ predicate hasBlockScope(Declaration decl) { exists(BlockStmt b | b.getADeclarati /** * identifiers in nested (named/nonglobal) namespaces are exceptions to hiding due to being able access via fully qualified ids */ -predicate excludedViaNestedNamespaces(UserDeclaration v2, UserDeclaration v1) { +predicate excludedViaNestedNamespaces(UserDeclaration outerDecl, UserDeclaration innerDecl) { exists(Namespace inner, Namespace outer | outer.getAChildNamespace+() = inner and //outer is not global not outer instanceof GlobalNamespace and not outer.isAnonymous() and not inner.isAnonymous() and - v2.getNamespace() = inner and - v1.getNamespace() = outer + innerDecl.getNamespace() = inner and + outerDecl.getNamespace() = outer ) } diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index a7719ba81..eaa239f49 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -12,35 +12,35 @@ abstract class IdentifierHiddenSharedQuery extends Query { } Query getQuery() { result instanceof IdentifierHiddenSharedQuery } /** - * There is a lambda that contains a declaration - * that hides something that is captured - * and the lambda exists in the function where this lamda is enclosed + * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` captured by the lambda. */ -predicate hiddenInLambda(UserDeclaration v2, UserDeclaration v1) { +predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { exists(Scope s, Closure le | - s.getADeclaration() = v2 and + //innerDecl declared inside of the lambda + s.getADeclaration() = innerDecl and s.getAnAncestor() = le and - le.getEnclosingFunction().getBasicBlock().(Scope) = v1.getParentScope() and - exists(LambdaCapture cap, Variable v | - v.getAnAccess() = cap.getInitializer().(VariableAccess) and - v = v1 and + le.getEnclosingFunction().getBasicBlock().(Scope) = outerDecl.getParentScope() and + exists(LambdaCapture cap | + outerDecl.getAnAccess() = cap.getInitializer().(VariableAccess) and le.getLambdaExpression().getACapture() = cap ) and - v2.getName() = v1.getName() + innerDecl.getName() = outerDecl.getName() ) } -query predicate problems(UserDeclaration v2, string message, UserDeclaration v1, string varName) { - not isExcluded(v1, getQuery()) and - not isExcluded(v2, getQuery()) and +query predicate problems( + UserDeclaration innerDecl, string message, UserDeclaration outerDecl, string varName +) { + not isExcluded(outerDecl, getQuery()) and + not isExcluded(innerDecl, getQuery()) and //ignore template variables for this rule - not v1 instanceof TemplateVariable and - not v2 instanceof TemplateVariable and - //ignore types for this rule - not v2 instanceof Type and - not v1 instanceof Type and - (hidesStrict(v1, v2) or hiddenInLambda(v2, v1)) and - not excludedViaNestedNamespaces(v2, v1) and - varName = v1.getName() and + not outerDecl instanceof TemplateVariable and + not innerDecl instanceof TemplateVariable and + //ignore types for this rule as the Misra C/C++ 23 version of this rule (rule 6.4.1 and 6.4.2) focuses solely on variables and functions + not innerDecl instanceof Type and + not outerDecl instanceof Type and + (hidesStrict(outerDecl, innerDecl) or hiddenInLambda(outerDecl, innerDecl)) and + not excludedViaNestedNamespaces(outerDecl, innerDecl) and + varName = outerDecl.getName() and message = "Declaration is hiding declaration $@." } From 908b6e94400e07d27bf25cf6ba05c6b95134af55 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 4 Apr 2024 12:50:01 -0400 Subject: [PATCH 14/22] IdentifierHidden: improve lambda handling case --- .../identifierhidden/IdentifierHidden.qll | 44 +++++++++++++++++-- .../IdentifierHidden.expected | 1 + .../test/rules/identifierhidden/test.cpp | 16 ++++++- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index eaa239f49..88bdf28e2 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -6,11 +6,22 @@ import cpp import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.Scope +import codingstandards.cpp.ConstHelpers abstract class IdentifierHiddenSharedQuery extends Query { } Query getQuery() { result instanceof IdentifierHiddenSharedQuery } +/** + * a `IntegralOrEnumType` that is nonvolatile and const + */ +class NonVolatileConstIntegralOrEnumType extends IntegralOrEnumType { + NonVolatileConstIntegralOrEnumType() { + not this.isVolatile() and + this.isConst() + } +} + /** * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` captured by the lambda. */ @@ -19,10 +30,35 @@ predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { //innerDecl declared inside of the lambda s.getADeclaration() = innerDecl and s.getAnAncestor() = le and - le.getEnclosingFunction().getBasicBlock().(Scope) = outerDecl.getParentScope() and - exists(LambdaCapture cap | - outerDecl.getAnAccess() = cap.getInitializer().(VariableAccess) and - le.getLambdaExpression().getACapture() = cap + //a variable can be accessed (therefore hide) another when: + //it is explicitly captured + ( + exists(LambdaCapture cap | + outerDecl.getAnAccess() = cap.getInitializer().(VariableAccess) and + le.getLambdaExpression().getACapture() = cap and + //captured variable (outerDecl) is in the same (function) scope as the lambda itself + outerDecl.getParentScope() = le.getEnclosingFunction().getBasicBlock().(Scope) + ) + or + //is non-local + outerDecl instanceof GlobalVariable + or + //has static or thread local storage duration + (outerDecl.isThreadLocal() or outerDecl.isStatic()) + or + //is a reference that has been initialized with a constant expression. + outerDecl.getType().stripTopLevelSpecifiers() instanceof ReferenceType and + exists(outerDecl.getInitializer().getExpr().getValue()) + or + //const non-volatile integral or enumeration type and has been initialized with a constant expression + outerDecl.getType() instanceof NonVolatileConstIntegralOrEnumType and + exists(outerDecl.getInitializer().getExpr().getValue()) + or + //is constexpr and has no mutable members + outerDecl.isConstexpr() and + not exists(Class c | + c = outerDecl.getType() and not c.getAMember() instanceof MutableVariable + ) ) and innerDecl.getName() = outerDecl.getName() ) diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index b857842ec..8cab1bf1d 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -10,3 +10,4 @@ | test.cpp:75:16:75:16 | i | Declaration is hiding declaration $@. | test.cpp:61:7:61:7 | i | i | | test.cpp:81:5:81:5 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a | | test.cpp:102:9:102:9 | b | Declaration is hiding declaration $@. | test.cpp:96:11:96:11 | b | b | +| test.cpp:114:9:114:17 | globalvar | Declaration is hiding declaration $@. | test.cpp:110:5:110:13 | globalvar | globalvar | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index e5e75d751..bd82d0952 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -105,4 +105,18 @@ void f4() { } void f5(int i) {} // COMPLIANT - exception - assume purposefully overloaded -void f5(double d) {} // COMPLIANT - exception - assume purposefully overloaded \ No newline at end of file +void f5(double d) {} // COMPLIANT - exception - assume purposefully overloaded + +int globalvar = 0; + +int f6() { + auto lambda_with_shadowing = []() { + int globalvar = 1; // NON_COMPLIANT - not an exception - not captured but + // still accessible + return globalvar + globalvar; + }; + + auto lambda_without_shadowing = []() { return globalvar + globalvar; }; + + return lambda_with_shadowing(); +} \ No newline at end of file From 6a21fa5a9e0479a6282a08c898844163a7f8b587 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Thu, 4 Apr 2024 14:16:39 -0700 Subject: [PATCH 15/22] Handle lambda scope using `Scope` class --- .../identifierhidden/IdentifierHidden.qll | 41 ++++++++----- .../IdentifierHidden.expected | 4 ++ .../test/rules/identifierhidden/test.cpp | 61 +++++++++++++++++++ 3 files changed, 90 insertions(+), 16 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 88bdf28e2..63c6127b4 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -23,43 +23,52 @@ class NonVolatileConstIntegralOrEnumType extends IntegralOrEnumType { } /** - * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` captured by the lambda. + * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` by the lambda. */ predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { - exists(Scope s, Closure le | - //innerDecl declared inside of the lambda - s.getADeclaration() = innerDecl and - s.getAnAncestor() = le and - //a variable can be accessed (therefore hide) another when: - //it is explicitly captured + exists( + Scope innerScope, LambdaExpression lambdaExpr, Scope lambdaExprScope, Scope outerScope, + Closure lambdaClosure + | + // The variable `innerDecl` is declared inside of the lambda. + innerScope.getADeclaration() = innerDecl and + // Because a lambda is compiled down to a closure, we need to use the closure to determine if the declaration + // is part of the lambda. + innerScope.getAnAncestor() = lambdaClosure and + // Next we determine the scope of the lambda expression to determine if `outerDecl` is visible in the scope of the lambda. + lambdaClosure.getLambdaExpression() = lambdaExpr and + lambdaExprScope.getAnExpr() = lambdaExpr and + outerScope.getADeclaration() = outerDecl and + lambdaExprScope.getStrictParent*() = outerScope and ( + // A definition can be hidden if it is in scope and it iscaptured by the lambda, exists(LambdaCapture cap | - outerDecl.getAnAccess() = cap.getInitializer().(VariableAccess) and - le.getLambdaExpression().getACapture() = cap and - //captured variable (outerDecl) is in the same (function) scope as the lambda itself - outerDecl.getParentScope() = le.getEnclosingFunction().getBasicBlock().(Scope) + lambdaExpr.getACapture() = cap and + // The outer declaration is captured by the lambda + outerDecl.getAnAccess() = cap.getInitializer() ) or - //is non-local + // it is is non-local, outerDecl instanceof GlobalVariable or - //has static or thread local storage duration + // it has static or thread local storage duration, (outerDecl.isThreadLocal() or outerDecl.isStatic()) or - //is a reference that has been initialized with a constant expression. + //it is a reference that has been initialized with a constant expression. outerDecl.getType().stripTopLevelSpecifiers() instanceof ReferenceType and exists(outerDecl.getInitializer().getExpr().getValue()) or - //const non-volatile integral or enumeration type and has been initialized with a constant expression + //it const non-volatile integral or enumeration type and has been initialized with a constant expression outerDecl.getType() instanceof NonVolatileConstIntegralOrEnumType and exists(outerDecl.getInitializer().getExpr().getValue()) or - //is constexpr and has no mutable members + //it is constexpr and has no mutable members outerDecl.isConstexpr() and not exists(Class c | c = outerDecl.getType() and not c.getAMember() instanceof MutableVariable ) ) and + // Finally, the variables must have the same names. innerDecl.getName() = outerDecl.getName() ) } diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index 8cab1bf1d..518d21ace 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -11,3 +11,7 @@ | test.cpp:81:5:81:5 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a | | test.cpp:102:9:102:9 | b | Declaration is hiding declaration $@. | test.cpp:96:11:96:11 | b | b | | test.cpp:114:9:114:17 | globalvar | Declaration is hiding declaration $@. | test.cpp:110:5:110:13 | globalvar | globalvar | +| test.cpp:133:11:133:11 | b | Declaration is hiding declaration $@. | test.cpp:127:13:127:13 | b | b | +| test.cpp:142:9:142:10 | a1 | Declaration is hiding declaration $@. | test.cpp:140:14:140:15 | a1 | a1 | +| test.cpp:147:9:147:10 | a2 | Declaration is hiding declaration $@. | test.cpp:145:20:145:21 | a2 | a2 | +| test.cpp:152:9:152:10 | a3 | Declaration is hiding declaration $@. | test.cpp:150:17:150:18 | a3 | a3 | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index bd82d0952..71b9f283c 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -119,4 +119,65 @@ int f6() { auto lambda_without_shadowing = []() { return globalvar + globalvar; }; return lambda_with_shadowing(); +} + +void f7(int p) { + // Introduce a nested scope to test scope comparison. + if (p != 0) { + int a1, b; + auto lambda1 = [a1]() { + int b = 10; // COMPLIANT - exception - non captured variable b + }; + + auto lambda2 = [b]() { + int b = 10; // NON_COMPLIANT - not an exception - captured + // variable b + }; + } +} + +void f8() { + static int a1; + auto lambda1 = []() { + int a1 = 10; // NON_COMPLIANT - Lambda can access static variable. + }; + + thread_local int a2; + auto lambda2 = []() { + int a2 = 10; // NON_COMPLIANT - Lambda can access thread local variable. + }; + + constexpr int a3 = 10; + auto lambda3 = []() { + int a3 = a3 + 1; // NON_COMPLIANT - Lambda can access const + // expression without mutable members. + }; + + const int &a4 = a3; + auto lambda4 = []() { + int a4 = a4 + 1; // NON_COMPLIANT[FALSE_NEGATIVE] - Lambda can access + // reference initialized with constant expression. + }; + + const int a5 = 10; + auto lambda5 = []() { + int a5 = a5 + 1; // NON_COMPLIANT[FALSE_NEGATIVE] - Lambda can access const + // non-volatile integral or enumeration type initialized + // with constant expression. + }; + + volatile const int a6 = 10; + auto lambda6 = []() { + int a6 = + a6 + 1; // COMPLIANT - Lambda cannot access const volatile integral or + // enumeration type initialized with constant expression. + }; +} + +void f9() { + auto lambda1 = []() { + int a1 = 10; // COMPLIANT + }; + + int a1 = 10; } \ No newline at end of file From 20ef634d649ebb4c56340e7bc47240de05250689 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Thu, 4 Apr 2024 14:33:03 -0700 Subject: [PATCH 16/22] Fix missing space in comment --- .../cpp/rules/identifierhidden/IdentifierHidden.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 63c6127b4..26cd40747 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -41,7 +41,7 @@ predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { outerScope.getADeclaration() = outerDecl and lambdaExprScope.getStrictParent*() = outerScope and ( - // A definition can be hidden if it is in scope and it iscaptured by the lambda, + // A definition can be hidden if it is in scope and it is captured by the lambda, exists(LambdaCapture cap | lambdaExpr.getACapture() = cap and // The outer declaration is captured by the lambda From 30aa0446e31321807692f82aa3ca897014710a20 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 4 Apr 2024 23:27:10 -0400 Subject: [PATCH 17/22] IdentifierHidden: reduce FN case and remove redundant testcase --- .../cpp/rules/identifierhidden/IdentifierHidden.qll | 12 +++++++----- .../rules/identifierhidden/IdentifierHidden.expected | 1 + cpp/common/test/rules/identifierhidden/test.cpp | 10 +--------- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 26cd40747..820bb986f 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -13,12 +13,14 @@ abstract class IdentifierHiddenSharedQuery extends Query { } Query getQuery() { result instanceof IdentifierHiddenSharedQuery } /** - * a `IntegralOrEnumType` that is nonvolatile and const + * a `Variable` that is nonvolatile and const + * and of type `IntegralOrEnumType` */ -class NonVolatileConstIntegralOrEnumType extends IntegralOrEnumType { - NonVolatileConstIntegralOrEnumType() { +class NonVolatileConstIntegralOrEnumVariable extends Variable { + NonVolatileConstIntegralOrEnumVariable() { not this.isVolatile() and - this.isConst() + this.isConst() and + this.getUnspecifiedType() instanceof IntegralOrEnumType } } @@ -59,7 +61,7 @@ predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { exists(outerDecl.getInitializer().getExpr().getValue()) or //it const non-volatile integral or enumeration type and has been initialized with a constant expression - outerDecl.getType() instanceof NonVolatileConstIntegralOrEnumType and + outerDecl instanceof NonVolatileConstIntegralOrEnumVariable and exists(outerDecl.getInitializer().getExpr().getValue()) or //it is constexpr and has no mutable members diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index 518d21ace..1b0d94d83 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -15,3 +15,4 @@ | test.cpp:142:9:142:10 | a1 | Declaration is hiding declaration $@. | test.cpp:140:14:140:15 | a1 | a1 | | test.cpp:147:9:147:10 | a2 | Declaration is hiding declaration $@. | test.cpp:145:20:145:21 | a2 | a2 | | test.cpp:152:9:152:10 | a3 | Declaration is hiding declaration $@. | test.cpp:150:17:150:18 | a3 | a3 | +| test.cpp:164:9:164:10 | a5 | Declaration is hiding declaration $@. | test.cpp:162:13:162:14 | a5 | a5 | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index 71b9f283c..ede4bb24d 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -161,7 +161,7 @@ void f8() { const int a5 = 10; auto lambda5 = []() { - int a5 = a5 + 1; // NON_COMPLIANT[FALSE_NEGATIVE] - Lambda can access const + int a5 = a5 + 1; // NON_COMPLIANT - Lambda can access const // non-volatile integral or enumeration type initialized // with constant expression. }; @@ -172,12 +172,4 @@ void f8() { a6 + 1; // COMPLIANT - Lambda cannot access const volatile integral or // enumeration type initialized with constant expression. }; -} - -void f9() { - auto lambda1 = []() { - int a1 = 10; // COMPLIANT - }; - - int a1 = 10; } \ No newline at end of file From 24247d25cc56de10803a17a1b8a2f9ca4663eee9 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 10 Apr 2024 17:51:50 -0400 Subject: [PATCH 18/22] Refactor A7-1-2 to extract constant expression logic --- .../rules/A7-1-2/VariableMissingConstexpr.ql | 75 +---------------- cpp/common/src/codingstandards/cpp/Expr.qll | 80 +++++++++++++++++++ 2 files changed, 82 insertions(+), 73 deletions(-) diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql index 3c2ae9a59..13272c816 100644 --- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql +++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql @@ -18,6 +18,7 @@ import codingstandards.cpp.autosar import codingstandards.cpp.TrivialType import codingstandards.cpp.SideEffect import semmle.code.cpp.controlflow.SSA +import codingstandards.cpp.Expr predicate isZeroInitializable(Variable v) { not exists(v.getInitializer().getExpr()) and @@ -34,78 +35,6 @@ predicate isTypeZeroInitializable(Type t) { t.getUnderlyingType() instanceof ArrayType } -/** - * An optimized set of expressions used to determine the flow through constexpr variables. - */ -class VariableAccessOrCallOrLiteral extends Expr { - VariableAccessOrCallOrLiteral() { - this instanceof VariableAccess or - this instanceof Call or - this instanceof Literal - } -} - -/** - * Holds if the value of source flows through compile time evaluated variables to target. - */ -predicate flowsThroughConstExprVariables( - VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target -) { - ( - source = target - or - source != target and - exists(SsaDefinition intermediateDef, StackVariable intermediate | - intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and - intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and - intermediateDef.getAVariable() = intermediate and - intermediate.isConstexpr() - | - DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and - flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target) - ) - ) -} - -/* - * Returns true if the given call may be evaluated at compile time and is compile time evaluated because - * all its arguments are compile time evaluated and its default values are compile time evaluated. - */ - -predicate isCompileTimeEvaluated(Call call) { - // 1. The call may be evaluated at compile time, because it is constexpr, and - call.getTarget().isConstexpr() and - // 2. all its arguments are compile time evaluated, and - forall(DataFlow::Node ultimateArgSource, DataFlow::Node argSource | - argSource = DataFlow::exprNode(call.getAnArgument()) and - DataFlow::localFlow(ultimateArgSource, argSource) and - not DataFlow::localFlowStep(_, ultimateArgSource) - | - ( - ultimateArgSource.asExpr() instanceof Literal - or - any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr() - ) and - // If the ultimate argument source is not the same as the argument source, then it must flow through - // constexpr variables. - ( - ultimateArgSource != argSource - implies - flowsThroughConstExprVariables(ultimateArgSource.asExpr(), argSource.asExpr()) - ) - ) and - // 3. all the default values used are compile time evaluated. - forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx | - parameterUsingDefaultValue = call.getTarget().getParameter(idx) and - not exists(call.getArgument(idx)) and - parameterUsingDefaultValue.getAnAssignedValue() = defaultValue - | - defaultValue instanceof Literal - or - any(Call c | isCompileTimeEvaluated(c)) = defaultValue - ) -} - from Variable v where not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and @@ -119,7 +48,7 @@ where ( v.getInitializer().getExpr().isConstant() or - any(Call call | isCompileTimeEvaluated(call)) = v.getInitializer().getExpr() + any(Call call | isCompileTimeEvaluatedCall(call)) = v.getInitializer().getExpr() or isZeroInitializable(v) or diff --git a/cpp/common/src/codingstandards/cpp/Expr.qll b/cpp/common/src/codingstandards/cpp/Expr.qll index 86d04e70d..5c19495f4 100644 --- a/cpp/common/src/codingstandards/cpp/Expr.qll +++ b/cpp/common/src/codingstandards/cpp/Expr.qll @@ -189,3 +189,83 @@ module MisraExpr { CValue() { isCValue(this) } } } + +/** + * An optimized set of expressions used to determine the flow through constexpr variables. + */ +class VariableAccessOrCallOrLiteral extends Expr { + VariableAccessOrCallOrLiteral() { + this instanceof VariableAccess and this.(VariableAccess).getTarget().isConstexpr() + or + this instanceof Call + or + this instanceof Literal + } +} + +/** + * Holds if the value of source flows through compile time evaluated variables to target. + */ +predicate flowsThroughConstExprVariables( + VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target +) { + ( + source = target + or + source != target and + exists(SsaDefinition intermediateDef, StackVariable intermediate | + intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and + intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and + intermediateDef.getAVariable() = intermediate and + intermediate.isConstexpr() + | + DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and + flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target) + ) + ) +} + +predicate isCompileTimeEvaluatedExpression(Expr expression) { + forall(DataFlow::Node ultimateSource, DataFlow::Node source | + source = DataFlow::exprNode(expression) and + DataFlow::localFlow(ultimateSource, source) and + not DataFlow::localFlowStep(_, ultimateSource) + | + isDirectCompileTimeEvaluatedExpression(ultimateSource.asExpr()) and + // If the ultimate source is not the same as the source, then it must flow through + // constexpr variables. + ( + ultimateSource != source + implies + flowsThroughConstExprVariables(ultimateSource.asExpr(), source.asExpr()) + ) + ) +} + +predicate isDirectCompileTimeEvaluatedExpression(Expr expression) { + expression instanceof Literal + or + any(Call c | isCompileTimeEvaluatedCall(c)) = expression +} + +/* + * Returns true if the given call may be evaluated at compile time and is compile time evaluated because + * all its arguments are compile time evaluated and its default values are compile time evaluated. + */ + +predicate isCompileTimeEvaluatedCall(Call call) { + // 1. The call may be evaluated at compile time, because it is constexpr, and + call.getTarget().isConstexpr() and + // 2. all its arguments are compile time evaluated, and + forall(Expr argSource | argSource = call.getAnArgument() | + isCompileTimeEvaluatedExpression(argSource) + ) and + // 3. all the default values used are compile time evaluated. + forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx | + parameterUsingDefaultValue = call.getTarget().getParameter(idx) and + not exists(call.getArgument(idx)) and + parameterUsingDefaultValue.getAnAssignedValue() = defaultValue + | + isDirectCompileTimeEvaluatedExpression(defaultValue) + ) +} From 22a80f64cdeaa042e8400dab38990e1569e0a989 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 10 Apr 2024 17:53:50 -0400 Subject: [PATCH 19/22] IdentifierHidden: use improved constant expression logic --- .../cpp/rules/identifierhidden/IdentifierHidden.qll | 5 +++-- .../test/rules/identifierhidden/IdentifierHidden.expected | 1 + cpp/common/test/rules/identifierhidden/test.cpp | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 820bb986f..91d9720c8 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -7,6 +7,7 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.Scope import codingstandards.cpp.ConstHelpers +import codingstandards.cpp.Expr abstract class IdentifierHiddenSharedQuery extends Query { } @@ -58,11 +59,11 @@ predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { or //it is a reference that has been initialized with a constant expression. outerDecl.getType().stripTopLevelSpecifiers() instanceof ReferenceType and - exists(outerDecl.getInitializer().getExpr().getValue()) + isCompileTimeEvaluatedExpression(outerDecl.getInitializer().getExpr()) or //it const non-volatile integral or enumeration type and has been initialized with a constant expression outerDecl instanceof NonVolatileConstIntegralOrEnumVariable and - exists(outerDecl.getInitializer().getExpr().getValue()) + isCompileTimeEvaluatedExpression(outerDecl.getInitializer().getExpr()) or //it is constexpr and has no mutable members outerDecl.isConstexpr() and diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index 1b0d94d83..3ed0ce6f9 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -15,4 +15,5 @@ | test.cpp:142:9:142:10 | a1 | Declaration is hiding declaration $@. | test.cpp:140:14:140:15 | a1 | a1 | | test.cpp:147:9:147:10 | a2 | Declaration is hiding declaration $@. | test.cpp:145:20:145:21 | a2 | a2 | | test.cpp:152:9:152:10 | a3 | Declaration is hiding declaration $@. | test.cpp:150:17:150:18 | a3 | a3 | +| test.cpp:158:9:158:10 | a4 | Declaration is hiding declaration $@. | test.cpp:156:14:156:15 | a4 | a4 | | test.cpp:164:9:164:10 | a5 | Declaration is hiding declaration $@. | test.cpp:162:13:162:14 | a5 | a5 | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index ede4bb24d..946063e6b 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -155,7 +155,7 @@ void f8() { const int &a4 = a3; auto lambda4 = []() { - int a4 = a4 + 1; // NON_COMPLIANT[FALSE_NEGATIVE] - Lambda can access + int a4 = a4 + 1; // NON_COMPLIANT - Lambda can access // reference initialized with constant expression. }; From b2c5896b6887e9868c76c1eb9bdca02c0e71e25e Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 10 Apr 2024 17:56:57 -0400 Subject: [PATCH 20/22] Reformat query --- cpp/common/src/codingstandards/cpp/Expr.qll | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/common/src/codingstandards/cpp/Expr.qll b/cpp/common/src/codingstandards/cpp/Expr.qll index 78e99b118..fe2877f84 100644 --- a/cpp/common/src/codingstandards/cpp/Expr.qll +++ b/cpp/common/src/codingstandards/cpp/Expr.qll @@ -268,9 +268,12 @@ predicate isCompileTimeEvaluatedCall(Call call) { | isDirectCompileTimeEvaluatedExpression(defaultValue) ) +} +/* * an operator that does not evaluate its operand */ + class UnevaluatedExprExtension extends Expr { UnevaluatedExprExtension() { this.getAChild().isUnevaluated() From a9f55d2e483af1b2d77952592175b040f1997b7c Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 24 Apr 2024 10:59:36 -0400 Subject: [PATCH 21/22] Scope lib: rename predicate --- cpp/common/src/codingstandards/cpp/Scope.qll | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index 4ac52e223..cfa2d062f 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -149,8 +149,8 @@ private UserDeclaration getPotentialScopeOfDeclaration_candidate(UserDeclaration ) } -/** Gets a Declarationthat is in the potential scope of Declaration `v`. */ -private UserDeclaration getOuterScopesOfDeclaration_candidate(UserDeclaration v) { +/** Gets a Declaration that is in the potential scope of Declaration `v`. */ +private UserDeclaration getPotentialScopeOfDeclarationStrict_candidate(UserDeclaration v) { exists(Scope s | result = s.getADeclaration() and ( @@ -175,7 +175,7 @@ predicate inSameTranslationUnit(File f1, File f2) { */ cached UserDeclaration getPotentialScopeOfDeclarationStrict(UserDeclaration v) { - result = getOuterScopesOfDeclaration_candidate(v) and + result = getPotentialScopeOfDeclarationStrict_candidate(v) and inSameTranslationUnit(v.getFile(), result.getFile()) } From 80432d87a8446fc5a99593c13e2c96ae5b2f079b Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 25 Apr 2024 12:26:51 -0400 Subject: [PATCH 22/22] IdentifierHidden: revert realisitic compile time constant model in lambda hiding due to performance the simpler heurisitic is better --- .../cpp/rules/identifierhidden/IdentifierHidden.qll | 7 +++---- .../test/rules/identifierhidden/IdentifierHidden.expected | 1 - cpp/common/test/rules/identifierhidden/test.cpp | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 91d9720c8..d5d8a0d93 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -7,7 +7,6 @@ import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions import codingstandards.cpp.Scope import codingstandards.cpp.ConstHelpers -import codingstandards.cpp.Expr abstract class IdentifierHiddenSharedQuery extends Query { } @@ -59,11 +58,11 @@ predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) { or //it is a reference that has been initialized with a constant expression. outerDecl.getType().stripTopLevelSpecifiers() instanceof ReferenceType and - isCompileTimeEvaluatedExpression(outerDecl.getInitializer().getExpr()) + outerDecl.getInitializer().getExpr() instanceof Literal or - //it const non-volatile integral or enumeration type and has been initialized with a constant expression + // //it const non-volatile integral or enumeration type and has been initialized with a constant expression outerDecl instanceof NonVolatileConstIntegralOrEnumVariable and - isCompileTimeEvaluatedExpression(outerDecl.getInitializer().getExpr()) + outerDecl.getInitializer().getExpr() instanceof Literal or //it is constexpr and has no mutable members outerDecl.isConstexpr() and diff --git a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected index 3ed0ce6f9..1b0d94d83 100644 --- a/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected +++ b/cpp/common/test/rules/identifierhidden/IdentifierHidden.expected @@ -15,5 +15,4 @@ | test.cpp:142:9:142:10 | a1 | Declaration is hiding declaration $@. | test.cpp:140:14:140:15 | a1 | a1 | | test.cpp:147:9:147:10 | a2 | Declaration is hiding declaration $@. | test.cpp:145:20:145:21 | a2 | a2 | | test.cpp:152:9:152:10 | a3 | Declaration is hiding declaration $@. | test.cpp:150:17:150:18 | a3 | a3 | -| test.cpp:158:9:158:10 | a4 | Declaration is hiding declaration $@. | test.cpp:156:14:156:15 | a4 | a4 | | test.cpp:164:9:164:10 | a5 | Declaration is hiding declaration $@. | test.cpp:162:13:162:14 | a5 | a5 | diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index 946063e6b..ede4bb24d 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -155,7 +155,7 @@ void f8() { const int &a4 = a3; auto lambda4 = []() { - int a4 = a4 + 1; // NON_COMPLIANT - Lambda can access + int a4 = a4 + 1; // NON_COMPLIANT[FALSE_NEGATIVE] - Lambda can access // reference initialized with constant expression. };