From 35d320b8fc9b22dce1d9a9788dbdf6bd223a322f Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Fri, 23 Jan 2026 13:02:18 -0800 Subject: [PATCH 1/5] First draft of Rule 0-1-1, unnecessary assignments. --- c/common/src/codingstandards/c/Objects.qll | 2 +- .../src/codingstandards/cpp/Iterators.qll | 79 -- .../cpp/dominance/SuccessorUnless.qll | 108 +++ .../cpp/exclusions/cpp/DeadCode5.qll | 26 + .../cpp/exclusions/cpp/RuleMetadata.qll | 3 + .../cpp/lifetimes/CppObjects.qll | 389 +++++++++ .../cpp/lifetimes/CppSubObjects.qll | 135 ++++ .../cpp/lifetimes}/StorageDuration.qll | 0 .../cpp/standardlibrary/Iterators.qll | 379 +++++++++ .../cpp/standardlibrary/STLContainers.qll | 346 ++++++++ .../test/includes/standard-library/algorithm | 2 - cpp/common/test/includes/standard-library/map | 12 +- .../test/includes/standard-library/memory.h | 6 + .../test/includes/standard-library/string | 6 - .../test/includes/standard-library/utility.h | 13 +- .../test/includes/standard-library/vector.h | 2 + .../UnnecessaryWriteToLocalObject.ql | 643 +++++++++++++++ .../UnnecessaryWriteToLocalObject.expected | 47 ++ .../UnnecessaryWriteToLocalObject.qlref | 1 + cpp/misra/test/rules/RULE-0-1-1/test.cpp | 749 ++++++++++++++++++ rule_packages/cpp/DeadCode5.json | 27 + rules.csv | 2 +- 22 files changed, 2883 insertions(+), 94 deletions(-) create mode 100644 cpp/common/src/codingstandards/cpp/dominance/SuccessorUnless.qll create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode5.qll create mode 100644 cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll create mode 100644 cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll rename {c/common/src/codingstandards/c => cpp/common/src/codingstandards/cpp/lifetimes}/StorageDuration.qll (100%) create mode 100644 cpp/common/src/codingstandards/cpp/standardlibrary/Iterators.qll create mode 100644 cpp/common/src/codingstandards/cpp/standardlibrary/STLContainers.qll create mode 100644 cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql create mode 100644 cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected create mode 100644 cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.qlref create mode 100644 cpp/misra/test/rules/RULE-0-1-1/test.cpp create mode 100644 rule_packages/cpp/DeadCode5.json diff --git a/c/common/src/codingstandards/c/Objects.qll b/c/common/src/codingstandards/c/Objects.qll index 9a0206771b..cdad9dfd4b 100644 --- a/c/common/src/codingstandards/c/Objects.qll +++ b/c/common/src/codingstandards/c/Objects.qll @@ -1,5 +1,5 @@ import cpp -import codingstandards.c.StorageDuration +import codingstandards.cpp.lifetimes.StorageDuration import codingstandards.c.IdentifierLinkage import semmle.code.cpp.valuenumbering.HashCons import codingstandards.cpp.Clvalues diff --git a/cpp/common/src/codingstandards/cpp/Iterators.qll b/cpp/common/src/codingstandards/cpp/Iterators.qll index c8c217aea4..38ebf3f7b7 100644 --- a/cpp/common/src/codingstandards/cpp/Iterators.qll +++ b/cpp/common/src/codingstandards/cpp/Iterators.qll @@ -252,78 +252,6 @@ class AdditiveOperatorFunctionCall extends FunctionCall { } } -/** - * Models a collection of STL container classes. - */ -class STLContainer extends Class { - STLContainer() { - getNamespace() instanceof StdNS and - getSimpleName() in [ - "vector", "list", "deque", "set", "multiset", "map", "multimap", "stack", "queue", - "priority_queue", "string", "forward_list", "unordered_set", "unordered_multiset", - "unordered_map", "unordered_multimap", "valarray", "string", "basic_string" - ] - or - getSimpleName() = "string" - or - getSimpleName() = "basic_string" - } - - /** - * Get a call to a named function of this class. - */ - FunctionCall getACallTo(string name) { - exists(Function f | - f = getAMemberFunction() and - f.hasName(name) and - result = f.getACallToThisFunction() - ) - } - - /** - * Gets all calls to all functions of this class. - */ - FunctionCall getACallToAFunction() { - exists(Function f | - f = getAMemberFunction() and - result = f.getACallToThisFunction() - ) - } - - FunctionCall getACallToSize() { result = getACallTo("size") } - - FunctionCall getANonConstIteratorBeginFunctionCall() { result = getACallTo("begin") } - - IteratorSource getAConstIteratorBeginFunctionCall() { result = getACallTo("cbegin") } - - IteratorSource getANonConstIteratorEndFunctionCall() { result = getACallTo("end") } - - IteratorSource getAConstIteratorEndFunctionCall() { result = getACallTo("cend") } - - IteratorSource getANonConstIteratorFunctionCall() { - result = getACallToAFunction() and - result.getTarget().getType() instanceof NonConstIteratorType - } - - IteratorSource getAConstIteratorFunctionCall() { - result = getACallToAFunction() and - result.getTarget().getType() instanceof ConstIteratorType - } - - IteratorSource getAnIteratorFunctionCall() { - result = getANonConstIteratorFunctionCall() or result = getAConstIteratorFunctionCall() - } - - IteratorSource getAnIteratorBeginFunctionCall() { - result = getANonConstIteratorBeginFunctionCall() or - result = getAConstIteratorBeginFunctionCall() - } - - IteratorSource getAnIteratorEndFunctionCall() { - result = getANonConstIteratorEndFunctionCall() or result = getAConstIteratorEndFunctionCall() - } -} - /** * Models the set of iterator sources. Useful for encapsulating dataflow coming * from a function call producing an iterator. @@ -338,13 +266,6 @@ class IteratorSource extends FunctionCall { Variable getOwner() { result = getQualifier().(VariableAccess).getTarget() } } -/** - * Models a variable that is a `STLContainer` - */ -class STLContainerVariable extends Variable { - STLContainerVariable() { getType() instanceof STLContainer } -} - /** * Usually an iterator range consists of two sequential iterator arguments, for * example, the start and end. However, there are exceptions to this rule so diff --git a/cpp/common/src/codingstandards/cpp/dominance/SuccessorUnless.qll b/cpp/common/src/codingstandards/cpp/dominance/SuccessorUnless.qll new file mode 100644 index 0000000000..9a5fe06fcd --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/dominance/SuccessorUnless.qll @@ -0,0 +1,108 @@ +import cpp + +/** + * The configuration signature for the SuccessorUnless module. + * + * Typically, the `Node` type should be a `ControlFlowNode`, but it can be overridden to enable + * other kinds of graphs. + */ +signature module SuccessorUnlessConfigSig { + /** The state type used to carry context through the CFG exploration. */ + class State; + + /** The node type used to represent CFG nodes. Overridable. */ + class Node { + Node getASuccessor(); + } + + predicate isStart(State state, Node node); + + predicate isUnless(State state, Node node); + + predicate isEnd(State state, Node node); +} + +/** + * A module that finds successor of a node -- unless there is an intermediate node that satisfies + * a given condition. + * + * Also accepts a state parameter to allow a context to flow through the CFG. + * + * ## Usage + * + * Implement the module `ConfigSig`, with some context type: + * + * ```ql + * module MyConfig implements SuccessorUnless::ConfigSig { + * predicate isStart(SomeContext state, ControlFlowNode node) { + * node = state.someStartCondition() + * } + * + * predicate isUnless(SomeContext state, ControlFlowNode node) { + * node = state.someUnlessCondition() + * } + * + * predicate isEnd(SomeContext state, ControlFlowNode node) { + * node = state.someEndCondition() + * } + * } + * + * import SuccessorUnless::Make as Successor + * ``` + * + * ## Rationale + * + * Why does this module exist? While it may be tempting to write: + * + * ```ql + * exists(ControlFlowNode start, ControlFlowNode end) { + * isStart(start) and + * isEnd(end) and + * end = start.getASuccessor*() and + * not exists(ControlFlowNode mid | + * mid = start.getASuccessor+() and + * end = mid.getASuccessor*() and + * isUnless(mid) + * ) + * } + * ``` + * + * This has an unintuitive trap case in looping CFGs: + * + * ```c + * while (cond) { + * start(); + * end(); + * unless(); + * } + * ``` + * + * In the above code, `unless()` is a successor of `start()`, and `end()` is also a successor of + * `unless()` (via the loop back edge). However, there is no path from `start()` to `end()` that + * does not pass through `unless()`. + * + * This module will correctly handle this case. Forward exploration through the graph will stop + * at the `unless()` nodes, such that only paths from `start()` to `end()` that do not pass through + * `unless()` nodes will be found. + */ +module SuccessorUnless { + predicate isSuccessor(Config::State state, Config::Node start, Config::Node end) { + isMid(state, start, end) and + Config::isEnd(state, end) + } + + private predicate isMid(Config::State state, Config::Node start, Config::Node mid) { + // TODO: Explore if forward-reverse pruning would be beneficial for performance here. + Config::isStart(state, start) and + ( + mid = start + or + exists(Config::Node prevMid | + isMid(state, start, prevMid) and + mid = prevMid.getASuccessor() and + not Config::isUnless(state, mid) and + not Config::isEnd(state, prevMid) + ) + ) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode5.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode5.qll new file mode 100644 index 0000000000..5f28bd8f01 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode5.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype DeadCode5Query = TUnnecessaryWriteToLocalObjectQuery() + +predicate isDeadCode5QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `unnecessaryWriteToLocalObject` query + DeadCode5Package::unnecessaryWriteToLocalObjectQuery() and + queryId = + // `@id` for the `unnecessaryWriteToLocalObject` query + "cpp/misra/unnecessary-write-to-local-object" and + ruleId = "RULE-0-1-1" and + category = "advisory" +} + +module DeadCode5Package { + Query unnecessaryWriteToLocalObjectQuery() { + //autogenerate `Query` type + result = + // `Query` type for `unnecessaryWriteToLocalObject` query + TQueryCPP(TDeadCode5PackageQuery(TUnnecessaryWriteToLocalObjectQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index 0c3cbcc28c..1ac09ca5ac 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -16,6 +16,7 @@ import Const import Conversions import Conversions2 import DeadCode +import DeadCode5 import Declarations import ExceptionSafety import Exceptions1 @@ -78,6 +79,7 @@ newtype TCPPQuery = TConversionsPackageQuery(ConversionsQuery q) or TConversions2PackageQuery(Conversions2Query q) or TDeadCodePackageQuery(DeadCodeQuery q) or + TDeadCode5PackageQuery(DeadCode5Query q) or TDeclarationsPackageQuery(DeclarationsQuery q) or TExceptionSafetyPackageQuery(ExceptionSafetyQuery q) or TExceptions1PackageQuery(Exceptions1Query q) or @@ -140,6 +142,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isConversionsQueryMetadata(query, queryId, ruleId, category) or isConversions2QueryMetadata(query, queryId, ruleId, category) or isDeadCodeQueryMetadata(query, queryId, ruleId, category) or + isDeadCode5QueryMetadata(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/src/codingstandards/cpp/lifetimes/CppObjects.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll new file mode 100644 index 0000000000..32dec453c6 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll @@ -0,0 +1,389 @@ +import cpp +import codingstandards.cpp.lifetimes.StorageDuration +import semmle.code.cpp.valuenumbering.HashCons +import codingstandards.cpp.Clvalues + +/** + * A library for handling "Objects" in C++. + * + * See `codingstandards.c.Objects` for the C equivalent. Changes specific to C++ are: + * - refactored class hierarchy with final extension to simplify overridden predicates + * - handling of references as non-objects + * + * Objects may be stored in registers or memory, they have an address, a type, a storage duration, + * and a lifetime (which is different than storage duration). Objects which are structs or arrays + * have subobjects, which share the storage duration and lifetime of the parent object. + * + * Note: lifetime analysis is not performed in this library, but is available in + * the module `codingstandards.cpp.lifetimes.LifetimeProfile`. In the future, these libraries could + * be merged for more complete analysis. + * + * To get objects in a project, use the `ObjectIdentity` class which finds the following types of + * objects: + * - global variables + * - local variables + * - literals + * - malloc calls + * - certain temporary object expressions + * + * And direct references to these objects can be found via the member predicate `getAnAccess()`. + * However, much of a project's code will not refer to these objects directly, but rather, refer to + * their subobjects. The class `ObjectIdentity` exposes several member predicates for finding when + * these subobjects are used: + * - `getASubobjectType()` + * - `getASubobjectAccess()` + * - `getASubobjectAddressExpr()` + * + * These methods do not use flow analysis, and will not return a conclusive list of accesses. To + * get better results here, this library should be integrated with flow analysis or the library + * `LifetimeProfile.qll`. + * + * Additionally, subobjects are currently not tracked individually. In the future subobjects could + * be tracked as a root object and an access chain to refer to them. For now, however, finding *any* + * subobject access is sufficient for many analyses. + * + * To get the storage duration, `ObjectIdentity` exposes the member predicate + * `getStorageDuration()` with the following options: + * - `obj.getStorageDuration().isAutomatic()`: Stack objects + * - `obj.getStorageDuration().isStatic()`: Global objects + * - `obj.getStorageDuration().isThread()`: Threadlocal objects + * - `obj.getStorageDuration().isAllocated()`: Dynamic objects + * + * Note that lifetimes are not storage durations. The only lifetime tracking currently implemented + * is `hasTemporaryLifetime()`, which is a subset of automatic storage duration objects, and may + * be filtered out, or selected directly with `TemporaryObjectIdentity`. + */ +final class ObjectIdentity = ObjectIdentityBase; + +/** + * A base class for objects in C++, along with the source location where the object can be + * identified in the project code (thus, this class extends `Element`), which may be variable, or + * may be an expression such as a literal or a malloc call. + * + * Extend this class to define a new type of object identity. To create a class which filters the + * set of object identities, users of this library should extend the final subclass + * `ObjectIdentity` instead. + */ +abstract class ObjectIdentityBase extends Element { + /** + * The type of this object. + * + * Note that for allocated objects, this is inferred from the sizeof() statement or the variable + * it is assigned to. + */ + abstract Type getType(); + + /* The storage duration of this object: static, thread, automatic, or allocated. */ + abstract StorageDuration getStorageDuration(); + + /** + * Get the nested objects within this object (members, array element types). + * + * Note that if a struct has a pointer member, the pointer itself is a subobject, but the value + * it points to is not. Therefore `struct { int* x; }` has a subobject of type `int*`, but not + * `int`. + */ + Type getASubObjectType() { result = getADirectSubobjectType*(getType()) } + + /** + * Get expressions which trivially access this object. Does not perform flow analysis. + * + * For dynamically allocated objects, this is a dereference of the malloc call. + */ + abstract Expr getAnAccess(); + + /** + * Get expressions which trivially access this object or a subobject of this object. Does not + * perform flow analysis. + * + * For dynamically allocated objects, this is a dereference of the malloc call or direct access + * of the result of dereferencing the malloc call. + */ + Expr getASubobjectAccess() { result = getASubobjectAccessOf(getAnAccess()) } + + /** + * Get expressions which trivially take the address of this object or a subobject of this object. + * Does not perform flow analysis. + */ + Expr getASubobjectAddressExpr() { + exists(Expr subobject | + subobject = getASubobjectAccess() and + ( + // Holds for address-of expressions. + result = any(AddressOfExpr e | e.getOperand() = subobject) + or + // Holds for array-to-pointer conversions, which evaluate to a usable subobject address. + exists(ArrayToPointerConversion c | c.getExpr() = subobject) and + // Note that `arr[x]` has an array-to-pointer conversion, and returns the `x`th item by + // value, not the address of the `x`th item. Therefore, exclude `arr` if `arr` is part of + // an expression `arr[x]`. + not exists(ArrayExpr a | a.getArrayBase() = subobject) and + result = subobject + ) + ) + } + + /** + * Finds cases such as a subobject such as `x.y` is taken as a reference. + * + * This is useful for alias analysis, as references create aliases that can invalidate certain + * assumptions about object accesses. + */ + Expr getASubobjectTakenAsReference() { + result = getASubobjectAccess() and + result.getFullyConverted().getUnderlyingType() instanceof ReferenceType + } +} + +/** + * Finds expressions `e.x` or `e[x]` for expression `e`, recursively. Does not resolve pointers. + * + * Note that this does not hold for `e->x` or `e[x]` where `e` is a pointer. + */ +private Expr getASubobjectAccessOf(Expr e) { + result = e + or + result.(DotFieldAccess).getQualifier() = getASubobjectAccessOf(e) + or + result.(ArrayExpr).getArrayBase() = getASubobjectAccessOf(e) and + not result.(ArrayExpr).getArrayBase().getUnspecifiedType() instanceof PointerType +} + +/** + * Find the object types that are embedded within the current type. + * + * For example, a block of memory with type `T[]` has subobjects of type `T`, and a struct with a + * member of `T member;` has a subobject of type `T`. + * + * Note that subobjects may be pointers, but the value they point to is not a subobject. For + * instance, `struct { int* x; }` has a subobject of type `int*`, but not `int`. + */ +Type getADirectSubobjectType(Type type) { + result = type.stripTopLevelSpecifiers().(Struct).getAMember().getADeclarationEntry().getType() + or + result = type.stripTopLevelSpecifiers().(ArrayType).getBaseType() +} + +/** + * An object in memory which may be identified by the variable that holds it. + * + * This may be a local variable, a global variable, or a parameter, etc. However, it cannot be a + * member of a struct or union, as these do not have storage duration. + */ +class VariableObjectIdentity extends ObjectIdentityBase instanceof Variable { + VariableObjectIdentity() { + // Exclude members; member definitions does not allocate storage and thus do not have a storage + // duration. They are therefore not objects. To get the storage duration of members, use one of + // the predicates related to sub objects, e.g. `getASubObjectType()`. + not this.(Variable).isMember() and + // The C++ is deliberately vague on the object properties of references. Technically, references + // are implemented as pointers, which are objects and have storage duration and lifetime. + // However, the underlying pointer is opaque, and only the referent is accessible. Therefore, + // a reference itself is not considered an object here. + not this.(Variable).getType() instanceof ReferenceType + } + + override StorageDuration getStorageDuration() { + exists(Variable v | v = this | + // 6.2.4.4, objects declared _Thread_local have thread storage duration. + v.isThreadLocal() and result.isThread() + or + // 6.2.4.3, Non _ThreadLocal objects with internal or external linkage or declared static have + // static storage duration. + not v.isThreadLocal() and + (v.isStatic() or v.isTopLevel()) and + result.isStatic() + or + // 6.2.4.3, Non _ThreadLocal objects no linkage that are not static have automatic storage + // duration. + not v.isThreadLocal() and + not v.isStatic() and + not v.isTopLevel() and + result.isAutomatic() + ) + } + + override Type getType() { + // Caution here: If we use `Variable.super.getType()` then override resolution is skipped, and + // it uses the base predicate defined as `none()`. By casting this to `Variable` and calling + // `getType()`, all overrides (harmlessly, *including this one*...) are considered, which means + // we defer to the subclasses such as `GlobalVariable` overrides of `getType()`, which is what + // we want. + result = this.(Variable).getType() + } + + override VariableAccess getAnAccess() { result = Variable.super.getAnAccess() } +} + +/** + * A string literal is an object with static storage duration. + * + * 6.4.5.6, multibyte character sequences initialize an array of static storage duration. + */ +class LiteralObjectIdentity extends Literal, ObjectIdentityBase { + override StorageDuration getStorageDuration() { result.isStatic() } + + override Type getType() { result = Literal.super.getType() } + + override Expr getAnAccess() { result = this } +} + +/** + * An object identifiable as a struct or array literal, which is an lvalue that may have static or + * automatic storage duration depending on context. + * + * 6.5.2.5.5, compound literals outside of a function have static storage duration, while literals + * inside a function have automatic storage duration. + */ +class AggregateLiteralObjectIdentity extends AggregateLiteral, ObjectIdentityBase { + override StorageDuration getStorageDuration() { + if exists(getEnclosingFunction()) then result.isAutomatic() else result.isStatic() + } + + override Type getType() { result = AggregateLiteral.super.getType() } + + override Expr getAnAccess() { result = this } +} + +/** + * An object identified by a call to `malloc`. + * + * Note: the malloc expression returns an address to this object, not the object itself. Therefore, + * `getAnAccess()` returns cases where this malloc result is dereferenced, and not the malloc call + * itself. + * + * Note that the predicates for tracking accesses, subobject accesses, and address expresisons may + * be less reliable as dynamic memory is fundamentally more difficult to track. However, this class + * attempts to give reasonable results. In the future, this could be improved by integrating with + * LifetimeProfile.qll or by integrating flow analysis. + * + * Additionally, the type of this object is inferred based on its size and use. + */ +class AllocatedObjectIdentity extends AllocationExpr, ObjectIdentityBase { + AllocatedObjectIdentity() { + this.(FunctionCall).getTarget().(AllocationFunction).requiresDealloc() + } + + override StorageDuration getStorageDuration() { result.isAllocated() } + + /** Attempt to infer the type of the allocated memory */ + override Type getType() { result = this.getAllocatedElementType() } + + /** Find dereferences of direct aliases of this pointer result. */ + override Expr getAnAccess() { result.(PointerDereferenceExpr).getOperand() = getAnAlias() } + + /** + * Find the following subobject accesses, given a pointer alias `x`: + * - `(*x)` + * - `(*x).y` + * - `(*x)[i]` + * - `x->y` + * - `x[i]` + * - `x->y.z` + * - `x[i].y` + * - all direct accesses (`foo.x`, `foo[i]`) of the above + */ + override Expr getASubobjectAccess() { + result = getASubobjectAccessOf(getAnAccess()) + or + exists(PointerFieldAccess pfa | + pfa.getQualifier() = getASubobjectAddressExpr() and + result = getASubobjectAccessOf(pfa) + ) + or + exists(ArrayExpr arrayExpr | + arrayExpr.getArrayBase() = getASubobjectAddressExpr() and + result = getASubobjectAccessOf(arrayExpr) + ) + } + + /** + * Given a pointer alias `x`, finds `x` itself. Additionally, defers to the default class + * behavior, which finds address-of (`&`) and array-to-pointer conversions of all subobject + * accesses. (See `AllocatedObjectIdentity.getASubobjectAccess()`.) + */ + override Expr getASubobjectAddressExpr() { + result = getAnAlias() + or + result = super.getASubobjectAddressExpr() + } + + /** + * Find an obvious direct reference to the result of a `malloc()` function call. This includes + * the function call itself, but additionally: + * - For `T* x = malloc(...)`, accesses to variable `x` are likely aliases of the malloc result + * - For `(expr) = malloc(...)` future lexically identical uses of `expr` are likely aliases of + * the malloc result. + * + * This is used so that member predicates such as `getAnAccess()`, `getASubobjectAccess()` can + * find cases such as: + * + * ```c + * int *x = malloc(sizeof(int)); + * return *x; // accesses the malloc result + * ``` + */ + Expr getAnAlias() { + result = this + or + exists(AssignExpr assignExpr | + assignExpr.getRValue() = this and + hashCons(result) = hashCons(assignExpr.getLValue()) + ) + or + exists(Variable v | + v.getInitializer().getExpr() = this and + result = v.getAnAccess() + ) + } +} + +/** + * A struct or union type that contains an array type, used to find objects with temporary + * lifetime. + */ +private class StructOrUnionTypeWithArrayField extends Struct { + StructOrUnionTypeWithArrayField() { + this.getAField().getUnspecifiedType() instanceof ArrayType + or + // nested struct or union containing an array type + this.getAField().getUnspecifiedType().(Struct) instanceof StructOrUnionTypeWithArrayField + } +} + +/** + * 6.2.4.7, A non-lvalue expression with struct or or union type that has a field member of array + * type, refers to an object with automatic storage duration (and has temporary lifetime). + * + * The spec uses the lanugage "refers to." This is likely intended to mean that the expression + * `foo().x` does not create a new temporary object, but rather "refers to" the temporary object + * storing the value of the expression `foo()`. + * + * Separate this predicate to avoid non-monotonic recursion (`C() { not exists(C c | ... ) }`). + */ +private class TemporaryObjectIdentityExpr extends Expr { + TemporaryObjectIdentityExpr() { + getType() instanceof StructOrUnionTypeWithArrayField and + not isCLValue(this) + } +} + +/** + * 6.2.4.7, A non-lvalue expression with struct or or union type that has a field member of array + * type, is an object with automatic storage duration (and has temporary lifetime). + */ +class TemporaryObjectIdentity extends ObjectIdentityBase instanceof TemporaryObjectIdentityExpr { + TemporaryObjectIdentity() { + // See comment in `TemporaryObjectIdentityExpr` for why we check `getASubobjectAccess()` here. + not exists(TemporaryObjectIdentityExpr parent | + this = getASubobjectAccessOf(parent) and + not this = parent + ) + } + + override StorageDuration getStorageDuration() { result.isAutomatic() } + + override Type getType() { result = this.(Expr).getType() } + + override Expr getAnAccess() { result = this } +} diff --git a/cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll new file mode 100644 index 0000000000..aabd513e6a --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll @@ -0,0 +1,135 @@ +/** + * A library for tracking "subobjects" of objects in C++. + * + * See `codingstandards.c.SubObjects` for the C equivalent. Changes specific to C++ are: + * - excluding fields of reference type as subobjects + * + * A library that expands upon the `Objects.qll` library, to support nested "Objects" such as + * `x.y.z` or `x[i][j]` within an object `x`. + * + * Objects in C are values in memory, that have a type and a storage duration. In the case of + * array objects and struct objects, the object will contain other objects. The these subobjects + * will share properties of the root object such as storage duration. This library can be used to, + * for instance, find all usages of a struct member to ensure that member is initialized before it + * is used. + * + * To use this library, select `SubObject` and find its usages in the AST via `getAnAccess()` (to + * find usages of the subobject by value) or `getAnAddressOfExpr()` (to find usages of the object + * by address). + * + * Note that a struct or array object may contain a pointer. In this case, the pointer itself is + * a subobject of the struct or array object, but the object that the pointer points to is not. + * This is because the pointed-to object does not necessarily have the same storage duration, + * lifetime, or linkage as the pointer and the object containing the pointer. + * + * Note as well that `getAnAccess()` on an array subobject will return all accesses to the array, + * not just accesses to a particular index. For this reason, `SubObject` exposes the predicate + * `isPrecise()`. If a subobject is precise, that means all results of `getAnAccess()` will + * definitely refer to the same object in memory. If it is not precise, the different accesses + * may refer to the same or different objects in memory. For instance, `x[i].y` and `x[j].y` are + * the same object if `i` and `j` are the same, but they are different objects if `i` and `j` are + * different. + */ + +import codingstandards.cpp.lifetimes.CppObjects + +newtype TSubObject = + TObjectRoot(ObjectIdentity i) or + TObjectMember(SubObject struct, MemberVariable m) { + m = struct.getType().(Class).getAMemberVariable() and + // References point to subobjects of the reference type, so fields of reference type are not + // subobjects of the containing object: + not m.getUnderlyingType() instanceof ReferenceType + } or + TObjectIndex(SubObject array) { array.getType() instanceof ArrayType } + +class SubObject extends TSubObject { + string toString() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i.toString() + ) + or + exists(SubObject struct, Variable m | + this = TObjectMember(struct, m) and + result = struct.toString() + "." + m.getName() + ) + or + exists(SubObject array | + this = TObjectIndex(array) and + result = array.toString() + ) + } + + Type getType() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i.getType() + ) + or + exists(Variable m | + this = TObjectMember(_, m) and + result = m.getType() + ) + or + exists(SubObject array | + this = TObjectIndex(array) and + result = array.getType().(ArrayType).getBaseType() + ) + } + + /** + * Holds for object roots and for member accesses on that root, not for array accesses. + * + * This is useful for cases where we do not wish to treat `x[y]` and `x[z]` as the same object. + */ + predicate isPrecise() { not getParent*() = TObjectIndex(_) } + + SubObject getParent() { + exists(SubObject struct, MemberVariable m | + this = TObjectMember(struct, m) and + result = struct + ) + or + exists(SubObject array | + this = TObjectIndex(array) and + result = array + ) + } + + Expr getAnAccess() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i.getAnAccess() + ) + or + exists(MemberVariable m | + this = TObjectMember(_, m) and + result = m.getAnAccess() and + // Only consider `DotFieldAccess`es, not `PointerFieldAccess`es, as the latter + // are not subobjects of the root object: + result.(DotFieldAccess).getQualifier() = getParent().getAnAccess() + ) + or + this = TObjectIndex(_) and + result.(ArrayExpr).getArrayBase() = getParent().getAnAccess() + } + + AddressOfExpr getAnAddressOfExpr() { result.getOperand() = this.getAnAccess() } + + /** + * Get the "root" object identity to which this subobject belongs. For instance, in the + * expression `x.y.z`, the root object is `x`. This subobject will share properties with the root + * object such as storage duration, lifetime, and linkage. + */ + ObjectIdentity getRootIdentity() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i + ) + or + result = getParent().getRootIdentity() + } + + predicate representsRootObject() { this = TObjectRoot(_) } +} diff --git a/c/common/src/codingstandards/c/StorageDuration.qll b/cpp/common/src/codingstandards/cpp/lifetimes/StorageDuration.qll similarity index 100% rename from c/common/src/codingstandards/c/StorageDuration.qll rename to cpp/common/src/codingstandards/cpp/lifetimes/StorageDuration.qll diff --git a/cpp/common/src/codingstandards/cpp/standardlibrary/Iterators.qll b/cpp/common/src/codingstandards/cpp/standardlibrary/Iterators.qll new file mode 100644 index 0000000000..f8ec8eb188 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/standardlibrary/Iterators.qll @@ -0,0 +1,379 @@ +/** + * A library for modeling iterators and various iterator operations. + */ + +import cpp +private import semmle.code.cpp.dataflow.DataFlow +private import semmle.code.cpp.dataflow.TaintTracking +import codingstandards.cpp.StdNamespace +import codingstandards.cpp.rules.containeraccesswithoutrangecheck.ContainerAccessWithoutRangeCheck as ContainerAccessWithoutRangeCheck +import semmle.code.cpp.controlflow.Guards +import semmle.code.cpp.valuenumbering.GlobalValueNumbering +import codingstandards.cpp.standardlibrary.STLContainers +import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils + +pragma[noinline, nomagic] +private predicate localTaint(DataFlow::Node n1, DataFlow::Node n2) { + TaintTracking::localTaint(n1, n2) +} + +/** + * Models a variable of type `const_iterator` or `iterator` + */ +class ContainerIteratorAccess extends ContainerAccess { + ContainerIteratorAccess() { getType() instanceof IteratorType } + + predicate isAdditiveOperation() { + exists(AdditiveOperatorFunctionCall fc | + // start with all accesses to this variable + // and restrict it to those accesses where + // the access is as a qualifier to a + // ++ or + operation. + this = fc.getQualifier() + ) + } + + override Variable getOwningContainer() { + exists(FunctionCall fc | + fc = getANearbyAssigningIteratorCall() and + result = fc.getQualifier().(VariableAccess).getTarget() + ) + } + + /** + * gets a function call to cbegin/begin that + * assigns its value to the iterator represented by this + * access + */ + FunctionCall getANearbyAssigningIteratorCall() { + // the underlying container for this variable is one wherein + // there is an assigned value of cbegin/cend + // find all the cbegin/begin calls + exists(STLContainer c, FunctionCall fc | + // all calls to cbegin/begin + fc = c.getAnIteratorFunctionCall() and + // wherein it has dataflow into this access + DataFlow::localFlow(DataFlow::exprNode(fc), DataFlow::exprNode(this)) and + result = fc + ) + } +} + +class ContainerRevalidationOperation extends FunctionCall { + ContainerRevalidationOperation() { getTarget().getType() instanceof IteratorType } + + Variable getOwningContainer() { result = getQualifier().(VariableAccess).getTarget() } + + VariableAccess getARevalidatedExpr() { + localTaint(DataFlow::exprNode(this), DataFlow::exprNode(result)) + } +} + +/** + * Models the set of operations that can invalidate a pointer, iterator or + * reference to a `STLContainer`. + */ +class ContainerInvalidationOperation extends FunctionCall { + ContainerInvalidationOperation() { + //std::deque + ( + exists(STLContainer c | + c.getSimpleName() = "deque" and + this = c.getACallToAFunction() and + getTarget().getName() in [ + "insert", "emplace_front", "emplace_back", "emplace", "push_front", "push_back", + "erase", "pop_back", "resize", "clear" + ] + ) + or + //std::forward_list + exists(STLContainer c | + c.getSimpleName() = "forward_list" and + this = c.getACallToAFunction() and + getTarget().getName() in ["erase_after", "pop_front", "resize", "remove", "unique", "clear"] + ) + or + //std::list + exists(STLContainer c | + c.getSimpleName() = "list" and + this = c.getACallToAFunction() and + getTarget().getName() in [ + "erase", "pop_front", "pop_back", "clear", "remove", "remove_if", "unique", "clear" + ] + ) + or + //std::vector + exists(STLContainer c | + c.getSimpleName() = "vector" and + this = c.getACallToAFunction() and + getTarget().getName() in [ + "reserve", "insert", "emplace_back", "emplace", "push_back", "erase", "pop_back", + "resize", "clear" + ] + ) + or + //std::set,multiset, map, multimap + exists(STLContainer c | + c.getSimpleName() in ["set", "multiset", "map", "multimap"] and + this = c.getACallToAFunction() and + getTarget().getName() in ["erase", "clear"] + ) + or + //std::unordered_set, unordered_multiset, unordered_map, unordered_multimap + exists(STLContainer c | + c.getSimpleName() in [ + "unordered_set", "unordered_multiset", "unordered_map", "unordered_multimap" + ] and + this = c.getACallToAFunction() and + getTarget().getName() in ["erase", "clear", "insert", "emplace", "rehash", "reserve"] + ) + or + //std::valarray + exists(STLContainer c | + c.getSimpleName() in ["valarray"] and + this = c.getACallToAFunction() and + getTarget().getName() in ["resize"] + ) + or + //std::string + ( + exists(STLContainer c | + c.getSimpleName() in ["string", "basic_string"] and + this = c.getACallToAFunction() and + // non-const member functions + not getTarget().hasSpecifier("const") and + // except + not getTarget().getName() in [ + "operator[]", "at", "front", "back", "begin", "rbegin", "end", "rend", "data" + ] + ) + or + exists(FunctionCall fc | + fc.getTarget().getNamespace() instanceof StdNS and + this.getTarget().getName() in ["swap", "operator>>", "getline"] + ) + ) + ) + } + + /** + * Holds if this operation taints a given `VariableAccess`. This is useful for + * tracking flow out of these functions (which may or may not happen). For + * example, if an iterator reference returned is not subsequently assigned. + */ + predicate taintsVariables() { + exists(VariableAccess e | localTaint(DataFlow::exprNode(this), DataFlow::exprNode(e))) + } + + /** + * All operations in this class happen on a variable. + */ + Variable getContainer() { result = getQualifier().(VariableAccess).getTarget() } +} + +/** An iterator type in the `std` namespace. */ +class StdIteratorType extends UserType { + StdIteratorType() { + this.getNamespace() instanceof StdNS and + getSimpleName().matches("%_iterator") and + not getSimpleName().matches("const_%") + } +} + +abstract class IteratorType extends Type { } + +/** + * Models a `const_iterator` datatype. + */ +class ConstIteratorType extends IteratorType { + ConstIteratorType() { + this.(CTypedefType).getName() = "const_iterator" + or + this.isConst() and + this.(SpecifiedType).getBaseType() instanceof StdIteratorType + } +} + +/** + * Models a `iterator` datatype. + */ +class NonConstIteratorType extends IteratorType { + NonConstIteratorType() { + this.(CTypedefType).getName() = "iterator" + or + this instanceof StdIteratorType + } +} + +/** + * Models a variable of type `const_iterator`. + */ +class ConstIteratorVariable extends Variable { + ConstIteratorVariable() { getType() instanceof ConstIteratorType } +} + +/** + * Models a call to an additive operator. + */ +class AdditiveOperatorFunctionCall extends FunctionCall { + AdditiveOperatorFunctionCall() { + getTarget().getName() = "operator+" or getTarget().getName() = "operator++" + } +} + +/** + * Models the set of iterator sources. Useful for encapsulating dataflow coming + * from a function call producing an iterator. + */ +class IteratorSource extends FunctionCall { + IteratorSource() { this.getType() instanceof IteratorType } + + predicate sourceFor(Expr e) { + DataFlow::localFlow(DataFlow::exprNode(this), DataFlow::exprNode(e)) + } + + Variable getOwner() { result = getQualifier().(VariableAccess).getTarget() } +} + +/** + * Usually an iterator range consists of two sequential iterator arguments, for + * example, the start and end. However, there are exceptions to this rule so + * rather than relying on a heuristic alone, this class is encoded with + * with exceptions to create a more accurate model. It is used with `IteratorRangeFunctionCall` + * to create this functionality. + */ +class IteratorRangeModel extends Function { + IteratorRangeModel() { hasQualifiedName("std", "lexicographical_compare") } + + int getAnIndexOfAStartRange() { + (hasQualifiedName("std", "lexicographical_compare") and result = [0, 1]) + } + + int getAnIndexOfAEndRange() { + (hasQualifiedName("std", "lexicographical_compare") and result = [2, 3]) + } + + int getAnIteratorArgumentIndex() { + (hasQualifiedName("std", "lexicographical_compare") and result = [0, 1, 2, 3]) + } + + predicate getAPairOfStartEndIndexes(int start, int end) { + hasQualifiedName("std", "lexicographical_compare") and start = 0 and end = 1 + or + hasQualifiedName("std", "lexicographical_compare") and start = 2 and end = 3 + } +} + +/** + * This definition takes the position that any function taking more than one + * iterator is a range function. This assumption appears to be reasonable + * based on testing on several large databases. + * + * We add special cases for functions where this assumption is not correct. + * These exceptions are modeled by the `IteratorRangeModel` class on which + * this class depends. + */ +class IteratorRangeFunctionCall extends FunctionCall { + IteratorRangeFunctionCall() { + count(Expr e | + e = getAnArgument() and + e.getType() instanceof IteratorType and + getTarget().getNamespace() instanceof StdNS and + not getTarget().getName() in ["operator==", "operator!="] + ) > 1 + } + + int getAnIteratorArgumentIndex() { + result = getTarget().(IteratorRangeModel).getAnIteratorArgumentIndex() + or + getArgument(result).getType() instanceof IteratorType + } + + // develop a function that gives indexes of which things are which + // then we basically theck that there is flow from cend/cbegin to those + // essentially that there exists one + int getAnIndexOfAStartRange() { + result = getTarget().(IteratorRangeModel).getAnIndexOfAStartRange() + or + result = min(getAnIteratorArgumentIndex()) + } + + int getAnIndexOfAEndRange() { + result = getTarget().(IteratorRangeModel).getAnIndexOfAEndRange() + or + result = getAnIteratorArgumentIndex() and not result = getAnIndexOfAStartRange() + } + + Expr getAStartRangeFunctionCallArgument() { result = getArgument(getAnIndexOfAStartRange()) } + + Expr getAEndRangeFunctionCallArgument() { result = getArgument(getAnIndexOfAEndRange()) } + + predicate getAStartEndRangeFunctionCallArgumentPairIndexes(int start, int end) { + getTarget().(IteratorRangeModel).getAPairOfStartEndIndexes(start, end) + or + end = getAnIndexOfAEndRange() and start = getAnIndexOfAStartRange() + } + + /** + * This predicate produces the set of (start,end) where `start` and `end` are + * start end pairs corresponding to some usage. Note this is modeled by `IteratorRangeModel`. + */ + predicate getAStartEndRangeFunctionCallArgumentPair(Expr start, Expr end) { + exists(int s, int e | + getAStartEndRangeFunctionCallArgumentPairIndexes(s, e) and + start = getArgument(s) and + end = getArgument(e) + ) + } +} + +/** + * This predicate holds if a given `ContainerInvalidationOperation` reaches a + * `ContainerAccess` without being revalidated. + */ +predicate invalidationReaches(ContainerInvalidationOperation op, ContainerAccess target) { + target.getOwningContainer() = op.getContainer() and + target = getANonInvalidatedSuccessor(op) and + not exists(ContainerRevalidationOperation rop | + rop.getARevalidatedExpr() = target and + rop = getANonInvalidatedSuccessor(op) and + rop = getANonInvalidatedPredecessor(target) + ) +} + +/** Get a predecessor of `target` that occurs without invalidation. */ +ControlFlowNode getANonInvalidatedPredecessor(ContainerAccess target) { + result = target + or + exists(ControlFlowNode mid | + mid = getANonInvalidatedPredecessor(target) and + result = mid.getAPredecessor() and + not result instanceof ContainerInvalidationOperation + ) +} + +/** Get a successor of `target` that occurs without invalidation. */ +ControlFlowNode getANonInvalidatedSuccessor(ContainerInvalidationOperation op) { + result = op + or + exists(ControlFlowNode mid | + mid = getANonInvalidatedSuccessor(op) and + result = mid.getASuccessor() and + not result instanceof ContainerInvalidationOperation + ) +} + +/** + * Guarded by a bounds check that ensures our destination is larger than "some" value + */ +predicate sizeCompareBoundsChecked(IteratorSource iteratorCreationCall, Expr guarded) { + exists( + GuardCondition guard, ContainerAccessWithoutRangeCheck::ContainerSizeCall sizeCall, + boolean branch + | + globalValueNumber(sizeCall.getQualifier()) = + globalValueNumber(iteratorCreationCall.getQualifier()) and + guard.controls(guarded.getBasicBlock(), branch) and + relOpWithSwapAndNegate(guard, sizeCall, _, Greater(), _, branch) + ) +} diff --git a/cpp/common/src/codingstandards/cpp/standardlibrary/STLContainers.qll b/cpp/common/src/codingstandards/cpp/standardlibrary/STLContainers.qll new file mode 100644 index 0000000000..df1635f261 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/standardlibrary/STLContainers.qll @@ -0,0 +1,346 @@ +import cpp +import codingstandards.cpp.StdNamespace +private import codingstandards.cpp.standardlibrary.Iterators +private import semmle.code.cpp.dataflow.DataFlow +private import semmle.code.cpp.dataflow.TaintTracking +private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis + +newtype TContainerKind = + TIndexableContainer() or + TCollectionContainer() or + TAssociativeContainer() or + TStringContainer() + +newtype TContainerTypeKind = + TContainerElementType() or + TContainerKeyType() or + TContainerCharType() + +newtype TIndexIdentity = + TBoundedIndex(float lower, float upper) { + exists(ArrayExpr arrayExpr | + lower = lowerBound(arrayExpr.getArrayOffset().getFullyConverted()) and + upper = upperBound(arrayExpr.getArrayOffset().getFullyConverted()) + ) + or + exists(STLContainerKnownElementAccess access | + access.getContainer().getContainerKind() = TIndexableContainer() and + lower = lowerBound(access.getElementExpr().getFullyConverted()) and + upper = upperBound(access.getElementExpr().getFullyConverted()) + ) + } + +class BoundedIndex extends TBoundedIndex { + float ub; + float lb; + + BoundedIndex() { this = TBoundedIndex(lb, ub) } + + float getLowerBound() { result = lb } + + float getUpperBound() { result = ub } + + predicate mayOverlap(BoundedIndex other) { + // The lower bound is contained by the other index range. + lb <= other.getUpperBound() and lb >= other.getLowerBound() + or + // or the upper bound is contained by the other index range. + ub <= other.getUpperBound() and ub >= other.getLowerBound() + } + + predicate forIndexExpr(Expr expr) { + this = TBoundedIndex(lowerBound(expr.getFullyConverted()), upperBound(expr.getFullyConverted())) + } + + string toString() { result = "Index bounded by [" + lb.toString() + ", " + ub.toString() + "]" } +} + +class STLContainerUnknownElementAccess extends FunctionCall { + STLContainer container; + Expr containerExpr; + + STLContainerUnknownElementAccess() { + this = + container + .getACallTo([ + "front", "back", "top", "count", "find", "contains", "lower_bound", "upper_bound", + "equal_range" + ]) and + containerExpr = this.getQualifier() + } + + STLContainer getContainer() { result = container } + + Expr getContainerExpr() { result = containerExpr } +} + +class STLContainerKnownElementAccess extends FunctionCall { + STLContainer container; + Expr containerExpr; + Expr elementExpr; + + STLContainerKnownElementAccess() { + this = container.getACallTo(["at", "operator[]"]) and + containerExpr = this.getQualifier() and + elementExpr = this.getArgument(0) + } + + STLContainer getContainer() { result = container } + + Expr getContainerExpr() { result = containerExpr } + + Expr getElementExpr() { result = elementExpr } +} + +class STLContainerIndexAccess extends STLContainerKnownElementAccess { + BoundedIndex index; + + STLContainerIndexAccess() { + container.getContainerKind() = TIndexableContainer() and + containerExpr = this.getQualifier() and + index.forIndexExpr(getElementExpr()) + } + + Expr getIndexExpr() { result = getElementExpr() } + + BoundedIndex getIndexIdentity() { result = index } +} + +class STLContainerKeyAccess extends STLContainerKnownElementAccess { } + +class STLContainerUnknownElementModification extends FunctionCall { + Expr keyExpr; + Expr containerExpr; + STLContainer container; + + STLContainerUnknownElementModification() { + this = + container + .getACallTo([ + "clear", "push_back", "pop_back", "pop_front", "insert", "emplace", "emplace_back", + "emplace_front", "emplace_hint", "erase", "resize", "assign", "assign_range", "swap", + "push", "pop", "fill", "extract", "merge", "insert_or_assign", "try_emplace", + "operaator+=", "replace", "replace_with_range", "copy" + ]) + } +} + +class STLContainerElementModification extends FunctionCall { + STLContainer container; + Expr elementExpr; + + STLContainerElementModification() { + this = container.getACallTo(["remove", "remove_if", "sort", "reverse", "unique"]) + } +} + +class STLContainerStorageAccess extends FunctionCall { + STLContainer container; + + Expr getAStorageAccess() { + result = + container + .getACallTo([ + "data", "c_str", "bucket_count", "max_bucket_count", "bucket_size", "bucket", + "load_factor", "max_load_factor", "capacity", "length", "size", "empty", "length", + "find", "rfind", "find_first_of", "find_last_of", "find_first_not_of", + "find_last_not_of", "substr", "compare" + ]) + } +} + +class STLContainerIndexAssignment extends FunctionCall { } + +class STLContainerKeyAssignment extends FunctionCall { } + +class STLContainerStorageModification extends FunctionCall { } + +/** + * Models a collection of STL container classes. + */ +class STLContainer extends Class { + TContainerKind containerKind; + + STLContainer() { + getNamespace() instanceof StdNS and + // TODO: add to change log that we added std::array + getSimpleName() in ["vector", "deque", "array", "valarray",] and + containerKind = TIndexableContainer() + or + getSimpleName() in [ + "set", "stack", "queue", "list", "priority_queue", "forward_list", "unordered_set" + ] and + containerKind = TCollectionContainer() + or + getNamespace() instanceof StdNS and + getSimpleName() in [ + "multiset", "map", "multimap", "unordered_multiset", "unordered_map", "unordered_multimap" + ] + or + // TODO: This intentionally doesn't check namespace::std, what are the implications? + // TODO: add support for `std::string_view`? + getSimpleName() in ["string", "basic_string"] and + containerKind = TStringContainer() + } + + /** + * Get the kind of this container, such as array-like or associative or string. + */ + TContainerKind getContainerKind() { result = containerKind } + + /** + * Get the type argument corresponding to the given kind, such as element type or key type. + * + * Note that this treats `T` in `std::set` (and similar containers) as the element type, though + * it may be referred to as the key type by some documentation. + */ + Type getTypeArgument(TContainerTypeKind kind) { + (containerKind = TIndexableContainer() or containerKind = TCollectionContainer()) and + kind = TContainerElementType() and + result = this.getTemplateArgument(0) + or + containerKind = TAssociativeContainer() and + kind = TContainerKeyType() and + result = this.getTemplateArgument(0) + or + containerKind = TAssociativeContainer() and + kind = TContainerElementType() and + result = this.getTemplateArgument(1) + or + containerKind = TStringContainer() and + kind = TContainerCharType() and + result = this.getTemplateArgument(0) + } + + /** + * Get the element type of this container, for instance `int` in `std::vector`. + * + * Note that for associative containers like `std::map`, this returns `V`. For the key type, + * use `getKeyType()`. + * + * For set-like containers such as `std::set`, this returns `T`, though it may be referred to as the key + * type by some documentation. + */ + Type getElementType() { result = getTypeArgument(TContainerElementType()) } + + /** + * Get the key type of this container, for instance `K` in `std::map`. + * + * This does not return a result for set-like containers such as `std::set`, though `T` may be + * referred to as the key type by some documentation. + */ + Type getKeyType() { result = getTypeArgument(TContainerKeyType()) } + + Type getStringCharType() { + // TODO: unwrap `std::string` to `std::basic_string` + result = getTypeArgument(TContainerCharType()) + } + + /** + * Get a call to a named function of this class. + */ + FunctionCall getACallTo(string name) { + exists(Function f | + f = getAMemberFunction() and + f.hasName(name) and + result = f.getACallToThisFunction() + ) + } + + /** + * Gets all calls to all functions of this class. + */ + FunctionCall getACallToAFunction() { + exists(Function f | + f = getAMemberFunction() and + result = f.getACallToThisFunction() + ) + } + + FunctionCall getACallToSize() { result = getACallTo("size") } + + FunctionCall getANonConstIteratorBeginFunctionCall() { result = getACallTo(["begin", "rbegin"]) } + + IteratorSource getAConstIteratorBeginFunctionCall() { result = getACallTo(["cbegin", "crbegin"]) } + + IteratorSource getANonConstIteratorEndFunctionCall() { result = getACallTo(["end", "rend"]) } + + IteratorSource getAConstIteratorEndFunctionCall() { result = getACallTo(["cend", "crend"]) } + + IteratorSource getANonConstIteratorFunctionCall() { + result = getACallToAFunction() and + result.getTarget().getType() instanceof NonConstIteratorType + } + + IteratorSource getAConstIteratorFunctionCall() { + result = getACallToAFunction() and + result.getTarget().getType() instanceof ConstIteratorType + } + + IteratorSource getAnIteratorFunctionCall() { + result = getANonConstIteratorFunctionCall() or result = getAConstIteratorFunctionCall() + } + + IteratorSource getAnIteratorBeginFunctionCall() { + result = getANonConstIteratorBeginFunctionCall() or + result = getAConstIteratorBeginFunctionCall() + } + + IteratorSource getAnIteratorEndFunctionCall() { + result = getANonConstIteratorEndFunctionCall() or result = getAConstIteratorEndFunctionCall() + } +} + +/** + * Models a variable that is a `STLContainer` + */ +class STLContainerVariable extends Variable { + STLContainerVariable() { getType() instanceof STLContainer } +} + +/** + * An access to a variable that refers to an STL container, or objects owned by such a container. + * + * Includes `ContainerPointerOrReferenceAccess`, and `ContainerIteratorAccess` from `Iterators.qll`, + * which tracks access to container elements via iterators. + */ +abstract class ContainerAccess extends VariableAccess { + abstract Variable getOwningContainer(); +} + +pragma[noinline, nomagic] +private predicate localTaint(DataFlow::Node n1, DataFlow::Node n2) { + TaintTracking::localTaint(n1, n2) +} + +/** + * An access that gets a pointer or reference to an element of an STL container. + * + * Defined as anything with dataflow FROM the container. + */ +class ContainerPointerOrReferenceAccess extends ContainerAccess { + Variable owningContainer; + + ContainerPointerOrReferenceAccess() { + exists(STLContainer c, FunctionCall fc | + fc = c.getACallToAFunction() and + // There are a few cases where the value is tainted + // but no actual link to the underlying container is established. + // For example, calling Vector.size() returns an int but the + // resulting variable doesn't depend on the underlying container + // anymore. + ( + fc.getTarget().getType() instanceof ReferenceType or + fc.getTarget().getType() instanceof PointerType or + fc.getTarget().getType() instanceof IteratorType + ) and + localTaint(DataFlow::exprNode(fc), DataFlow::exprNode(this)) and + (getUnderlyingType() instanceof ReferenceType or getUnderlyingType() instanceof PointerType) and + fc.getQualifier().(VariableAccess).getTarget() = owningContainer and + // Exclude cases where we see taint into the owning container + not this = owningContainer.getAnAccess() + ) + } + + override Variable getOwningContainer() { result = owningContainer } +} diff --git a/cpp/common/test/includes/standard-library/algorithm b/cpp/common/test/includes/standard-library/algorithm index 43534aa5cc..12226668da 100644 --- a/cpp/common/test/includes/standard-library/algorithm +++ b/cpp/common/test/includes/standard-library/algorithm @@ -3,8 +3,6 @@ #include namespace std { -template struct pair {}; - template constexpr const T &min(const T &a, const T &b); template diff --git a/cpp/common/test/includes/standard-library/map b/cpp/common/test/includes/standard-library/map index 74d952f554..e10b7a8c6b 100644 --- a/cpp/common/test/includes/standard-library/map +++ b/cpp/common/test/includes/standard-library/map @@ -1,4 +1,6 @@ #include "iterator.h" +#include +#include #include namespace std { @@ -7,8 +9,9 @@ template , typename _Alloc = std::allocator<_Key>> class multimap { public: - typedef std::iterator iterator; - typedef std::iterator const_iterator; + typedef std::pair value_type; + typedef std::__iterator iterator; + typedef std::__iterator const_iterator; iterator begin(); iterator end(); @@ -22,8 +25,9 @@ template , typename _Alloc = std::allocator<_Key>> class map { public: - typedef std::iterator iterator; - typedef std::iterator const_iterator; + typedef std::pair value_type; + typedef std::__iterator iterator; + typedef std::__iterator const_iterator; iterator begin(); iterator end(); diff --git a/cpp/common/test/includes/standard-library/memory.h b/cpp/common/test/includes/standard-library/memory.h index 494f428422..eec0566d4c 100644 --- a/cpp/common/test/includes/standard-library/memory.h +++ b/cpp/common/test/includes/standard-library/memory.h @@ -5,6 +5,12 @@ namespace std { +template class allocator { +public: + allocator() throw(); + typedef size_t size_type; +}; + template T *addressof(T &arg) noexcept; template struct default_delete { diff --git a/cpp/common/test/includes/standard-library/string b/cpp/common/test/includes/standard-library/string index 3f60c1838c..8874e913a4 100644 --- a/cpp/common/test/includes/standard-library/string +++ b/cpp/common/test/includes/standard-library/string @@ -88,12 +88,6 @@ template <> struct char_traits { static int_type eof(); }; -template class allocator { -public: - allocator() throw(); - typedef size_t size_type; -}; - template , class Allocator = allocator> class basic_string { diff --git a/cpp/common/test/includes/standard-library/utility.h b/cpp/common/test/includes/standard-library/utility.h index b9bf47b8ce..400ce2bf38 100644 --- a/cpp/common/test/includes/standard-library/utility.h +++ b/cpp/common/test/includes/standard-library/utility.h @@ -1,4 +1,6 @@ #include "type_traits.h" +#ifndef _GHLIBCPP_UTILITY +#define _GHLIBCPP_UTILITY namespace std { template constexpr T &&forward(remove_reference_t &t) noexcept; @@ -8,4 +10,13 @@ template constexpr remove_reference_t &&move(T &&t) noexcept; template typename add_rvalue_reference::type declval() noexcept; template void swap(T &a, T &b) noexcept; -} // namespace std \ No newline at end of file + +template struct pair { + T1 first; + T2 second; + pair(const T1& a, const T2& b); +}; + +} // namespace std + +#endif _GHLIBCPP_UTILITY \ No newline at end of file diff --git a/cpp/common/test/includes/standard-library/vector.h b/cpp/common/test/includes/standard-library/vector.h index 6d0293f8f5..deb645ea73 100644 --- a/cpp/common/test/includes/standard-library/vector.h +++ b/cpp/common/test/includes/standard-library/vector.h @@ -60,6 +60,8 @@ template > class vector { const_reference front() const; reference back(); const_reference back() const; + + ~vector(); }; } // namespace std diff --git a/cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql b/cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql new file mode 100644 index 0000000000..83ebf84282 --- /dev/null +++ b/cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql @@ -0,0 +1,643 @@ +/** + * @id cpp/misra/unnecessary-write-to-local-object + * @name RULE-0-1-1: A value should not be unnecessarily written to a local object + * @description Unused writes to objects may harm performance, complicate code comprehension, or + * indicate logical errors in the program. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-0-1-1 + * scope/system + * correctness + * maintainability + * performance + * external/misra/enforcement/undecidable + * external/misra/obligation/advisory + */ + +import cpp +import semmle.code.cpp.dataflow.DataFlow +import semmle.code.cpp.dataflow.internal.TaintTrackingUtil as Taint +import codingstandards.cpp.misra +import codingstandards.cpp.lifetimes.CppObjects +import codingstandards.cpp.lifetimes.CppSubObjects +import codingstandards.cpp.dominance.BehavioralSet +import codingstandards.cpp.dominance.SuccessorUnless +import codingstandards.cpp.types.TrivialType +import codingstandards.cpp.standardlibrary.STLContainers +import semmle.code.cpp.ir.dataflow.internal.SsaInternalsCommon as Ssa +import semmle.code.cpp.dataflow.new.DataFlow as NewDataFlow + +/** + * A type that is relevant for analysis by this rule. + * + * Objects tracked by this rule must have a trivially destructible type, or be a container of + * trivially destructible types (either a standard library container or an array). + * + * Non-class types such as pointer types, built-in types, and enumerations are trivially + * destructible. + * + * Note that `T*` is a trivially destructible type, regardless of whether `T` is trivially + * destructible. + */ +class RelevantType extends Type { + RelevantType() { + // Include trivially destructible types + hasTrivialDestructor(this) + or + // Include scalar types, but arrays must be checked recursively. + not this instanceof ArrayType and + not this instanceof Class + or + // Arrays of trivial types are trivial + this.(ArrayType).getBaseType() instanceof RelevantType + or + // TODO exclude arrays of containers + // Containers of trivial types are trivial + // Exclude containers of containers because we can't model those well yet. + exists(RelevantType elemType | + elemType = this.(STLContainer).getElementType() and + not elemType instanceof STLContainer + ) + } +} + +/** + * Certain types of behavior are not correctly tracked and therefore we exclude functions + * containing them from analysis. + * + * Try-catch blocks can complicate control flow in ways that are not currently modeled. + * + * Lambda captures by reference are not yet tracked, so we exclude functions that define lambdas + * entirely. Additionally, the bodies of those lambdas had some odd results during testing, so those + * functions are also excluded. + */ +class ExcludedFunction extends Function { + ExcludedFunction() { + exists(TryStmt try | try.getEnclosingFunction() = this) + or + exists(LambdaExpression l | l.getEnclosingFunction() = this) + or + exists(Closure c | c.getLambdaFunction() = this) + } +} + +/** + * For performance reasons, we limit the analyzed control flow nodes to the ones that are not in + * excluded functions. + */ +class RelevantControlFlowNode extends ControlFlowNode { + RelevantControlFlowNode() { + not this.(Element).getEnclosingElement+() instanceof ExcludedFunction + } +} + +/** + * For performance reasons, we define this class to be the set of objects that we want to track + * writes and reads for to satisfy the rule. + */ +class RelevantObject extends ObjectIdentity { + RelevantObject() { + getStorageDuration().isAutomatic() and + getType() instanceof RelevantType + } +} + +newtype TTrackedObject = + TJustSubObject(SubObject subObj) { subObj.getRootIdentity() instanceof RelevantObject } or + /** + * An indexed element of a container or array, tracked with a `BoundedIndex`, see `TrackedObject` + * for details. + */ + TContainerElementSubObject(SubObject container, BoundedIndex index) { + exists(STLContainerIndexAccess access | + access.getContainer() = container.getType() and + access.getContainerExpr() = container.getAnAccess() and + index = access.getIndexIdentity() + ) + or + container.getType() instanceof ArrayType and + exists(ArrayExpr access | + access.getArrayBase() = container.getAnAccess() and + index.forIndexExpr(access.getArrayOffset()) + ) + } + +/** + * A `TrackedObject` is an expansion of `SubObject` that allows array and container index tracking. + * + * In `CppSubObjects.qll`, an array member is represented as a single `SubObject`, to minimize + * performance overhead. Most rules can treat `x[i]` and `x[j]` as the same subobject `x[]`, and + * rely on `SubObject`'s `isPrecise()` predicate to handle the nuance in that type of case. However, + * this rule explicitly desires us to track writes to different indices of the same array or + * container, so we use `TrackedObject` to augment `SubObject` with index tracking. + * + * To distinguish `arr[x]` and `arr[y]`, we use a `BoundedIndex` to represent the index expression, + * which represents a lower and upper bound for the index value. Therefore, `arr[0]` and `arr[1]` + * will be separate `TrackedObject`s. Additionally, `arr[i]` and `arr[j]` will be the same + * `TrackedObject` when the bounds of `i` and `j` are the same. Lastly, two different expressions + * `arr[i]` and `arr[i]` with different source locations may be different `TrackedObject`s if their + * `BoundedIndex`es differ. + * + * Note that a `TrackedObject` will only track a single index for a given container or array. For a + * multidimensional array access such as `arr[i][j]`, the leaf `TrackedObject` will precisely track + * `j`, but its parent `TrackedObject` will only track `arr[]`, not `i`, via the `SubObject` API. + * + * Therefore, tracking equality of two `TrackedObject`s has some subtleties, see `mayOverlap` and + * `mustOverlap` for implementations. + */ +class TrackedObject extends TTrackedObject { + ObjectIdentity getRootIdentity() { + exists(SubObject subObj | + this = TJustSubObject(subObj) and result = subObj.getRootIdentity() + or + this = TContainerElementSubObject(subObj, _) and + result = subObj.getRootIdentity() + ) + } + + Expr getAnAccess() { + exists(SubObject subObj | + this = TJustSubObject(subObj) and result = subObj.getAnAccess() + or + exists(BoundedIndex index | + this = TContainerElementSubObject(subObj, index) and + ( + exists(STLContainerIndexAccess access | + access.getContainer() = subObj.getType() and + access.getContainerExpr() = subObj.getAnAccess() and + access.getIndexIdentity() = index and + result = access + ) + or + exists(ArrayExpr arrayExpr | + arrayExpr.getArrayBase() = subObj.getAnAccess() and + // Note the pragma on `getArrayOffset` for performance. The join orderer may begin its + // its search from `index` or `subObj` in `TContainerElementSubObject(subObj, index)`, + // in order to find `arrayExpr`. We want it to begin from `subObj` instead of `index`, + // and `pragma[only_bind_out]` does this by disallowing `forIndexExpr` to "write into," + // or bind into, `getArrayOffset()`. + index.forIndexExpr(pragma[only_bind_out](arrayExpr.getArrayOffset())) and + result = arrayExpr + ) + ) + ) + ) + } + + /** + * Holds if this `TrackedObject` represents the root `ObjectIdentity` itself, rather than a subobject, + * which also have corresponding `TrackedObject`s. + */ + predicate representsRootObject() { + exists(SubObject subObj | this = TJustSubObject(subObj) and subObj.representsRootObject()) + } + + /** Gets the parent `TrackedObject`, if any. */ + TrackedObject getParent() { + exists(SubObject subObj | + this = TJustSubObject(subObj) and + result = TJustSubObject(subObj.getParent()) + or + this = TContainerElementSubObject(subObj, _) and + result = TJustSubObject(subObj) + ) + } + + /** + * Two different `TrackedObject`s may be the same object even if they are tracked separately. + * + * For example, `arr[x]` and `arr[y]` are different `TrackedObject`s, but may overlap if `x` and `y` + * may be the same index. + */ + predicate mayOverlap(TrackedObject other) { + this = other + or + exists(BoundedIndex index1, BoundedIndex index2, SubObject subObj | + this = TContainerElementSubObject(subObj, index1) and + other = TContainerElementSubObject(subObj, index2) and + index1.mayOverlap(index2) + ) + } + + /** + * Holds if this and the other `TrackedObject`s are known to be the same object. + * + * This predicate is more strict than `mayOverlap`, and while it implies equality, it is not + * equivalent to it. + * + * For example, `arr[x]` and `arr[y]` must be the same object if `x` and `y` are equal constant + * values. Otherwise, even if the upper and lower bounds of `x` and `y` overlap, we cannot be sure + * they are exactly the same. Even `arr[x]` and `arr[x]` may not be considered the exact same + * object if `x` is not known to be a precise value, as the value of `x` may change between uses. + * In the future we could use GlobalValueNumbering to be more precise for that case. + */ + predicate mustOverlap(TrackedObject other) { + this.isPrecise() and this = other + or + exists(BoundedIndex index1, BoundedIndex index2, SubObject subObj | + this = TContainerElementSubObject(subObj, index1) and + other = TContainerElementSubObject(subObj, index2) and + // TODO: make this a member predicate of BoundedIndex? + index1.getLowerBound() = index1.getUpperBound() and + index1 = index2 and + subObj.isPrecise() + ) + } + + predicate isPrecise() { + exists(SubObject subObj | + this = TJustSubObject(subObj) and subObj.isPrecise() + // Container elements are not precise + ) + } + + string toString() { + this = TJustSubObject(_) and result = "tracked subobject" + or + exists(BoundedIndex index | + this = TContainerElementSubObject(_, index) and + result = "tracked container element " + index.toString() + ) + } +} + +/** + * A recap of the rule: + * - Writes to objects must be observed, either directly or indirectly through pointers/references. + * - Observing any part of an object or container is considered to observe the whole object. + * - Writes may not immediately follow other writes without a read in between. + * - The rule text states that the above cases must occur on every feasible path to be considered + * a violation. + * - Objects that have non trivial destructors or containers of objects with non-trivial destructors + * are exempt. + * + * Note that the rule has provides an example that may be considered contradictory: + * + * ```c + * int m; + * for ( ... ) { + * m = 10; + * m++; + * } + * return m; + * ``` + * + * Here, each write to `m` has a feasible path to an observation (the return), but the rule states + * that the assignment (`m = 10;`) is non compliant as it overwrites the value from the previous + * iteration. + * + * It's unclear what part of the CFG indicates that the assignment is non compliant. The write to + * `m` via increment is not postdominated by an overwrite, and the overwrite does not dominate the + * increment. The clearest explanation is that this is assuming the loop is unrolled, but we do not + * attempt to model that here. + */ +predicate isRead(TrackedObject object, CrementAwareNode cfn) { + cfn.asExprRead() = object.getAnAccess() and + not exists(AssignExpr ae | ae.getLValue() = cfn.asExprRead() and ae.getOperator() = "=") + or + isObserved(object.getRootIdentity(), cfn) +} + +newtype TCrementAwareNode = + /** A control flow node that does not involve postfix ++ or -- */ + TNonCrementNode(RelevantControlFlowNode n) { not n instanceof PostfixCrementOperation } or + /** + * A control flow node representing the value of a crement node as it is at usage site, before + * being modified. + */ + TPreCrementNode(CrementOperation n) { n instanceof PostfixCrementOperation } or + /** + * A control flow node representing the value of a crement node after its value was modified. + */ + TPostCrementNode(CrementOperation n) { n instanceof PostfixCrementOperation } + +/** + * A control flow node that distinguishes between the read and write phases of a postfix + * crement operation like `i++` or `i--`. + */ +class CrementAwareNode extends TCrementAwareNode { + /** The underlying control flow node. */ + RelevantControlFlowNode n; + + CrementAwareNode() { + this = TNonCrementNode(n) + or + this = TPreCrementNode(n) + or + this = TPostCrementNode(n) + } + + /** + * Whether this node represents the beginning of a control flow node. + * + * For non-crement nodes, this is always true. For crement nodes, this holds for the "pre-crement" + * node only. + */ + predicate beginsControlFlowNode() { + this = TNonCrementNode(_) or + this = TPreCrementNode(_) + } + + /** Gets the underlying control flow node. */ + RelevantControlFlowNode getControlFlowNode() { result = n } + + /** + * Gets the successor node in the control flow graph, accounting for crement nodes. + */ + CrementAwareNode getASuccessor() { + if this = TPreCrementNode(n) + then result = TPostCrementNode(n) + else ( + result.beginsControlFlowNode() and + result.getControlFlowNode() = n.getASuccessor() + ) + } + + /** + * Can be used to get the crement-aware control flow node for a given expression, treated as a + * read operation. + * + * For non-crement expressions, this is just the expression itself. For crement expressions, this + * is the "pre-crement" node, and does not hold for the "post-crement" node. + */ + Expr asExprRead() { + result = this.getControlFlowNode() and + this.beginsControlFlowNode() + } + + /** + * Can be used to get the crement-aware control flow node for a given expression, treated as a + * write operation. + * + * For non-crement expressions, this is just the expression itself. For crement expressions, this + * is the "post-crement" node, and does not hold for the "pre-crement" node. + */ + Expr asExprWrite() { + // Pre-crement nodes are not writes, and have no result. + not this = TPreCrementNode(n) and + result = this.getControlFlowNode() + } + + string toString() { + this = TNonCrementNode(_) and result = "non crement node" + or + this = TPreCrementNode(_) and result = "pre crement node" + or + this = TPostCrementNode(_) and result = "pre crement node" + } + + Location getLocation() { result = getControlFlowNode().getLocation() } +} + +predicate isWrite(RelevantObject obj, TrackedObject subObj, CrementAwareNode cfn) { + subObj.getRootIdentity() = obj and + ( + cfn.asExprWrite() = obj.(Variable).getInitializer().getExpr() and + // Parameter initializers are default argument values, not writes. + not obj instanceof Parameter and + subObj.representsRootObject() + or + exists(AssignExpr ae | + ae.getLValue() = cfn.asExprWrite() and + ae.getOperator() = "=" and + ae.getLValue() = subObj.getAnAccess() + ) + or + exists(CrementOperation crement | + cfn.asExprWrite() = crement and + crement.getOperand() = subObj.getAnAccess() + ) + ) +} + +/** + * An expression that functions as a condition in a program's control flow. + * + * These expressions are control flow nodes that have a "true" and/or "false" successor. + */ +class ConditionalExpr extends Expr { + RelevantControlFlowNode cfn; + + ConditionalExpr() { + this = cfn and + ( + exists(cfn.getAFalseSuccessor()) + or + exists(cfn.getATrueSuccessor()) + ) + } +} + +/** + * Finds the objects observed via an rvalue expression in an assignment. + * + * This is differentiated from `exprIsObserved` so that we can exclude reciprocal assignments, such + * as `x = x + 1`. This is unique to assignment, note that `return x + 1;` cannot be reciprocal and + * always observes `x`. + * + * This predicate does not find all reciprocal assignments, such as `*(p + 1) = *(p + 1) + 1`. It + * only finds reciprocal assignments where the lvalue is a direct access to the object, such as + * `x = x + 1`, `arr[i] = arr[i] + 1`, or `x.field = x.field + 1`, though the lvalue and rvalue may + * refer to different subobjects of the same root object, such as `x.a = x.b`. + */ +predicate assignmentObservesObject(Assignment assign, RelevantObject object) { + exists( + // Objects accessed in an rvalue are observed except for reciprocal assignments. + Expr rvaluePart + | + assign.getRValue() = rvaluePart.getParent*() and + rvaluePart = object.getASubobjectAccess() and + // An assignment like x = x + 1 does not observe x. + not exists(TrackedObject lvalueObj | + assign.getLValue() = lvalueObj.getAnAccess() and lvalueObj.getRootIdentity() = object + ) + ) +} + +/** + * Finds the certain types of lvalue expressions that indirectly observe their subexpressions. + * + * For example, in the expression `*p = 42`, the pointer dereference `*p` observes `p`, and the + * array index expressions `arr[i] = 42` or `p[i] = 42` observe both `i` and `p`, when `p` is a + * pointer type. + * + * In cases such as `arr[i + 1] = 42`, this predicate holds for the expression `i + 1` but not the + * inner expression `i`. This is because this predicate does not track which subexpression values + * ultimately affect the lvalue, or which subexpressions correspond to which objects. + * + * This predicate could be more pessimistic in cases such as `p[*p] = p`, which could be said to not + * observe `p`. However it does not distinguish these cases for simplicity. + */ +predicate assignmentObservesLvalueExpr(Expr assign, Expr observed) { + exists(Expr lvaluePart, Expr assignBase | + // Using `getParent*()` is not strictly correct in certain edge cases such as lambda expressions + // and comma operators, but is sufficient for our purposes here. + lvaluePart.getParent*() = assignBase and + ( + assignBase = assign.(Assignment).getLValue() + or + // Handle cases like `*p++ = 42;` + assignBase = assign.(CrementOperation).getOperand() + ) and + ( + lvaluePart.(PointerDereferenceExpr).getOperand() = observed + or + lvaluePart.(PointerFieldAccess).getQualifier() = observed + or + lvaluePart.(ArrayExpr).getArrayOffset() = observed + or + lvaluePart.(ArrayExpr).getArrayBase() = observed and + // An array's address is not part of its value per se, so `x[y]` does not observe `x` when + // `x` is an array type. However, if `x` is a pointer, then `x[y]` uses its value and thus + // observes it. + observed.getType().getUnderlyingType() instanceof PointerType + ) + ) +} + +/** + * By rule definition, an object is observed when its value affects the control flow of the program + * or its value affects the value external program state. + * + * This predicate finds syntactic constructs that induce an observation of an expression, without + * analyzing the observed subexpressions or which subexpressions may be from which objects. + */ +predicate exprIsObserved(Expr e) { + e instanceof ConditionalExpr + or + exists(SwitchStmt switch | switch.getControllingExpr() = e) + or + exists(Initializer i | i.getExpr() = e) + or + assignmentObservesLvalueExpr(_, e) + or + exists(Call c, Expr arg | + c.getAnArgument() = arg and + arg = e + ) + or + exists(Call c | + c.getQualifier() = e and + // Exclude container element lookup calls, as those do not observe the container. + not exists(STLContainer container | + container = c.getQualifier().getType() and + c instanceof STLContainerIndexAccess + ) and + // Exclude destructor calls, as those do not observe the object, which is either a trivially + // destructible type or a container. + not c instanceof DestructorCall + ) + or + exists(ExprCall ec | ec.getExpr() = e) + or + exists(ReturnStmt rs | rs.getExpr() = e) + or + exists(ThrowExpr ts | ts.getExpr() = e) + or + exists(DeleteExpr de | de.getExpr() = e) +} + +predicate childExprTaintStep(Expr child, Expr parent) { + parent.(Operation).getAnOperand() = child + or + parent.(ArrayExpr).getArrayBase() = child + or + parent.(ArrayExpr).getArrayOffset() = child + or + parent.(AggregateLiteral).getAChild() = child + or + // We need this to model containers, where vec.at(i) is tainted by vec. + parent.(Call).getQualifier() = child + or + // a->b is tainted by a. + // We don't model a.b tainting b, because the 'SubObject' captures this relationship. + parent.(PointerFieldAccess).getQualifier() = child +} + +predicate isObserved(RelevantObject obj, CrementAwareNode cfn) { + exists(Expr directlyObservedExpr, Expr accessExpr | + cfn.asExprRead() = directlyObservedExpr and + accessExpr = obj.getASubobjectAccess() and + childExprTaintStep*(accessExpr, directlyObservedExpr) and + exprIsObserved(directlyObservedExpr) + ) + or + assignmentObservesObject(cfn.asExprWrite(), obj) +} + +/** + * Writes that occur after taking the address of or a reference to a subobject are excluded from + * consideration, as the address/reference may be used later to observe the object. + * + * This could be tracked more precisely in the future, however, for the moment this case is simply + * excluded. + */ +predicate isExcludedWrite(RelevantControlFlowNode node, RelevantObject obj) { + node.getAPredecessor+() = [obj.getASubobjectAddressExpr(), obj.getASubobjectTakenAsReference()] +} + +module WriteWithReadConfig implements SuccessorUnlessConfigSig { + class State = TrackedObject; + + class Node = CrementAwareNode; + + predicate isStart(TrackedObject state, CrementAwareNode node) { + exists(RelevantObject rootObj | + isWrite(rootObj, state, node) and + not isExcludedWrite(node.getControlFlowNode(), rootObj) + ) + } + + predicate isUnless(TrackedObject state, CrementAwareNode node) { + exists(TrackedObject rewritten | + rewritten = state.getParent*() and + isWrite(_, rewritten, node) and + // Don't stop at writes to imprecise subobjects, as they may overlap. + state.isPrecise() + or + isWrite(_, rewritten, node) and + rewritten.mustOverlap(state) + ) + } + + predicate isEnd(TrackedObject obj, CrementAwareNode node) { + exists(TrackedObject readObj | + isRead(readObj, node) and + readObj.mayOverlap(obj) + ) + } +} + +predicate isWriteWithRead = SuccessorUnless::isSuccessor/3; + +predicate isWriteWithObserve(RelevantObject obj, CrementAwareNode node) { + isWrite(obj, _, node) and + isObserved(obj, node.getASuccessor*()) and + not isExcludedWrite(node.getControlFlowNode(), obj) +} + +from + RelevantControlFlowNode cfn, CrementAwareNode crementNode, RelevantObject obj, string explanation +where + not isExcluded(cfn, DeadCode5Package::unnecessaryWriteToLocalObjectQuery()) and + // Exclude macro expansions, with variables that may be unused only in certain expansions. + not cfn.isInMacroExpansion() and + // Exclude compiler generated variables and nodes. + not cfn.(Expr).isCompilerGenerated() and + not obj.(Variable).isCompilerGenerated() and + // Exclude variables with no enclosing element, indicating odd cases such as structured bindings + // where CodeQL modeling is incomplete. + exists(obj.getEnclosingElement()) and + crementNode.getControlFlowNode() = cfn and + exists(TrackedObject subObj | + isWrite(obj, subObj, crementNode) and not isExcludedWrite(cfn, obj) + | + if not isWriteWithObserve(obj, crementNode) + then explanation = "is never observed" + else + if not isWriteWithRead(subObj, crementNode, _) + then explanation = "is not read before subsequent writes" + else none() + ) +select cfn, "Unnecessary write to object $@ " + explanation + ".", obj, obj.toString() diff --git a/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected b/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected new file mode 100644 index 0000000000..0c0ee0907f --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected @@ -0,0 +1,47 @@ +| test.cpp:10:11:10:12 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:10:7:10:8 | l0 | l0 | +| test.cpp:60:12:60:13 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:60:7:60:9 | l13 | l13 | +| test.cpp:61:3:61:5 | l13 | Unnecessary write to object $@ is never observed. | test.cpp:60:7:60:9 | l13 | l13 | +| test.cpp:116:6:116:9 | ... -- | Unnecessary write to object $@ is never observed. | test.cpp:115:7:115:8 | l2 | l2 | +| test.cpp:118:6:118:9 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:117:7:117:8 | l3 | l3 | +| test.cpp:126:11:126:12 | 0 | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:126:7:126:8 | l0 | l0 | +| test.cpp:141:20:141:20 | second | Unnecessary write to object $@ is never observed. | test.cpp:141:13:141:13 | l1 | l1 | +| test.cpp:149:18:149:18 | call to operator* | Unnecessary write to object $@ is never observed. | test.cpp:149:13:149:14 | l3 | l3 | +| test.cpp:167:13:167:22 | new | Unnecessary write to object $@ is never observed. | test.cpp:167:8:167:9 | l3 | l3 | +| test.cpp:172:11:172:12 | 1 | Unnecessary write to object $@ is never observed. | test.cpp:172:7:172:8 | l1 | l1 | +| test.cpp:178:11:178:12 | 1 | Unnecessary write to object $@ is never observed. | test.cpp:178:7:178:8 | l1 | l1 | +| test.cpp:199:15:199:24 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:199:11:199:12 | t0 | t0 | +| test.cpp:218:15:218:24 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:218:11:218:12 | t4 | t4 | +| test.cpp:219:17:219:29 | new | Unnecessary write to object $@ is never observed. | test.cpp:219:12:219:13 | t5 | t5 | +| test.cpp:222:6:222:6 | a | Unnecessary write to object $@ is never observed. | test.cpp:221:11:221:12 | t6 | t6 | +| test.cpp:235:16:235:25 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:235:11:235:13 | t10 | t10 | +| test.cpp:238:18:238:30 | new | Unnecessary write to object $@ is never observed. | test.cpp:238:12:238:14 | t11 | t11 | +| test.cpp:241:18:241:40 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:241:11:241:13 | t12 | t12 | +| test.cpp:255:13:255:14 | l0 | Unnecessary write to object $@ is never observed. | test.cpp:255:8:255:9 | l1 | l1 | +| test.cpp:265:4:265:7 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:264:8:264:9 | l4 | l4 | +| test.cpp:280:6:280:6 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:275:11:275:12 | t0 | t0 | +| test.cpp:284:6:284:6 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:275:11:275:12 | t0 | t0 | +| test.cpp:300:8:300:8 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:299:5:299:6 | t2 | t2 | +| test.cpp:311:24:311:33 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:311:20:311:21 | v1 | v1 | +| test.cpp:317:20:317:21 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:317:20:317:21 | v3 | v3 | +| test.cpp:318:5:318:5 | call to operator[] | Unnecessary write to object $@ is never observed. | test.cpp:317:20:317:21 | v3 | v3 | +| test.cpp:326:5:326:5 | call to operator[] | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:325:20:325:21 | v5 | v5 | +| test.cpp:345:29:345:51 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:345:24:345:26 | v10 | v10 | +| test.cpp:355:3:355:4 | p1 | Unnecessary write to object $@ is never observed. | test.cpp:351:22:351:23 | p1 | p1 | +| test.cpp:362:6:362:6 | a | Unnecessary write to object $@ is never observed. | test.cpp:358:30:358:31 | t1 | t1 | +| test.cpp:372:5:372:5 | call to operator[] | Unnecessary write to object $@ is never observed. | test.cpp:367:56:367:57 | v1 | v1 | +| test.cpp:387:3:387:4 | p1 | Unnecessary write to object $@ is never observed. | test.cpp:377:24:377:25 | p1 | p1 | +| test.cpp:388:3:388:4 | p2 | Unnecessary write to object $@ is never observed. | test.cpp:377:37:377:38 | p2 | p2 | +| test.cpp:389:3:389:4 | p3 | Unnecessary write to object $@ is never observed. | test.cpp:377:53:377:54 | p3 | p3 | +| test.cpp:390:3:390:4 | p4 | Unnecessary write to object $@ is never observed. | test.cpp:378:32:378:33 | p4 | p4 | +| test.cpp:404:13:404:22 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:404:7:404:8 | l1 | l1 | +| test.cpp:409:17:409:39 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:409:11:409:12 | l3 | l3 | +| test.cpp:420:3:420:7 | access to array | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:419:7:419:8 | l6 | l6 | +| test.cpp:425:3:425:9 | access to array | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:424:7:424:10 | l6_1 | l6_1 | +| test.cpp:441:3:441:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | +| test.cpp:442:3:442:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | +| test.cpp:443:3:443:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | +| test.cpp:451:3:451:8 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:450:7:450:9 | l11 | l11 | +| test.cpp:452:3:452:10 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:450:7:450:9 | l11 | l11 | +| test.cpp:516:7:516:7 | call to operator[] | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:515:22:515:23 | v7 | v7 | +| test.cpp:558:13:558:19 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:558:8:558:9 | l0 | l0 | +| test.cpp:560:13:560:19 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:560:8:560:9 | l1 | l1 | diff --git a/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.qlref b/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.qlref new file mode 100644 index 0000000000..c7f7a36f95 --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.qlref @@ -0,0 +1 @@ +rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-0-1-1/test.cpp b/cpp/misra/test/rules/RULE-0-1-1/test.cpp new file mode 100644 index 0000000000..571a3d0a34 --- /dev/null +++ b/cpp/misra/test/rules/RULE-0-1-1/test.cpp @@ -0,0 +1,749 @@ +#include +#include +#include + +int g0 = 0; // COMPLIANT -- not automatic storage duration + +bool f0(int x); + +void f1() { + int l0 = 0; // NON_COMPLIANT + static int l1 = 0; // COMPLIANT -- not automatic storage duration +} + +void f2() { + int l0 = 0; // COMPLIANT -- used as function parameter + f0(l0); +} + +void f3() { + int l0 = 0; // COMPLIANT + if (l0) { + } + + int l1 = 0; // COMPLIANT + while (l1) { + } + + int l2 = 0; // COMPLIANT + for (; l2;) { + } + + int l3 = 0; // COMPLIANT + switch (l3) {} + + int l4 = 0; // COMPLIANT + do { + } while (l4); + + int l5 = 0; // COMPLIANT + l5 ? f0(0) : f0(1); + + bool l6; // COMPLIANT + l6 &&f0(0); + + bool l7; // COMPLIANT + l7 || f0(0); + + auto l8 = f0; // COMPLIANT + l8(0); + + int l9 = 0; // COMPLIANT + int l10 = l9; // COMPLIANT + f0(l10); + + int l11 = 0; // COMPLIANT + int l12 = 0; // COMPLIANT + l11 += l12; // COMPLIANT + f0(l11); + + int l13 = 0; + l13 = l13 + 1; // NON_COMPLIANT -- read but not observed + + int l14 = 0; + l14 = l14 + 1; // COMPLIANT -- read and observed + f0(l14); +} + +void f4() { + int l0 = 0; // COMPLIANT + f0(l0 + 1); + int l1 = 0; // COMPLIANT + f0(l1 - 1); + int l2 = 0; // COMPLIANT + f0(l2 * 2); // COMPLIANT + int l3 = 0; // COMPLIANT + f0(l3 / 2); // COMPLIANT + int l4 = 0; // COMPLIANT + f0(l4 % 2); // COMPLIANT + int l5 = 0; // COMPLIANT + f0(l5 & 2); // COMPLIANT + int l6 = 0; // COMPLIANT + f0(l6 | 2); // COMPLIANT + int l7 = 0; // COMPLIANT + f0(l7 ^ 2); // COMPLIANT + int l8 = 0; // COMPLIANT + f0(l8 << 1); // COMPLIANT + int l9 = 0; // COMPLIANT + f0(l9 >> 1); // COMPLIANT + int l10 = 0; // COMPLIANT + f0(-l10); // COMPLIANT + int l11 = 0; // COMPLIANT + f0(~l11); // COMPLIANT + int l12 = 0; // COMPLIANT + f0(!l12); // COMPLIANT + int l13 = 0; // COMPLIANT + f0(+l13); // COMPLIANT + + int l14 = 0; // COMPLIANT + int l15[] = {l14}; // COMPLIANT + int l16 = 10; // COMPLIANT + f0(l15[l16]); + + // Copy the above test, but with tainted values instead of direct uses + int l17 = 0; // COMPLIANT + int l18[] = {l17 + 1}; // COMPLIANT + int l19 = 0; // COMPLIANT + f0((l18 + 1)[l19 + 1]); // COMPLIANT +} + +void f5() { + int l0 = 0; // COMPLIANT + f0(++l0); // COMPLIANT -- modified value immediately observed + int l1 = 0; // COMPLIANT + f0(--l1); // COMPLIANT -- modified value immediately observed + int l2 = 0; // COMPLIANT + f0(l2--); // NON_COMPLIANT -- modified value not observed + int l3 = 0; // COMPLIANT + f0(l3++); // NON_COMPLIANT -- modified value not observed + + int l4 = 0; // COMPLIANT + f0(l4++); // COMPLIANT -- modified value observed later + f0(l4); +} + +void f6() { + int l0 = 0; // NON_COMPLIANT -- overwritten before being read + f0(l0 = 1); // COMPLIANT -- value observed + int l1 = 0; // COMPLIANT + f0(l1 = l1); // COMPLIANT -- value observed + int l2 = 0; // COMPLIANT + f0(l2 = l2 + 1); // COMPLIANT -- value observed + int l3 = 0; // COMPLIANT + f0(l3 += 1); // COMPLIANT -- value observed +} + +void f7(int x = 0) { // COMPLIANT -- default parameter value +} + +void f8() { + std::pair p = {1, 2}; + auto [l0, l1] = p; // NON_COMPLIANT -- 'l1' is unused + f0(l0); + + std::vector v; + for (auto l2 : v) { // COMPLIANT -- 'l2' is used + f0(l2); + } + + for (auto l3 : v) { // NON_COMPLIANT -- 'l3' is unused + } + + std::map m; + for (auto [a, b] : m) { // COMPLIANT + // Technically 'a' is unused, but this is a convenient cost free pattern + f0(b); + } +} + +void f9() { + int l0 = 0; // COMPLIANT -- observed via address-of + int *l1 = &l0; // COMPLIANT -- observed via deref + f0(*l1); + + int *l2 = new int(0); // COMPLIANT -- dynamic storage duration + delete l2; // COMPLIANT + + int *l3 = new int(0); // NON_COMPLIANT -- unused +} + +int f10() { + int l0 = 0; // COMPLIANT -- observed via return + int l1 = 1; // NON_COMPLIANT -- unused + return l0; +} + +int f11() { + int l0 = 0; // COMPLIANT -- observed via throw + int l1 = 1; // NON_COMPLIANT -- unused + throw l0; +} + +class Trivial { +public: + int a; + int b; + void member(); +}; + +class NonTrivial { +public: + ~NonTrivial() {} + int a; + void member(); +}; + +void use(Trivial t); + +void f12() { + Trivial t0 = Trivial(); // NON_COMPLIANT -- unused + NonTrivial t1 = NonTrivial(); // COMPLIANT -- non-trivial type +} + +void f13() { + Trivial t0; + t0.a = 0; // COMPLIANT -- write to a is observed through t0 + use(t0); + + Trivial t1 = Trivial(); // COMPLIANT -- used via member() call + t1.member(); + + Trivial *t2 = new Trivial(); + t2->a = 0; // COMPLIANT -- write to a is observed through t2 + use(*t2); + + Trivial *t3 = new Trivial(); // COMPLIANT -- used via member() call + t3->member(); + + Trivial t4 = Trivial(); // NON_COMPLIANT -- unused + Trivial *t5 = new Trivial(); // NON_COMPLIANT -- unused + + Trivial t6; + t6.a = 0; // NON_COMPLIANT -- write to a is not observed + + Trivial *t7 = new Trivial(); // COMPLIANT + t7->a = 0; // write to a triggers observation of *t7 through deref assignment. + + Trivial t8 = Trivial(); + f0(t8.a); // COMPLIANT -- observation of subobject 'a' counts as observing the + // whole t8 object + + Trivial *t9 = new Trivial(); + f0(t9->a); // COMPLIANT -- observation of subobject 'a' counts as observing + // the whole *t9 object + + Trivial t10 = Trivial(); // NON_COMPLIANT + t10.a; // reading a does not count as observing t10 + + Trivial *t11 = new Trivial(); // NON_COMPLIANT + t11->a; // reading a does not count as observing *t11 + + Trivial t12[] = {Trivial(), Trivial()}; // NON_COMPLIANT -- unused + NonTrivial t13[] = {NonTrivial(), + NonTrivial()}; // COMPLIANT -- non-trivial type +} + +void f14() { + // Test that dereferencing can observe the pointer base, but only when + // assigned to. + // + // Note that the cases involve assigment to dereferenced pointers could be + // fully modeled to count as an assignment to the pointee object, but we don't + // currently do that advanced of an analysis. + int l0[] = {0, 1, 2}; // COMPLIANT + + int *l1 = l0; // NON_COMPLIANT -- dereference occurs but is not observed + *l1; + + int *l2 = l0; // COMPLIANT + *l2 = 0; // dereference observed via assignment to pointee. + + int *l3 = l0; // COMPLIANT + *(l3 + 1) = 0; + + int *l4 = l0; // COMPLIANT + *l4++ = 0; // NON_COMPLIANT -- increment not observed + + int *l5 = l0; // COMPLIANT + *++l5 = 0; + + int *l6 = l0; // COMPLIANT + *l6 += 1; +} + +void f15() { + Trivial t0; + t0.a = 0; // COMPLIANT -- not overwritten by assignment to b + t0.b = 1; // COMPLIANT -- used via containing object t0. + use(t0); + + t0.a = 0; // NON_COMPLIANT -- overwritten before being read + t0.a = 1; // COMPLIANT -- used via containing object t0. + use(t0); + + t0.a = 0; // NON_COMPLIANT -- overwritten before being read + t0 = Trivial(); + use(t0); + + struct { + Trivial t; + int x; + } t1; + t1.t.a = 0; // COMPLIANT -- not overwritten + t1.x = 1; // COMPLIANT -- observing t1.t counts as observing t1.x by rule text + use(t1.t); // use t1.t containing subobject t1.t.a + + struct { + Trivial t; + int x; + } t2; + t2.t.a = 0; // NON_COMPLIANT -- overwritten before being read + t2.t = Trivial(); + use(t2.t); +} + +template void useVec(std::vector &v); + +void f16() { + std::vector v0 = {0, 1, 2}; // COMPLIANT + useVec(v0); + + std::vector v1 = {0, 1, 2}; // NON_COMPLIANT -- unused + + std::vector v2; + v2[0] = 0; // COMPLIANT -- write to element is observed via useVec + useVec(v2); + + std::vector v3; // NON_COMPLIANT -- unobserved + v3[0] = 0; // NON_COMPLIANT -- unused + + std::vector v4; + v4[0] = 0; // COMPLIANT + v4[1] = 0; // COMPLIANT + useVec(v4); + + std::vector v5; + v5[0] = 0; // NON_COMPLIANT + v5[0] = 0; // COMPLIANT + useVec(v5); + + std::vector v6 = {0, 1, 2}; // COMPLIANT + f0(v6[0]); + + std::vector v7; + v7[0] = 1; // COMPLIANT + f0(v7[0]); + + std::vector v8; + v8[0] = + 1; // COMPLIANT -- observing v8[1] counts as observing the whole v8 object + f0(v8[1]); + + std::vector v9 = {NonTrivial(), + NonTrivial()}; // COMPLIANT -- non-trivial type + + std::vector v10 = {Trivial(), Trivial()}; // NON_COMPLIANT -- unused + + std::vector v11 = {Trivial(), Trivial()}; // COMPLIANT + useVec(v11); +} + +void f17(int p0, int p1) { + p0 = 0; // COMPLIANT + f0(p0); + + p1 = 0; // NON_COMPLIANT +} + +void f18(Trivial t0, Trivial t1, NonTrivial t2) { + t0.a = 0; // COMPLIANT + use(t0); + + t1.a = 0; // NON_COMPLIANT + + t2.a = 0; // COMPLIANT -- non-trivial type +} + +void f19(std::vector v0, std::vector v1, + std::vector v2) { + v0[0] = Trivial(); // COMPLIANT + useVec(v0); + + v1[0] = Trivial(); // NON_COMPLIANT + + v2[0] = NonTrivial(); // COMPLIANT -- non-trivial type +} + +void f20(int *p0, int *p1, Trivial *p2, NonTrivial *p3, + std::vector *p4) { + *p0 = 0; // COMPLIANT -- not an assignment to a local object + *p2 = Trivial(); // COMPLIANT + *p3 = NonTrivial(); // COMPLIANT + *p4 = std::vector(); // COMPLIANT + + p0 = nullptr; // COMPLIANT + f0(*p0); + + p1 = nullptr; // NON_COMPLIANT -- the pointer is a local object + p2 = nullptr; // NON_COMPLIANT + p3 = nullptr; // NON_COMPLIANT + p4 = nullptr; // NON_COMPLIANT -- a pointer to a non-trivial type is itself a + // local trivial object +} + +void f20(int &p0, int &p1, Trivial &p2, std::vector &p3) { + p0 = 0; // COMPLIANT + p2 = Trivial(); // COMPLIANT + p3 = std::vector(); // COMPLIANT +} + +void f21() { + int l0[] = {0, 1, 2}; // COMPLIANT + f0(l0[0]); // Observes entire object + + int l1[] = {0, 1, 2}; // NON_COMPLIANT -- unused + + Trivial l2[] = {Trivial(), Trivial()}; // COMPLIANT + use(l2[0]); // Observes entire object + + Trivial l3[] = {Trivial(), Trivial()}; // NON_COMPLIANT -- unused + + NonTrivial l4[] = {NonTrivial(), + NonTrivial()}; // COMPLIANT -- non-trivial type + + int l5[10]; + l5[0] = 0; // COMPLIANT + l5[1] = 0; // COMPLIANT + f0(l5[0]); // Observes entire object + + int l6[10]; + l6[0] = 0; // NON_COMPLIANT + l6[0] = 0; // COMPLIANT + f0(l6[0]); // Observes entire object + + int l6_1[10]; + l6_1[0] = 0; // NON_COMPLIANT + l6_1[1] = l6_1[2]; // COMPLIANT + l6_1[0] = 0; // COMPLIANT + f0(l6_1[0]); // Observes entire object + + int l7[10]; + l7[0] = 0; // COMPLIANT + l7[0] += 1; // COMPLIANT + f0(l7[0]); // Observes entire object + + int l8[10]; + l8[0] = 0; // COMPLIANT + l8[0] = l8[0] + 1; // COMPLIANT + f0(l8[0]); // Observes entire object + + int l9[10]; + l9[0] = 0; // NON_COMPLIANT -- not observed + l9[1] = 0; // NON_COMPLIANT -- not observed + l9[1] = l9[1] + 1; // NON_COMPLIANT -- not observed + + int l10[10]; + l10[0] = 0; // COMPLIANT + l10[0]++; // COMPLIANT + f0(l10[0]); // Observes entire object + + int l11[10]; + l11[0] = 0; // NON_COMPLIANT -- not observed + l11[0]++; // NON_COMPLIANT -- not observed +} + +void f21(size_t p0, size_t p1) { + int l0[10]; + l0[p0] = 0; // COMPLIANT + l0[p1] = 0; // COMPLIANT + f0(l0[0]); // Observes entire object + + int l1[10]; + l1[0] = 0; // COMPLIANT + l1[p0] = 0; // COMPLIANT + f0(l1[0]); // Observes entire object +} + +void f22(size_t p0, size_t p1) { + std::vector v0(10); + v0[p0] = 0; // COMPLIANT + v0[p1] = 0; // COMPLIANT + useVec(v0); + + std::vector v1(10); + v1[p0] = 0; // COMPLIANT + v1[p0] += 1; // COMPLIANT + useVec(v1); + + // This case can be caught without too much difficulty using global value + // numbering, but this has not been done yet, and may be slow. + // + // Many cases below are similar false negatives, just in more complicated + // circumstances. + std::vector v2(10); + v2[p0] = 0; // NON_COMPLIANT[False negative] + v2[p0] = 1; // COMPLIANT + useVec(v2); + + /* Test index aliasing */ + std::vector v3(10); + v3[0] = 0; // COMPLIANT -- may be read in next line when p0 == 1 + v3[1] = v3[p0]; // COMPLIANT + v3[0] = 2; // COMPLIANT + useVec(v3); + + std::vector v4(10); + v4[p0] = 0; // COMPLIANT -- may be read in next line when p0 == p1 + v4[0] = v4[p1]; // COMPLIANT + v4[p0] = 2; // COMPLIANT + useVec(v4); + + std::vector v5(10); + v5[p0] = 0; // COMPLIANT -- may be read in next line when p0 == 0 + v5[1] = v5[0]; // COMPLIANT + v5[p0] = 2; // COMPLIANT + useVec(v5); + + /* Set it so that p0 and p1 cannot alias */ + if (p0 < 10 && p1 > 10) { + std::vector v6(10); + v6[0] = 0; // COMPLIANT -- can be read in next line when p0 == 0 + v6[1] = v6[p0]; // COMPLIANT + v6[0] = 2; // COMPLIANT + useVec(v6); + + std::vector v7(10); + v7[0] = 0; // NON_COMPLIANT -- p1 cannot be equal to 0 + v7[1] = v7[p1]; // COMPLIANT + v7[0] = 2; // COMPLIANT + useVec(v7); + + std::vector v8(10); + v8[p0] = 0; // NON_COMPLIANT[False negative] -- cannot be read in next line + v8[0] = v8[p1]; // COMPLIANT + v8[p0] = 2; // COMPLIANT + useVec(v8); + + std::vector v9(10); + v9[p0] = 0; // NON_COMPLIANT[False negative] -- cannot be read in next line + // p0 cannot be 11 + v9[0] = v9[11]; // COMPLIANT + v9[p0] = 2; // COMPLIANT + useVec(v9); + + std::vector v10(10); + v10[p0] = 0; // COMPLIANT -- can be read in next line + // p0 can be equal to 0 + v10[1] = v10[0]; // COMPLIANT + v10[p0] = 2; // COMPLIANT + useVec(v10); + + std::vector v11(10); + v11[p1] = 0; // COMPLIANT -- can be read in next line + // p1 may be equal to be 11 + v11[0] = v11[11]; // COMPLIANT + v11[p1] = 2; // COMPLIANT + useVec(v11); + + std::vector v12(10); + v12[p1] = 0; // NON_COMPLIANT[False negative] -- cannot be read in next line + // p1 cannot be equal to 0 + v12[1] = v12[0]; // COMPLIANT + v12[p1] = 2; // COMPLIANT + useVec(v12); + } +} + +void f23() { + int *l0 = nullptr; // NON_COMPLIANT + + int *l1 = nullptr; // NON_COMPLIANT + l1[0]; // read but not observed + + int *l2 = nullptr; // COMPLIANT + l2[0] = 0; // write observed via assignment +} + +void f24() { + int l0 = 0; // COMPLIANT + std::vector l1(10); + l1[l0 + 1] = 0; // COMPLIANT + useVec(l1); + + int l2 = 0; // COMPLIANT + int *l3; // COMPLIANT + l3[l2 * 2] = 0; // COMPLIANT + f0(l3[0]); + + int l4 = 0; // COMPLIANT + int l5[10]; + l5[l4 % 10] = 0; // COMPLIANT + f0(l5[0]); + + int l6 = 0; // COMPLIANT + Trivial *l7; // COMPLIANT + l7[l6 & 3].a = 0; // COMPLIANT + use(l7[0]); + + int l8 = 0; // COMPLIANT + std::vector l9(10); + l9[l8 | 1].a = 0; // COMPLIANT + useVec(l9); + + int l10 = 0; // COMPLIANT + int l11[10]; + l11[l10]++; // COMPLIANT + f0(l11[0]); + + int l12 = 0; // COMPLIANT + int l13[10]; + l13[l12 + 1]++; // COMPLIANT + f0(l13[0]); +} + +void f25() { + // True positive case + int l0 = 0; // COMPLIANT + int *l1 = &l0; + l0 = 1; // COMPLIANT -- observed via pointer + f0(*l1); + + // False positive case + int l2 = 0; // COMPLIANT + int *l3 = &l2; // COMPLIANT + f0(*l3); + l2 = 1; // NON_COMPLIANT[False negative] -- not observed + + // True positive case + // Handles case where l4 is written twice without read + int l4 = 0; // COMPLIANT + int *l5 = &l4; // COMPLIANT + l4 = 1; // COMPLIANT + f0(*l5); + l4 = 2; // COMPLIANT + f0(l4); // observe l4 + + // False positive case + // Handles case where l6 is written twice without read + int l6 = 0; // COMPLIANT + int *l7 = &l6; // COMPLIANT + f0(*l7); + l6 = 1; // NON_COMPLIANT[False negative] + l6 = 2; // COMPLIANT + f0(l6); // observe l6 + + // Test reference variables + int l8 = 0; // COMPLIANT + int &l9 = l8; // COMPLIANT + l8 = 1; // COMPLIANT + f0(l9); + + int l10 = 0; // COMPLIANT + int &l11 = l10; // COMPLIANT + f0(l11); + l10 = 1; // NON_COMPLIANT[False negative] +} + +void f26() { + /** + * The following tests show the limitations of our STL container modeling. + * + * These FNs are preferred over FPs. + */ + std::vector v0 = {0, 1, 2}; // COMPLIANT[False negative] + v0.size(); // finding size is currently treated as observes whole object + + std::vector v1 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v1.begin(); // obtaining iterator is currently treated as observing whole object + + std::vector v2 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v2.end(); // obtaining iterator is currently treated as observing whole object + + std::vector v3 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v3.empty(); // checking emptiness is currently treated as observing whole object + + //std::vector v4 = {0, 1, 2}; // COMPLIANT[False negative] + //v4.capacity(); // checking capacity is currently treated as observing whole object + + /** + * Testing calls to methods that modify the container. + * + * These could be treated as a possible assignment to any index of that + * container. Therefore, never treated as an overwrite, but must be observed. + * + * This is not currently modeled, so all are FNs. + */ + std::vector v5 = {0, 1, 2}; // NON_COMPLIANT[False negative] + // We don't model that this must modify indexes 0 through 2. + v5.clear(); // NON_COMPLIANT[False negative] -- not observed + + std::vector v6 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v6.clear(); // COMPLIANT -- observed + useVec(v6); + + std::vector v7 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v7.push_back(3); // NON_COMPLIANT[False negative] -- not observed + + std::vector v8 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v8.resize(10); // NON_COMPLIANT[False negative] -- not observed + + std::vector v9 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v9.pop_back(); // NON_COMPLIANT[False negative] -- not observed + + //std::vector v10 = {0, 1, 2}; // NON_COMPLIANT[False negative] + //v10.assign(5, 0); // NON_COMPLIANT[False negative] -- not observed + + //std::vector v11 = {0, 1, 2}; // NON_COMPLIANT[False negative] + //std::vector v12 = {3, 4, 5}; // NON_COMPLIANT[False negative] + //v11.swap(v12); // NON_COMPLIANT[False negative] -- not observed + + std::vector v13 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v13.emplace(v13.begin(), 3); // NON_COMPLIANT[False negative] -- not observed + + std::vector v14 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v14.emplace_back(3); // NON_COMPLIANT[False negative] -- not observed + + std::vector v15 = {0, 1, 2}; // NON_COMPLIANT[False negative] + v15.insert(v15.begin(), 3); // NON_COMPLIANT[False negative] -- not observed + + //std::vector v16 = {0, 1, 2}; // NON_COMPLIANT[False negative] + //v16.erase(v16.begin()); // NON_COMPLIANT[False negative] -- not observed +} + +void f27() { + // Lambdas have been excluded for now for simplicity. + int l0 = 0; // COMPLIANT + auto l1 = [&l0]() { return l0 + 1; }; // COMPLIANT + f0(l1()); + + // All of f27 is excluded from analysis. + int l2 = 0; // NON_COMPLIANT[False negative] + + []() { + // Some lambda bodies had strange results, so lambda bodies are excluded. + int l3 = 0; // NON_COMPLIANT[False negative] + }; +} + +void f28() { + // Try catch blocks are excluded from analysis for simplicity. + int l0, l1; + try { + l0 = 0; // NON_COMPLIANT[False negative] + l1 = 0; // COMPLIANT + } catch (...) { + f0(l1); + } + + // All of f28 is excluded from analysis. + int l2 = 0; // COMPLIANT +} + +int f29() { + struct { + int arr[10]; + } x; + + int l0 = 0; // COMPLIANT + f0(x.arr[l0]); +} \ No newline at end of file diff --git a/rule_packages/cpp/DeadCode5.json b/rule_packages/cpp/DeadCode5.json new file mode 100644 index 0000000000..9298341b06 --- /dev/null +++ b/rule_packages/cpp/DeadCode5.json @@ -0,0 +1,27 @@ +{ + "MISRA-C++-2023": { + "RULE-0-1-1": { + "properties": { + "enforcement": "undecidable", + "obligation": "advisory" + }, + "queries": [ + { + "description": "Unused writes to objects may harm performance, complicate code comprehension, or indicate logical errors in the program.", + "kind": "problem", + "name": "A value should not be unnecessarily written to a local object", + "precision": "very-high", + "severity": "error", + "short_name": "UnnecessaryWriteToLocalObject", + "tags": [ + "scope/system", + "correctness", + "maintainability", + "performance" + ] + } + ], + "title": "A value should not be unnecessarily written to a local object" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index a2eb7eddd8..812bb89ff1 100644 --- a/rules.csv +++ b/rules.csv @@ -825,7 +825,7 @@ c,MISRA-C-2012,RULE-23-7,Yes,Advisory,,,A generic selection that is expanded fro c,MISRA-C-2012,RULE-23-8,Yes,Required,,,A default association shall appear as either the first or the last association of a generic selection,,Generics,Easy, cpp,MISRA-C++-2023,RULE-0-0-1,Yes,Required,Decidable,Single Translation Unit,A function shall not contain unreachable statements,M0-1-1,DeadCode2,Medium, cpp,MISRA-C++-2023,RULE-0-0-2,Yes,Advisory,Undecidable,System,Controlling expressions should not be invariant,M0-1-2,DeadCode2,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-1,Yes,Advisory,Undecidable,System,A value should not be unnecessarily written to a local object,A0-1-1,DeadCode5,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-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, From c9b5dbe3dc7063444cd4c9d0ba1daa0cba24b771 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Fri, 30 Jan 2026 09:05:36 -0800 Subject: [PATCH 2/5] format --- cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll | 2 +- cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll index 32dec453c6..4ef3122805 100644 --- a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll +++ b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll @@ -125,7 +125,7 @@ abstract class ObjectIdentityBase extends Element { /** * Finds cases such as a subobject such as `x.y` is taken as a reference. - * + * * This is useful for alias analysis, as references create aliases that can invalidate certain * assumptions about object accesses. */ diff --git a/cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll index aabd513e6a..ca1628e09a 100644 --- a/cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll +++ b/cpp/common/src/codingstandards/cpp/lifetimes/CppSubObjects.qll @@ -3,7 +3,7 @@ * * See `codingstandards.c.SubObjects` for the C equivalent. Changes specific to C++ are: * - excluding fields of reference type as subobjects - * + * * A library that expands upon the `Objects.qll` library, to support nested "Objects" such as * `x.y.z` or `x[i][j]` within an object `x`. * From 75874561c08a71a14b3836700ae9c77afc1765dd Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Fri, 30 Jan 2026 09:07:14 -0800 Subject: [PATCH 3/5] Reduce precision --- cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql | 2 +- rule_packages/cpp/DeadCode5.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql b/cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql index 83ebf84282..ef85736b58 100644 --- a/cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql +++ b/cpp/misra/src/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.ql @@ -4,7 +4,7 @@ * @description Unused writes to objects may harm performance, complicate code comprehension, or * indicate logical errors in the program. * @kind problem - * @precision very-high + * @precision high * @problem.severity error * @tags external/misra/id/rule-0-1-1 * scope/system diff --git a/rule_packages/cpp/DeadCode5.json b/rule_packages/cpp/DeadCode5.json index 9298341b06..cac2afe994 100644 --- a/rule_packages/cpp/DeadCode5.json +++ b/rule_packages/cpp/DeadCode5.json @@ -10,7 +10,7 @@ "description": "Unused writes to objects may harm performance, complicate code comprehension, or indicate logical errors in the program.", "kind": "problem", "name": "A value should not be unnecessarily written to a local object", - "precision": "very-high", + "precision": "high", "severity": "error", "short_name": "UnnecessaryWriteToLocalObject", "tags": [ From dae4d25f2e2c5142279c5da8e84c56a4de13ca20 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sat, 31 Jan 2026 11:11:07 -0800 Subject: [PATCH 4/5] Fix Iterators.qll imports, add change note --- change_notes/2026-01-31-move-iterators-shared-library.md | 4 ++++ .../A23-0-1/IteratorImplicitlyConvertedToConstIterator.ql | 2 +- .../GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql | 2 +- cpp/cert/src/rules/CTR53-CPP/UseValidIteratorRanges.ql | 2 +- .../CTR54-CPP/DoNotSubtractIteratorsForDifferentContainers.ql | 2 +- .../rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql | 2 +- .../ValidContainerElementAccess.qll | 2 +- 7 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 change_notes/2026-01-31-move-iterators-shared-library.md diff --git a/change_notes/2026-01-31-move-iterators-shared-library.md b/change_notes/2026-01-31-move-iterators-shared-library.md new file mode 100644 index 0000000000..1121f7b23a --- /dev/null +++ b/change_notes/2026-01-31-move-iterators-shared-library.md @@ -0,0 +1,4 @@ +- `A-23-0-1`, `A-23-0-2`, `CTR-51-CPP`, `CTR-52-CPP`, `CTR-53-CPP`, `CTR-54-CPP`, `CTR-55-CPP`, `STR-52-CPP` - `IteratorImplicitlyConvertedToConstIterator.ql`, `ValidContainerElementAccess.ql`, `UsesValidContainerElementAccess.ql`, `GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql`, `UseValidIteratorRanges.ql`, `DoNotSubtractIteratorsForDifferentContainers.ql`, `DoNotUseAnAdditiveOperatorOnAnIterator.ql`, `UseValidReferencesForElementsOfString.ql`: + - Iterator access methods `rbegin`, `rend`, `crbegin`, `crend` are now recognized on containers. + - Shared library `Iterators.qll` has been refactored by splitting out container type logic into a separate library and add logic to differentiate types of containers, such as associative, indexed, and strings. + - Shared library `Iterators.qll`, used by many queries, has been moved. \ No newline at end of file diff --git a/cpp/autosar/src/rules/A23-0-1/IteratorImplicitlyConvertedToConstIterator.ql b/cpp/autosar/src/rules/A23-0-1/IteratorImplicitlyConvertedToConstIterator.ql index d67058868c..2bba3ba7eb 100644 --- a/cpp/autosar/src/rules/A23-0-1/IteratorImplicitlyConvertedToConstIterator.ql +++ b/cpp/autosar/src/rules/A23-0-1/IteratorImplicitlyConvertedToConstIterator.ql @@ -13,7 +13,7 @@ import cpp import codingstandards.cpp.autosar -import codingstandards.cpp.Iterators +import codingstandards.cpp.standardlibrary.Iterators /* * Due to inconsistent typedefs across STL containers the way this is parsed diff --git a/cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql b/cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql index b022869136..ad905d5cc6 100644 --- a/cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql +++ b/cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql @@ -18,7 +18,7 @@ import cpp import codingstandards.cpp.cert -import codingstandards.cpp.Iterators +import codingstandards.cpp.standardlibrary.Iterators import codingstandards.cpp.rules.containeraccesswithoutrangecheck.ContainerAccessWithoutRangeCheck as ContainerAccessWithoutRangeCheck import semmle.code.cpp.controlflow.Guards import semmle.code.cpp.dataflow.TaintTracking diff --git a/cpp/cert/src/rules/CTR53-CPP/UseValidIteratorRanges.ql b/cpp/cert/src/rules/CTR53-CPP/UseValidIteratorRanges.ql index 1512a7fd99..4ba3307213 100644 --- a/cpp/cert/src/rules/CTR53-CPP/UseValidIteratorRanges.ql +++ b/cpp/cert/src/rules/CTR53-CPP/UseValidIteratorRanges.ql @@ -18,7 +18,7 @@ import cpp import codingstandards.cpp.cert -import codingstandards.cpp.Iterators +import codingstandards.cpp.standardlibrary.Iterators import semmle.code.cpp.dataflow.DataFlow predicate startEndArgumentsDoNotPointToTheSameContainer( diff --git a/cpp/cert/src/rules/CTR54-CPP/DoNotSubtractIteratorsForDifferentContainers.ql b/cpp/cert/src/rules/CTR54-CPP/DoNotSubtractIteratorsForDifferentContainers.ql index 2401bcbf54..7045a35679 100644 --- a/cpp/cert/src/rules/CTR54-CPP/DoNotSubtractIteratorsForDifferentContainers.ql +++ b/cpp/cert/src/rules/CTR54-CPP/DoNotSubtractIteratorsForDifferentContainers.ql @@ -18,7 +18,7 @@ import cpp import codingstandards.cpp.cert -import codingstandards.cpp.Iterators +import codingstandards.cpp.standardlibrary.Iterators /** Models the subtraction operator */ class SubtractionOperator extends Function { diff --git a/cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql b/cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql index c6ea2c4518..182f718f6f 100644 --- a/cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql +++ b/cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql @@ -18,7 +18,7 @@ import cpp import codingstandards.cpp.cert -import codingstandards.cpp.Iterators +import codingstandards.cpp.standardlibrary.Iterators import semmle.code.cpp.controlflow.Dominance import semmle.code.cpp.dataflow.DataFlow diff --git a/cpp/common/src/codingstandards/cpp/rules/validcontainerelementaccess/ValidContainerElementAccess.qll b/cpp/common/src/codingstandards/cpp/rules/validcontainerelementaccess/ValidContainerElementAccess.qll index 93e121d44c..8f8625baa2 100644 --- a/cpp/common/src/codingstandards/cpp/rules/validcontainerelementaccess/ValidContainerElementAccess.qll +++ b/cpp/common/src/codingstandards/cpp/rules/validcontainerelementaccess/ValidContainerElementAccess.qll @@ -25,7 +25,7 @@ import cpp import codingstandards.cpp.Customizations import codingstandards.cpp.Exclusions -import codingstandards.cpp.Iterators +import codingstandards.cpp.standardlibrary.Iterators abstract class ValidContainerElementAccessSharedQuery extends Query { } From fe33f61c1af7a79c57931351e99e05653950286a Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sat, 31 Jan 2026 11:11:26 -0800 Subject: [PATCH 5/5] Autoformat cpp, fix test expectations --- .../UnnecessaryWriteToLocalObject.expected | 94 +++++++++---------- cpp/misra/test/rules/RULE-0-1-1/test.cpp | 67 ++++++------- 2 files changed, 82 insertions(+), 79 deletions(-) diff --git a/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected b/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected index 0c0ee0907f..5a935853dd 100644 --- a/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected +++ b/cpp/misra/test/rules/RULE-0-1-1/UnnecessaryWriteToLocalObject.expected @@ -1,47 +1,47 @@ -| test.cpp:10:11:10:12 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:10:7:10:8 | l0 | l0 | -| test.cpp:60:12:60:13 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:60:7:60:9 | l13 | l13 | -| test.cpp:61:3:61:5 | l13 | Unnecessary write to object $@ is never observed. | test.cpp:60:7:60:9 | l13 | l13 | -| test.cpp:116:6:116:9 | ... -- | Unnecessary write to object $@ is never observed. | test.cpp:115:7:115:8 | l2 | l2 | -| test.cpp:118:6:118:9 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:117:7:117:8 | l3 | l3 | -| test.cpp:126:11:126:12 | 0 | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:126:7:126:8 | l0 | l0 | -| test.cpp:141:20:141:20 | second | Unnecessary write to object $@ is never observed. | test.cpp:141:13:141:13 | l1 | l1 | -| test.cpp:149:18:149:18 | call to operator* | Unnecessary write to object $@ is never observed. | test.cpp:149:13:149:14 | l3 | l3 | -| test.cpp:167:13:167:22 | new | Unnecessary write to object $@ is never observed. | test.cpp:167:8:167:9 | l3 | l3 | -| test.cpp:172:11:172:12 | 1 | Unnecessary write to object $@ is never observed. | test.cpp:172:7:172:8 | l1 | l1 | -| test.cpp:178:11:178:12 | 1 | Unnecessary write to object $@ is never observed. | test.cpp:178:7:178:8 | l1 | l1 | -| test.cpp:199:15:199:24 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:199:11:199:12 | t0 | t0 | -| test.cpp:218:15:218:24 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:218:11:218:12 | t4 | t4 | -| test.cpp:219:17:219:29 | new | Unnecessary write to object $@ is never observed. | test.cpp:219:12:219:13 | t5 | t5 | -| test.cpp:222:6:222:6 | a | Unnecessary write to object $@ is never observed. | test.cpp:221:11:221:12 | t6 | t6 | -| test.cpp:235:16:235:25 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:235:11:235:13 | t10 | t10 | -| test.cpp:238:18:238:30 | new | Unnecessary write to object $@ is never observed. | test.cpp:238:12:238:14 | t11 | t11 | -| test.cpp:241:18:241:40 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:241:11:241:13 | t12 | t12 | -| test.cpp:255:13:255:14 | l0 | Unnecessary write to object $@ is never observed. | test.cpp:255:8:255:9 | l1 | l1 | -| test.cpp:265:4:265:7 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:264:8:264:9 | l4 | l4 | -| test.cpp:280:6:280:6 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:275:11:275:12 | t0 | t0 | -| test.cpp:284:6:284:6 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:275:11:275:12 | t0 | t0 | -| test.cpp:300:8:300:8 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:299:5:299:6 | t2 | t2 | -| test.cpp:311:24:311:33 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:311:20:311:21 | v1 | v1 | -| test.cpp:317:20:317:21 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:317:20:317:21 | v3 | v3 | -| test.cpp:318:5:318:5 | call to operator[] | Unnecessary write to object $@ is never observed. | test.cpp:317:20:317:21 | v3 | v3 | -| test.cpp:326:5:326:5 | call to operator[] | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:325:20:325:21 | v5 | v5 | -| test.cpp:345:29:345:51 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:345:24:345:26 | v10 | v10 | -| test.cpp:355:3:355:4 | p1 | Unnecessary write to object $@ is never observed. | test.cpp:351:22:351:23 | p1 | p1 | -| test.cpp:362:6:362:6 | a | Unnecessary write to object $@ is never observed. | test.cpp:358:30:358:31 | t1 | t1 | -| test.cpp:372:5:372:5 | call to operator[] | Unnecessary write to object $@ is never observed. | test.cpp:367:56:367:57 | v1 | v1 | -| test.cpp:387:3:387:4 | p1 | Unnecessary write to object $@ is never observed. | test.cpp:377:24:377:25 | p1 | p1 | -| test.cpp:388:3:388:4 | p2 | Unnecessary write to object $@ is never observed. | test.cpp:377:37:377:38 | p2 | p2 | -| test.cpp:389:3:389:4 | p3 | Unnecessary write to object $@ is never observed. | test.cpp:377:53:377:54 | p3 | p3 | -| test.cpp:390:3:390:4 | p4 | Unnecessary write to object $@ is never observed. | test.cpp:378:32:378:33 | p4 | p4 | -| test.cpp:404:13:404:22 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:404:7:404:8 | l1 | l1 | -| test.cpp:409:17:409:39 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:409:11:409:12 | l3 | l3 | -| test.cpp:420:3:420:7 | access to array | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:419:7:419:8 | l6 | l6 | -| test.cpp:425:3:425:9 | access to array | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:424:7:424:10 | l6_1 | l6_1 | -| test.cpp:441:3:441:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | -| test.cpp:442:3:442:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | -| test.cpp:443:3:443:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | -| test.cpp:451:3:451:8 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:450:7:450:9 | l11 | l11 | -| test.cpp:452:3:452:10 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:450:7:450:9 | l11 | l11 | -| test.cpp:516:7:516:7 | call to operator[] | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:515:22:515:23 | v7 | v7 | -| test.cpp:558:13:558:19 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:558:8:558:9 | l0 | l0 | -| test.cpp:560:13:560:19 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:560:8:560:9 | l1 | l1 | +| test.cpp:10:11:10:12 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:10:7:10:8 | l0 | l0 | +| test.cpp:60:12:60:13 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:60:7:60:9 | l13 | l13 | +| test.cpp:61:3:61:5 | l13 | Unnecessary write to object $@ is never observed. | test.cpp:60:7:60:9 | l13 | l13 | +| test.cpp:116:6:116:9 | ... -- | Unnecessary write to object $@ is never observed. | test.cpp:115:7:115:8 | l2 | l2 | +| test.cpp:118:6:118:9 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:117:7:117:8 | l3 | l3 | +| test.cpp:126:11:126:12 | 0 | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:126:7:126:8 | l0 | l0 | +| test.cpp:141:20:141:20 | second | Unnecessary write to object $@ is never observed. | test.cpp:141:13:141:13 | l1 | l1 | +| test.cpp:149:18:149:18 | call to operator* | Unnecessary write to object $@ is never observed. | test.cpp:149:13:149:14 | l3 | l3 | +| test.cpp:167:13:167:22 | new | Unnecessary write to object $@ is never observed. | test.cpp:167:8:167:9 | l3 | l3 | +| test.cpp:172:11:172:12 | 1 | Unnecessary write to object $@ is never observed. | test.cpp:172:7:172:8 | l1 | l1 | +| test.cpp:178:11:178:12 | 1 | Unnecessary write to object $@ is never observed. | test.cpp:178:7:178:8 | l1 | l1 | +| test.cpp:199:15:199:24 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:199:11:199:12 | t0 | t0 | +| test.cpp:218:15:218:24 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:218:11:218:12 | t4 | t4 | +| test.cpp:219:17:219:29 | new | Unnecessary write to object $@ is never observed. | test.cpp:219:12:219:13 | t5 | t5 | +| test.cpp:222:6:222:6 | a | Unnecessary write to object $@ is never observed. | test.cpp:221:11:221:12 | t6 | t6 | +| test.cpp:235:16:235:25 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:235:11:235:13 | t10 | t10 | +| test.cpp:238:18:238:30 | new | Unnecessary write to object $@ is never observed. | test.cpp:238:12:238:14 | t11 | t11 | +| test.cpp:241:18:241:40 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:241:11:241:13 | t12 | t12 | +| test.cpp:255:13:255:14 | l0 | Unnecessary write to object $@ is never observed. | test.cpp:255:8:255:9 | l1 | l1 | +| test.cpp:265:4:265:7 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:264:8:264:9 | l4 | l4 | +| test.cpp:280:6:280:6 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:275:11:275:12 | t0 | t0 | +| test.cpp:284:6:284:6 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:275:11:275:12 | t0 | t0 | +| test.cpp:300:8:300:8 | a | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:299:5:299:6 | t2 | t2 | +| test.cpp:311:24:311:33 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:311:20:311:21 | v1 | v1 | +| test.cpp:317:20:317:21 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:317:20:317:21 | v3 | v3 | +| test.cpp:318:5:318:5 | call to operator[] | Unnecessary write to object $@ is never observed. | test.cpp:317:20:317:21 | v3 | v3 | +| test.cpp:326:5:326:5 | call to operator[] | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:325:20:325:21 | v5 | v5 | +| test.cpp:345:29:345:51 | call to vector | Unnecessary write to object $@ is never observed. | test.cpp:345:24:345:26 | v10 | v10 | +| test.cpp:355:3:355:4 | p1 | Unnecessary write to object $@ is never observed. | test.cpp:351:22:351:23 | p1 | p1 | +| test.cpp:362:6:362:6 | a | Unnecessary write to object $@ is never observed. | test.cpp:358:30:358:31 | t1 | t1 | +| test.cpp:372:5:372:5 | call to operator[] | Unnecessary write to object $@ is never observed. | test.cpp:367:56:367:57 | v1 | v1 | +| test.cpp:387:3:387:4 | p1 | Unnecessary write to object $@ is never observed. | test.cpp:377:24:377:25 | p1 | p1 | +| test.cpp:388:3:388:4 | p2 | Unnecessary write to object $@ is never observed. | test.cpp:377:37:377:38 | p2 | p2 | +| test.cpp:389:3:389:4 | p3 | Unnecessary write to object $@ is never observed. | test.cpp:377:53:377:54 | p3 | p3 | +| test.cpp:390:3:390:4 | p4 | Unnecessary write to object $@ is never observed. | test.cpp:378:32:378:33 | p4 | p4 | +| test.cpp:404:13:404:22 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:404:7:404:8 | l1 | l1 | +| test.cpp:409:17:409:39 | {...} | Unnecessary write to object $@ is never observed. | test.cpp:409:11:409:12 | l3 | l3 | +| test.cpp:420:3:420:7 | access to array | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:419:7:419:8 | l6 | l6 | +| test.cpp:425:3:425:9 | access to array | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:424:7:424:10 | l6_1 | l6_1 | +| test.cpp:441:3:441:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | +| test.cpp:442:3:442:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | +| test.cpp:443:3:443:7 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:440:7:440:8 | l9 | l9 | +| test.cpp:451:3:451:8 | access to array | Unnecessary write to object $@ is never observed. | test.cpp:450:7:450:9 | l11 | l11 | +| test.cpp:452:3:452:10 | ... ++ | Unnecessary write to object $@ is never observed. | test.cpp:450:7:450:9 | l11 | l11 | +| test.cpp:516:7:516:7 | call to operator[] | Unnecessary write to object $@ is not read before subsequent writes. | test.cpp:515:22:515:23 | v7 | v7 | +| test.cpp:558:13:558:19 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:558:8:558:9 | l0 | l0 | +| test.cpp:560:13:560:19 | 0 | Unnecessary write to object $@ is never observed. | test.cpp:560:8:560:9 | l1 | l1 | diff --git a/cpp/misra/test/rules/RULE-0-1-1/test.cpp b/cpp/misra/test/rules/RULE-0-1-1/test.cpp index 571a3d0a34..b6152dbdae 100644 --- a/cpp/misra/test/rules/RULE-0-1-1/test.cpp +++ b/cpp/misra/test/rules/RULE-0-1-1/test.cpp @@ -315,7 +315,7 @@ void f16() { useVec(v2); std::vector v3; // NON_COMPLIANT -- unobserved - v3[0] = 0; // NON_COMPLIANT -- unused + v3[0] = 0; // NON_COMPLIANT -- unused std::vector v4; v4[0] = 0; // COMPLIANT @@ -422,10 +422,10 @@ void f21() { f0(l6[0]); // Observes entire object int l6_1[10]; - l6_1[0] = 0; // NON_COMPLIANT + l6_1[0] = 0; // NON_COMPLIANT l6_1[1] = l6_1[2]; // COMPLIANT - l6_1[0] = 0; // COMPLIANT - f0(l6_1[0]); // Observes entire object + l6_1[0] = 0; // COMPLIANT + f0(l6_1[0]); // Observes entire object int l7[10]; l7[0] = 0; // COMPLIANT @@ -570,18 +570,18 @@ void f24() { l1[l0 + 1] = 0; // COMPLIANT useVec(l1); - int l2 = 0; // COMPLIANT - int *l3; // COMPLIANT + int l2 = 0; // COMPLIANT + int *l3; // COMPLIANT l3[l2 * 2] = 0; // COMPLIANT f0(l3[0]); - + int l4 = 0; // COMPLIANT int l5[10]; l5[l4 % 10] = 0; // COMPLIANT f0(l5[0]); - int l6 = 0; // COMPLIANT - Trivial *l7; // COMPLIANT + int l6 = 0; // COMPLIANT + Trivial *l7; // COMPLIANT l7[l6 & 3].a = 0; // COMPLIANT use(l7[0]); @@ -609,23 +609,23 @@ void f25() { f0(*l1); // False positive case - int l2 = 0; // COMPLIANT + int l2 = 0; // COMPLIANT int *l3 = &l2; // COMPLIANT f0(*l3); l2 = 1; // NON_COMPLIANT[False negative] -- not observed // True positive case // Handles case where l4 is written twice without read - int l4 = 0; // COMPLIANT + int l4 = 0; // COMPLIANT int *l5 = &l4; // COMPLIANT - l4 = 1; // COMPLIANT + l4 = 1; // COMPLIANT f0(*l5); l4 = 2; // COMPLIANT f0(l4); // observe l4 // False positive case // Handles case where l6 is written twice without read - int l6 = 0; // COMPLIANT + int l6 = 0; // COMPLIANT int *l7 = &l6; // COMPLIANT f0(*l7); l6 = 1; // NON_COMPLIANT[False negative] @@ -633,12 +633,12 @@ void f25() { f0(l6); // observe l6 // Test reference variables - int l8 = 0; // COMPLIANT + int l8 = 0; // COMPLIANT int &l9 = l8; // COMPLIANT - l8 = 1; // COMPLIANT + l8 = 1; // COMPLIANT f0(l9); - int l10 = 0; // COMPLIANT + int l10 = 0; // COMPLIANT int &l11 = l10; // COMPLIANT f0(l11); l10 = 1; // NON_COMPLIANT[False negative] @@ -647,30 +647,33 @@ void f25() { void f26() { /** * The following tests show the limitations of our STL container modeling. - * + * * These FNs are preferred over FPs. */ std::vector v0 = {0, 1, 2}; // COMPLIANT[False negative] v0.size(); // finding size is currently treated as observes whole object std::vector v1 = {0, 1, 2}; // NON_COMPLIANT[False negative] - v1.begin(); // obtaining iterator is currently treated as observing whole object + v1.begin(); // obtaining iterator is currently treated as observing whole + // object std::vector v2 = {0, 1, 2}; // NON_COMPLIANT[False negative] v2.end(); // obtaining iterator is currently treated as observing whole object std::vector v3 = {0, 1, 2}; // NON_COMPLIANT[False negative] - v3.empty(); // checking emptiness is currently treated as observing whole object + v3.empty(); // checking emptiness is currently treated as observing whole + // object - //std::vector v4 = {0, 1, 2}; // COMPLIANT[False negative] - //v4.capacity(); // checking capacity is currently treated as observing whole object + // std::vector v4 = {0, 1, 2}; // COMPLIANT[False negative] + // v4.capacity(); // checking capacity is currently treated as observing whole + // object /** * Testing calls to methods that modify the container. - * + * * These could be treated as a possible assignment to any index of that * container. Therefore, never treated as an overwrite, but must be observed. - * + * * This is not currently modeled, so all are FNs. */ std::vector v5 = {0, 1, 2}; // NON_COMPLIANT[False negative] @@ -678,7 +681,7 @@ void f26() { v5.clear(); // NON_COMPLIANT[False negative] -- not observed std::vector v6 = {0, 1, 2}; // NON_COMPLIANT[False negative] - v6.clear(); // COMPLIANT -- observed + v6.clear(); // COMPLIANT -- observed useVec(v6); std::vector v7 = {0, 1, 2}; // NON_COMPLIANT[False negative] @@ -690,12 +693,12 @@ void f26() { std::vector v9 = {0, 1, 2}; // NON_COMPLIANT[False negative] v9.pop_back(); // NON_COMPLIANT[False negative] -- not observed - //std::vector v10 = {0, 1, 2}; // NON_COMPLIANT[False negative] - //v10.assign(5, 0); // NON_COMPLIANT[False negative] -- not observed + // std::vector v10 = {0, 1, 2}; // NON_COMPLIANT[False negative] + // v10.assign(5, 0); // NON_COMPLIANT[False negative] -- not observed - //std::vector v11 = {0, 1, 2}; // NON_COMPLIANT[False negative] - //std::vector v12 = {3, 4, 5}; // NON_COMPLIANT[False negative] - //v11.swap(v12); // NON_COMPLIANT[False negative] -- not observed + // std::vector v11 = {0, 1, 2}; // NON_COMPLIANT[False negative] + // std::vector v12 = {3, 4, 5}; // NON_COMPLIANT[False negative] + // v11.swap(v12); // NON_COMPLIANT[False negative] -- not observed std::vector v13 = {0, 1, 2}; // NON_COMPLIANT[False negative] v13.emplace(v13.begin(), 3); // NON_COMPLIANT[False negative] -- not observed @@ -706,13 +709,13 @@ void f26() { std::vector v15 = {0, 1, 2}; // NON_COMPLIANT[False negative] v15.insert(v15.begin(), 3); // NON_COMPLIANT[False negative] -- not observed - //std::vector v16 = {0, 1, 2}; // NON_COMPLIANT[False negative] - //v16.erase(v16.begin()); // NON_COMPLIANT[False negative] -- not observed + // std::vector v16 = {0, 1, 2}; // NON_COMPLIANT[False negative] + // v16.erase(v16.begin()); // NON_COMPLIANT[False negative] -- not observed } void f27() { // Lambdas have been excluded for now for simplicity. - int l0 = 0; // COMPLIANT + int l0 = 0; // COMPLIANT auto l1 = [&l0]() { return l0 + 1; }; // COMPLIANT f0(l1());