diff --git a/c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql b/c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql index e0617c266d..6d9caacc5c 100644 --- a/c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql +++ b/c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql @@ -14,30 +14,39 @@ import cpp import codingstandards.c.cert +import codingstandards.c.Objects import codingstandards.cpp.Concurrency import codingstandards.cpp.dataflow.TaintTracking import codingstandards.cpp.dataflow.DataFlow import semmle.code.cpp.commons.Alloc -from C11ThreadCreateCall tcc, StackVariable sv, Expr arg, Expr acc +from C11ThreadCreateCall tcc, Expr arg where not isExcluded(tcc, Concurrency4Package::appropriateThreadObjectStorageDurationsQuery()) and tcc.getArgument(2) = arg and - sv.getAnAccess() = acc and - // a stack variable that is given as an argument to a thread - TaintTracking::localTaint(DataFlow::exprNode(acc), DataFlow::exprNode(arg)) and - // or isn't one of the allowed usage patterns - not exists(Expr mfc | - isAllocationExpr(mfc) and - sv.getAnAssignedValue() = mfc and - acc.getAPredecessor*() = mfc - ) and - not exists(TSSGetFunctionCall tsg, TSSSetFunctionCall tss, DataFlow::Node src | - sv.getAnAssignedValue() = tsg and - acc.getAPredecessor*() = tsg and - // there should be dataflow from somewhere (the same somewhere) - // into each of the first arguments - DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and - DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0))) + ( + exists(ObjectIdentity obj, Expr acc | + obj.getASubobjectAccess() = acc and + obj.getStorageDuration().isAutomatic() and + exists(DataFlow::Node addrNode | + ( + addrNode = DataFlow::exprNode(any(AddressOfExpr e | e.getOperand() = acc)) + or + addrNode = DataFlow::exprNode(acc) and exists(ArrayToPointerConversion c | c.getExpr() = acc) + ) and + TaintTracking::localTaint(addrNode, DataFlow::exprNode(arg)) + ) + ) + or + // TODO: Remove/replace with tss_t type check, see #801. + exists(TSSGetFunctionCall tsg | + TaintTracking::localTaint(DataFlow::exprNode(tsg), DataFlow::exprNode(arg)) and + not exists(TSSSetFunctionCall tss, DataFlow::Node src | + // there should be dataflow from somewhere (the same somewhere) + // into each of the first arguments + DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and + DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0))) + ) + ) ) select tcc, "$@ not declared with appropriate storage duration", arg, "Shared object" diff --git a/c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql b/c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql index 9097f14297..1e1e19c7c6 100644 --- a/c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql +++ b/c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql @@ -13,10 +13,15 @@ import cpp import codingstandards.c.cert +import codingstandards.c.Objects import codingstandards.cpp.dataflow.DataFlow -class Source extends StackVariable { - Source() { not this instanceof Parameter } +class Source extends Expr { + ObjectIdentity rootObject; + Source() { + rootObject.getStorageDuration().isAutomatic() + and this = rootObject.getASubobjectAddressExpr() + } } class Sink extends DataFlow::Node { @@ -40,7 +45,7 @@ from DataFlow::Node src, DataFlow::Node sink where not isExcluded(sink.asExpr(), Declarations8Package::appropriateStorageDurationsFunctionReturnQuery()) and - exists(Source s | src.asExpr() = s.getAnAccess()) and + exists(Source s | src.asExpr() = s) and sink instanceof Sink and DataFlow::localFlow(src, sink) select sink, "$@ with automatic storage may be accessible outside of its lifetime.", src, diff --git a/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql b/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql index 6a018ed8c4..3689aa4397 100644 --- a/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql +++ b/c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql @@ -13,15 +13,13 @@ import cpp import codingstandards.c.cert -import codingstandards.cpp.lifetimes.CLifetimes +import codingstandards.c.Objects // Note: Undefined behavior is possible regardless of whether the accessed field from the returned // struct is an array or a scalar (i.e. arithmetic and pointer types) member, according to the standard. -from FieldAccess fa, FunctionCall fc +from FieldAccess fa, TemporaryObjectIdentity tempObject where not isExcluded(fa, InvalidMemory2Package::doNotModifyObjectsWithTemporaryLifetimeQuery()) and - not fa.getQualifier().isLValue() and - fa.getQualifier().getUnconverted() = fc and - fa.getQualifier().getUnconverted().getUnspecifiedType() instanceof StructOrUnionTypeWithArrayField -select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", fc, - "function call" + fa.getQualifier().getUnconverted() = tempObject +select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", tempObject, + "temporary object" diff --git a/c/cert/src/rules/MEM33-C/AllocStructsWithAFlexibleArrayMemberDynamically.ql b/c/cert/src/rules/MEM33-C/AllocStructsWithAFlexibleArrayMemberDynamically.ql index 620c4486a9..4e4ccc2171 100644 --- a/c/cert/src/rules/MEM33-C/AllocStructsWithAFlexibleArrayMemberDynamically.ql +++ b/c/cert/src/rules/MEM33-C/AllocStructsWithAFlexibleArrayMemberDynamically.ql @@ -14,6 +14,7 @@ import cpp import codingstandards.c.cert import codingstandards.c.Variable +import codingstandards.c.Objects import semmle.code.cpp.models.interfaces.Allocation import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis @@ -21,7 +22,7 @@ abstract class FlexibleArrayAlloc extends Element { /** * Returns the `Variable` being allocated. */ - abstract Variable getVariable(); + abstract Element getReportElement(); } /** @@ -53,18 +54,25 @@ class FlexibleArrayStructDynamicAlloc extends FlexibleArrayAlloc, FunctionCall { ) } - override Variable getVariable() { result = v } + override Element getReportElement() { result = v } } /** * A `Variable` of type `FlexibleArrayStructType` that is not allocated dynamically. */ -class FlexibleArrayNonDynamicAlloc extends FlexibleArrayAlloc, Variable { +class FlexibleArrayNonDynamicAlloc extends FlexibleArrayAlloc { + ObjectIdentity object; FlexibleArrayNonDynamicAlloc() { - this.getUnspecifiedType().getUnspecifiedType() instanceof FlexibleArrayStructType + this = object and + not object.getStorageDuration().isAllocated() and + // Exclude temporaries. Though they should violate this rule, in practice these results are + // often spurious and redundant, such as (*x = *x) which creates an unused temporary object. + not object.hasTemporaryLifetime() and + object.getType().getUnspecifiedType() instanceof FlexibleArrayStructType + and not exists(Variable v | v.getInitializer().getExpr() = this) } - override Variable getVariable() { result = this } + override Element getReportElement() { result = object } } from FlexibleArrayAlloc alloc, string message @@ -77,4 +85,4 @@ where alloc instanceof FlexibleArrayNonDynamicAlloc and message = "$@ contains a flexible array member but is not dynamically allocated." ) -select alloc, message, alloc.getVariable(), alloc.getVariable().getName() +select alloc, message, alloc.getReportElement(), alloc.getReportElement().toString() diff --git a/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected b/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected index f14ab4de4a..3fb10f3267 100644 --- a/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected +++ b/c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected @@ -1,4 +1,4 @@ -| test.c:65:18:65:18 | a | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:65:9:65:14 | call to get_s1 | function call | -| test.c:67:18:67:19 | s1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:67:9:67:14 | call to get_s3 | function call | -| test.c:68:18:68:19 | i1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:68:9:68:14 | call to get_s3 | function call | -| test.c:69:18:69:21 | af12 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:69:9:69:14 | call to get_s4 | function call | +| test.c:65:18:65:18 | a | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:65:9:65:14 | call to get_s1 | temporary object | +| test.c:67:18:67:19 | s1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:67:9:67:14 | call to get_s3 | temporary object | +| test.c:68:18:68:19 | i1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:68:9:68:14 | call to get_s3 | temporary object | +| test.c:69:18:69:21 | af12 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:69:9:69:14 | call to get_s4 | temporary object | diff --git a/c/common/src/codingstandards/c/IdentifierLinkage.qll b/c/common/src/codingstandards/c/IdentifierLinkage.qll new file mode 100644 index 0000000000..085ebf5a7b --- /dev/null +++ b/c/common/src/codingstandards/c/IdentifierLinkage.qll @@ -0,0 +1,47 @@ +import cpp + +newtype TIdentifierLinkage = + TIdentifierLinkageExternal() or + TIdentifierLinkageInternal() or + TIdentifierLinkageNone() + +/** + * In C, identifiers have internal linkage, or external linkage, or no linkage (6.2.2.1). + * + * The linkage of an identifier is used to, among other things, determine the storage duration + * and/or lifetime of that identifier. Storage durations and lifetimes are often used to define + * rules in the various coding standards. + */ +class IdentifierLinkage extends TIdentifierLinkage { + predicate isExternal() { this = TIdentifierLinkageExternal() } + + predicate isInternal() { this = TIdentifierLinkageInternal() } + + predicate isNone() { this = TIdentifierLinkageNone() } + + string toString() { + this.isExternal() and result = "external linkage" + or + this.isInternal() and result = "internal linkage" + or + this.isNone() and result = "no linkage" + } +} + +/** + * Determine the linkage of a variable: external, or static, or none. + * + * The linkage of a variable is determined by its scope and storage class. Note that other types of + * identifiers (e.g. functions) may also have linkage, but that behavior is not covered in this + * predicate. + */ +IdentifierLinkage linkageOfVariable(Variable v) { + // 6.2.2.3, file scope identifiers marked static have internal linkage. + v.isTopLevel() and v.isStatic() and result.isInternal() + or + // 6.2.2.4 describes generally non-static file scope identifiers, which have external linkage. + v.isTopLevel() and not v.isStatic() and result.isExternal() + or + // Note: Not all identifiers have linkage, see 6.2.2.6 + not v.isTopLevel() and result.isNone() +} diff --git a/c/common/src/codingstandards/c/Objects.qll b/c/common/src/codingstandards/c/Objects.qll new file mode 100644 index 0000000000..105291c688 --- /dev/null +++ b/c/common/src/codingstandards/c/Objects.qll @@ -0,0 +1,393 @@ +import cpp +import codingstandards.c.StorageDuration +import codingstandards.c.IdentifierLinkage +import semmle.code.cpp.valuenumbering.HashCons +import codingstandards.cpp.Clvalues + +/** + * A libary for handling "Objects" in C. + * + * 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 + ( + result = any(AddressOfExpr e | e.getOperand() = subobject) + or + exists(ArrayToPointerConversion c | c.getExpr() = subobject) and + not exists(ArrayExpr a | a.getArrayBase() = subobject) and + result = subobject + ) + ) + } + + /** + * Holds if the object has temporary lifetime. This is not a storage duration, but only objects + * with automatic storage duration have temporary lifetime. + */ + abstract predicate hasTemporaryLifetime(); +} + +/** + * 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 Variable, ObjectIdentityBase { + 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 isMember() + } + + override StorageDuration getStorageDuration() { + // 6.2.4.4, objects declared _Thread_local have thread storage duration. + 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 isThreadLocal() and + (hasLinkage() or isStatic()) and + result.isStatic() + or + // 6.2.4.3, Non _ThreadLocal objects no linkage that are not static have automatic storage + // duration. + not isThreadLocal() and + not hasLinkage() and + not isStatic() 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() + } + + /* The storage duration of a variable depends on its linkage. */ + IdentifierLinkage getLinkage() { result = linkageOfVariable(this) } + + predicate hasLinkage() { not getLinkage().isNone() } + + override VariableAccess getAnAccess() { result = Variable.super.getAnAccess() } + + override predicate hasTemporaryLifetime() { + none() // Objects identified by a variable do not have temporary lifetime. + } +} + +/** + * 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 } + + override predicate hasTemporaryLifetime() { + none() // String literal objects do not have temporary lifetime. + } +} + +/** + * 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 } + + override predicate hasTemporaryLifetime() { + // Confusing; a struct literal is an lvalue, and therefore does not have temporary lifetime. + none() + } +} + +/** + * 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() + ) + } + + override predicate hasTemporaryLifetime() { + none() // Allocated objects do not have "temporary lifetime." + } +} + +/** + * 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 } + + override predicate hasTemporaryLifetime() { any() } +} diff --git a/c/common/src/codingstandards/c/StorageDuration.qll b/c/common/src/codingstandards/c/StorageDuration.qll new file mode 100644 index 0000000000..4669d467bb --- /dev/null +++ b/c/common/src/codingstandards/c/StorageDuration.qll @@ -0,0 +1,31 @@ +import cpp + +class DeclarationWithStorageDuration extends Declaration { } + +newtype TStorageDuration = + StorageDurationStatic() or + StorageDurationAutomatic() or + StorageDurationThread() or + StorageDurationAllocated() + +class StorageDuration extends TStorageDuration { + predicate isStatic() { this = StorageDurationStatic() } + + predicate isAutomatic() { this = StorageDurationAutomatic() } + + predicate isThread() { this = StorageDurationThread() } + + predicate isAllocated() { this = StorageDurationAllocated() } + + string toString() { result = getStorageTypeName() + " storage duration" } + + string getStorageTypeName() { + isStatic() and result = "static" + or + isAutomatic() and result = "automatic" + or + isThread() and result = "thread" + or + isAllocated() and result = "allocated" + } +} diff --git a/c/common/src/codingstandards/c/UninitializedMutex.qll b/c/common/src/codingstandards/c/UninitializedMutex.qll new file mode 100644 index 0000000000..e69de29bb2 diff --git a/c/common/test/library/identifierlinkage/IdentifierLinkage.expected b/c/common/test/library/identifierlinkage/IdentifierLinkage.expected new file mode 100644 index 0000000000..c3f1bc39ef --- /dev/null +++ b/c/common/test/library/identifierlinkage/IdentifierLinkage.expected @@ -0,0 +1,10 @@ +| identifierlinkage.c:2:5:2:10 | g_ext1 | external linkage | +| identifierlinkage.c:3:12:3:17 | g_ext2 | external linkage | +| identifierlinkage.c:6:12:6:17 | g_int1 | internal linkage | +| identifierlinkage.c:9:5:9:10 | g_ext3 | external linkage | +| identifierlinkage.c:12:12:12:17 | g_int2 | internal linkage | +| identifierlinkage.c:15:12:15:12 | p | no linkage | +| identifierlinkage.c:16:7:16:13 | l_none1 | no linkage | +| identifierlinkage.c:17:14:17:20 | l_none2 | no linkage | +| identifierlinkage.c:18:14:18:19 | l_ext1 | external linkage | +| identifierlinkage.c:24:7:24:7 | m | no linkage | diff --git a/c/common/test/library/identifierlinkage/IdentifierLinkage.ql b/c/common/test/library/identifierlinkage/IdentifierLinkage.ql new file mode 100644 index 0000000000..37e5b4cd58 --- /dev/null +++ b/c/common/test/library/identifierlinkage/IdentifierLinkage.ql @@ -0,0 +1,5 @@ +import codingstandards.c.IdentifierLinkage + +from Variable v +where not v.getLocation().toString() = "file://:0:0:0:0" +select v, linkageOfVariable(v) diff --git a/c/common/test/library/identifierlinkage/identifierlinkage.c b/c/common/test/library/identifierlinkage/identifierlinkage.c new file mode 100644 index 0000000000..cf6b439797 --- /dev/null +++ b/c/common/test/library/identifierlinkage/identifierlinkage.c @@ -0,0 +1,30 @@ +// Simple external linkage +int g_ext1; +extern int g_ext2; + +// Simple internal linkage +static int g_int1; + +// Redefined maintaining linkage +int g_ext3; +extern int g_ext3; + +static int g_int2; +extern int g_int2; + +void f(int p) { + int l_none1; + static int l_none2; + extern int l_ext1; +} + +// Structs are not variables +struct s { + // Struct members are variables with no linkage. + int m; +}; + +// Enums and enum constants are not variables and have no linkage. +enum e { + E1 +}; \ No newline at end of file diff --git a/c/common/test/library/objects/ObjectIdentity.expected b/c/common/test/library/objects/ObjectIdentity.expected new file mode 100644 index 0000000000..34be1974f5 --- /dev/null +++ b/c/common/test/library/objects/ObjectIdentity.expected @@ -0,0 +1,21 @@ +| objectidentity.c:3:5:3:14 | g_statstg1 | static storage duration | file://:0:0:0:0 | int | +| objectidentity.c:4:12:4:21 | g_statstg2 | static storage duration | file://:0:0:0:0 | int | +| objectidentity.c:5:12:5:21 | g_statstg3 | static storage duration | file://:0:0:0:0 | int | +| objectidentity.c:8:13:8:22 | p_autostg1 | automatic storage duration | file://:0:0:0:0 | int | +| objectidentity.c:8:31:8:40 | l_autostg2 | automatic storage duration | file://:0:0:0:0 | int | +| objectidentity.c:12:14:12:23 | l_statstg1 | static storage duration | file://:0:0:0:0 | int | +| objectidentity.c:13:14:13:23 | l_statstg2 | static storage duration | file://:0:0:0:0 | int | +| objectidentity.c:17:15:17:24 | g_thrdstg1 | thread storage duration | file://:0:0:0:0 | int | +| objectidentity.c:18:22:18:31 | g_thrdstg2 | thread storage duration | file://:0:0:0:0 | int | +| objectidentity.c:19:22:19:31 | g_thrdstg3 | thread storage duration | file://:0:0:0:0 | int | +| objectidentity.c:21:24:21:33 | l_statstg3 | thread storage duration | file://:0:0:0:0 | int | +| objectidentity.c:22:24:22:33 | l_statstg4 | thread storage duration | file://:0:0:0:0 | int | +| objectidentity.c:35:11:35:20 | g_statstg4 | static storage duration | file://:0:0:0:0 | s * | +| objectidentity.c:35:35:35:37 | {...} | static storage duration | objectidentity.c:27:8:27:8 | s | +| objectidentity.c:35:36:35:36 | 0 | static storage duration | file://:0:0:0:0 | int | +| objectidentity.c:36:7:36:16 | g_statstg5 | static storage duration | file://:0:0:0:0 | char * | +| objectidentity.c:36:20:36:26 | hello | static storage duration | file://:0:0:0:0 | char[6] | +| objectidentity.c:38:3:38:3 | 1 | static storage duration | file://:0:0:0:0 | int | +| objectidentity.c:39:3:39:9 | hello | static storage duration | file://:0:0:0:0 | char[6] | +| objectidentity.c:40:3:40:15 | {...} | automatic storage duration | objectidentity.c:27:8:27:8 | s | +| objectidentity.c:40:14:40:14 | 1 | static storage duration | file://:0:0:0:0 | int | diff --git a/c/common/test/library/objects/ObjectIdentity.ql b/c/common/test/library/objects/ObjectIdentity.ql new file mode 100644 index 0000000000..28e6832bb2 --- /dev/null +++ b/c/common/test/library/objects/ObjectIdentity.ql @@ -0,0 +1,5 @@ +import codingstandards.c.Objects + +from ObjectIdentity obj +where obj.getFile().getBaseName() = "objectidentity.c" +select obj, obj.getStorageDuration(), obj.getType() \ No newline at end of file diff --git a/c/common/test/library/objects/objectidentity.c b/c/common/test/library/objects/objectidentity.c new file mode 100644 index 0000000000..066f68b1fd --- /dev/null +++ b/c/common/test/library/objects/objectidentity.c @@ -0,0 +1,41 @@ +#include "threads.h" +// Basic static storage duration +int g_statstg1; +extern int g_statstg2; +static int g_statstg3; + +// Basic automatic storage duration +void f1(int p_autostg1) { int l_autostg2; } + +// Block identifiers with static storage duration +void f2(void) { + extern int l_statstg1; + static int l_statstg2; +} + +// Thread storage duration +_Thread_local g_thrdstg1; +extern _Thread_local g_thrdstg2; +static _Thread_local g_thrdstg3; +void f3() { + extern _Thread_local l_statstg3; + static _Thread_local l_statstg4; +} + +// Struct declarations do not allocate storage, and do not have a storage +// duration. +struct s { + int m; +}; + +// Enums and enum constants are not variables and have no linkage. +enum e { E1 }; + +// Various literals: +struct s *g_statstg4 = &(struct s){0}; +char *g_statstg5 = "hello"; +void f4(void) { + 1; + "hello"; + (struct s){1}; +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.ql b/c/misra/src/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.ql index 5317966f3b..c65890f6bc 100644 --- a/c/misra/src/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.ql +++ b/c/misra/src/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.ql @@ -15,7 +15,7 @@ import cpp import codingstandards.c.misra -import codingstandards.cpp.lifetimes.CLifetimes +import codingstandards.c.Objects /** * Holds if the value of an expression is used or stored. @@ -59,11 +59,11 @@ Expr temporaryObjectFlowStep(Expr e) { } from - TemporaryLifetimeArrayAccess fa, TemporaryLifetimeExpr temporary, + FieldAccess fa, TemporaryObjectIdentity temporary, ArrayToPointerConversion conversion where not isExcluded(conversion, InvalidMemory3Package::arrayToPointerConversionOfTemporaryObjectQuery()) and - fa.getTemporary() = temporary and + fa = temporary.getASubobjectAccess() and conversion.getExpr() = fa and isUsedOrStored(temporaryObjectFlowStep*(conversion.getExpr())) select conversion, "Array to pointer conversion of array $@ from temporary object $@.", diff --git a/c/misra/src/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.ql b/c/misra/src/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.ql index f8a341b9bd..e10b478ee7 100644 --- a/c/misra/src/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.ql +++ b/c/misra/src/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.ql @@ -15,45 +15,31 @@ import cpp import codingstandards.c.misra -import codingstandards.cpp.lifetimes.CLifetimes +import codingstandards.c.Objects +import codeql.util.Boolean -class TemporaryLifetimeArrayExpr extends ArrayExpr { - TemporaryLifetimeArrayAccess member; - Type elementType; - - TemporaryLifetimeArrayExpr() { - member = getArrayBase() and - elementType = member.getType().(ArrayType).getBaseType() - or - exists(TemporaryLifetimeArrayExpr inner | - inner = getArrayBase() and - member = inner.getMember() and - elementType = inner.getElementType().(ArrayType).getBaseType() - ) - } - - TemporaryLifetimeArrayAccess getMember() { result = member } - - Type getElementType() { result = elementType } -} - -predicate usedAsModifiableLvalue(Expr expr) { +predicate usedAsModifiableLvalue(Expr expr, Boolean allowArrayAccess) { exists(Assignment parent | parent.getLValue() = expr) or exists(CrementOperation parent | parent.getOperand() = expr) or exists(AddressOfExpr parent | parent.getOperand() = expr) or - exists(FieldAccess parent | parent.getQualifier() = expr and usedAsModifiableLvalue(parent)) + // Don't report `x.y[0].m[0]++` twice. Recurse with `allowArrayAccess` set to false. + exists(FieldAccess parent | parent.getQualifier() = expr and usedAsModifiableLvalue(parent, false)) + or + allowArrayAccess = true and + exists(ArrayExpr parent | parent.getArrayBase() = expr and usedAsModifiableLvalue(parent, true)) } -from TemporaryLifetimeArrayExpr expr, TemporaryLifetimeArrayAccess member +from ArrayExpr expr, FieldAccess fieldAccess, TemporaryObjectIdentity tempObject where not isExcluded(expr, InvalidMemory3Package::modifiableLValueSubscriptedWithTemporaryLifetimeQuery()) and - member = expr.getMember() and + expr = tempObject.getASubobjectAccess() and + fieldAccess = expr.getArrayBase() and not expr.isUnevaluated() and - usedAsModifiableLvalue(expr) + usedAsModifiableLvalue(expr, true) select expr, "Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ ", - member, member.getTarget().getName(), member.getTemporary(), member.getTemporary().toString() + fieldAccess, fieldAccess.getTarget().getName(), tempObject, tempObject.toString() diff --git a/c/misra/src/rules/RULE-22-12/NonstandardUseOfThreadingObject.ql b/c/misra/src/rules/RULE-22-12/NonstandardUseOfThreadingObject.ql new file mode 100644 index 0000000000..d92b4ccea6 --- /dev/null +++ b/c/misra/src/rules/RULE-22-12/NonstandardUseOfThreadingObject.ql @@ -0,0 +1,54 @@ +/** + * @id c/misra/nonstandard-use-of-threading-object + * @name RULE-22-12: Standard library threading objects (mutexes, threads, etc.) shall only be accessed by the appropriate Standard Library functions + * @description Thread objects, thread synchronization objects, and thread-specific storage pointers + * shall only be accessed by the appropriate Standard Library functions. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-22-12 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Concurrency +import codingstandards.cpp.Type + +predicate isThreadingObject(Type t) { t instanceof PossiblySpecified::Type } + +predicate validUseOfStdThreadObject(Expr e) { + e.getParent() instanceof AddressOfExpr + or + exists(Call c | + c.getTarget().hasName(["tss_get", "tss_set", "tss_delete"]) and + e = c.getArgument(0) + ) +} + +predicate isStdThreadObjectPtr(Type t) { isThreadingObject(t.(PointerType).getBaseType()) } + +predicate invalidStdThreadObjectUse(Expr e) { + // Invalid use of mtx_t, etc. + isThreadingObject(e.getType()) and + not validUseOfStdThreadObject(e) + or + // Invalid cast from mtx_t* to void*, etc. + isStdThreadObjectPtr(e.getType()) and + exists(Cast cast | + cast.getExpr() = e and + not isStdThreadObjectPtr(cast.getType()) + ) +} + +from Expr e +where + not isExcluded(e, Concurrency8Package::nonstandardUseOfThreadingObjectQuery()) and + invalidStdThreadObjectUse(e) and + // Deduplicate results: (mtx = mtx) is an expression of mtx type, but don't flag the equality + // check, only flag the two `mtx` references. + not invalidStdThreadObjectUse(e.getAChild+()) +select e, "Invalid usage of standard thread object type '" + e.getType().toString() + "'." diff --git a/c/misra/src/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.ql b/c/misra/src/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.ql new file mode 100644 index 0000000000..066cf3c295 --- /dev/null +++ b/c/misra/src/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.ql @@ -0,0 +1,31 @@ +/** + * @id c/misra/threading-object-with-invalid-storage-duration + * @name RULE-22-13: Threading objects (mutexes, threads, etc). shall have not have automatic or thread storage duration + * @description Thread objects, thread synchronization objects, and thread specific storage pointers + * shall have appropriate storage duration. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-22-13 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.Objects +import codingstandards.cpp.Concurrency +import codingstandards.cpp.Type + +from ObjectIdentity obj, StorageDuration storageDuration, Type type +where + not isExcluded(obj, Concurrency8Package::threadingObjectWithInvalidStorageDurationQuery()) and + storageDuration = obj.getStorageDuration() and + not storageDuration.isStatic() and + type = obj.getASubObjectType() and + type instanceof PossiblySpecified::Type +select obj, + "Object of type '" + obj.getType().getName() + "' has invalid storage duration type '" + + storageDuration.getStorageTypeName() + "'." diff --git a/c/misra/src/rules/RULE-22-14/MutexInitWithInvalidMutexType.ql b/c/misra/src/rules/RULE-22-14/MutexInitWithInvalidMutexType.ql new file mode 100644 index 0000000000..a122a0bec4 --- /dev/null +++ b/c/misra/src/rules/RULE-22-14/MutexInitWithInvalidMutexType.ql @@ -0,0 +1,36 @@ +/** + * @id c/misra/mutex-init-with-invalid-mutex-type + * @name RULE-22-14: Mutexes shall be initialized with a valid mutex type + * @description Mutexes shall be initialized with a valid mutex type. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-14 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Concurrency + +predicate isBaseMutexType(EnumConstantAccess access) { + access.getTarget().hasName(["mtx_plain", "mtx_timed"]) +} + +predicate isValidMutexType(Expr expr) { + isBaseMutexType(expr) + or + exists(BinaryBitwiseOperation binOr | binOr = expr | + isBaseMutexType(binOr.getLeftOperand()) and + binOr.getRightOperand().(EnumConstantAccess).getTarget().hasName("mtx_recursive") + ) +} + +from C11MutexSource init +where + not isExcluded(init, Concurrency8Package::mutexInitWithInvalidMutexTypeQuery()) and + not isValidMutexType(init.getMutexTypeExpr()) +select init, "Mutex initialized with incorrect type expression." diff --git a/c/misra/src/rules/RULE-22-14/MutexInitializedInsideThread.ql b/c/misra/src/rules/RULE-22-14/MutexInitializedInsideThread.ql new file mode 100644 index 0000000000..649c941e66 --- /dev/null +++ b/c/misra/src/rules/RULE-22-14/MutexInitializedInsideThread.ql @@ -0,0 +1,27 @@ +/** + * @id c/misra/mutex-initialized-inside-thread + * @name RULE-22-14: Thread synchronization objects shall be initialized deterministically + * @description Mutex and condition objects initialized inside of threads may result in + * indeterministic state + * @kind problem + * @precision high + * @problem.severity recommendation + * @tags external/misra/id/rule-22-14 + * readability + * maintainability + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Concurrency + +from C11MutexSource mutexCreate, ThreadedFunction thread +where + not isExcluded(mutexCreate, Concurrency8Package::mutexInitializedInsideThreadQuery()) and + thread.calls*(mutexCreate.getEnclosingFunction()) +select +mutexCreate, "Mutex initialization reachable from threaded function '$@'.", +thread, thread.getName() diff --git a/c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql b/c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql new file mode 100644 index 0000000000..ed4226c207 --- /dev/null +++ b/c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql @@ -0,0 +1,130 @@ +/** + * @id c/misra/mutex-not-initialized-before-use + * @name RULE-22-14: Thread synchronization objects shall be initialized before being accessed + * @description Mutex and condition objects shall be initialized with the standard library functions + * before using them. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-14 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.Objects +import codingstandards.cpp.Concurrency +import codingstandards.cpp.Type + +/** A function which is not called or started as a thread */ +class RootFunction extends Function { + RootFunction() { + not exists(Function f | f.calls(this)) and + not this instanceof ThreadedFunction + } +} + +/** A function call which initializes a mutex or a condition */ +class ThreadObjectInitialization extends FunctionCall { + ObjectIdentity owningObject; + + ThreadObjectInitialization() { + this.(C11MutexSource).getMutexExpr() = owningObject.getASubobjectAddressExpr() + or + exists(CConditionOperation condOp | + this = condOp and + condOp.isInit() and + condOp.getConditionExpr() = owningObject.getASubobjectAddressExpr() + ) + } + + ObjectIdentity getOwningObject() { result = owningObject } +} + +/** + * A function argument where that argument is used as a mutex or condition object. + */ +class ThreadObjectUse extends Expr { + ObjectIdentity owningObject; + string typeString; + ThreadObjectUse() { + owningObject.getASubobjectAddressExpr() = this and + ( + exists(CMutexFunctionCall mutexUse | this = mutexUse.getLockExpr()) and + typeString = "Mutex" + or + exists(CConditionOperation condOp | this = condOp.getMutexExpr()) and + typeString = "Mutex" + or + exists(CConditionOperation condOp | + condOp.isUse() and + this = condOp.getConditionExpr() and + typeString = "Condition" + ) + ) + } + + ObjectIdentity getOwningObject() { result = owningObject } + + string getDescription() { + if + getOwningObject().getType() instanceof PossiblySpecified::Type or + getOwningObject().getType() instanceof PossiblySpecified::Type + then result = typeString + else result = typeString + " in object" + } +} + + +predicate requiresInitializedMutexObject( + Function func, ThreadObjectUse mutexUse, ObjectIdentity owningObject +) { + mutexUse.getEnclosingFunction() = func and + owningObject = mutexUse.getOwningObject() and + not exists(ThreadObjectInitialization init | + init.getEnclosingFunction() = func and + init.getOwningObject() = owningObject and + mutexUse.getAPredecessor+() = init + ) + or + exists(FunctionCall call | + func = call.getEnclosingFunction() and + requiresInitializedMutexObject(call.getTarget(), mutexUse, owningObject) and + not exists(ThreadObjectInitialization init | + call.getAPredecessor*() = init and + init.getOwningObject() = owningObject + ) + ) + or + exists(C11ThreadCreateCall call | + func = call.getEnclosingFunction() and + not owningObject.getStorageDuration().isThread() and + requiresInitializedMutexObject(call.getFunction(), mutexUse, owningObject) and + not exists(ThreadObjectInitialization init | + call.getAPredecessor*() = init and + init.getOwningObject() = owningObject + ) + ) +} + +from ThreadObjectUse objUse, ObjectIdentity obj, Function callRoot +where + not isExcluded(objUse, Concurrency8Package::mutexNotInitializedBeforeUseQuery()) and + obj = objUse.getOwningObject() and + requiresInitializedMutexObject(callRoot, objUse, obj) and + ( + if obj.getStorageDuration().isAutomatic() + then obj.getEnclosingElement+() = callRoot + else ( + obj.getStorageDuration().isThread() and callRoot instanceof ThreadedFunction + or + callRoot instanceof RootFunction + ) + ) +select objUse, + objUse.getDescription() + + " '$@' possibly used before initialization, from entry point function '$@'.", obj, + obj.toString(), callRoot, callRoot.getName() diff --git a/c/misra/src/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.ql b/c/misra/src/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.ql new file mode 100644 index 0000000000..c86c9b9d9d --- /dev/null +++ b/c/misra/src/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.ql @@ -0,0 +1,52 @@ +/** + * @id c/misra/mutex-objects-not-always-unlocked + * @name RULE-22-16: All mutex objects locked by a thread shall be explicitly unlocked by the same thread + * @description Mutex not unlocked by thread on all execution paths in current thread after being + * locked. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-16 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Concurrency +import codingstandards.cpp.resources.ResourceLeakAnalysis + +module MutexLeakConfig implements ResourceLeakConfigSig { + predicate isAllocate(ControlFlowNode allocPoint, DataFlow::Node node) { + exists(MutexFunctionCall lock | + allocPoint = lock and + lock.isLock() and + node.asDefiningArgument() = lock.getLockExpr() + ) + } + + predicate isFree(ControlFlowNode node, DataFlow::Node resource) { + exists(MutexFunctionCall mfc | + node = mfc and + mfc.isUnlock() and + mfc.getLockExpr() = resource.asExpr() + ) + } +} + +string describeMutex(Expr mutexExpr) { + if mutexExpr instanceof AddressOfExpr + then result = mutexExpr.(AddressOfExpr).getOperand().toString() + else result = mutexExpr.toString() +} + +from MutexFunctionCall lockCall, string mutexDescription +where + not isExcluded(lockCall, Concurrency8Package::mutexObjectsNotAlwaysUnlockedQuery()) and + //lockCall.getLockExpr() = mutexNode.asDefiningArgument() and + exists(ResourceLeak::getALeak(lockCall)) and + mutexDescription = describeMutex(lockCall.getLockExpr()) +select lockCall, + "Mutex '" + mutexDescription + "' is locked here and may not always be subsequently unlocked." diff --git a/c/misra/test/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.expected b/c/misra/test/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.expected index 688dde4650..34bec03490 100644 --- a/c/misra/test/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.expected +++ b/c/misra/test/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.expected @@ -21,10 +21,10 @@ | test.c:65:15:65:32 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:65:15:65:20 | call to get_s1 | call to get_s1 | | test.c:66:16:66:33 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:66:16:66:21 | call to get_s1 | call to get_s1 | | test.c:67:23:67:40 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:67:23:67:28 | call to get_s1 | call to get_s1 | -| test.c:89:3:89:30 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:89:12:89:20 | member_s1 | member_s1 | -| test.c:90:3:90:36 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:90:3:90:26 | access to array | access to array | -| test.c:91:15:91:42 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:91:24:91:32 | member_s1 | member_s1 | -| test.c:92:15:92:48 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:92:15:92:38 | access to array | access to array | +| test.c:89:3:89:30 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:89:3:89:8 | call to get_s2 | call to get_s2 | +| test.c:90:3:90:36 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:90:3:90:8 | call to get_s2 | call to get_s2 | +| test.c:91:15:91:42 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:91:15:91:20 | call to get_s2 | call to get_s2 | +| test.c:92:15:92:48 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:92:15:92:20 | call to get_s2 | call to get_s2 | | test.c:111:15:111:33 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:111:16:111:22 | ... = ... | ... = ... | | test.c:113:15:113:37 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:113:16:113:26 | ... ? ... : ... | ... ? ... : ... | | test.c:114:15:114:31 | array to pointer conversion | Array to pointer conversion of array $@ from temporary object $@. | test.c:3:13:3:21 | const_arr | const_arr | test.c:114:16:114:20 | ... , ... | ... , ... | diff --git a/c/misra/test/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.expected b/c/misra/test/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.expected index ae140dcd59..19604e2092 100644 --- a/c/misra/test/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.expected +++ b/c/misra/test/rules/RULE-18-9/ModifiableLValueSubscriptedWithTemporaryLifetime.expected @@ -3,13 +3,13 @@ | test.c:82:3:82:17 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:82:12:82:14 | arr | arr | test.c:82:3:82:8 | call to get_s1 | call to get_s1 | | test.c:83:3:83:17 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:83:12:83:14 | arr | arr | test.c:83:3:83:8 | call to get_s1 | call to get_s1 | | test.c:84:5:84:19 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:84:14:84:16 | arr | arr | test.c:84:5:84:10 | call to get_s1 | call to get_s1 | -| test.c:93:3:93:27 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:93:22:93:24 | arr | arr | test.c:93:12:93:20 | member_s1 | member_s1 | -| test.c:94:3:94:27 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:94:22:94:24 | arr | arr | test.c:94:3:94:20 | access to array | access to array | +| test.c:93:3:93:27 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:93:22:93:24 | arr | arr | test.c:93:3:93:8 | call to get_s2 | call to get_s2 | +| test.c:94:3:94:27 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:94:22:94:24 | arr | arr | test.c:94:3:94:8 | call to get_s2 | call to get_s2 | | test.c:137:3:137:23 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:137:12:137:20 | arr_union | arr_union | test.c:137:3:137:8 | call to get_s3 | call to get_s3 | | test.c:138:3:138:24 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:138:12:138:21 | arr_struct | arr_struct | test.c:138:3:138:8 | call to get_s3 | call to get_s3 | | test.c:139:3:139:24 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:139:12:139:21 | arr_struct | arr_struct | test.c:139:3:139:8 | call to get_s3 | call to get_s3 | | test.c:140:3:140:24 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:140:12:140:21 | arr_struct | arr_struct | test.c:140:3:140:8 | call to get_s3 | call to get_s3 | | test.c:141:3:141:24 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:141:12:141:21 | arr_struct | arr_struct | test.c:141:3:141:8 | call to get_s3 | call to get_s3 | | test.c:142:4:142:25 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:142:13:142:22 | arr_struct | arr_struct | test.c:142:4:142:9 | call to get_s3 | call to get_s3 | -| test.c:146:3:146:22 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:146:12:146:16 | arr2d | arr2d | test.c:146:3:146:8 | call to get_s3 | call to get_s3 | +| test.c:146:3:146:19 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:146:12:146:16 | arr2d | arr2d | test.c:146:3:146:8 | call to get_s3 | call to get_s3 | | test.c:147:4:147:20 | access to array | Modifiable lvalue produced by subscripting array member $@ of temporary lifetime object $@ | test.c:147:13:147:17 | arr2d | arr2d | test.c:147:4:147:9 | call to get_s3 | call to get_s3 | diff --git a/c/misra/test/rules/RULE-22-12/NonstandardUseOfThreadingObject.expected b/c/misra/test/rules/RULE-22-12/NonstandardUseOfThreadingObject.expected new file mode 100644 index 0000000000..62a740f960 --- /dev/null +++ b/c/misra/test/rules/RULE-22-12/NonstandardUseOfThreadingObject.expected @@ -0,0 +1,27 @@ +| test.c:37:3:37:7 | mutex | Invalid usage of standard thread object type 'mtx_t'. | +| test.c:37:11:37:21 | * ... | Invalid usage of standard thread object type 'mtx_t'. | +| test.c:38:3:38:8 | thread | Invalid usage of standard thread object type 'thrd_t'. | +| test.c:38:12:38:23 | * ... | Invalid usage of standard thread object type 'thrd_t'. | +| test.c:39:3:39:13 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:39:17:39:27 | * ... | Invalid usage of standard thread object type 'tss_t'. | +| test.c:40:3:40:11 | condition | Invalid usage of standard thread object type 'cnd_t'. | +| test.c:40:15:40:25 | * ... | Invalid usage of standard thread object type 'cnd_t'. | +| test.c:43:10:43:15 | & ... | Invalid usage of standard thread object type 'mtx_t *'. | +| test.c:44:10:44:16 | & ... | Invalid usage of standard thread object type 'thrd_t *'. | +| test.c:45:10:45:21 | & ... | Invalid usage of standard thread object type 'tss_t *'. | +| test.c:46:10:46:19 | & ... | Invalid usage of standard thread object type 'cnd_t *'. | +| test.c:48:3:48:13 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:49:3:49:13 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:50:3:50:13 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:54:3:54:8 | thread | Invalid usage of standard thread object type 'thrd_t'. | +| test.c:54:13:54:18 | thread | Invalid usage of standard thread object type 'thrd_t'. | +| test.c:55:3:55:8 | thread | Invalid usage of standard thread object type 'thrd_t'. | +| test.c:55:13:55:18 | thread | Invalid usage of standard thread object type 'thrd_t'. | +| test.c:56:3:56:13 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:56:18:56:28 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:57:3:57:13 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:57:18:57:28 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:61:14:61:18 | mutex | Invalid usage of standard thread object type 'mtx_t'. | +| test.c:62:15:62:20 | thread | Invalid usage of standard thread object type 'thrd_t'. | +| test.c:63:20:63:30 | threadlocal | Invalid usage of standard thread object type 'tss_t'. | +| test.c:64:18:64:26 | condition | Invalid usage of standard thread object type 'cnd_t'. | diff --git a/c/misra/test/rules/RULE-22-12/NonstandardUseOfThreadingObject.qlref b/c/misra/test/rules/RULE-22-12/NonstandardUseOfThreadingObject.qlref new file mode 100644 index 0000000000..d1d345420d --- /dev/null +++ b/c/misra/test/rules/RULE-22-12/NonstandardUseOfThreadingObject.qlref @@ -0,0 +1 @@ +rules/RULE-22-12/NonstandardUseOfThreadingObject.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-12/test.c b/c/misra/test/rules/RULE-22-12/test.c new file mode 100644 index 0000000000..1c59c43d8c --- /dev/null +++ b/c/misra/test/rules/RULE-22-12/test.c @@ -0,0 +1,65 @@ +#include "string.h" +#include "threads.h" + +mtx_t mutex; +thrd_t thread; +tss_t threadlocal; +cnd_t condition; + +extern void use_mutex(mtx_t *m); +extern void use_thread(thrd_t *t); +extern void use_threadlocal(tss_t *t); +extern void use_condition(cnd_t *c); + +void valid_usages(void) { + mtx_init(&mutex, mtx_plain); // COMPLIANT + mtx_lock(&mutex); // COMPLIANT + mtx_unlock(&mutex); // COMPLIANT + thrd_create(&thread, NULL, NULL); // COMPLIANT + tss_create(&threadlocal, NULL); // COMPLIANT + tss_set(threadlocal, NULL); // COMPLIANT + cnd_init(&condition); // COMPLIANT + cnd_signal(&condition); // COMPLIANT + cnd_wait(&condition, &mutex); // COMPLIANT + + use_mutex(&mutex); // COMPLIANT + use_thread(&thread); // COMPLIANT + use_threadlocal(&threadlocal); // COMPLIANT + use_condition(&condition); // COMPLIANT +} + +extern void copy_mutex(mtx_t m); +extern void copy_thread(thrd_t t); +extern void copy_threadlocal(tss_t t); +extern void copy_condition(cnd_t t); + +void invalid_usages(void) { + mutex = *(mtx_t *)0; // NON-COMPLIANT + thread = *(thrd_t *)0; // NON-COMPLIANT + threadlocal = *(tss_t *)0; // NON-COMPLIANT + condition = *(cnd_t *)0; // NON-COMPLIANT + + int *buf; + memcpy(&mutex, buf, sizeof(mtx_t)); // NON-COMPLIANT + memcpy(&thread, buf, sizeof(thrd_t)); // NON-COMPLIANT + memcpy(&threadlocal, buf, sizeof(tss_t)); // NON-COMPLIANT + memcpy(&condition, buf, sizeof(cnd_t)); // NON-COMPLIANT + + threadlocal++; // NON-COMPLIANT + threadlocal += 1; // NON-COMPLIANT + threadlocal + 1; // NON-COMPLIANT + + // mutex == mutex; // NON-COMPLIANT + // mutex != mutex; // NON-COMPLIANT + thread == thread; // NON-COMPLIANT + thread != thread; // NON-COMPLIANT + threadlocal == threadlocal; // NON-COMPLIANT + threadlocal != threadlocal; // NON-COMPLIANT + // condition == condition; // NON-COMPLIANT + // condition != condition; // NON-COMPLIANT + + copy_mutex(mutex); // COMPLIANT + copy_thread(thread); // COMPLIANT + copy_threadlocal(threadlocal); // COMPLIANT + copy_condition(condition); // COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.expected b/c/misra/test/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.expected new file mode 100644 index 0000000000..0fe033366d --- /dev/null +++ b/c/misra/test/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.expected @@ -0,0 +1,13 @@ +| test.c:14:21:14:22 | g9 | Object of type 'mtx_t' has invalid storage duration type 'thread'. | +| test.c:15:22:15:24 | g10 | Object of type 'thrd_t' has invalid storage duration type 'thread'. | +| test.c:16:21:16:23 | g11 | Object of type 'tss_t' has invalid storage duration type 'thread'. | +| test.c:17:21:17:23 | g12 | Object of type 'cnd_t' has invalid storage duration type 'thread'. | +| test.c:34:9:34:10 | l1 | Object of type 'mtx_t' has invalid storage duration type 'automatic'. | +| test.c:35:10:35:11 | l2 | Object of type 'thrd_t' has invalid storage duration type 'automatic'. | +| test.c:36:9:36:10 | l3 | Object of type 'tss_t' has invalid storage duration type 'automatic'. | +| test.c:37:9:37:10 | l4 | Object of type 'cnd_t' has invalid storage duration type 'automatic'. | +| test.c:44:9:44:10 | l9 | Object of type 'mtx_t[10]' has invalid storage duration type 'automatic'. | +| test.c:46:13:46:15 | l11 | Object of type 'has_mtx_t' has invalid storage duration type 'automatic'. | +| test.c:48:13:48:15 | l13 | Object of type 'has_mtx_t[10]' has invalid storage duration type 'automatic'. | +| test.c:51:9:51:14 | call to malloc | Object of type 'mtx_t' has invalid storage duration type 'allocated'. | +| test.c:52:9:52:14 | call to malloc | Object of type 'mtx_t' has invalid storage duration type 'allocated'. | diff --git a/c/misra/test/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.qlref b/c/misra/test/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.qlref new file mode 100644 index 0000000000..9c054fc623 --- /dev/null +++ b/c/misra/test/rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.qlref @@ -0,0 +1 @@ +rules/RULE-22-13/ThreadingObjectWithInvalidStorageDuration.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-13/test.c b/c/misra/test/rules/RULE-22-13/test.c new file mode 100644 index 0000000000..0a5a84167d --- /dev/null +++ b/c/misra/test/rules/RULE-22-13/test.c @@ -0,0 +1,53 @@ +#include "stdlib.h" +#include "threads.h" + +mtx_t g1; // COMPLIANT +thrd_t g2; // COMPLIANT +tss_t g3; // COMPLIANT +cnd_t g4; // COMPLIANT + +static mtx_t g5; // COMPLIANT +static thrd_t g6; // COMPLIANT +static tss_t g7; // COMPLIANT +static cnd_t g8; // COMPLIANT + +_Thread_local mtx_t g9; // NON-COMPLIANT +_Thread_local thrd_t g10; // NON-COMPLIANT +_Thread_local tss_t g11; // NON-COMPLIANT +_Thread_local cnd_t g12; // NON-COMPLIANT + +typedef struct { + mtx_t m; +} has_mtx_t; + +typedef struct { + mtx_t *m; +} has_ptr_mtx_t; + +mtx_t g13[10]; // COMPLIANT +mtx_t *g14; // COMPLIANT +has_mtx_t g15; // COMPLIANT +has_ptr_mtx_t g16; // COMPLIANT +has_mtx_t g17[10]; // COMPLIANT + +void f1(void) { + mtx_t l1; // NON-COMPLIANT + thrd_t l2; // NON-COMPLIANT + tss_t l3; // NON-COMPLIANT + cnd_t l4; // NON-COMPLIANT + + static mtx_t l5; // COMPLIANT + static thrd_t l6; // COMPLIANT + static tss_t l7; // COMPLIANT + static cnd_t l8; // COMPLIANT + + mtx_t l9[10]; // NON-COMPLIANT + mtx_t *l10; // COMPLIANT + has_mtx_t l11; // NON-COMPLIANT + has_ptr_mtx_t l12; // COMPLIANT + has_mtx_t l13[10]; // NON-COMPLIANT + + l10 = &g1; // COMPLIANT + l10 = malloc(sizeof(mtx_t)); // NON-COMPLIANT + l10 = malloc(sizeof(mtx_t) * 4); // NON-COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-14/MutexInitWithInvalidMutexType.expected b/c/misra/test/rules/RULE-22-14/MutexInitWithInvalidMutexType.expected new file mode 100644 index 0000000000..1ea39b6b1d --- /dev/null +++ b/c/misra/test/rules/RULE-22-14/MutexInitWithInvalidMutexType.expected @@ -0,0 +1,5 @@ +| test.c:140:3:140:10 | call to mtx_init | Mutex initialized with incorrect type expression. | +| test.c:141:3:141:10 | call to mtx_init | Mutex initialized with incorrect type expression. | +| test.c:142:3:142:10 | call to mtx_init | Mutex initialized with incorrect type expression. | +| test.c:144:3:144:10 | call to mtx_init | Mutex initialized with incorrect type expression. | +| test.c:145:3:145:10 | call to mtx_init | Mutex initialized with incorrect type expression. | diff --git a/c/misra/test/rules/RULE-22-14/MutexInitWithInvalidMutexType.qlref b/c/misra/test/rules/RULE-22-14/MutexInitWithInvalidMutexType.qlref new file mode 100644 index 0000000000..32b57cfd07 --- /dev/null +++ b/c/misra/test/rules/RULE-22-14/MutexInitWithInvalidMutexType.qlref @@ -0,0 +1 @@ +rules/RULE-22-14/MutexInitWithInvalidMutexType.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-14/MutexInitializedInsideThread.expected b/c/misra/test/rules/RULE-22-14/MutexInitializedInsideThread.expected new file mode 100644 index 0000000000..360c02f622 --- /dev/null +++ b/c/misra/test/rules/RULE-22-14/MutexInitializedInsideThread.expected @@ -0,0 +1 @@ +| test.c:110:3:110:10 | call to mtx_init | Mutex initialization reachable from threaded function '$@'. | test.c:105:6:105:32 | from_root7_use_thread_local | from_root7_use_thread_local | diff --git a/c/misra/test/rules/RULE-22-14/MutexInitializedInsideThread.qlref b/c/misra/test/rules/RULE-22-14/MutexInitializedInsideThread.qlref new file mode 100644 index 0000000000..83ada06139 --- /dev/null +++ b/c/misra/test/rules/RULE-22-14/MutexInitializedInsideThread.qlref @@ -0,0 +1 @@ +rules/RULE-22-14/MutexInitializedInsideThread.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-14/MutexNotInitializedBeforeUse.expected b/c/misra/test/rules/RULE-22-14/MutexNotInitializedBeforeUse.expected new file mode 100644 index 0000000000..b9560165ce --- /dev/null +++ b/c/misra/test/rules/RULE-22-14/MutexNotInitializedBeforeUse.expected @@ -0,0 +1,14 @@ +| test.c:6:12:6:14 | & ... | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:5:9:5:10 | l1 | l1 | test.c:4:6:4:19 | use_local_mtxs | use_local_mtxs | +| test.c:13:12:13:16 | & ... | Mutex in object '$@' possibly used before initialization, from entry point function '$@'. | test.c:12:5:12:6 | l2 | l2 | test.c:4:6:4:19 | use_local_mtxs | use_local_mtxs | +| test.c:18:12:18:17 | & ... | Mutex in object '$@' possibly used before initialization, from entry point function '$@'. | test.c:17:9:17:10 | l3 | l3 | test.c:4:6:4:19 | use_local_mtxs | use_local_mtxs | +| test.c:23:12:23:13 | l4 | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:22:15:22:20 | call to malloc | call to malloc | test.c:28:6:28:31 | root1_calls_use_local_mtxs | root1_calls_use_local_mtxs | +| test.c:41:12:41:14 | & ... | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:34:7:34:8 | g1 | g1 | test.c:40:6:40:30 | root2_uses_global_mutexes | root2_uses_global_mutexes | +| test.c:42:12:42:17 | & ... | Mutex in object '$@' possibly used before initialization, from entry point function '$@'. | test.c:37:3:37:4 | g2 | g2 | test.c:40:6:40:30 | root2_uses_global_mutexes | root2_uses_global_mutexes | +| test.c:43:12:43:13 | g3 | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:56:8:56:13 | call to malloc | call to malloc | test.c:40:6:40:30 | root2_uses_global_mutexes | root2_uses_global_mutexes | +| test.c:64:12:64:14 | & ... | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:34:7:34:8 | g1 | g1 | test.c:67:6:67:45 | root4_call_thread_without_initialization | root4_call_thread_without_initialization | +| test.c:88:12:88:14 | & ... | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:87:9:87:10 | l1 | l1 | test.c:86:6:86:36 | island1_use_uninitialized_mutex | island1_use_uninitialized_mutex | +| test.c:100:12:100:14 | & ... | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:98:21:98:22 | g5 | g5 | test.c:99:6:99:27 | root6_use_thread_local | root6_use_thread_local | +| test.c:107:12:107:14 | & ... | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:98:21:98:22 | g5 | g5 | test.c:105:6:105:32 | from_root7_use_thread_local | from_root7_use_thread_local | +| test.c:124:12:124:13 | & ... | Condition '$@' possibly used before initialization, from entry point function '$@'. | test.c:122:9:122:9 | c | c | test.c:121:6:121:28 | root8_uninitialized_cnd | root8_uninitialized_cnd | +| test.c:124:16:124:17 | & ... | Mutex '$@' possibly used before initialization, from entry point function '$@'. | test.c:123:9:123:9 | m | m | test.c:121:6:121:28 | root8_uninitialized_cnd | root8_uninitialized_cnd | +| test.c:127:12:127:13 | & ... | Condition '$@' possibly used before initialization, from entry point function '$@'. | test.c:122:9:122:9 | c | c | test.c:121:6:121:28 | root8_uninitialized_cnd | root8_uninitialized_cnd | diff --git a/c/misra/test/rules/RULE-22-14/MutexNotInitializedBeforeUse.qlref b/c/misra/test/rules/RULE-22-14/MutexNotInitializedBeforeUse.qlref new file mode 100644 index 0000000000..2827a9c571 --- /dev/null +++ b/c/misra/test/rules/RULE-22-14/MutexNotInitializedBeforeUse.qlref @@ -0,0 +1 @@ +rules/RULE-22-14/MutexNotInitializedBeforeUse.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-14/test.c b/c/misra/test/rules/RULE-22-14/test.c new file mode 100644 index 0000000000..d8f1770ad8 --- /dev/null +++ b/c/misra/test/rules/RULE-22-14/test.c @@ -0,0 +1,146 @@ +#include "stdlib.h" +#include "threads.h" + +void use_local_mtxs(int x, int y) { + mtx_t l1; + mtx_lock(&l1); // NON-COMPLIANT + mtx_init(&l1, mtx_plain); + mtx_lock(&l1); // COMPLIANT + + struct { + mtx_t m; + } l2; + mtx_lock(&l2.m); // NON-COMPLIANT + mtx_init(&l2.m, mtx_plain); + mtx_lock(&l2.m); // COMPLIANT + + mtx_t l3[10]; + mtx_lock(&l3[y]); // NON-COMPLIANT + mtx_init(&l3[x], mtx_plain); + mtx_lock(&l3[y]); // COMPLIANT + + mtx_t *l4 = malloc(sizeof(mtx_t)); + mtx_lock(l4); // NON-COMPLIANT + mtx_init(l4, mtx_plain); + mtx_lock(l4); // COMPLIANT +} + +void root1_calls_use_local_mtxs() { + // Since a function exists which calls use_local_mtxs(), that function is not + // a root function. The query should still report unused locals in this case. + use_local_mtxs(1, 2); +} + +mtx_t g1; +struct { + mtx_t m1; +} g2; +mtx_t *g3; + +void root2_uses_global_mutexes() { + mtx_lock(&g1); // NON-COMPLIANT + mtx_lock(&g2.m1); // NON-COMPLIANT + mtx_lock(g3); // NON-COMPLIANT +} + +void from_root3_use_global_mutexes() { + mtx_lock(&g1); // COMPLIANT + mtx_lock(&g2.m1); // COMPLIANT + mtx_lock(g3); // COMPLIANT +} + +void root3_initializes_and_uses_global_mutexes() { + // Init global mutex with an allocated storage duration object. The existence + // of this malloc() is not checked by the query, but if its exists, the object + // and its uses should be trackable as a nice bonus. + g3 = malloc(sizeof(mtx_t)); + mtx_init(&g1, mtx_plain); + mtx_init(&g2.m1, mtx_plain); + mtx_init(g3, mtx_plain); + from_root3_use_global_mutexes(); +} + +void from_root4_use_global_mutex(void *arg) { + mtx_lock(&g1); // NON-COMPLIANT +} + +void root4_call_thread_without_initialization() { + thrd_t t; + thrd_create(&t, &from_root4_use_global_mutex, NULL); +} + +void from_root5_use_global_mutex(void *arg) { + mtx_lock(&g1); // COMPLIANT +} + +void root5_thread_with_initialization() { + mtx_init(&g1, mtx_plain); + thrd_t t; + thrd_create(&t, &from_root5_use_global_mutex, NULL); +} + +// Set up two functions such that a calls b and b calls a. This means there is +// no root function, but we should still report unused locals. +void island2_call_island1(); + +void island1_use_uninitialized_mutex() { + mtx_t l1; + mtx_lock(&l1); // NON-COMPLIANT + + // Globals are hard to detect + mtx_lock(&g1); // NON-COMPLIANT[False negative] + + island2_call_island1(); +} + +void island2_call_island1() { island1_use_uninitialized_mutex(); } + +_Thread_local mtx_t g5; +void root6_use_thread_local() { + mtx_lock(&g5); // NON-COMPLIANT + mtx_init(&g5, mtx_plain); + mtx_lock(&g5); // COMPLIANT +} + +void from_root7_use_thread_local() { + // Invalid, thread local g5 hasn't been initialized in this thread. + mtx_lock(&g5); // NON-COMPLIANT + + // Violates recommendation, mutexes initialized within a thread. + mtx_init(&g5, mtx_plain); // NON-COMPLIANT + + // Valid if we except the above initialization. + mtx_lock(&g5); // COMPLIANT +} + +void root7_spawn_thread_uninitialized_thread_local() { + thrd_t t; + thrd_create(&t, &from_root7_use_thread_local, NULL); +} + +void root8_uninitialized_cnd() { + cnd_t c; + mtx_t m; + cnd_wait(&c, &m); // NON-COMPLIANT + + mtx_init(&m, mtx_plain); + cnd_wait(&c, &m); // NON-COMPLIANT + + cnd_init(&c); + cnd_wait(&c, &m); // COMPLIANT +} + +void invalid_mtx_init_types() { + mtx_t m; + mtx_init(&m, mtx_plain); // COMPLIANT + mtx_init(&m, mtx_plain | mtx_recursive); // COMPLIANT + mtx_init(&m, mtx_timed); // COMPLIANT + mtx_init(&m, mtx_timed | mtx_recursive); // COMPLIANT + + mtx_init(&m, mtx_recursive); // NON-COMPLIANT + mtx_init(&m, mtx_plain | mtx_timed); // NON-COMPLIANT + mtx_init(&m, mtx_plain | mtx_plain); // NON-COMPLIANT + mtx_init(&m, mtx_plain & mtx_recursive); // NON-COMPLIANT + mtx_init(&m, mtx_plain * mtx_recursive); // NON-COMPLIANT + mtx_init(&m, -1); // NON-COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.expected b/c/misra/test/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.expected new file mode 100644 index 0000000000..126e75bca7 --- /dev/null +++ b/c/misra/test/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.expected @@ -0,0 +1,7 @@ +| test.c:16:3:16:10 | call to mtx_lock | Mutex 'm' is locked here and may not always be subsequently unlocked. | +| test.c:21:3:21:10 | call to mtx_lock | Mutex 'm' is locked here and may not always be subsequently unlocked. | +| test.c:39:3:39:10 | call to mtx_lock | Mutex 'm' is locked here and may not always be subsequently unlocked. | +| test.c:55:3:55:10 | call to mtx_lock | Mutex 'm' is locked here and may not always be subsequently unlocked. | +| test.c:72:3:72:10 | call to mtx_lock | Mutex 'g1' is locked here and may not always be subsequently unlocked. | +| test.c:79:3:79:10 | call to mtx_lock | Mutex 'm' is locked here and may not always be subsequently unlocked. | +| test.c:101:5:101:12 | call to mtx_lock | Mutex 'm' is locked here and may not always be subsequently unlocked. | diff --git a/c/misra/test/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.qlref b/c/misra/test/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.qlref new file mode 100644 index 0000000000..a1877f944d --- /dev/null +++ b/c/misra/test/rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.qlref @@ -0,0 +1 @@ +rules/RULE-22-16/MutexObjectsNotAlwaysUnlocked.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-16/test.c b/c/misra/test/rules/RULE-22-16/test.c new file mode 100644 index 0000000000..b9a19165e8 --- /dev/null +++ b/c/misra/test/rules/RULE-22-16/test.c @@ -0,0 +1,107 @@ +#include "threads.h" + +void f1() { + mtx_t m; + mtx_lock(&m); // COMPLIANT + mtx_unlock(&m); +} + +void f2() { + mtx_t m; + mtx_unlock(&m); // COMPLIANT +} + +void f3() { + mtx_t m; + mtx_lock(&m); // NON-COMPLIANT +} + +void f4(int p) { + mtx_t m; + mtx_lock(&m); // NON-COMPLIANT + if (p) { + mtx_unlock(&m); + } +} + +void f5(int p) { + mtx_t m; + mtx_lock(&m); // COMPLIANT + if (p) { + mtx_unlock(&m); + } else { + mtx_unlock(&m); + } +} + +void f6(int p) { + mtx_t m; + mtx_lock(&m); // NON-COMPLIANT + if (p) { + goto skipped; + } + mtx_unlock(&m); + skipped: +} + +void f7(int p) { + mtx_t *m; + mtx_lock(m); // COMPLIANT + mtx_unlock(m); +} + +void f8(int p) { + mtx_t *m; + mtx_lock(m); // NON-COMPLIANT +} + +void f9(int p) { + mtx_t m; + mtx_lock(&m); // COMPLIANT + mtx_t *ptr_m = &m; + mtx_unlock(ptr_m); +} + +mtx_t g1; +void f10() { + mtx_lock(&g1); // COMPLIANT + mtx_unlock(&g1); +} + +void f11() { + mtx_lock(&g1); // NON-COMPLIANT +} + +void f12() { + struct { + mtx_t m; + } s; + mtx_lock(&s.m); // NON-COMPLIANT +} + +void f13() { + struct { + mtx_t m; + } s; + mtx_lock(&s.m); // COMPLIANT + mtx_unlock(&s.m); +} + +void f14() { + for(;;) { + mtx_t m; + mtx_lock(&m); // COMPLIANT + mtx_unlock(&m); + } +} + +void f15(int p) { + for(;;) { + mtx_t m; + mtx_lock(&m); // NON-COMPLIANT + if (p) { + break; + } + mtx_unlock(&m); + } +} \ No newline at end of file diff --git a/change_notes/2024-11-27-c-object-refactor.md b/change_notes/2024-11-27-c-object-refactor.md new file mode 100644 index 0000000000..511ce1b7ce --- /dev/null +++ b/change_notes/2024-11-27-c-object-refactor.md @@ -0,0 +1,21 @@ +- `CON34-C` - `AppropriateThreadObjectStorageDurations.ql`: + - Improved analysis for detecting objects with automatic storage duration + - New reports will include `a.x`, `a[x]` for object `a` with automatic storage duration +- `DCL30-C` - `AppropriateStorageDurationsFunctionReturn.ql`: + - Improved analysis for detecting objects with automatic storage duration + - New reports will include `a.x`, `a[x]` for object `a` with automatic storage duration + - False positives related to returning copying pointer values +- `EXP35-C` - `DoNotModifyObjectsWithTemporaryLifetime.ql`: + - Improved analysis for detecting objects with temporary lifetime + - More non-lvalue expressions that produce temporary objects detected, for instance `(x = y).x`, previously only `f().x` discovered +- `MEM33-C` - `AllocStructsWithAFlexibleArrayMemberDynamically.ql`: + - Improved analysis for detecting objects with automatic storage duration + - New reports will include struct literals with a flexible array member +- `RULE-18-9` - `ModifiableLValueSubscriptedWithTemporaryLifetime.ql`: + - Problems will be reported at more obviously non-lvalue locations + - Implementation refactored to be shared with other libraries + - No other changes expected +- `RULE-18-9` - `ArrayToPointerConversionOfTemporaryLifetime.ql`: + - Problems will be reported at more obviously non-lvalue locations + - Implementation refactored to be shared with other libraries + - No other changes expected \ No newline at end of file diff --git a/change_notes/2024-11-27-raii-concurrency-analysis-perf.md b/change_notes/2024-11-27-raii-concurrency-analysis-perf.md new file mode 100644 index 0000000000..3a08427808 --- /dev/null +++ b/change_notes/2024-11-27-raii-concurrency-analysis-perf.md @@ -0,0 +1,2 @@ + - `Concurrency` - for all queries related to RAII-style mutexes + - These types of locks have been refactored to improve performance in some queries. No change in query results expected. \ No newline at end of file diff --git a/change_notes/2024-11-27-resource-leak-analysis-refactor.md b/change_notes/2024-11-27-resource-leak-analysis-refactor.md new file mode 100644 index 0000000000..8f2799b543 --- /dev/null +++ b/change_notes/2024-11-27-resource-leak-analysis-refactor.md @@ -0,0 +1,10 @@ +- `ERR57-CPP` - `DoNotLeakResourcesWhenHandlingExceptions.ql`: + - Resource leak detection code refactored for sharing across queries + - Control flow no longer uses "cut nodes." This could impact performance positively or negatively, however measurements have been taken that indicate no significant change + - Some false positives have been suppressed due to slightly different control flow approach + - Leaked mutex locks and open files are reported at slightly different location, reported at call site (e.g. `f.open(...)`, `m.lock()`) rather than on the variable itself (`f` and `m`). +- `A15-1-4` - `ValidResourcesStateBeforeThrow.ql`: + - Resource leak detection code refactored for sharing across queries + - Control flow no longer uses "cut nodes." This could impact performance positively or negatively, however measurements have been taken that indicate no significant change + - Some false positives have been suppressed due to slightly different control flow approach + - Leaked mutex locks and open files are reported at slightly different location, reported at call site (e.g. `f.open(...)`, `m.lock()`) rather than on the variable itself (`f` and `m`). \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Clvalues.qll b/cpp/common/src/codingstandards/cpp/Clvalues.qll index 73fcd65eb1..157041f13b 100644 --- a/cpp/common/src/codingstandards/cpp/Clvalues.qll +++ b/cpp/common/src/codingstandards/cpp/Clvalues.qll @@ -14,4 +14,7 @@ predicate isCLValue(Expr expr) { not expr instanceof AssignExpr and not expr instanceof CommaExpr and not exists(Cast c | c = expr.getConversion*()) + or + // 6.5.2.5.4: Compound literals are always lvalues. + expr instanceof AggregateLiteral } diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index d856fa4515..f68553dfd9 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -69,6 +69,42 @@ class C11ThreadCreateCall extends ThreadCreationFunction { override ControlFlowNode getNext() { result = getFunction().getEntryPoint() } } +class C11MutexType extends TypedefType { + C11MutexType() { + this.hasName("mtx_t") + } +} + +class C11ThreadType extends TypedefType { + C11ThreadType() { + this.hasName("thrd_t") + } +} + +class C11ConditionType extends TypedefType { + C11ConditionType() { + this.hasName("cnd_t") + } +} + +class C11ThreadStorageType extends TypedefType { + C11ThreadStorageType() { + this.hasName("tss_t") + } +} + +class C11ThreadingObjectType extends TypedefType { + C11ThreadingObjectType() { + this instanceof C11MutexType + or + this instanceof C11ThreadType + or + this instanceof C11ConditionType + or + this instanceof C11ThreadStorageType + } +} + /** * Common base class providing an interface into function call * based mutex locks. @@ -317,14 +353,14 @@ abstract class LockingOperation extends FunctionCall { */ class RAIIStyleLock extends LockingOperation { VariableAccess lock; - Element e; RAIIStyleLock() { ( getTarget().getDeclaringType().hasQualifiedName("std", "lock_guard") or getTarget().getDeclaringType().hasQualifiedName("std", "unique_lock") or getTarget().getDeclaringType().hasQualifiedName("std", "scoped_lock") - ) + ) and + lock = getArgument(0).getAChild*() } /** @@ -333,7 +369,6 @@ class RAIIStyleLock extends LockingOperation { override predicate isLock() { not isLockingOperationWithinLockingOperation(this) and this instanceof ConstructorCall and - lock = getArgument(0).getAChild*() and // defer_locks don't cause a lock not exists(Expr exp | exp = getArgument(1) and @@ -463,6 +498,28 @@ class CConditionalWait extends ConditionalWait { CConditionalWait() { getTarget().getName() in ["cnd_wait"] } } +/** + * Models a function which uses a c condition variable. Not integrated into the thread aware CFG. + */ +class CConditionOperation extends FunctionCall { + CConditionOperation() { + getTarget().hasName(["cnd_broadcast", "cnd_signal", "cnd_timedwait", "cnd_wait", "cnd_init"]) + } + + predicate isInit() { + getTarget().hasName("cnd_init") + } + + predicate isUse() { + not isInit() + } + + Expr getConditionExpr() { result = getArgument(0) } + + /* Note: only holds for `cnd_wait()` and `cnd_timedwait()` */ + Expr getMutexExpr() { result = getArgument(1) } +} + /** * Models a call to a `std::thread` constructor that depends on a mutex. */ @@ -531,6 +588,10 @@ class CPPMutexSource extends MutexSource, ConstructorCall { */ class C11MutexSource extends MutexSource, FunctionCall { C11MutexSource() { getTarget().hasName("mtx_init") } + + Expr getMutexExpr() { result = getArgument(0) } + + Expr getMutexTypeExpr() { result = getArgument(1) } } /** diff --git a/cpp/common/src/codingstandards/cpp/Type.qll b/cpp/common/src/codingstandards/cpp/Type.qll index 4199b4a12d..4f72f5612e 100644 --- a/cpp/common/src/codingstandards/cpp/Type.qll +++ b/cpp/common/src/codingstandards/cpp/Type.qll @@ -60,6 +60,30 @@ Type stripSpecifiers(Type type) { else result = type } +signature class PossiblySpecifiedBaseType extends Type; + +/** + * This module defines a class `Type` which holds for types `T` and `const/volatile T` etc. + * + * Similar to `getUnspecifiedType()`, but does not resolve typedefs. Useful for matching + * potentially qualified versions of standard typedef types, such as `const mtx_t`. + * + * Example usage: `someType.(PossiblySpecified::Type).strip()` + */ +module PossiblySpecified { + import cpp as cpp + final class CppType = cpp::Type; + + class Type extends CppType { + BaseType baseType; + Type() { + baseType = stripSpecifiers(this) + } + + BaseType strip() { result = baseType } + } +} + /** * Get the precision of an integral type, where precision is defined as the number of bits * that can be used to represent the numeric value. @@ -69,4 +93,4 @@ int getPrecision(IntegralType type) { type.isExplicitlyUnsigned() and result = type.getSize() * 8 or type.isExplicitlySigned() and result = type.getSize() * 8 - 1 -} +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency8.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency8.qll new file mode 100644 index 0000000000..677b35d12b --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency8.qll @@ -0,0 +1,112 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Concurrency8Query = + TNonstandardUseOfThreadingObjectQuery() or + TThreadingObjectWithInvalidStorageDurationQuery() or + TMutexNotInitializedBeforeUseQuery() or + TMutexInitializedInsideThreadQuery() or + TMutexInitWithInvalidMutexTypeQuery() or + TMutexObjectsNotAlwaysUnlockedQuery() + +predicate isConcurrency8QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `nonstandardUseOfThreadingObject` query + Concurrency8Package::nonstandardUseOfThreadingObjectQuery() and + queryId = + // `@id` for the `nonstandardUseOfThreadingObject` query + "c/misra/nonstandard-use-of-threading-object" and + ruleId = "RULE-22-12" and + category = "mandatory" + or + query = + // `Query` instance for the `threadingObjectWithInvalidStorageDuration` query + Concurrency8Package::threadingObjectWithInvalidStorageDurationQuery() and + queryId = + // `@id` for the `threadingObjectWithInvalidStorageDuration` query + "c/misra/threading-object-with-invalid-storage-duration" and + ruleId = "RULE-22-13" and + category = "required" + or + query = + // `Query` instance for the `mutexNotInitializedBeforeUse` query + Concurrency8Package::mutexNotInitializedBeforeUseQuery() and + queryId = + // `@id` for the `mutexNotInitializedBeforeUse` query + "c/misra/mutex-not-initialized-before-use" and + ruleId = "RULE-22-14" and + category = "mandatory" + or + query = + // `Query` instance for the `mutexInitializedInsideThread` query + Concurrency8Package::mutexInitializedInsideThreadQuery() and + queryId = + // `@id` for the `mutexInitializedInsideThread` query + "c/misra/mutex-initialized-inside-thread" and + ruleId = "RULE-22-14" and + category = "mandatory" + or + query = + // `Query` instance for the `mutexInitWithInvalidMutexType` query + Concurrency8Package::mutexInitWithInvalidMutexTypeQuery() and + queryId = + // `@id` for the `mutexInitWithInvalidMutexType` query + "c/misra/mutex-init-with-invalid-mutex-type" and + ruleId = "RULE-22-14" and + category = "mandatory" + or + query = + // `Query` instance for the `mutexObjectsNotAlwaysUnlocked` query + Concurrency8Package::mutexObjectsNotAlwaysUnlockedQuery() and + queryId = + // `@id` for the `mutexObjectsNotAlwaysUnlocked` query + "c/misra/mutex-objects-not-always-unlocked" and + ruleId = "RULE-22-16" and + category = "required" +} + +module Concurrency8Package { + Query nonstandardUseOfThreadingObjectQuery() { + //autogenerate `Query` type + result = + // `Query` type for `nonstandardUseOfThreadingObject` query + TQueryC(TConcurrency8PackageQuery(TNonstandardUseOfThreadingObjectQuery())) + } + + Query threadingObjectWithInvalidStorageDurationQuery() { + //autogenerate `Query` type + result = + // `Query` type for `threadingObjectWithInvalidStorageDuration` query + TQueryC(TConcurrency8PackageQuery(TThreadingObjectWithInvalidStorageDurationQuery())) + } + + Query mutexNotInitializedBeforeUseQuery() { + //autogenerate `Query` type + result = + // `Query` type for `mutexNotInitializedBeforeUse` query + TQueryC(TConcurrency8PackageQuery(TMutexNotInitializedBeforeUseQuery())) + } + + Query mutexInitializedInsideThreadQuery() { + //autogenerate `Query` type + result = + // `Query` type for `mutexInitializedInsideThread` query + TQueryC(TConcurrency8PackageQuery(TMutexInitializedInsideThreadQuery())) + } + + Query mutexInitWithInvalidMutexTypeQuery() { + //autogenerate `Query` type + result = + // `Query` type for `mutexInitWithInvalidMutexType` query + TQueryC(TConcurrency8PackageQuery(TMutexInitWithInvalidMutexTypeQuery())) + } + + Query mutexObjectsNotAlwaysUnlockedQuery() { + //autogenerate `Query` type + result = + // `Query` type for `mutexObjectsNotAlwaysUnlocked` query + TQueryC(TConcurrency8PackageQuery(TMutexObjectsNotAlwaysUnlockedQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index 1562ba7894..f91f80f17a 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -12,6 +12,7 @@ import Concurrency2 import Concurrency3 import Concurrency4 import Concurrency5 +import Concurrency8 import Contracts1 import Contracts2 import Contracts3 @@ -91,6 +92,7 @@ newtype TCQuery = TConcurrency3PackageQuery(Concurrency3Query q) or TConcurrency4PackageQuery(Concurrency4Query q) or TConcurrency5PackageQuery(Concurrency5Query q) or + TConcurrency8PackageQuery(Concurrency8Query q) or TContracts1PackageQuery(Contracts1Query q) or TContracts2PackageQuery(Contracts2Query q) or TContracts3PackageQuery(Contracts3Query q) or @@ -170,6 +172,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isConcurrency3QueryMetadata(query, queryId, ruleId, category) or isConcurrency4QueryMetadata(query, queryId, ruleId, category) or isConcurrency5QueryMetadata(query, queryId, ruleId, category) or + isConcurrency8QueryMetadata(query, queryId, ruleId, category) or isContracts1QueryMetadata(query, queryId, ruleId, category) or isContracts2QueryMetadata(query, queryId, ruleId, category) or isContracts3QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/lifetimes/CLifetimes.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CLifetimes.qll deleted file mode 100644 index 9282260fb9..0000000000 --- a/cpp/common/src/codingstandards/cpp/lifetimes/CLifetimes.qll +++ /dev/null @@ -1,48 +0,0 @@ -import cpp -import codingstandards.cpp.Clvalues - -/** - * A struct or union type that contains an array type. - */ -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 - } -} - -/** - * A non-lvalue expression with struct or or union type that has a field member - * of array type, has a temporary lifetime. - * - * The array members are also part of that object, and thus also have temporary - * lifetime. - */ -class TemporaryLifetimeExpr extends Expr { - TemporaryLifetimeExpr() { - getUnconverted().getUnspecifiedType() instanceof StructOrUnionTypeWithArrayField and - not isCLValue(this) - or - this.getUnconverted().(ArrayExpr).getArrayBase() instanceof TemporaryLifetimeArrayAccess - } -} - -/** - * A field access on a temporary object that returns an array member. - */ -class TemporaryLifetimeArrayAccess extends FieldAccess { - // The temporary lifetime object which owns the array that is returned. - TemporaryLifetimeExpr temporary; - - TemporaryLifetimeArrayAccess() { - getQualifier().getUnconverted() = temporary and - getUnspecifiedType() instanceof ArrayType - } - - /** - * Get the temporary lifetime object which own the array that is returned. - */ - Expr getTemporary() { result = temporary } -} diff --git a/cpp/common/src/codingstandards/cpp/resources/ResourceLeakAnalysis.qll b/cpp/common/src/codingstandards/cpp/resources/ResourceLeakAnalysis.qll new file mode 100644 index 0000000000..4623b3e95e --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/resources/ResourceLeakAnalysis.qll @@ -0,0 +1,101 @@ +import cpp +import codingstandards.cpp.dataflow.DataFlow +import semmle.code.cpp.valuenumbering.GlobalValueNumbering +import semmle.code.cpp.controlflow.Dominance +import codeql.util.Boolean + +/** + * A library for detecting leaked resources. + * + * To use this library, implement `ResourceLeakConfigSig`: + * + * ``` + * class UnjoinedThreadConfig implements ResourceLeakConfigSig { + * predicate isResource(DataFlow::Node node) { + * node.asExpr().isThreadCreate() + * } + * + * predicate isFree(ControlFlowNode node, DataFlow::Node resource) { + * node.asExpr().isThreadJoin(resource.asExpr()) + * } + * } + * ``` + * + * You can now check if a resource is leaked through the module predicate + * `ResourceLeak::isLeaked(resource)`. + * + * The leak analysis finds the exit point of the function in which the resource is is declared, and + * then reverses execution from there using `getAPredecessor()`. When this backwards walk discovers + * a control flow node that frees the resource, that exploration stops. If any exploration reaches + * a resource, that resource may be leaked via that path. + * + * Uses `DataFlow::Node` in order to track aliases of the resource to better detect when the + * resource is freed. + * + * This library by default assumes that resources are expression nodes. To use it with other kinds + * of nodes requires overriding `resourceInitPoint`. + */ +signature module ResourceLeakConfigSig { + predicate isAllocate(ControlFlowNode node, DataFlow::Node resource); + + predicate isFree(ControlFlowNode node, DataFlow::Node resource); + + default DataFlow::Node getAnAlias(DataFlow::Node node) { + DataFlow::localFlow(node, result) or + exists(Expr current, Expr after | + current in [node.asExpr(), node.asDefiningArgument()] and + after in [result.asExpr(), result.asDefiningArgument()] and + globalValueNumber(current) = globalValueNumber(after) and + strictlyDominates(current, after) + ) + } + + /* A point at which a resource is considered to have leaked if it has not been freed. */ + default ControlFlowNode outOfScope(ControlFlowNode allocPoint) { + result = allocPoint.(Expr).getEnclosingFunction().getBlock().getLastStmt() + } +} + +module ResourceLeak { + private newtype TResource = TJustResource(DataFlow::Node resource, ControlFlowNode cfgNode) { + Config::isAllocate(cfgNode, resource) + } + + /** + * Get an alias of a resource, and aliases of nodes that are aliased by a resource. + */ + private DataFlow::Node getAnAliasRecursive(DataFlow::Node node) { + result = Config::getAnAlias(node) and + Config::isAllocate(_, node) + or + exists(DataFlow::Node parent | + node = getAnAliasRecursive(parent) and + result = Config::getAnAlias(parent) + ) + } + + private predicate isLeakedAtControlPoint(TResource resource, ControlFlowNode cfgNode) { + // Holds if this control point is where the resource was allocated (and therefore not freed). + resource = TJustResource(_, cfgNode) + or + // Holds if this control point does not free the resource, and is reachable from a point that + // does not free the resource. + isLeakedAtControlPoint(resource, cfgNode.getAPredecessor()) and + not exists(DataFlow::Node freed, DataFlow::Node resourceNode | + Config::isFree(cfgNode, freed) and + freed = getAnAliasRecursive(resourceNode) and + resource = TJustResource(resourceNode, _) + ) + } + + /** + * Holds if `resource` is leaked. Use this module predicate to find leaked resources. + */ + ControlFlowNode getALeak(ControlFlowNode allocPoint) { + exists(TResource resourceWrapper, DataFlow::Node resource | + resourceWrapper = TJustResource(resource, allocPoint) and + result = Config::outOfScope(allocPoint) and + isLeakedAtControlPoint(resourceWrapper, result) + ) + } +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/resources/ResourceManagement.qll b/cpp/common/src/codingstandards/cpp/resources/ResourceManagement.qll index 0798575495..93c26e0a46 100644 --- a/cpp/common/src/codingstandards/cpp/resources/ResourceManagement.qll +++ b/cpp/common/src/codingstandards/cpp/resources/ResourceManagement.qll @@ -1,58 +1,45 @@ import cpp import codingstandards.cpp.dataflow.DataFlow +import codingstandards.cpp.resources.ResourceLeakAnalysis -/** - * The `ResourceAcquisitionExpr` abstract class models resource - * acquisition and release expressions - */ -abstract class ResourceAcquisitionExpr extends Expr { - abstract Expr getReleaseExpr(); -} - -// allocation - deallocation -class AllocExpr extends ResourceAcquisitionExpr { - AllocExpr() { this.(AllocationExpr).requiresDealloc() } - - override Expr getReleaseExpr() { - exists(DeallocationExpr d | result = d.getFreedExpr()) and - DataFlow::localFlow(DataFlow::exprNode(this), DataFlow::exprNode(result)) - } -} - -// file open-close -class FstreamAcquisitionExpr extends ResourceAcquisitionExpr { - FstreamAcquisitionExpr() { +module ResourceLeakConfig implements ResourceLeakConfigSig { + predicate isAllocate(ControlFlowNode allocPoint, DataFlow::Node node) { + exists(AllocationExpr alloc | + allocPoint = alloc and + alloc.requiresDealloc() and + node.asExpr() = alloc + ) + or exists(FunctionCall f | - f.getTarget().hasQualifiedName("std", "basic_fstream", "open") and this = f.getQualifier() + f.getTarget().hasQualifiedName("std", "basic_fstream", "open") + and allocPoint = f + and node.asDefiningArgument() = f.getQualifier() ) - } - - override Expr getReleaseExpr() { + or exists(FunctionCall f | - f.getTarget().hasQualifiedName("std", "basic_fstream", "close") and result = f.getQualifier() - ) and - exists(DataFlow::Node def | - def.asDefiningArgument() = this and - DataFlow::localFlow(def, DataFlow::exprNode(result)) + f.getTarget().hasQualifiedName("std", "mutex", "lock") and + allocPoint = f and + node.asDefiningArgument() = f.getQualifier() ) } -} -// mutex lock unlock -class MutexAcquisitionExpr extends ResourceAcquisitionExpr { - MutexAcquisitionExpr() { + predicate isFree(ControlFlowNode node, DataFlow::Node resource) { + exists(DeallocationExpr d, Expr freedExpr| + freedExpr = d.getFreedExpr() and + node = d and + resource.asExpr() = freedExpr + ) + or exists(FunctionCall f | - f.getTarget().hasQualifiedName("std", "mutex", "lock") and this = f.getQualifier() + f.getTarget().hasQualifiedName("std", "basic_fstream", "close") + and node = f and + resource.asExpr() = f.getQualifier() ) - } - - override Expr getReleaseExpr() { + or exists(FunctionCall f | - f.getTarget().hasQualifiedName("std", "mutex", "unlock") and result = f.getQualifier() - ) and - exists(DataFlow::Node def | - def.asDefiningArgument() = this and - DataFlow::localFlow(def, DataFlow::exprNode(result)) + f.getTarget().hasQualifiedName("std", "mutex", "unlock") + and node = f and + resource.asExpr() = f.getQualifier() ) } -} +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.qll b/cpp/common/src/codingstandards/cpp/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.qll index 16f86f78be..8ed5342def 100644 --- a/cpp/common/src/codingstandards/cpp/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.qll +++ b/cpp/common/src/codingstandards/cpp/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.qll @@ -4,42 +4,17 @@ */ import cpp -import semmle.code.cpp.controlflow.SubBasicBlocks import codingstandards.cpp.Exclusions import codingstandards.cpp.Customizations import codingstandards.cpp.exceptions.ExceptionFlow import codingstandards.cpp.ExceptionSafety import codingstandards.cpp.resources.ResourceManagement +import codingstandards.cpp.resources.ResourceLeakAnalysis abstract class ExceptionSafetyValidStateSharedQuery extends Query { } Query getQuery() { result instanceof ExceptionSafetyValidStateSharedQuery } -/** - * Ensures that `UncaughtThrowExpr` and `Expr` appear at the start of a `SubBasicBlock`. - */ -class SafetyValidStateSubBasicBlock extends SubBasicBlockCutNode { - SafetyValidStateSubBasicBlock() { - this instanceof ResourceAcquisitionExpr or - this = any(ResourceAcquisitionExpr rae).getReleaseExpr() or - this instanceof UncaughtThrowExpr - } -} - -/** - * Execution continues from an allocation expression - * without releasing the resource - */ -SubBasicBlock followsInitialized(ResourceAcquisitionExpr src) { - result = src - or - exists(SubBasicBlock mid | - mid = followsInitialized(src) and - result = mid.getASuccessor() and - //stop recursion on resource release - not result = src.getReleaseExpr() - ) -} /** * `UncaughtThrowExpr` models a `throw` expression that is not handled @@ -48,14 +23,40 @@ class UncaughtThrowExpr extends ThrowExpr { UncaughtThrowExpr() { getASuccessor() = getEnclosingFunction() } } +module ThrowLeakConfig implements ResourceLeakConfigSig { + + predicate isAllocate(ControlFlowNode node, DataFlow::Node resource) { + //exists(ResourceAcquisitionExpr rae | + // node = rae and resource.asExpr() = rae + //) + ResourceLeakConfig::isAllocate(node, resource) + } + + predicate isFree(ControlFlowNode node, DataFlow::Node resource) { + //exists(ResourceAcquisitionExpr rae | + // node = rae.getReleaseExpr() and resource.asExpr() = rae + //) + ResourceLeakConfig::isFree(node, resource) + } + + ControlFlowNode outOfScope(ControlFlowNode allocPoint) { + result.(UncaughtThrowExpr).getEnclosingFunction() = allocPoint.(Expr).getEnclosingFunction() + } + + DataFlow::Node getAnAlias(DataFlow::Node node) { + DataFlow::localFlow(node, result) + } +} + query predicate problems( - UncaughtThrowExpr te, string message, ResourceAcquisitionExpr e, string eDescription + UncaughtThrowExpr te, string message, Element e, string eDescription ) { not isExcluded(te, getQuery()) and - exists(SubBasicBlock sbb | - sbb.getANode() = e and - te = followsInitialized(sbb) - ) and + //exists(SubBasicBlock sbb | + // sbb.getANode() = e and + // te = followsInitialized(sbb) + //) and + te = ResourceLeak::getALeak(e) and message = "The $@ is not released explicitly before throwing an exception." and eDescription = "allocated resource" } diff --git a/cpp/common/test/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.expected b/cpp/common/test/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.expected index 18689c333b..b8a8c17435 100644 --- a/cpp/common/test/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.expected +++ b/cpp/common/test/rules/exceptionsafetyvalidstate/ExceptionSafetyValidState.expected @@ -6,7 +6,7 @@ | test.cpp:103:5:103:9 | re-throw exception | The $@ is not released explicitly before throwing an exception. | test.cpp:99:12:99:21 | new | allocated resource | | test.cpp:125:5:125:37 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:124:9:124:18 | new | allocated resource | | test.cpp:134:5:134:37 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:133:23:133:32 | new | allocated resource | -| test.cpp:142:3:142:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:141:3:141:4 | fs | allocated resource | -| test.cpp:154:3:154:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:152:3:152:4 | fs | allocated resource | -| test.cpp:160:3:160:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:159:3:159:5 | mtx | allocated resource | -| test.cpp:172:3:172:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:170:3:170:5 | mtx | allocated resource | +| test.cpp:142:3:142:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:141:6:141:9 | call to open | allocated resource | +| test.cpp:154:3:154:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:152:6:152:9 | call to open | allocated resource | +| test.cpp:160:3:160:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:159:7:159:10 | call to lock | allocated resource | +| test.cpp:172:3:172:35 | throw ... | The $@ is not released explicitly before throwing an exception. | test.cpp:170:7:170:10 | call to lock | allocated resource | diff --git a/rule_packages/c/Concurrency8.json b/rule_packages/c/Concurrency8.json new file mode 100644 index 0000000000..c27563f81e --- /dev/null +++ b/rule_packages/c/Concurrency8.json @@ -0,0 +1,115 @@ +{ + "MISRA-C-2012": { + "RULE-22-12": { + "properties": { + "obligation": "mandatory" + }, + "queries": [ + { + "description": "Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions.", + "kind": "problem", + "name": "Standard library threading objects (mutexes, threads, etc.) shall only be accessed by the appropriate Standard Library functions", + "precision": "very-high", + "severity": "error", + "short_name": "NonstandardUseOfThreadingObject", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions" + }, + "RULE-22-13": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration.", + "kind": "problem", + "name": "Threading objects (mutexes, threads, etc). shall have not have automatic or thread storage duration", + "precision": "very-high", + "severity": "error", + "short_name": "ThreadingObjectWithInvalidStorageDuration", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration" + }, + "RULE-22-14": { + "properties": { + "obligation": "mandatory" + }, + "queries": [ + { + "description": "Mutex and condition objects shall be initialized with the standard library functions before using them.", + "kind": "problem", + "name": "Thread synchronization objects shall be initialized before being accessed", + "precision": "high", + "severity": "error", + "short_name": "MutexNotInitializedBeforeUse", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + }, + { + "description": "Mutex and condition objects initialized inside of threads may result in indeterministic state", + "kind": "problem", + "name": "Thread synchronization objects shall be initialized deterministically", + "precision": "high", + "severity": "recommendation", + "short_name": "MutexInitializedInsideThread", + "tags": [ + "readability", + "maintainability", + "concurrency", + "external/misra/c/2012/amendment4" + ] + }, + { + "description": "Mutexes shall be initialized with a valid mutex type.", + "kind": "problem", + "name": "Mutexes shall be initialized with a valid mutex type", + "precision": "high", + "severity": "error", + "short_name": "MutexInitWithInvalidMutexType", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "Thread synchronization objects shall be initialized before being accessed" + }, + "RULE-22-16": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Mutex not unlocked by thread on all execution paths in current thread after being locked.", + "kind": "problem", + "name": "All mutex objects locked by a thread shall be explicitly unlocked by the same thread", + "precision": "high", + "severity": "error", + "short_name": "MutexObjectsNotAlwaysUnlocked", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "All mutex objects locked by a thread shall be explicitly unlocked by the same thread" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 475ea1d66c..624de41885 100644 --- a/rules.csv +++ b/rules.csv @@ -803,11 +803,11 @@ c,MISRA-C-2012,RULE-22-8,Yes,Required,,,The value of errno shall be set to zero c,MISRA-C-2012,RULE-22-9,Yes,Required,,,The value of errno shall be tested against zero after calling an errno-setting-function,,Contracts3,Medium, c,MISRA-C-2012,RULE-22-10,Yes,Required,,,The value of errno shall only be tested when the last function to be called was an errno-setting-function,,Contracts3,Medium, c,MISRA-C-2012,RULE-22-11,Yes,Required,,,A thread that was previously either joined or detached shall not be subsequently joined nor detached,CON39-C,Concurrency6,Import, -c,MISRA-C-2012,RULE-22-12,Yes,Mandatory,,,"Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions",,Concurrency6,Medium, -c,MISRA-C-2012,RULE-22-13,Yes,Required,,,"Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration",EXP54-CPP and CON34-C,Concurrency6,Medium, -c,MISRA-C-2012,RULE-22-14,Yes,Mandatory,,,Thread synchronization objects shall be initialized before being accessed,EXP53-CPP,Concurrency6,Hard, +c,MISRA-C-2012,RULE-22-12,Yes,Mandatory,,,"Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions",,Concurrency8,Medium, +c,MISRA-C-2012,RULE-22-13,Yes,Required,,,"Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration",EXP54-CPP and CON34-C,Concurrency8,Medium, +c,MISRA-C-2012,RULE-22-14,Yes,Mandatory,,,Thread synchronization objects shall be initialized before being accessed,EXP53-CPP,Concurrency9,Hard, c,MISRA-C-2012,RULE-22-15,Yes,Required,,,Thread synchronization objects and thread-specific storage pointers shall not be destroyed until after all threads accessing them have terminated,,Concurrency6,Hard, -c,MISRA-C-2012,RULE-22-16,Yes,Required,,,All mutex objects locked by a thread shall be explicitly unlocked by the same thread,MEM51-CPP,Concurrency6,Hard, +c,MISRA-C-2012,RULE-22-16,Yes,Required,,,All mutex objects locked by a thread shall be explicitly unlocked by the same thread,MEM51-CPP,Concurrency9,Hard, c,MISRA-C-2012,RULE-22-17,Yes,Required,,,No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked before,Rule 22.2,Concurrency6,Medium, c,MISRA-C-2012,RULE-22-18,Yes,Required,,,Non-recursive mutexes shall not be recursively locked,CON56-CPP,Concurrency6,Medium, c,MISRA-C-2012,RULE-22-19,Yes,Required,,,A condition variable shall be associated with at most one mutex object,,Concurrency6,Medium,