diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index 88bdf28e24..26cd40747f 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 is captured 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 8cab1bf1d9..518d21ace0 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 bd82d09525..71b9f283ce 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