diff --git a/change_notes/2026-02-12-linkage-changes-and-unused-variable-reorganization.md b/change_notes/2026-02-12-linkage-changes-and-unused-variable-reorganization.md new file mode 100644 index 000000000..4be15535c --- /dev/null +++ b/change_notes/2026-02-12-linkage-changes-and-unused-variable-reorganization.md @@ -0,0 +1,7 @@ + - All rules using `Linkage.qll`: + - `extern const` global variables are now properly analyzed as having external linkage, rather than internal linkage. + - Linkage analysis has been fixed to properly handle nested classes, including anonymous and typedefs of anonymous classes. + - Linkage for names within classes with internal linkage is now properly inherited as internal, rather than external. + - `M0-1-3`, `RULE-2-8` - `UnusedLocalVariable.ql`, `UnusedMemberVariable.ql`, `UnusedGlobalOrNamespaceVariable.ql`, `UnusedObjectDefinition.ql`, `UnusedObjectDefinitionStrict.ql`: + - The organization of unused variable analysis has been reorganized to be usable in MISRA C++ rule 0.2.1, with no expected noticeable change in results. + - New filtering passes begin by filtering out variables that have an existing access (`.getAnAccess()`), instead of performing such a filter step last, with a measured small reduction in overall tuple counts and improved overall query performance. \ No newline at end of file diff --git a/cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql b/cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql index ba6b6df20..06f57119a 100644 --- a/cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql +++ b/cpp/autosar/src/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.ql @@ -18,11 +18,6 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.deadcode.UnusedVariables -from PotentiallyUnusedGlobalOrNamespaceVariable v -where - not isExcluded(v, DeadCodePackage::unusedGlobalOrNamespaceVariableQuery()) and - // No variable access - not exists(v.getAnAccess()) and - // Exclude members whose value is compile time and is potentially used to inintialize a template - not maybeACompileTimeTemplateArgument(v) +from ThirdPassUnused::UnusedGlobalOrNamespaceVariable v +where not isExcluded(v, DeadCodePackage::unusedGlobalOrNamespaceVariableQuery()) select v, "Variable '" + v.getQualifiedName() + "' is unused." diff --git a/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql b/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql index 8a9090e71..7ac014a75 100644 --- a/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql +++ b/cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql @@ -18,58 +18,10 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.deadcode.UnusedVariables -// Collect constant values that we should use to exclude otherwise unused constexpr variables. -// -// For constexpr variables used as template arguments or in static_asserts, we don't see accesses -// (just the appropriate literals). We therefore take a conservative approach and do not report -// constexpr variables whose values are used in such contexts. -// -// For performance reasons, these special values should be collected in a single pass. -predicate excludedConstantValue(string value) { - value = any(ClassTemplateInstantiation cti).getTemplateArgument(_).(Expr).getValue() - or - value = any(StaticAssert sa).getCondition().getAChild*().getValue() -} - -/** - * Defines the local variables that should be excluded from the unused variable analysis based - * on their constant value. - * - * See `excludedConstantValue` for more details. - */ -predicate excludeVariableByValue(Variable variable) { - variable.isConstexpr() and - excludedConstantValue(getConstExprValue(variable)) -} - -// TODO: This predicate may be possible to merge with M0-1-4's getUseCount(). These two rules -// diverged to handle `excludeVariableByValue`, but may be possible to merge. -int getUseCountConservatively(Variable v) { - result = - count(VariableAccess access | access = v.getAnAccess()) + - count(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) + - // In case an array type uses a constant in the same scope as the constexpr variable, - // consider it as used. - countUsesInLocalArraySize(v) -} - -predicate isConservativelyUnused(Variable v) { - getUseCountConservatively(v) = 0 and - not excludeVariableByValue(v) -} - -from PotentiallyUnusedLocalVariable v -where - not isExcluded(v, DeadCodePackage::unusedLocalVariableQuery()) and - // Local variable is never accessed - not exists(v.getAnAccess()) and - // Sometimes multiple objects representing the same entities are created in - // the AST. Check if those are not accessed as well. Refer issue #658 - not exists(LocalScopeVariable another | - another.getDefinitionLocation() = v.getDefinitionLocation() and - another.hasName(v.getName()) and - exists(another.getAnAccess()) and - another != v - ) and - isConservativelyUnused(v) +// 10.1, 41.0 +// 10.9, 45.9 +// 11.0, 52.3 +// 13.1, 59.3 +from ThirdPassUnused::UnusedLocalVariable v +where not isExcluded(v, DeadCodePackage::unusedLocalVariableQuery()) select v, "Local variable '" + v.getName() + "' in '" + v.getFunction().getName() + "' is not used." diff --git a/cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql b/cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql index a27f9cbca..e48e3831c 100644 --- a/cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql +++ b/cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql @@ -19,13 +19,6 @@ import codingstandards.cpp.autosar import codingstandards.cpp.FunctionEquivalence import codingstandards.cpp.deadcode.UnusedVariables -from PotentiallyUnusedMemberVariable v -where - not isExcluded(v, DeadCodePackage::unusedMemberVariableQuery()) and - // No variable access - not exists(v.getAnAccess()) and - // No explicit initialization in a constructor - not exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) and - // Exclude members whose value is compile time and is potentially used to inintialize a template - not maybeACompileTimeTemplateArgument(v) +from ThirdPassUnused::UnusedMemberVariable v +where not isExcluded(v, DeadCodePackage::unusedMemberVariableQuery()) select v, "Member variable '" + v.getName() + "' is unused." diff --git a/cpp/common/src/codingstandards/cpp/Linkage.qll b/cpp/common/src/codingstandards/cpp/Linkage.qll index c9ad700c9..6ecab3f6b 100644 --- a/cpp/common/src/codingstandards/cpp/Linkage.qll +++ b/cpp/common/src/codingstandards/cpp/Linkage.qll @@ -5,15 +5,27 @@ import cpp import codingstandards.cpp.Scope +/** + * [basic]/6 uses a specific definition of "variable" that excludes non-static data members. + */ +private predicate isSpecificationVariable(Variable v) { + v.(MemberVariable).isStatic() + or + not v instanceof MemberVariable +} + /** Holds if `elem` has internal linkage. */ predicate hasInternalLinkage(Element elem) { + // An unnamed namespace or a namespace declared directly or indirectly within an unnamed namespace has internal linkage + directlyOrIndirectlyUnnnamedNamespace(elem) + or exists(Declaration decl | decl = elem | // A name having namespace scope has internal linkage if it is the name of hasNamespaceScope(decl) and ( // a variable, function or function template ( - decl instanceof Variable + isSpecificationVariable(decl) or decl instanceof Function // TemplateFunction is a subclass of Function so this captures both. ) and @@ -21,7 +33,7 @@ predicate hasInternalLinkage(Element elem) { decl.isStatic() or // a non-volatile variable - decl instanceof Variable and + isSpecificationVariable(decl) and not decl.(Variable).isVolatile() and // that is explicitly declared const or constexpr and (decl.(Variable).isConst() or decl.(Variable).isConstexpr()) and @@ -32,131 +44,93 @@ predicate hasInternalLinkage(Element elem) { exists(Union u | hasNamespaceScope(u) and u.isAnonymous() | decl = u.getACanonicalMemberVariable() ) - or - // A name having namespace scope that has not been given internal linkage above has the same linkage as the enclosing namespace if it is the name of - hasInternalLinkage(decl.getNamespace()) and - ( - // a variable; - decl instanceof Variable - or - // a function - decl instanceof Function - or - // a named class, or an unnamed class defined in a typedef declartion in which the class has the typedef name for linkage purposes - exists(Class klass | decl = klass | - not klass.isAnonymous() - or - klass.isAnonymous() and exists(TypedefType typedef | typedef.getADeclaration() = klass) - ) - or - // a named enumeration, or an unnamed enumeration defined in a typedef declaration in which the enumeration has the typedef name for linkage purposes - exists(Enum enum | enum = decl | - not enum.isAnonymous() - or - enum.isAnonymous() and exists(TypedefType typedef | typedef.getADeclaration() = enum) - ) - or - // an enumerator beloning to an enumeration with linkage - exists(Enum enum | enum.getADeclaration() = decl | hasInternalLinkage(enum)) - or - // a template - decl instanceof TemplateClass - or - decl instanceof TemplateFunction - ) ) or - decl instanceof GlobalVariable and - ( - decl.(GlobalVariable).isStatic() or - decl.(GlobalVariable).isConst() - ) and - not decl.(GlobalVariable).hasSpecifier("external") - ) - or - // An unnamed namespace or a namespace declared directly or indirectly within an unnamed namespace has internal linkage - exists(Namespace ns | ns = elem | - ns.isAnonymous() + directlyOrIndirectlyUnnnamedNamespace(decl.getNamespace()) and + inheritsLinkageOfNamespace(decl.getNamespace(), decl) or - not ns.isAnonymous() and - exists(Namespace parent | parent.isAnonymous() and parent.getAChildNamespace+() = ns) + exists(Class klass | + hasInternalLinkage(klass) and + inheritsLinkageOfClass(klass, decl) + ) ) - or - elem instanceof TopLevelFunction and - elem.(Function).isStatic() } /** Holds if `elem` has external linkage. */ predicate hasExternalLinkage(Element elem) { + elem instanceof Namespace and + not directlyOrIndirectlyUnnnamedNamespace(elem) + or not hasInternalLinkage(elem) and - ( - exists(Declaration decl | decl = elem | - hasNamespaceScope(decl) and - // A name having namespace scope that has not been given internal linkage above has the same linkage as the enclosing namespace if it is the name of - not hasInternalLinkage(decl.getNamespace()) and - ( - // a variable; - decl instanceof Variable - or - // a function - decl instanceof Function - or - // a named class, or an unnamed class defined in a typedef declaration in which the class has the typedef name for linkage purposes - exists(Class klass | decl = klass | - not klass.isAnonymous() - or - klass.isAnonymous() and exists(TypedefType typedef | typedef.getADeclaration() = klass) - ) - or - // a named enumeration, or an unnamed enumeration defined in a typedef declaration in which the enumeration has the typedef name for linkage purposes - exists(Enum enum | enum = decl | - not enum.isAnonymous() - or - enum.isAnonymous() and exists(TypedefType typedef | typedef.getADeclaration() = enum) - ) - or - // an enumerator beloning to an enumeration with linkage - exists(Enum enum | enum.getADeclaration() = decl | hasInternalLinkage(enum)) - or - // a template - decl instanceof TemplateClass - or - decl instanceof TemplateFunction - ) - or - // In addition, - hasClassScope(decl) and - ( - // a member function, - decl instanceof MemberFunction - or - // static data member - decl instanceof MemberVariable and decl.(MemberVariable).isStatic() - or - // a named class, or an unnamed class defined in a typedef declartion in which the class has the typedef name for linkage purposes - exists(Class klass | decl = klass | - not klass.isAnonymous() - or - klass.isAnonymous() and exists(TypedefType typedef | typedef.getADeclaration() = klass) - ) - or - // a named enumeration, or an unnamed enumeration defined in a typedef declaration in which the enumeration has the typedef name for linkage purposes - exists(Enum enum | enum = decl | - not enum.isAnonymous() - or - enum.isAnonymous() and exists(TypedefType typedef | typedef.getADeclaration() = enum) - ) - ) and - // has external linkage if the name of the class has external linkage - hasExternalLinkage(decl.getDeclaringType()) - or - decl instanceof GlobalVariable and - not decl.(GlobalVariable).isStatic() and - not decl.(GlobalVariable).isConst() + exists(Declaration decl | decl = elem | + // A name having namespace scope that has not been given internal linkage above has the same linkage as the enclosing namespace if it is the name of + not directlyOrIndirectlyUnnnamedNamespace(decl.getNamespace()) and + inheritsLinkageOfNamespace(decl.getNamespace(), decl) + or + exists(Class klass | + hasExternalLinkage(klass) and + inheritsLinkageOfClass(klass, decl) ) + ) +} + +private predicate directlyOrIndirectlyUnnnamedNamespace(Namespace ns) { + exists(Namespace anonymous | + anonymous.isAnonymous() and + ns = anonymous.getAChildNamespace*() + ) +} + +private predicate hasLinkageOfTypedef(TypedefType typedef, Element decl) { + // an unnamed class defined in a typedef declartion in which the class has the typedef name for linkage purposes + decl.(Class).isAnonymous() and typedef.getADeclaration() = decl + or + // an unnamed enumeration defined in a typedef declaration in which the enumeration has the typedef name for linkage purposes + decl.(Enum).isAnonymous() and typedef.getADeclaration() = decl +} + +private predicate inheritsLinkageOfNamespace(Namespace ns, Declaration decl) { + hasNamespaceScope(decl) and + ns = decl.getNamespace() and + ( + // A name having namespace scope that has not been given internal linkage above has the same linkage as the enclosing namespace if it is the name of + // a variable; + isSpecificationVariable(decl) + or + // a function + decl instanceof Function + or + decl instanceof Class and not decl.(Class).isAnonymous() // a named class + or + decl instanceof Enum and not decl.(Enum).isAnonymous() // a named enumeration + or + // a template + decl instanceof TemplateClass + or + decl instanceof TemplateFunction + or + decl instanceof TemplateVariable + ) + or + hasNamespaceScope(decl) and + exists(TypedefType typedef | hasLinkageOfTypedef(typedef, decl) and ns = typedef.getNamespace()) +} + +private predicate inheritsLinkageOfClass(Class klass, Element decl) { + hasClassScope(decl) and + ( + // a member function, + decl.(MemberFunction).getDeclaringType() = klass + or + // static data member + decl.(MemberVariable).isStatic() and decl.(MemberVariable).getDeclaringType() = klass or - elem instanceof Namespace + decl.(Class).getDeclaringType() = klass and not decl.(Class).isAnonymous() or - elem instanceof TopLevelFunction + decl.(Enum).getDeclaringType() = klass and not decl.(Enum).isAnonymous() + or + exists(TypedefType typedef | + hasLinkageOfTypedef(typedef, decl) and klass = typedef.getDeclaringType() + ) ) } diff --git a/cpp/common/src/codingstandards/cpp/deadcode/UnusedObjects.qll b/cpp/common/src/codingstandards/cpp/deadcode/UnusedObjects.qll index 94ae16ec4..10b313974 100644 --- a/cpp/common/src/codingstandards/cpp/deadcode/UnusedObjects.qll +++ b/cpp/common/src/codingstandards/cpp/deadcode/UnusedObjects.qll @@ -17,18 +17,15 @@ import codingstandards.cpp.alertreporting.DeduplicateMacroResults class UnusedObjectDefinition extends VariableDeclarationEntry { UnusedObjectDefinition() { ( - getVariable() instanceof BasePotentiallyUnusedLocalVariable + getVariable() instanceof FirstPassUnused::UnusedLocalVariable or - getVariable() instanceof BasePotentiallyUnusedGlobalOrNamespaceVariable + getVariable() instanceof FirstPassUnused::UnusedGlobalOrNamespaceVariable ) and - not exists(VariableAccess access | access.getTarget() = getVariable()) and getVariable().getDefinition() = this } /* Dead objects with these attributes are reported in the "strict" queries. */ - predicate hasAttrUnused() { - getVariable().getAnAttribute().hasName(["unused", "used", "maybe_unused", "cleanup"]) - } + predicate hasAttrUnused() { hasAttrUnused(getVariable()) } } /* Configuration to use the `DedupMacroResults` module to reduce alert noise */ diff --git a/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll b/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll index a7accd525..99ceb801d 100644 --- a/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll +++ b/cpp/common/src/codingstandards/cpp/deadcode/UnusedVariables.qll @@ -23,6 +23,10 @@ class TemplateDependentType extends Type { } } +predicate hasAttrUnused(Variable v) { + v.getAnAttribute().hasName(["unused", "used", "maybe_unused", "cleanup"]) +} + /** * A variable whose declaration has, or may have, side effects. */ @@ -36,36 +40,6 @@ predicate declarationHasSideEffects(Variable v) { v.getType() instanceof TemplateDependentType } -/** - * A `LocalVariable` which is a candidate for being unused, and may or may not be defined in a macro. - */ -class BasePotentiallyUnusedLocalVariable extends LocalVariable { - BasePotentiallyUnusedLocalVariable() { - // Ignore variables where initializing the variable has side effects - not declarationHasSideEffects(this) and // TODO non POD types with initializers? Also, do something different with templates? - exists(Function f | f = getFunction() | - // Ignore functions with assembly, as the assembly may use the variable - not exists(AsmStmt s | f = s.getEnclosingFunction()) and - // Ignore functions with error expressions as they indicate expressions that the extractor couldn't process - not any(ErrorExpr e).getEnclosingFunction() = f - ) and - // exclude uninstantiated template members - not this.isFromUninstantiatedTemplate(_) and - // Do not report compiler generated variables - not this.isCompilerGenerated() - } -} - -/** - * A `LocalVariable` which is a candidate for being unused, and not defined in a macro. - */ -class PotentiallyUnusedLocalVariable extends BasePotentiallyUnusedLocalVariable { - PotentiallyUnusedLocalVariable() { - // Ignore variables declared in macro expansions - not exists(DeclStmt ds | ds.getADeclaration() = this and ds.isInMacroExpansion()) - } -} - /** Holds if `mf` is "defined" in this database. */ predicate isDefined(MemberFunction mf) { exists(MemberFunction definedMemberFunction | @@ -79,6 +53,26 @@ predicate isDefined(MemberFunction mf) { ) } +/** Gets the constant value of a constexpr/const variable. */ +string getConstExprValue(Variable v) { + result = v.getInitializer().getExpr().getValue() and + (v.isConst() or v.isConstexpr()) +} + +/** + * Counts uses of `Variable` v in a local array of size `n`. + * + * For non `LocalVariable`s, this will return as zero. This is intentional. + */ +int countUsesInLocalArraySize(Variable v) { + result = + count(ArrayType at, LocalVariable arrayVariable | + arrayVariable.getType().resolveTypedefs() = at and + v.(LocalVariable).getFunction() = arrayVariable.getFunction() and + at.getArraySize().toString() = getConstExprValue(v) + ) +} + /** A `Class` where all non compiler generated member functions are defined in the current database. */ class FullyDefinedClass extends Class { FullyDefinedClass() { @@ -93,50 +87,110 @@ class FullyDefinedClass extends Class { } } -/** A `MemberVariable` which is potentially unused. */ -class PotentiallyUnusedMemberVariable extends MemberVariable { - PotentiallyUnusedMemberVariable() { - // Not an unnamed bitfield, which is permitted to be unused - not getName() = "(unnamed bitfield)" and - // Must be defined in this database - hasDefinition() and - // Declaration is not affected by a macro - not getADeclarationEntry().isAffectedByMacro() and - // No side effects from instantiating the class TODO is this right?! - I think this will exclude non-template arguments - not declarationHasSideEffects(this) and - // Must be in a fully defined class, otherwise one of the undefined functions may use the variable - getDeclaringType() instanceof FullyDefinedClass and - // Lambda captures are not "real" member variables - it's an implementation detail that they are represented that way - not this = any(LambdaCapture lc).getField() and - // exclude uninstantiated template members - not this.isFromUninstantiatedTemplate(_) and - // Do not report compiler generated variables - not this.isCompilerGenerated() +/** + * This module defines unused variables as defined by all standards, including MISRA C, C++, and + * AUTOSAR. + * + * This pass excludes variables that can be implicitly used in many ways. It notably does not + * exclude variables that are used in macros, which MISRA C supports specially. + */ +module FirstPassUnused { + final private class FinalLocalVariable = LocalVariable; + + final private class FinalGlobalOrNspVariable = GlobalOrNamespaceVariable; + + /** + * A `LocalVariable` which is a candidate for being unused, and may or may not be defined in a macro. + */ + class UnusedLocalVariable extends FinalLocalVariable { + UnusedLocalVariable() { + not exists(getAnAccess()) and + // Ignore variables where initializing the variable has side effects + not declarationHasSideEffects(this) and // TODO non POD types with initializers? Also, do something different with templates? + exists(Function f | f = getFunction() | + // Ignore functions with assembly, as the assembly may use the variable + not exists(AsmStmt s | f = s.getEnclosingFunction()) and + // Ignore functions with error expressions as they indicate expressions that the extractor couldn't process + not any(ErrorExpr e).getEnclosingFunction() = f + ) and + // exclude uninstantiated template members + not this.isFromUninstantiatedTemplate(_) and + // Do not report compiler generated variables + not this.isCompilerGenerated() + } } -} -/** A `GlobalOrNamespaceVariable` which is potentially unused and may or may not be defined in a macro */ -class BasePotentiallyUnusedGlobalOrNamespaceVariable extends GlobalOrNamespaceVariable { - BasePotentiallyUnusedGlobalOrNamespaceVariable() { - // A non-defined variable may never be used - hasDefinition() and - // No side-effects from declaration - not declarationHasSideEffects(this) and - // exclude uninstantiated template members - not this.isFromUninstantiatedTemplate(_) and - // Do not report compiler generated variables - not this.isCompilerGenerated() + /** A `GlobalOrNamespaceVariable` which is potentially unused and may or may not be defined in a macro */ + class UnusedGlobalOrNamespaceVariable extends FinalGlobalOrNspVariable { + UnusedGlobalOrNamespaceVariable() { + not exists(getAnAccess()) and + // A non-defined variable may never be used + hasDefinition() and + // No side-effects from declaration + not declarationHasSideEffects(this) and + // exclude uninstantiated template members + not this.isFromUninstantiatedTemplate(_) and + // Do not report compiler generated variables + not this.isCompilerGenerated() + } } } /** - * A `GlobalOrNamespaceVariable` which is potentially unused, and is not defined in a macro. + * This module defines an intermediate conservative pass for filtering out unused variables. + * + * This pass filters out variables defined in macros, and is used by AUTOSAR and MISRA, before a + * more expensive third pass. */ -class PotentiallyUnusedGlobalOrNamespaceVariable extends BasePotentiallyUnusedGlobalOrNamespaceVariable -{ - PotentiallyUnusedGlobalOrNamespaceVariable() { - // Not declared in a macro expansion - not isInMacroExpansion() +module SecondPassUnused { + final private class FirstPassLocalVariable = FirstPassUnused::UnusedLocalVariable; + + final private class FirstPassGlobalOrNspVariable = + FirstPassUnused::UnusedGlobalOrNamespaceVariable; + + final private class FinalMemberVariable = MemberVariable; + + /** + * A `LocalVariable` which is a candidate for being unused, and not defined in a macro. + */ + class UnusedLocalVariable extends FirstPassLocalVariable { + UnusedLocalVariable() { + // Ignore variables declared in macro expansions + not exists(DeclStmt ds | ds.getADeclaration() = this and ds.isInMacroExpansion()) + } + } + + /** A `MemberVariable` which is potentially unused. */ + class UnusedMemberVariable extends FinalMemberVariable { + UnusedMemberVariable() { + not exists(getAnAccess()) and + // Not an unnamed bitfield, which is permitted to be unused + not getName() = "(unnamed bitfield)" and + // Must be defined in this database + hasDefinition() and + // Declaration is not affected by a macro + not getADeclarationEntry().isAffectedByMacro() and + // No side effects from instantiating the class TODO is this right?! - I think this will exclude non-template arguments + not declarationHasSideEffects(this) and + // Must be in a fully defined class, otherwise one of the undefined functions may use the variable + getDeclaringType() instanceof FullyDefinedClass and + // Lambda captures are not "real" member variables - it's an implementation detail that they are represented that way + not this = any(LambdaCapture lc).getField() and + // exclude uninstantiated template members + not this.isFromUninstantiatedTemplate(_) and + // Do not report compiler generated variables + not this.isCompilerGenerated() + } + } + + /** + * A `GlobalOrNamespaceVariable` which is potentially unused, and is not defined in a macro. + */ + class UnusedGlobalOrNamespaceVariable extends FirstPassGlobalOrNspVariable { + UnusedGlobalOrNamespaceVariable() { + // Not declared in a macro expansion + not isInMacroExpansion() + } } } @@ -145,45 +199,111 @@ predicate isUnused(Variable variable) { variable.getInitializer().fromSource() } -class UserProvidedConstructorFieldInit extends ConstructorFieldInit { - UserProvidedConstructorFieldInit() { - not isCompilerGenerated() and - not getEnclosingFunction().isCompilerGenerated() - } -} - /** - * Holds if `v` may hold a compile time value and is accessible to a template instantiation that - * receives a constant value as an argument equal to the value of `v`. + * This module is the final pass for unused variables in AUTOSAR, and an intermediate pass for MISRA. + * + * This pass looks for odd cases such as constexpr variables whose usage was inlined by the compiler + * and therefore not visible to us. */ -predicate maybeACompileTimeTemplateArgument(Variable v) { - v.isConstexpr() and - exists(ClassTemplateInstantiation cti, TranslationUnit tu | - cti.getATemplateArgument().(Expr).getValue() = v.getInitializer().getExpr().getValue() and - ( - cti.getFile() = tu and +module ThirdPassUnused { + class UserProvidedConstructorFieldInit extends ConstructorFieldInit { + UserProvidedConstructorFieldInit() { + not isCompilerGenerated() and + not getEnclosingFunction().isCompilerGenerated() + } + } + + /** + * Holds if `v` may hold a compile time value and is accessible to a template instantiation that + * receives a constant value as an argument equal to the value of `v`. + */ + predicate maybeACompileTimeTemplateArgument(Variable v) { + v.isConstexpr() and + exists(ClassTemplateInstantiation cti, TranslationUnit tu | + cti.getATemplateArgument().(Expr).getValue() = v.getInitializer().getExpr().getValue() and ( - v.getADeclarationEntry().getFile() = tu or - tu.getATransitivelyIncludedFile() = v.getADeclarationEntry().getFile() + cti.getFile() = tu and + ( + v.getADeclarationEntry().getFile() = tu or + tu.getATransitivelyIncludedFile() = v.getADeclarationEntry().getFile() + ) ) ) - ) -} + } -/** Gets the constant value of a constexpr/const variable. */ -string getConstExprValue(Variable v) { - result = v.getInitializer().getExpr().getValue() and - (v.isConst() or v.isConstexpr()) -} + // Collect constant values that we should use to exclude otherwise unused constexpr variables. + // + // For constexpr variables used as template arguments or in static_asserts, we don't see accesses + // (just the appropriate literals). We therefore take a conservative approach and do not report + // constexpr variables whose values are used in such contexts. + // + // For performance reasons, these special values should be collected in a single pass. + predicate excludedConstantValue(string value) { + value = any(ClassTemplateInstantiation cti).getTemplateArgument(_).(Expr).getValue() + or + value = any(StaticAssert sa).getCondition().getAChild*().getValue() + } -/** - * Counts uses of `Variable` v in a local array of size `n` - */ -int countUsesInLocalArraySize(Variable v) { - result = - count(ArrayType at, LocalVariable arrayVariable | - arrayVariable.getType().resolveTypedefs() = at and - v.(PotentiallyUnusedLocalVariable).getFunction() = arrayVariable.getFunction() and - at.getArraySize().toString() = getConstExprValue(v) - ) + /** + * Defines the local variables that should be excluded from the unused variable analysis based + * on their constant value. + * + * See `excludedConstantValue` for more details. + */ + predicate excludeVariableByValue(Variable variable) { + variable.isConstexpr() and + excludedConstantValue(getConstExprValue(variable)) + } + + // TODO: This predicate may be possible to merge with M0-1-4's getUseCount(). These two rules + // diverged to handle `excludeVariableByValue`, but may be possible to merge. + int getUseCountConservatively(Variable v) { + result = + count(VariableAccess access | access = v.getAnAccess()) + + count(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) + + // In case an array type uses a constant in the same scope as the constexpr variable, + // consider it as used. + countUsesInLocalArraySize(v) + } + + predicate isConservativelyUnused(Variable v) { + getUseCountConservatively(v) = 0 and + not excludeVariableByValue(v) + } + + final class SecondPassLocalVariable = SecondPassUnused::UnusedLocalVariable; + + final class SecondPassGlobalOrNspVariable = SecondPassUnused::UnusedGlobalOrNamespaceVariable; + + final class SecondPassMemberVariable = SecondPassUnused::UnusedMemberVariable; + + class UnusedLocalVariable extends SecondPassLocalVariable { + UnusedLocalVariable() { + // Sometimes multiple objects representing the same entities are created in + // the AST. Check if those are not accessed as well. Refer issue #658 + not exists(LocalScopeVariable another | + another.getDefinitionLocation() = this.getDefinitionLocation() and + another.hasName(this.getName()) and + exists(another.getAnAccess()) and + another != this + ) and + isConservativelyUnused(this) + } + } + + class UnusedGlobalOrNamespaceVariable extends SecondPassGlobalOrNspVariable { + UnusedGlobalOrNamespaceVariable() { + // Exclude members whose value is compile time and is potentially used to inintialize a template + not maybeACompileTimeTemplateArgument(this) + } + } + + class UnusedMemberVariable extends SecondPassMemberVariable { + UnusedMemberVariable() { + // No explicit initialization in a constructor + not exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = this) and + // Exclude members whose value is compile time and is potentially used to inintialize a template + not maybeACompileTimeTemplateArgument(this) + } + } } diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode7.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode7.qll new file mode 100644 index 000000000..9b6b1a876 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode7.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype DeadCode7Query = TUnusedLimitedVisibilityVariableQuery() + +predicate isDeadCode7QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `unusedLimitedVisibilityVariable` query + DeadCode7Package::unusedLimitedVisibilityVariableQuery() and + queryId = + // `@id` for the `unusedLimitedVisibilityVariable` query + "cpp/misra/unused-limited-visibility-variable" and + ruleId = "RULE-0-2-1" and + category = "advisory" +} + +module DeadCode7Package { + Query unusedLimitedVisibilityVariableQuery() { + //autogenerate `Query` type + result = + // `Query` type for `unusedLimitedVisibilityVariable` query + TQueryCPP(TDeadCode7PackageQuery(TUnusedLimitedVisibilityVariableQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index cad86d228..c557c8c03 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -19,6 +19,7 @@ import Conversions2 import DeadCode import DeadCode3 import DeadCode4 +import DeadCode7 import Declarations import ExceptionSafety import Exceptions1 @@ -91,6 +92,7 @@ newtype TCPPQuery = TDeadCodePackageQuery(DeadCodeQuery q) or TDeadCode3PackageQuery(DeadCode3Query q) or TDeadCode4PackageQuery(DeadCode4Query q) or + TDeadCode7PackageQuery(DeadCode7Query q) or TDeclarationsPackageQuery(DeclarationsQuery q) or TExceptionSafetyPackageQuery(ExceptionSafetyQuery q) or TExceptions1PackageQuery(Exceptions1Query q) or @@ -163,6 +165,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isDeadCodeQueryMetadata(query, queryId, ruleId, category) or isDeadCode3QueryMetadata(query, queryId, ruleId, category) or isDeadCode4QueryMetadata(query, queryId, ruleId, category) or + isDeadCode7QueryMetadata(query, queryId, ruleId, category) or isDeclarationsQueryMetadata(query, queryId, ruleId, category) or isExceptionSafetyQueryMetadata(query, queryId, ruleId, category) or isExceptions1QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/test/Linkage/ExternalLinkage.expected b/cpp/common/test/Linkage/ExternalLinkage.expected index a61da6a1b..2b5a8e65f 100644 --- a/cpp/common/test/Linkage/ExternalLinkage.expected +++ b/cpp/common/test/Linkage/ExternalLinkage.expected @@ -4,6 +4,7 @@ | file://:0:0:0:0 | operator= | Element has external linkage | | test.cpp:1:5:1:6 | g1 | Element has external linkage | | test.cpp:2:12:2:13 | g2 | Element has external linkage | +| test.cpp:4:18:4:19 | g4 | Element has external linkage | | test.cpp:7:11:7:13 | ns1 | Element has external linkage | | test.cpp:8:5:8:6 | l1 | Element has external linkage | | test.cpp:10:6:10:7 | f1 | Element has external linkage | @@ -18,15 +19,32 @@ | test.cpp:28:8:28:9 | m3 | Element has external linkage | | test.cpp:30:8:30:9 | e1 | Element has external linkage | | test.cpp:31:16:31:16 | e2 | Element has external linkage | +| test.cpp:33:9:33:9 | C2 | Element has external linkage | | test.cpp:33:9:33:9 | operator= | Element has external linkage | | test.cpp:33:9:33:9 | operator= | Element has external linkage | | test.cpp:33:9:33:10 | C2 | Element has external linkage | -| test.cpp:34:17:34:17 | operator= | Element has external linkage | -| test.cpp:34:17:34:17 | operator= | Element has external linkage | -| test.cpp:34:17:34:18 | C3 | Element has external linkage | -| test.cpp:38:15:38:15 | (unnamed constructor) | Element has external linkage | -| test.cpp:38:15:38:15 | operator= | Element has external linkage | -| test.cpp:38:15:38:15 | operator= | Element has external linkage | -| test.cpp:38:15:38:16 | C2 | Element has external linkage | -| test.cpp:43:7:43:8 | C3 | Element has external linkage | -| test.cpp:86:6:86:6 | g | Element has external linkage | +| test.cpp:35:16:35:17 | m2 | Element has external linkage | +| test.cpp:36:10:36:11 | m3 | Element has external linkage | +| test.cpp:39:17:39:17 | (unnamed constructor) | Element has external linkage | +| test.cpp:39:17:39:17 | operator= | Element has external linkage | +| test.cpp:39:17:39:17 | operator= | Element has external linkage | +| test.cpp:39:17:39:18 | C3 | Element has external linkage | +| test.cpp:42:10:42:11 | m3 | Element has external linkage | +| test.cpp:52:15:52:15 | (unnamed constructor) | Element has external linkage | +| test.cpp:52:15:52:15 | operator= | Element has external linkage | +| test.cpp:52:15:52:15 | operator= | Element has external linkage | +| test.cpp:52:15:52:16 | C2 | Element has external linkage | +| test.cpp:55:8:55:9 | m3 | Element has external linkage | +| test.cpp:57:9:57:9 | C2 | Element has external linkage | +| test.cpp:57:9:57:9 | operator= | Element has external linkage | +| test.cpp:57:9:57:9 | operator= | Element has external linkage | +| test.cpp:57:9:57:10 | C2 | Element has external linkage | +| test.cpp:60:10:60:11 | m3 | Element has external linkage | +| test.cpp:63:17:63:17 | (unnamed constructor) | Element has external linkage | +| test.cpp:63:17:63:17 | operator= | Element has external linkage | +| test.cpp:63:17:63:17 | operator= | Element has external linkage | +| test.cpp:63:17:63:18 | C3 | Element has external linkage | +| test.cpp:66:10:66:11 | m3 | Element has external linkage | +| test.cpp:77:7:77:8 | C3 | Element has external linkage | +| test.cpp:160:6:160:6 | g | Element has external linkage | +| test.cpp:169:6:169:16 | block_scope | Element has external linkage | diff --git a/cpp/common/test/Linkage/InternalLinkage.expected b/cpp/common/test/Linkage/InternalLinkage.expected index b3f40fae8..c74361ab7 100644 --- a/cpp/common/test/Linkage/InternalLinkage.expected +++ b/cpp/common/test/Linkage/InternalLinkage.expected @@ -1,16 +1,44 @@ | test.cpp:3:11:3:12 | g3 | Element has internal linkage | -| test.cpp:4:18:4:19 | g4 | Element has internal linkage | | test.cpp:5:12:5:13 | g5 | Element has internal linkage | -| test.cpp:48:1:48:9 | (unnamed namespace) | Element has internal linkage | -| test.cpp:49:11:49:13 | (unnamed namespace)::ns2 | Element has internal linkage | -| test.cpp:50:5:50:6 | l1 | Element has internal linkage | -| test.cpp:52:6:52:7 | f1 | Element has internal linkage | -| test.cpp:54:6:54:7 | e1 | Element has internal linkage | -| test.cpp:59:14:59:14 | e2 | Element has internal linkage | -| test.cpp:65:6:65:7 | f2 | Element has internal linkage | -| test.cpp:67:7:67:8 | C1 | Element has internal linkage | -| test.cpp:71:15:71:16 | C2 | Element has internal linkage | -| test.cpp:76:7:76:8 | C3 | Element has internal linkage | -| test.cpp:84:13:84:13 | f | Element has internal linkage | -| test.cpp:85:12:85:12 | i | Element has internal linkage | -| test.cpp:87:15:87:15 | f | Element has internal linkage | +| test.cpp:82:1:82:9 | (unnamed namespace) | Element has internal linkage | +| test.cpp:83:11:83:13 | (unnamed namespace)::ns2 | Element has internal linkage | +| test.cpp:84:5:84:6 | l1 | Element has internal linkage | +| test.cpp:86:6:86:7 | f1 | Element has internal linkage | +| test.cpp:88:6:88:7 | e1 | Element has internal linkage | +| test.cpp:93:14:93:14 | e2 | Element has internal linkage | +| test.cpp:99:6:99:7 | f2 | Element has internal linkage | +| test.cpp:101:7:101:7 | C1 | Element has internal linkage | +| test.cpp:101:7:101:7 | operator= | Element has internal linkage | +| test.cpp:101:7:101:7 | operator= | Element has internal linkage | +| test.cpp:101:7:101:8 | C1 | Element has internal linkage | +| test.cpp:103:14:103:15 | m2 | Element has internal linkage | +| test.cpp:104:8:104:9 | m3 | Element has internal linkage | +| test.cpp:106:9:106:9 | C2 | Element has internal linkage | +| test.cpp:106:9:106:9 | operator= | Element has internal linkage | +| test.cpp:106:9:106:9 | operator= | Element has internal linkage | +| test.cpp:106:9:106:10 | C2 | Element has internal linkage | +| test.cpp:108:16:108:17 | m2 | Element has internal linkage | +| test.cpp:109:10:109:11 | m3 | Element has internal linkage | +| test.cpp:112:18:112:18 | operator= | Element has internal linkage | +| test.cpp:112:18:112:18 | operator= | Element has internal linkage | +| test.cpp:112:18:112:19 | C3 | Element has internal linkage | +| test.cpp:115:10:115:11 | m3 | Element has internal linkage | +| test.cpp:125:15:125:15 | (unnamed constructor) | Element has internal linkage | +| test.cpp:125:15:125:15 | operator= | Element has internal linkage | +| test.cpp:125:15:125:15 | operator= | Element has internal linkage | +| test.cpp:125:15:125:16 | C2 | Element has internal linkage | +| test.cpp:128:8:128:9 | m3 | Element has internal linkage | +| test.cpp:130:9:130:9 | C2 | Element has internal linkage | +| test.cpp:130:9:130:9 | operator= | Element has internal linkage | +| test.cpp:130:9:130:9 | operator= | Element has internal linkage | +| test.cpp:130:9:130:10 | C2 | Element has internal linkage | +| test.cpp:133:10:133:11 | m3 | Element has internal linkage | +| test.cpp:136:17:136:17 | (unnamed constructor) | Element has internal linkage | +| test.cpp:136:17:136:17 | operator= | Element has internal linkage | +| test.cpp:136:17:136:17 | operator= | Element has internal linkage | +| test.cpp:136:17:136:18 | C3 | Element has internal linkage | +| test.cpp:139:10:139:11 | m3 | Element has internal linkage | +| test.cpp:150:7:150:8 | C3 | Element has internal linkage | +| test.cpp:158:13:158:13 | f | Element has internal linkage | +| test.cpp:159:12:159:12 | i | Element has internal linkage | +| test.cpp:161:15:161:15 | f | Element has internal linkage | diff --git a/cpp/common/test/Linkage/test.cpp b/cpp/common/test/Linkage/test.cpp index 6c2d2e489..89192563b 100644 --- a/cpp/common/test/Linkage/test.cpp +++ b/cpp/common/test/Linkage/test.cpp @@ -30,13 +30,47 @@ class C1 { // inherits external linkage from enclosing namespace enum e1 { E1_1 }; // inherits external linkage from class scope typedef enum { E2_1 } e2; // inherits external linkage from class scope - class C2 {}; // inherits external linkage from class scope + class C2 { // inherits external linkage from class scope + int m1; + static int m2; // inherits external linkage from class scope + void m3() {} // inherits external linkage from class scope + }; // inherits external linkage from class scope + typedef class { - } C3; // inherits external linkage from class scope + int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits external linkage from class scope + } C3; // inherits external linkage from class scope + + class { + int m1; + // static int m2 not allowed in unnamed class + void m3() {} // inherits no linkage from class scope + } m4; // anonymous class outside typedef has no linkage }; typedef class { // inherits external linkage from enclosing namespace int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits external linkage from class scope + + class C2 { // inherits external linkage from enclosing class scope + int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits external linkage from class scope + }; + + typedef class { // inherits external linkage from enclosing namespace + int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits external linkage from class scope + } C3; // inherits external linkage from enclosing namespace + + class { + int m1; + // static int m2 not allowed in unnamed class + void m3() {} // inherits no linkage from class scope + } m4; // anonymous class outside typedef has no linkage } C2; template @@ -66,10 +100,50 @@ void f2(T p1) {} // inherits internal linkage from enclosing namespace class C1 { // inherits internal linkage from enclosing namespace int m1; + static int m2; // inherits internal linkage from class scope + void m3() {} // inherits internal linkage from class scope + + class C2 { + int m1; + static int m2; // inherits internal linkage from class scope + void m3() {} // inherits internal linkage from class scope + }; // inherits internal linkage from class scope + + typedef struct { + int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits internal linkage from class scope + } C3; // inherits internal linkage from class scope + + class { + int m1; + // static int m2 not allowed in unnamed class + void m3() {} // inherits no linkage from class scope + } m4; // anonymous class outside typedef has no linkage }; typedef class { // inherits internal linkage from enclosing namespace int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits internal linkage from class scope + + class C2 { + int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits internal linkage from class scope + }; // inherits internal linkage from class scope + + typedef class { // inherits internal linkage from enclosing namespace + int m1; + // static int m2 not allowed in anonymous class + void m3() {} // inherits internal linkage from class scope + } C3; // inherits internal linkage from enclosing namespace + + class { + int m1; + // static int m2 not allowed in unnamed class + void m3() {} // inherits no linkage from class scope + } m4; // anonymous class outside typedef has no linkage } C2; template @@ -90,4 +164,16 @@ void g() { { extern int i; // object with static storage duration and external linkage } +} + +void block_scope() { + struct S { // No linkage, block scope + int m1; + void member() {} // No linkage, block scope + }; + + typedef struct { // No linkage, block scope + int m1; + void member() {} // No linkage, block scope + } S2; } \ No newline at end of file diff --git a/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/ExternalLinkageNotDeclaredInHeaderFile.expected b/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/ExternalLinkageNotDeclaredInHeaderFile.expected index e5657e9ee..4dfcba071 100644 --- a/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/ExternalLinkageNotDeclaredInHeaderFile.expected +++ b/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/ExternalLinkageNotDeclaredInHeaderFile.expected @@ -1,5 +1,6 @@ | test.cpp:3:5:3:6 | definition of g1 | Externally linked object 'g1' not declared in header file. | | test.cpp:4:12:4:13 | declaration of g2 | Externally linked object 'g2' not declared in header file. | +| test.cpp:6:18:6:19 | definition of g4 | Externally linked object 'g4' not declared in header file. | | test.cpp:10:5:10:6 | definition of l1 | Externally linked object 'l1' not declared in header file. | | test.cpp:11:6:11:7 | definition of f1 | Externally linked function 'f1' not declared in header file. | | test.cpp:22:5:22:5 | definition of f | Externally linked function 'f' not declared in header file. | diff --git a/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/test.cpp b/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/test.cpp index 992399f4c..ee935d53b 100644 --- a/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/test.cpp +++ b/cpp/common/test/rules/externallinkagenotdeclaredinheaderfile/test.cpp @@ -3,7 +3,7 @@ int g1; // NON_COMPLIANT extern int g2; // NON_COMPLIANT const int g3 = 0; // COMPLIANT -extern const int g4 = 0; // COMPLIANT +extern const int g4 = 0; // NON_COMPLIANT static int g5; // COMPLIANT namespace n1 { // named namespace as external linkage diff --git a/cpp/misra/src/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.ql b/cpp/misra/src/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.ql new file mode 100644 index 000000000..4faf2ae8d --- /dev/null +++ b/cpp/misra/src/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.ql @@ -0,0 +1,48 @@ +/** + * @id cpp/misra/unused-limited-visibility-variable + * @name RULE-0-2-1: Variables with limited visibility should be used at least once + * @description Variables that do not affect the runtime of a program indicate potential errors and + * interfere with code readability. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-0-2-1 + * scope/single-translation-unit + * correctness + * readability + * external/misra/enforcement/decidable + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.deadcode.UnusedVariables +import codingstandards.cpp.Linkage + +class UnusedVariable extends Variable { + string description; + + UnusedVariable() { + not hasExternalLinkage(this) and + not hasAttrUnused(this) and + ( + this instanceof ThirdPassUnused::UnusedGlobalOrNamespaceVariable and + description = "Variable '" + this.getQualifiedName() + "' is unused." + or + this instanceof ThirdPassUnused::UnusedLocalVariable and + description = "Variable '" + this.getName() + "' is unused." + or + this instanceof ThirdPassUnused::UnusedMemberVariable and + description = "Member variable '" + this.getQualifiedName() + "' is unused." + ) and + not (isConstant(this) and this.getFile() instanceof HeaderFile and hasNamespaceScope(this)) + } + + string getProblemDescription() { result = description } +} + +predicate isConstant(Variable v) { v.isConst() or v.isConstexpr() or v.isConstinit() } + +from UnusedVariable var +where not isExcluded(var, DeadCode7Package::unusedLimitedVisibilityVariableQuery()) +select var, var.getProblemDescription() diff --git a/cpp/misra/test/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.expected b/cpp/misra/test/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.expected new file mode 100644 index 000000000..648243ea6 --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.expected @@ -0,0 +1,15 @@ +| test.cpp:7:7:7:7 | y | Variable 'y' is unused. | +| test.cpp:20:7:20:7 | z | Variable 'z' is unused. | +| test.cpp:26:5:26:5 | t | Variable 't' is unused. | +| test.cpp:26:5:26:5 | t | Variable 't' is unused. | +| test.cpp:47:6:47:6 | a | Variable 'a' is unused. | +| test.cpp:94:5:94:5 | t | Variable 't' is unused. | +| test_global_or_namespace.cpp:3:12:3:14 | sg0 | Variable 'sg0' is unused. | +| test_global_or_namespace.cpp:4:11:4:13 | cg0 | Variable 'cg0' is unused. | +| test_global_or_namespace.cpp:14:5:14:6 | g3 | Variable '(unnamed namespace)::g3' is unused. | +| test_global_or_namespace.cpp:29:4:29:4 | a | Variable '(unnamed namespace)::a' is unused. | +| test_global_or_namespace.cpp:37:5:37:6 | x3 | Variable '(unnamed namespace)::N1::x3' is unused. | +| test_global_or_namespace.cpp:47:5:47:5 | a | Variable '(unnamed namespace)::N1::a' is unused. | +| test_header.hpp:22:13:22:14 | m1 | Member variable 'HeaderClass::m1' is unused. | +| test_member.cpp:4:7:4:8 | m1 | Member variable 'test::A::m1' is unused. | +| test_member.cpp:31:7:31:8 | m1 | Member variable 'test::C::m1' is unused. | diff --git a/cpp/misra/test/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.qlref b/cpp/misra/test/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.qlref new file mode 100644 index 000000000..1a0df0d0d --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.qlref @@ -0,0 +1 @@ +rules/RULE-0-2-1/UnusedLimitedVisibilityVariable.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-0-2-1/test.cpp b/cpp/misra/test/rules/RULE-0-2-1/test.cpp new file mode 100644 index 000000000..a34e43256 --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-2-1/test.cpp @@ -0,0 +1,120 @@ +/** Test cases for `UnusedLocalVariable.ql` */ + +#define m1 int mx1 = 0; + +int test_simple() { + int x = 1; // COMPLIANT - used below + int y = 2; // NON_COMPLIANT - never used + m1 // we ignore unused variables in macros + return x; +} + +int test_maybe_unused() { + [[maybe_unused]] + int l0; // COMPLIANT - has attr unused +} + +int test_const() { + const int x = 1; // COMPLIANT - used below + const int y = 2; // COMPLIANT - used in array initialization, + int z[y]; // NON_COMPLIANT - never used + return x; +} + +template int f1() { + int x = 1; // COMPLIANT - used in return value + T t; // NON_COMPLIANT - t is never used in any instantiation + // Note: this gets reported twice, once for each instantiation + return x; +} + +class LA {}; +class LB {}; + +void test_f1() { + f1(); + f1(); +} + +int gc1 = 0; + +class LC { +public: + LC() { gc1++; } +}; + +void test_side_effect_init() { + LA a; // NON_COMPLIANT - no constructor called + LC c; // COMPLIANT - constructor called which is considered to potentially + // have side effects +} + +#include +#include +template class CharBuffer { +public: + int member[t]; + CharBuffer() : member{0} {} +}; + +int test_constexpr_in_template_inst() { + constexpr int line_length = 1024U; // COMPLIANT - used in template inst. + // of buffer. + CharBuffer buffer{}; + return buffer.member[0]; +} + +enum DataType : unsigned char { + int8, + int16, +}; + +template int test_constexpr_in_static_assert() { + const std::array lldts{int8}; + const std::array llams{int16}; + constexpr std::size_t mssu = 64 * 1024; // COMPLIANT - used in static assert. + static_assert((sizeof(lldts) + sizeof(llams)) <= mssu, "assert"); + return 0; +} + +int baz() { + test_constexpr_in_static_assert(); + return 0; +} + +template extern constexpr bool all_of_v = true; // COMPLIANT + +template +extern constexpr bool all_of_v = + B1 && all_of_v; // COMPLIANT + +void test_template_variable() { all_of_v; } + +template void template_function() { + T t; // NON_COMPLIANT - t is never used + T t2; // COMPLIANT - t is used + t2.test(); // Call may not be resolved in uninstantiated template +} + +class ClassT { +public: + void test() {} +}; + +void test_template_function() { template_function(); } + +int foo() { + constexpr int arrayDim = 10; // COMPLIANT - used in array size below + static int array[arrayDim]{}; + return array[4]; +} + +template static T another_templ_function() { return T(); } + +template +static T another_templ_function(const First &first, const Rest &...rest) { + return first + + another_templ_function(rest...); // COMPLIANT - 'rest' is used here +} + +static int templ_fnc2() { return another_templ_function(1, 2, 3, 4, 5); } diff --git a/cpp/misra/test/rules/RULE-0-2-1/test_global_or_namespace.cpp b/cpp/misra/test/rules/RULE-0-2-1/test_global_or_namespace.cpp new file mode 100644 index 000000000..e1c445ce5 --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-2-1/test_global_or_namespace.cpp @@ -0,0 +1,74 @@ + +int g0; // COMPLIANT -- external linkage +static int sg0; // NON_COMPLIANT -- internal linkage, but never used +const int cg0 = 0; // NON_COMPLIANT -- internal linkage, but never used, not in + // header +extern const int ecg0 = 0; // COMPLIANT -- external linkage +// tests compliant case of unused constant variable in header file +#include "test_header.hpp" + +// Introduce anonymous namespace to define variables with internal linkage +namespace { +extern int g1; // ignored, declaration only +int g2; // COMPLIANT - used in test_gs() +int g3; // NON_COMPLIANT - never used +int g4; // COMPLIANT - used in `B` constructor + +#define m1() \ + int mx1; \ + int f2() { return mx1; } + +m1(); // ignore dead code in macros + +class GA {}; +class GB { +public: + GB() { g4 = 10; } +}; + +GA a; // NON_COMPLIANT - never used, and does not have a constructor call +GB b; // COMPLIANT - has a constructor call + +void test_gs() { g2 = 1; } + +namespace N1 { +extern int x1; // ignored, declaration only +int x2; // COMPLIANT - used in test_gs() +int x3; // NON_COMPLIANT - never used +int x4; // COMPLIANT - used in `B` constructor +int x5; // COMPLIANT - used outside `N1`, in `test_access_variable` + +class N1A {}; +class N1B { +public: + N1B() { x4 = 10; } +}; + +N1A a; // NON_COMPLIANT - never used, and does not have a constructor call +N1B b; // COMPLIANT - has a constructor call + +void test_ns() { x2 = 1; } + +m1(); // ignore dead code in macros +} // namespace N1 + +int test_access_variable() { return N1::x5; } + +template struct C1 { + int array[t]; // COMPLIANT +}; + +constexpr int g5 = 1; // COMPLIANT - used as template parameter + +namespace ns1 { +constexpr int m1 = 1; // COMPLIANT - used a template parameter +} + +void test_fp_reported_in_384() { + struct C1 l1; + struct C1 l2; + + l1.array[0] = 1; + l2.array[0] = 1; +} +} // namespace \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-0-2-1/test_header.hpp b/cpp/misra/test/rules/RULE-0-2-1/test_header.hpp new file mode 100644 index 000000000..1edeae9a1 --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-2-1/test_header.hpp @@ -0,0 +1,24 @@ +const int cg1 = + 3; // COMPLIANT - unused constant variable defined in header file. +constexpr int cg2 = + 4; // COMPLIANT - unused constant variable defined in header file. +extern const int cg3; // COMPLIANT - unused constant variable with external + // linkage, defined in header file. +inline const int cg4 = 5; // COMPLIANT - unused constant variable defined in + // header file, with inline specifier. +static const int cg5 = 6; // COMPLIANT - unused constant variable defined in + // header file, with static specifier. + +template +constexpr int cg6 = 7; // COMPLIANT - unused constant variable defined in header + // file, with template. + +namespace cg8ns { +const int cg8 = 8; // COMPLIANT - unused constant variable defined in header + // file, in a namespace. +} + +class HeaderClass { + const int m1 = 9; // NON_COMPLIANT -- not namespace scope + static const int m2 = 30234; // NON_COMPLIANT -- not namespace scope +}; \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-0-2-1/test_member.cpp b/cpp/misra/test/rules/RULE-0-2-1/test_member.cpp new file mode 100644 index 000000000..7aff9a423 --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-2-1/test_member.cpp @@ -0,0 +1,64 @@ +namespace test { +class A { +public: + int m1; // NON_COMPLIANT - unused + int m2; // COMPLIANT - initialized in constructor + int m3; // COMPLIANT - written in method + int m4; // COMPLIANT - written outside class + A(int x) : m2(x) {} + void setm3(int x) { m3 = x; } +}; + +void setm4(A *a) { a->m4 = 0; } + +void test_nested_struct() { + struct s1 { + int sm1 : 7; // COMPLIANT - used + int pad : 1; // NON_COMPLIANT - padding bytes, so not used, but is not + // unnamed + int sm2 : 6; // NON_COMPLIANT - unused + int : 2; // COMPLIANT - unnamed padding bytes + } l1; + l1.sm1; // use of `sm1` +} + +class B { + int m1; // ignored, f1 is not defined and may access `m1` + void f1(); +}; + +class C { + int m1; // NON_COMPLIANT - unused + virtual void f1() = 0; // pure virtual function, so consider to be defined +}; + +template class D { +public: + T m1; // NON_COMPLIANT[FALSE_NEGATIVE] - unused, but excluded because of + // templates + T m2; // COMPLIANT - used + D(T p1) : m2(p1) {} + T getT() { return m2; } +}; + +void test_d() { + B b; + D d = D(b); + d.getT(); +} + +template struct C1 { + int array[t]; // COMPLIANT +}; + +struct C2 { + static constexpr int m1 = 1; // COMPLIANT - used as template parameter +}; + +void test_fp_reported_in_384() { + struct C1 l1; + + l1.array[0] = 1; +} + +} // namespace test \ No newline at end of file diff --git a/rule_packages/cpp/DeadCode7.json b/rule_packages/cpp/DeadCode7.json new file mode 100644 index 000000000..8d5a7eb05 --- /dev/null +++ b/rule_packages/cpp/DeadCode7.json @@ -0,0 +1,26 @@ +{ + "MISRA-C++-2023": { + "RULE-0-2-1": { + "properties": { + "enforcement": "decidable", + "obligation": "advisory" + }, + "queries": [ + { + "description": "Variables that do not affect the runtime of a program indicate potential errors and interfere with code readability.", + "kind": "problem", + "name": "Variables with limited visibility should be used at least once", + "precision": "high", + "severity": "error", + "short_name": "UnusedLimitedVisibilityVariable", + "tags": [ + "scope/single-translation-unit", + "correctness", + "readability" + ] + } + ], + "title": "Variables with limited visibility should be used at least once" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index dcc5ee5c5..29744533a 100644 --- a/rules.csv +++ b/rules.csv @@ -827,7 +827,7 @@ cpp,MISRA-C++-2023,RULE-0-0-1,Yes,Required,Decidable,Single Translation Unit,A f cpp,MISRA-C++-2023,RULE-0-0-2,Yes,Advisory,Undecidable,System,Controlling expressions should not be invariant,M0-1-2,DeadCode4,Easy, cpp,MISRA-C++-2023,RULE-0-1-1,Yes,Advisory,Undecidable,System,A value should not be unnecessarily written to a local object,A0-1-1,DeadCode2,Medium, cpp,MISRA-C++-2023,RULE-0-1-2,Yes,Required,Decidable,Single Translation Unit,The value returned by a function shall be used,A0-1-2,DeadCode2,Easy, -cpp,MISRA-C++-2023,RULE-0-2-1,Yes,Advisory,Decidable,Single Translation Unit,Variables with limited visibility should be used at least once,M0-1-3,DeadCode2,Easy, +cpp,MISRA-C++-2023,RULE-0-2-1,Yes,Advisory,Decidable,Single Translation Unit,Variables with limited visibility should be used at least once,M0-1-3,DeadCode7,Easy, cpp,MISRA-C++-2023,RULE-0-2-2,Yes,Required,Decidable,Single Translation Unit,A named function parameter shall be used at least once,"A0-1-4, A0-1-5",DeadCode2,Easy, cpp,MISRA-C++-2023,RULE-0-2-3,Yes,Advisory,Decidable,Single Translation Unit,Types with limited visibility should be used at least once,A0-1-6,DeadCode2,Easy, cpp,MISRA-C++-2023,RULE-0-2-4,Yes,Advisory,Decidable,System,Functions with limited visibility should be used at least once,A0-1-3,DeadCode2,Easy,