diff --git a/c/common/src/codingstandards/c/SubObjects.qll b/c/common/src/codingstandards/c/SubObjects.qll new file mode 100644 index 0000000000..66f15cd18c --- /dev/null +++ b/c/common/src/codingstandards/c/SubObjects.qll @@ -0,0 +1,125 @@ +/** + * A library that expands upon the `Objects.qll` library, to support nested "Objects" such as + * `x.y.z` or `x[i][j]` within an object `x`. + * + * Objects in C are values in memory, that have a type and a storage duration. In the case of + * array objects and struct objects, the object will contain other objects. The these subobjects + * will share properties of the root object such as storage duration. This library can be used to, + * for instance, find all usages of a struct member to ensure that member is initialized before it + * is used. + * + * To use this library, select `SubObject` and find its usages in the AST via `getAnAccess()` (to + * find usages of the subobject by value) or `getAnAddressOfExpr()` (to find usages of the object + * by address). + * + * Note that a struct or array object may contain a pointer. In this case, the pointer itself is + * a subobject of the struct or array object, but the object that the pointer points to is not. + * This is because the pointed-to object does not necessarily have the same storage duration, + * lifetime, or linkage as the pointer and the object containing the pointer. + * + * Note as well that `getAnAccess()` on an array subobject will return all accesses to the array, + * not just accesses to a particular index. For this reason, `SubObject` exposes the predicate + * `isPrecise()`. If a subobject is precise, that means all results of `getAnAccess()` will + * definitely refer to the same object in memory. If it is not precise, the different accesses + * may refer to the same or different objects in memory. For instance, `x[i].y` and `x[j].y` are + * the same object if `i` and `j` are the same, but they are different objects if `i` and `j` are + * different. + */ + +import codingstandards.c.Objects + +newtype TSubObject = + TObjectRoot(ObjectIdentity i) or + TObjectMember(SubObject struct, MemberVariable m) { + m = struct.getType().(Struct).getAMemberVariable() + } or + TObjectIndex(SubObject array) { array.getType() instanceof ArrayType } + +class SubObject extends TSubObject { + string toString() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i.toString() + ) + or + exists(SubObject struct, Variable m | + this = TObjectMember(struct, m) and + result = struct.toString() + "." + m.getName() + ) + or + exists(SubObject array | + this = TObjectIndex(array) and + result = array.toString() + ) + } + + Type getType() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i.getType() + ) + or + exists(Variable m | + this = TObjectMember(_, m) and + result = m.getType() + ) + or + exists(SubObject array | + this = TObjectIndex(array) and + result = array.getType().(ArrayType).getBaseType() + ) + } + + /** + * Holds for object roots and for member accesses on that root, not for array accesses. + * + * This is useful for cases where we do not wish to treat `x[y]` and `x[z]` as the same object. + */ + predicate isPrecise() { not getParent*() = TObjectIndex(_) } + + SubObject getParent() { + exists(SubObject struct, MemberVariable m | + this = TObjectMember(struct, m) and + result = struct + ) + or + exists(SubObject array | + this = TObjectIndex(array) and + result = array + ) + } + + Expr getAnAccess() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i.getAnAccess() + ) + or + exists(MemberVariable m | + this = TObjectMember(_, m) and + result = m.getAnAccess() and + // Only consider `DotFieldAccess`es, not `PointerFieldAccess`es, as the latter + // are not subobjects of the root object: + result.(DotFieldAccess).getQualifier() = getParent().getAnAccess() + ) + or + this = TObjectIndex(_) and + result.(ArrayExpr).getArrayBase() = getParent().getAnAccess() + } + + AddressOfExpr getAnAddressOfExpr() { result.getOperand() = this.getAnAccess() } + + /** + * Get the "root" object identity to which this subobject belongs. For instance, in the + * expression `x.y.z`, the root object is `x`. This subobject will share properties with the root + * object such as storage duration, lifetime, and linkage. + */ + ObjectIdentity getRootIdentity() { + exists(ObjectIdentity i | + this = TObjectRoot(i) and + result = i + ) + or + result = getParent().getRootIdentity() + } +} diff --git a/c/common/src/codingstandards/c/initialization/GlobalInitializationAnalysis.qll b/c/common/src/codingstandards/c/initialization/GlobalInitializationAnalysis.qll new file mode 100644 index 0000000000..2906883ae9 --- /dev/null +++ b/c/common/src/codingstandards/c/initialization/GlobalInitializationAnalysis.qll @@ -0,0 +1,95 @@ +import cpp +import codingstandards.c.Objects +import codingstandards.cpp.Concurrency +import codingstandards.cpp.Type + +signature module GlobalInitializationAnalysisConfigSig { + /** A function which is not called or started as a thread */ + default predicate isRootFunction(Function f) { + not exists(Function f2 | f2.calls(f)) and + not f instanceof ThreadedFunction and + // Exclude functions which are used as function pointers. + not exists(FunctionAccess access | f = access.getTarget()) + } + + ObjectIdentity getAnInitializedObject(Expr e); + + ObjectIdentity getAUsedObject(Expr e); +} + +module GlobalInitalizationAnalysis<GlobalInitializationAnalysisConfigSig Config> { + final class FinalFunction = Function; + + final class FinalExpr = Expr; + + class RootFunction extends FinalFunction { + RootFunction() { Config::isRootFunction(this) } + } + + /** A function call which initializes a mutex or a condition */ + class ObjectInit extends FinalExpr { + ObjectIdentity owningObject; + + ObjectInit() { owningObject = Config::getAnInitializedObject(this) } + + ObjectIdentity getOwningObject() { result = owningObject } + } + + /** + * A function argument where that argument is used as a mutex or condition object. + */ + class ObjectUse extends FinalExpr { + ObjectIdentity owningObject; + + ObjectUse() { owningObject = Config::getAUsedObject(this) } + + ObjectIdentity getOwningObject() { result = owningObject } + } + + predicate requiresInitializedMutexObject( + Function func, ObjectUse mutexUse, ObjectIdentity owningObject + ) { + mutexUse.getEnclosingFunction() = func and + owningObject = mutexUse.getOwningObject() and + not exists(ObjectInit 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(ObjectInit 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(ObjectInit init | + call.getAPredecessor*() = init and + init.getOwningObject() = owningObject + ) + ) + } + + predicate uninitializedFrom(Expr e, ObjectIdentity obj, Function callRoot) { + exists(ObjectUse use | use = e | + obj = use.getOwningObject() and + requiresInitializedMutexObject(callRoot, use, obj) and + ( + if obj.getStorageDuration().isAutomatic() + then obj.getEnclosingElement+() = callRoot + else ( + obj.getStorageDuration().isThread() and callRoot instanceof ThreadedFunction + or + callRoot instanceof RootFunction + ) + ) + ) + } +} diff --git a/c/common/test/includes/standard-library/stdlib.h b/c/common/test/includes/standard-library/stdlib.h index b54a051fe9..1af95223d1 100644 --- a/c/common/test/includes/standard-library/stdlib.h +++ b/c/common/test/includes/standard-library/stdlib.h @@ -49,6 +49,7 @@ int at_quick_exit (void (*) (void)); _Noreturn void quick_exit (int); char *getenv (const char *); +char *getenv_s (size_t *restrict len, char *restrict value, size_t valuesz, const char *restrict name); int system (const char *); diff --git a/c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql b/c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql new file mode 100644 index 0000000000..edf3705a9b --- /dev/null +++ b/c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql @@ -0,0 +1,162 @@ +/** + * @id c/misra/possible-data-race-between-threads + * @name DIR-5-1: There shall be no data races between threads + * @description Threads shall not access the same memory location concurrently without utilization + * of thread synchronization objects. + * @kind problem + * @precision medium + * @problem.severity error + * @tags external/misra/id/dir-5-1 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.Objects +import codingstandards.c.SubObjects +import codingstandards.cpp.Concurrency + +newtype TNonReentrantOperation = + TReadWrite(SubObject object) { + object.getRootIdentity().getStorageDuration().isStatic() + or + object.getRootIdentity().getStorageDuration().isAllocated() + } or + TStdFunctionCall(FunctionCall call) { + call.getTarget() + .hasName([ + "setlocale", "tmpnam", "rand", "srand", "getenv", "getenv_s", "strok", "strerror", + "asctime", "ctime", "gmtime", "localtime", "mbrtoc16", "c16rtomb", "mbrtoc32", + "c32rtomb", "mbrlen", "mbrtowc", "wcrtomb", "mbsrtowcs", "wcsrtombs" + ]) + } + +class NonReentrantOperation extends TNonReentrantOperation { + string toString() { + exists(SubObject object | + this = TReadWrite(object) and + result = object.toString() + ) + or + exists(FunctionCall call | + this = TStdFunctionCall(call) and + result = call.getTarget().getName() + ) + } + + Expr getAReadExpr() { + exists(SubObject object | + this = TReadWrite(object) and + result = object.getAnAccess() + ) + or + this = TStdFunctionCall(result) + } + + Expr getAWriteExpr() { + exists(SubObject object, Assignment assignment | + this = TReadWrite(object) and + result = assignment and + assignment.getLValue() = object.getAnAccess() + ) + or + this = TStdFunctionCall(result) + } + + string getReadString() { + this = TReadWrite(_) and + result = "read operation" + or + this = TStdFunctionCall(_) and + result = "call to non-reentrant function" + } + + string getWriteString() { + this = TReadWrite(_) and + result = "write to object" + or + this = TStdFunctionCall(_) and + result = "call to non-reentrant function" + } + + Element getSourceElement() { + exists(SubObject object | + this = TReadWrite(object) and + result = object.getRootIdentity() + ) + or + this = TStdFunctionCall(result) + } +} + +class WritingThread extends ThreadedFunction { + NonReentrantOperation aWriteObject; + Expr aWriteExpr; + + WritingThread() { + aWriteExpr = aWriteObject.getAWriteExpr() and + // This function directly contains the write expression, or transitively calls the function + // that contains the write expression. + this.calls*(aWriteExpr.getEnclosingFunction()) and + // The write isn't synchronized with a mutex or condition object. + not aWriteExpr instanceof LockProtectedControlFlowNode and + // The write doesn't seem to be during a special initialization phase of the program. + not aWriteExpr.getEnclosingFunction().getName().matches(["%init%", "%boot%", "%start%"]) + } + + Expr getAWriteExpr() { result = aWriteExpr } +} + +class ReadingThread extends ThreadedFunction { + Expr aReadExpr; + + ReadingThread() { + exists(NonReentrantOperation op | + aReadExpr = op.getAReadExpr() and + this.calls*(aReadExpr.getEnclosingFunction()) and + not aReadExpr instanceof LockProtectedControlFlowNode + ) + } + + Expr getAReadExpr() { result = aReadExpr } +} + +predicate mayBeDataRace(Expr write, Expr read, NonReentrantOperation operation) { + exists(WritingThread wt | + wt.getAWriteExpr() = write and + write = operation.getAWriteExpr() and + exists(ReadingThread rt | + read = rt.getAReadExpr() and + read = operation.getAReadExpr() and + ( + wt.isMultiplySpawned() or + not wt = rt + ) + ) + ) +} + +from + WritingThread wt, ReadingThread rt, Expr write, Expr read, NonReentrantOperation operation, + string message, string writeString, string readString +where + not isExcluded(write, Concurrency9Package::possibleDataRaceBetweenThreadsQuery()) and + mayBeDataRace(write, read, operation) and + wt = min(WritingThread f | f.getAWriteExpr() = write | f order by f.getName()) and + rt = min(ReadingThread f | f.getAReadExpr() = read | f order by f.getName()) and + writeString = operation.getWriteString() and + readString = operation.getReadString() and + if wt.isMultiplySpawned() + then + message = + "Threaded " + writeString + + " $@ not synchronized from thread function $@ spawned from a loop." + else + message = + "Threaded " + writeString + + " $@ from thread function $@ is not synchronized with $@ from thread function $@." +select write, message, operation.getSourceElement(), operation.toString(), wt, wt.getName(), read, + "concurrent " + readString, rt, rt.getName() diff --git a/c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql b/c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql index f02891d5d0..f78c25f981 100644 --- a/c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql +++ b/c/misra/src/rules/RULE-22-14/MutexNotInitializedBeforeUse.ql @@ -18,113 +18,61 @@ import codingstandards.c.misra import codingstandards.c.Objects import codingstandards.cpp.Concurrency import codingstandards.cpp.Type +import codingstandards.c.initialization.GlobalInitializationAnalysis -/** 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 +module MutexInitializationConfig implements GlobalInitializationAnalysisConfigSig { + ObjectIdentity getAnInitializedObject(Expr e) { + e.(C11MutexSource).getMutexExpr() = result.getASubobjectAddressExpr() } -} - -/** 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 getAUsedObject(Expr e) { + result.getASubobjectAddressExpr() = e and + ( + exists(CMutexFunctionCall mutexUse | e = mutexUse.getLockExpr()) + or + exists(CConditionOperation condOp | e = condOp.getMutexExpr()) ) } - - 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" - ) +module ConditionInitializationConfig implements GlobalInitializationAnalysisConfigSig { + ObjectIdentity getAnInitializedObject(Expr e) { + exists(CConditionOperation condOp | + e = condOp and + condOp.isInit() and + condOp.getConditionExpr() = result.getASubobjectAddressExpr() ) } - ObjectIdentity getOwningObject() { result = owningObject } - - string getDescription() { - if - getOwningObject().getType() instanceof PossiblySpecified<C11MutexType>::Type or - getOwningObject().getType() instanceof PossiblySpecified<C11ConditionType>::Type - then result = typeString - else result = typeString + " in object" + ObjectIdentity getAUsedObject(Expr e) { + result.getASubobjectAddressExpr() = e and + exists(CConditionOperation condOp | + condOp.isUse() and + e = condOp.getConditionExpr() + ) } } -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 - ) - ) -} +import GlobalInitalizationAnalysis<MutexInitializationConfig> as MutexInitAnalysis +import GlobalInitalizationAnalysis<ConditionInitializationConfig> as CondInitAnalysis -from ThreadObjectUse objUse, ObjectIdentity obj, Function callRoot +from Expr objUse, ObjectIdentity obj, Function callRoot, string typeString, string description 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 - ) + MutexInitAnalysis::uninitializedFrom(objUse, obj, callRoot) and + typeString = "Mutex" + or + CondInitAnalysis::uninitializedFrom(objUse, obj, callRoot) and + typeString = "Condition" + ) and + ( + if + obj.getType() instanceof PossiblySpecified<C11MutexType>::Type or + obj.getType() instanceof PossiblySpecified<C11ConditionType>::Type + then description = typeString + else description = typeString + " in object" ) select objUse, - objUse.getDescription() + - " '$@' possibly used before initialization, from entry point function '$@'.", obj, + description + " '$@' possibly used before initialization, from entry point function '$@'.", obj, obj.toString(), callRoot, callRoot.getName() diff --git a/c/misra/src/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.ql b/c/misra/src/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.ql new file mode 100644 index 0000000000..ec4631ef1b --- /dev/null +++ b/c/misra/src/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.ql @@ -0,0 +1,113 @@ +/** + * @id c/misra/thread-resource-disposed-before-threads-joined + * @name RULE-22-15: Thread synchronization objects and thread-specific storage pointers shall not be disposed unsafely + * @description Thread synchronization objects and thread-specific storage pointers shall not be + * destroyed until after all threads accessing them have terminated. + * @kind problem + * @precision medium + * @problem.severity error + * @tags external/misra/id/rule-22-15 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.SubObjects +import codingstandards.cpp.Concurrency + +newtype TThreadKind = + TSpawned(C11ThreadCreateCall tcc) or + TMainThread() + +TThreadKind getThreadKind(FunctionCall operation) { + if + not exists(C11ThreadCreateCall tcc | + getAThreadContextAwareSuccessor(tcc.getFunction().getEntryPoint()) = operation + ) + then result = TMainThread() + else + exists(C11ThreadCreateCall tcc | + getAThreadContextAwareSuccessor(tcc.getFunction().getEntryPoint()) = operation and + result = TSpawned(tcc) + ) +} + +bindingset[tcc, thread] +predicate followsMainThreadTcc(C11ThreadCreateCall tcc, TThreadKind thread) { + thread = TMainThread() + or + exists(C11ThreadCreateCall tcc2 | + getAThreadContextAwareSuccessor(tcc) = tcc2 and + thread = TSpawned(tcc2) + ) +} + +string describeThread(TThreadKind thread) { + thread = TMainThread() and + result = "main thread" + or + exists(C11ThreadCreateCall tcc2 | + thread = TSpawned(tcc2) and + result = tcc2.getFunction().getName() + ) +} + +bindingset[alternative] +Element elementOr(TThreadKind thread, Element alternative) { + thread = TMainThread() and + result = alternative + or + exists(C11ThreadCreateCall tcc2 | + thread = TSpawned(tcc2) and + result = tcc2 + ) +} + +from + FunctionCall dispose, FunctionCall use, C11ThreadCreateCall tcc, TThreadKind disposeThread, + TThreadKind useThread, SubObject usedAndDestroyed +where + not isExcluded(dispose, Concurrency9Package::threadResourceDisposedBeforeThreadsJoinedQuery()) and + // `tcc` may be the thread that uses the resource, or the thread that disposes it. What matters + // for the query is that `tcc` is before the use and the dispose. + dispose = getAThreadContextAwareSuccessor(tcc) and + ( + // Lock and dispose of mtx_t: + exists(CMutexFunctionCall mfc, C11MutexDestroyer md | dispose = md and use = mfc | + mfc = getAThreadContextAwareSuccessor(tcc) and + mfc.getLockExpr() = usedAndDestroyed.getAnAddressOfExpr() and + md.getMutexExpr() = usedAndDestroyed.getAnAddressOfExpr() + ) + or + // Read/store and dispose of tss_t: + exists(ThreadSpecificStorageFunctionCall tssfc, TSSDeleteFunctionCall td | + dispose = td and use = tssfc + | + tssfc = getAThreadContextAwareSuccessor(tcc) and + tssfc.getKey() = usedAndDestroyed.getAnAddressOfExpr() and + td.getKey() = usedAndDestroyed.getAnAddressOfExpr() + ) + or + // Wait and dispose of cnd_t: + exists(CConditionOperation cndop, C11ConditionDestroyer cd | dispose = cd and use = cndop | + cndop = getAThreadContextAwareSuccessor(tcc) and + cndop.getConditionExpr() = usedAndDestroyed.getAnAddressOfExpr() and + cd.getConditionExpr() = usedAndDestroyed.getAnAddressOfExpr() + ) + ) and + // Dispose could be in the main thread or in a spawned thread. + disposeThread = getThreadKind(dispose) and + // Dispose could be in the main thread or in a spawned thread. + useThread = getThreadKind(use) and + // Exclude a thread that does not concurrently share the resource it disposed (unlikely). + not useThread = disposeThread and + followsMainThreadTcc(tcc, useThread) and + followsMainThreadTcc(tcc, disposeThread) and + // If there is a join between the use and the dispose, the code is compliant. + not getAThreadContextAwarePredecessor(elementOr(useThread, use), dispose) instanceof C11ThreadWait +select dispose, "Thread resource $@ disposed before joining thread $@ which uses it.", + usedAndDestroyed.getRootIdentity(), usedAndDestroyed.toString(), elementOr(useThread, use), + describeThread(useThread) diff --git a/c/misra/src/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.ql b/c/misra/src/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.ql new file mode 100644 index 0000000000..252b4a7d9f --- /dev/null +++ b/c/misra/src/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.ql @@ -0,0 +1,68 @@ +/** + * @id c/misra/invalid-operation-on-unlocked-mutex + * @name RULE-22-17: No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked + * @description No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it + * has not locked before. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-17 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.SubObjects +import codingstandards.cpp.Concurrency +import codingstandards.cpp.dominance.BehavioralSet +import semmle.code.cpp.dataflow.new.DataFlow::DataFlow as NewDF + +/* A call to mtx_unlock() or cnd_wait() or cnd_timedwait(), which require a locked mutex */ +class RequiresLockOperation extends FunctionCall { + SubObject mutex; + + RequiresLockOperation() { + exists(CMutexFunctionCall mutexCall | this = mutexCall | + mutexCall.isUnlock() and + mutex.getAnAddressOfExpr() = mutexCall.getLockExpr() + ) + or + exists(CConditionOperation condOp | this = condOp | + mutex.getAnAddressOfExpr() = condOp.getMutexExpr() + ) + } + + SubObject getMutex() { result = mutex } +} + +/* A config to search for a dominating set that locks the mutex before the operation */ +module LockDominatingSetConfig implements DominatingSetConfigSig<RequiresLockOperation> { + predicate isTargetBehavior(ControlFlowNode node, RequiresLockOperation op) { + exists(CMutexFunctionCall mutexCall | node = mutexCall | + mutexCall.isLock() and + mutexCall.getLockExpr() = op.getMutex().getAnAddressOfExpr() + ) + } + + predicate isBlockingBehavior(ControlFlowNode node, RequiresLockOperation op) { + // If we find a branch that explicitly unlocks the mutex, we should not look for an earlier + // call to lock that mutex. + exists(CMutexFunctionCall mutexCall | node = mutexCall | + mutexCall.isUnlock() and + mutexCall.getLockExpr() = op.getMutex().getAnAddressOfExpr() + ) + } +} + +import DominatingBehavioralSet<RequiresLockOperation, LockDominatingSetConfig> as DominatingSet + +from RequiresLockOperation operation, SubObject mutex +where + not isExcluded(operation, Concurrency9Package::invalidOperationOnUnlockedMutexQuery()) and + mutex = operation.getMutex() and + not DominatingSet::isDominatedByBehavior(operation) +select operation, "Invalid operation on mutex '$@' not locked by the current thread.", + mutex.getRootIdentity(), mutex.toString() diff --git a/c/misra/src/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.ql b/c/misra/src/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.ql new file mode 100644 index 0000000000..17762b3eee --- /dev/null +++ b/c/misra/src/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.ql @@ -0,0 +1,37 @@ +/** + * @id c/misra/non-recursive-mutex-recursively-locked + * @name RULE-22-18: Non-recursive mutexes shall not be recursively locked + * @description Mutexes initialized with mtx_init() without mtx_recursive shall not be locked by a + * thread that has previously locked it. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-22-18 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.SubObjects +import codingstandards.cpp.Concurrency +import codingstandards.cpp.Type + +from + LockProtectedControlFlowNode n, CMutexFunctionCall lockCall, SubObject mutex, + CMutexFunctionCall coveredByLock +where + not isExcluded(n, Concurrency9Package::nonRecursiveMutexRecursivelyLockedQuery()) and + lockCall = n and + coveredByLock = n.coveredByLock() and + not coveredByLock = lockCall and + mutex.isPrecise() and + coveredByLock.getLockExpr() = mutex.getAnAddressOfExpr() and + lockCall.getLockExpr() = mutex.getAnAddressOfExpr() and + forex(C11MutexSource init | init.getMutexExpr() = mutex.getAnAddressOfExpr() | + not init.isRecursive() + ) +select n, "Non-recursive mutex " + mutex.toString() + " locked after it is $@.", coveredByLock, + "already locked" diff --git a/c/misra/src/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.ql b/c/misra/src/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.ql new file mode 100644 index 0000000000..7e002585b6 --- /dev/null +++ b/c/misra/src/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.ql @@ -0,0 +1,60 @@ +/** + * @id c/misra/non-recursive-mutex-recursively-locked-audit + * @name RULE-22-18: (Audit) Non-recursive mutexes shall not be recursively locked + * @description Mutexes that may be initialized without mtx_recursive shall not be locked by a + * thread that may have previously locked it. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-18 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/audit + * external/misra/obligation/required + */ + +import cpp +import codeql.util.Boolean +import codingstandards.c.misra +import codingstandards.c.SubObjects +import codingstandards.cpp.Concurrency +import codingstandards.cpp.Type + +predicate isTrackableMutex(CMutexFunctionCall lockCall, Boolean recursive) { + exists(SubObject mutex | + lockCall.getLockExpr() = mutex.getAnAddressOfExpr() and + mutex.isPrecise() and + forex(C11MutexSource init | init.getMutexExpr() = mutex.getAnAddressOfExpr() | + if init.isRecursive() then recursive = true else recursive = false + ) + ) +} + +predicate definitelyDifferentMutexes(CMutexFunctionCall lockCall, CMutexFunctionCall coveredByLock) { + exists(SubObject a, SubObject b | + lockCall.getLockExpr() = a.getAnAddressOfExpr() and + coveredByLock.getLockExpr() = b.getAnAddressOfExpr() and + not a = b + ) +} + +from LockProtectedControlFlowNode n, CMutexFunctionCall lockCall, CMutexFunctionCall coveredByLock +where + not isExcluded(n, Concurrency9Package::nonRecursiveMutexRecursivelyLockedAuditQuery()) and + lockCall = n and + coveredByLock = n.coveredByLock() and + not coveredByLock = lockCall and + // If mutexes are provably different objects, they do not need to be audited + not definitelyDifferentMutexes(lockCall, coveredByLock) and + ( + // If either mutex is not trackable, it should be audited + not isTrackableMutex(lockCall, _) or + not isTrackableMutex(coveredByLock, _) + ) and + not ( + // If either mutex is definitely recursive, it does not need to be audited + isTrackableMutex(lockCall, true) or + isTrackableMutex(coveredByLock, true) + ) +select n, "Mutex locked after it was already $@.", coveredByLock, "previously locked" diff --git a/c/misra/src/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.ql b/c/misra/src/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.ql new file mode 100644 index 0000000000..0d5aa5399f --- /dev/null +++ b/c/misra/src/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.ql @@ -0,0 +1,69 @@ +/** + * @id c/misra/condition-variable-used-with-multiple-mutexes + * @name RULE-22-19: A condition variable shall be associated with at most one mutex object + * @description Standard library functions cnd_wait() and cnd_timedwait() shall specify the same + * mutex object for each condition object in all calls. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-22-19 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.c.SubObjects +import codingstandards.cpp.Concurrency + +bindingset[cond, mutex] +int countMutexesForConditionVariable(SubObject cond, SubObject mutex) { + result = + count(CConditionOperation call | + call.getConditionExpr() = cond.getAnAddressOfExpr() and + call.getMutexExpr() = mutex.getAnAddressOfExpr() + ) +} + +bindingset[cond, mutex] +predicate conditionVariableUsesMutex(SubObject cond, SubObject mutex) { + countMutexesForConditionVariable(cond, mutex) > 0 +} + +bindingset[cond, n] +SubObject nthMutexForConditionVariable(SubObject cond, int n) { + result = + rank[n](SubObject mutex | + conditionVariableUsesMutex(cond, mutex) + | + mutex order by countMutexesForConditionVariable(cond, mutex), mutex.toString() + ) +} + +bindingset[cond, mutex] +CConditionOperation firstCallForConditionMutex(SubObject cond, SubObject mutex) { + result = + rank[1](CConditionOperation call | + call.getConditionExpr() = cond.getAnAddressOfExpr() and + call.getMutexExpr() = mutex.getAnAddressOfExpr() + | + call order by call.getFile().getAbsolutePath(), call.getLocation().getStartLine() + ) +} + +from + SubObject cond, CConditionOperation useOne, SubObject mutexOne, CConditionOperation useTwo, + SubObject mutexTwo +where + not isExcluded(cond.getRootIdentity(), + Concurrency9Package::conditionVariableUsedWithMultipleMutexesQuery()) and + mutexOne = nthMutexForConditionVariable(cond, 1) and + mutexTwo = nthMutexForConditionVariable(cond, 2) and + useOne = firstCallForConditionMutex(cond, mutexOne) and + useTwo = firstCallForConditionMutex(cond, mutexOne) +select useOne, + "Condition variable $@ associated with multiple mutexes, operation uses mutex $@ while $@ uses other mutex $@.", + cond.getRootIdentity(), cond.toString(), mutexOne.getRootIdentity(), mutexOne.toString(), useTwo, + "another operation", mutexTwo.getRootIdentity(), mutexTwo.toString() diff --git a/c/misra/src/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.ql b/c/misra/src/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.ql new file mode 100644 index 0000000000..1edf4aa9c3 --- /dev/null +++ b/c/misra/src/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.ql @@ -0,0 +1,44 @@ +/** + * @id c/misra/thread-storage-not-initialized-before-use + * @name RULE-22-20: Thread-specific storage pointers shall be created before being accessed + * @description Thread specific storage pointers shall be initialized with the standard library + * functions before using them. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-20 + * 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 +import codingstandards.c.initialization.GlobalInitializationAnalysis + +module ThreadStoreInitializationConfig implements GlobalInitializationAnalysisConfigSig { + ObjectIdentity getAnInitializedObject(Expr e) { + e.(TSSCreateFunctionCall).getKey() = result.getASubobjectAddressExpr() + } + + ObjectIdentity getAUsedObject(Expr e) { + result.getASubobjectAddressExpr() = e and + exists(ThreadSpecificStorageFunctionCall use | + not use instanceof TSSCreateFunctionCall and e = use.getKey() + ) + } +} + +import GlobalInitalizationAnalysis<ThreadStoreInitializationConfig> as InitAnalysis + +from Expr objUse, ObjectIdentity obj, Function callRoot +where + not isExcluded(objUse, Concurrency9Package::threadStorageNotInitializedBeforeUseQuery()) and + InitAnalysis::uninitializedFrom(objUse, obj, callRoot) +select objUse, + "Thread specific storage pointer '$@' used before initialization from entry point function '$@'.", + obj, obj.toString(), callRoot, callRoot.getName() diff --git a/c/misra/src/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.ql b/c/misra/src/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.ql new file mode 100644 index 0000000000..3c40ea7116 --- /dev/null +++ b/c/misra/src/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.ql @@ -0,0 +1,27 @@ +/** + * @id c/misra/thread-storage-pointer-initialized-inside-thread + * @name RULE-22-20: Thread specific storage pointers shall be initialized deterministically + * @description Thread specific storage pointers initialized inside of threads may result in + * indeterministic state. + * @kind problem + * @precision very-high + * @problem.severity recommendation + * @tags external/misra/id/rule-22-20 + * readability + * maintainability + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Concurrency + +from TSSCreateFunctionCall tssCreate, ThreadedFunction thread +where + not isExcluded(tssCreate, Concurrency8Package::mutexInitializedInsideThreadQuery()) and + thread.calls*(tssCreate.getEnclosingFunction()) +select tssCreate, + "Thread specific storage object initialization reachable from threaded function '$@'.", thread, + thread.getName() diff --git a/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected b/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected new file mode 100644 index 0000000000..e1c0e9389d --- /dev/null +++ b/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected @@ -0,0 +1,24 @@ +| test.c:31:3:31:8 | ... = ... | Threaded write to object $@ from thread function $@ is not synchronized with $@ from thread function $@. | test.c:11:5:11:6 | g2 | g2 | test.c:30:6:30:29 | single_thread4_writes_g2 | single_thread4_writes_g2 | test.c:27:3:27:4 | g2 | concurrent read operation | test.c:26:6:26:28 | single_thread3_reads_g2 | single_thread3_reads_g2 | +| test.c:35:3:35:8 | ... = ... | Threaded write to object $@ not synchronized from thread function $@ spawned from a loop. | test.c:12:5:12:6 | g3 | g3 | test.c:34:6:34:27 | many_thread5_writes_g3 | many_thread5_writes_g3 | test.c:35:3:35:4 | g3 | concurrent read operation | test.c:34:6:34:27 | many_thread5_writes_g3 | many_thread5_writes_g3 | +| test.c:71:3:71:11 | ... = ... | Threaded write to object $@ from thread function $@ is not synchronized with $@ from thread function $@. | test.c:68:3:68:4 | g7 | g7.m1 | test.c:70:6:70:33 | single_thread11_writes_g7_m1 | single_thread11_writes_g7_m1 | test.c:75:6:75:7 | m1 | concurrent read operation | test.c:74:6:74:33 | single_thread12_writes_g7_m1 | single_thread12_writes_g7_m1 | +| test.c:75:3:75:11 | ... = ... | Threaded write to object $@ from thread function $@ is not synchronized with $@ from thread function $@. | test.c:68:3:68:4 | g7 | g7.m1 | test.c:74:6:74:33 | single_thread12_writes_g7_m1 | single_thread12_writes_g7_m1 | test.c:71:6:71:7 | m1 | concurrent read operation | test.c:70:6:70:33 | single_thread11_writes_g7_m1 | single_thread11_writes_g7_m1 | +| test.c:79:3:79:11 | call to setlocale | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:79:3:79:11 | call to setlocale | setlocale | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:79:3:79:11 | call to setlocale | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:80:3:80:8 | call to tmpnam | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:80:3:80:8 | call to tmpnam | tmpnam | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:80:3:80:8 | call to tmpnam | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:81:3:81:6 | call to rand | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:81:3:81:6 | call to rand | rand | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:81:3:81:6 | call to rand | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:82:3:82:7 | call to srand | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:82:3:82:7 | call to srand | srand | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:82:3:82:7 | call to srand | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:83:3:83:8 | call to getenv | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:83:3:83:8 | call to getenv | getenv | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:83:3:83:8 | call to getenv | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:84:3:84:10 | call to getenv_s | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:84:3:84:10 | call to getenv_s | getenv_s | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:84:3:84:10 | call to getenv_s | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:86:3:86:10 | call to strerror | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:86:3:86:10 | call to strerror | strerror | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:86:3:86:10 | call to strerror | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:87:3:87:9 | call to asctime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:87:3:87:9 | call to asctime | asctime | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:87:3:87:9 | call to asctime | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:88:3:88:7 | call to ctime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:88:3:88:7 | call to ctime | ctime | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:88:3:88:7 | call to ctime | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:89:3:89:8 | call to gmtime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:89:3:89:8 | call to gmtime | gmtime | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:89:3:89:8 | call to gmtime | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:90:3:90:11 | call to localtime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:90:3:90:11 | call to localtime | localtime | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:90:3:90:11 | call to localtime | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:91:3:91:10 | call to mbrtoc16 | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:91:3:91:10 | call to mbrtoc16 | mbrtoc16 | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:91:3:91:10 | call to mbrtoc16 | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:92:3:92:10 | call to mbrtoc32 | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:92:3:92:10 | call to mbrtoc32 | mbrtoc32 | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:92:3:92:10 | call to mbrtoc32 | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:93:3:93:10 | call to c16rtomb | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:93:3:93:10 | call to c16rtomb | c16rtomb | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:93:3:93:10 | call to c16rtomb | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:94:3:94:10 | call to c32rtomb | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:94:3:94:10 | call to c32rtomb | c32rtomb | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:94:3:94:10 | call to c32rtomb | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:95:3:95:8 | call to mbrlen | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:95:3:95:8 | call to mbrlen | mbrlen | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:95:3:95:8 | call to mbrlen | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:96:3:96:9 | call to mbrtowc | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:96:3:96:9 | call to mbrtowc | mbrtowc | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:96:3:96:9 | call to mbrtowc | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:97:3:97:9 | call to wcrtomb | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:97:3:97:9 | call to wcrtomb | wcrtomb | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:97:3:97:9 | call to wcrtomb | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:98:3:98:11 | call to mbsrtowcs | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:98:3:98:11 | call to mbsrtowcs | mbsrtowcs | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:98:3:98:11 | call to mbsrtowcs | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.c:99:3:99:11 | call to wcsrtombs | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.c:99:3:99:11 | call to wcsrtombs | wcsrtombs | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.c:99:3:99:11 | call to wcsrtombs | concurrent call to non-reentrant function | test.c:78:6:78:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | diff --git a/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.qlref b/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.qlref new file mode 100644 index 0000000000..737cf79505 --- /dev/null +++ b/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.qlref @@ -0,0 +1 @@ +rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql \ No newline at end of file diff --git a/c/misra/test/rules/DIR-5-1/test.c b/c/misra/test/rules/DIR-5-1/test.c new file mode 100644 index 0000000000..5e568cc95c --- /dev/null +++ b/c/misra/test/rules/DIR-5-1/test.c @@ -0,0 +1,132 @@ +#include "locale.h" +#include "stdio.h" +#include "stdlib.h" +#include "string.h" +#include "threads.h" +#include "time.h" +#include "uchar.h" +#include "wchar.h" + +int g1; +int g2; +int g3; +int g4; +mtx_t g4_lock; +int g5; +mtx_t g5_lock; + +void single_thread1_reads_g1(void *p) { + g1; // COMPLIANT +} + +void many_thread2_reads_g1(void *p) { + g1; // COMPLIANT +} + +void single_thread3_reads_g2(void *p) { + g2; // COMPLIANT +} + +void single_thread4_writes_g2(void *p) { + g2 = 1; // NON-COMPLIANT +} + +void many_thread5_writes_g3(void *p) { + g3 = 1; // NON-COMPLIANT +} + +void single_thread6_reads_g4_locked(void *p) { + mtx_lock(&g4_lock); + g4; // COMPLIANT +} + +void single_thread7_writes_g4_locked(void *p) { + mtx_lock(&g4_lock); + g4 = 1; // COMPLIANT +} + +void many_thread8_writes_g5_locked(void *p) { + mtx_lock(&g5_lock); + g5 = 1; // COMPLIANT +} + +struct { + int m1; + int m2; +} g6; + +void single_thread9_writes_g6_m1(void *p) { + g6.m1 = 1; // COMPLIANT +} + +void single_thread10_writes_g6_m2(void *p) { + g6.m2 = 1; // COMPLIANT +} + +struct { + int m1; +} g7; + +void single_thread11_writes_g7_m1(void *p) { + g7.m1 = 1; // NON-COMPLIANT +} + +void single_thread12_writes_g7_m1(void *p) { + g7.m1 = 1; // NON-COMPLIANT +} + +void many_thread13_calls_nonreentrant_funcs(void *p) { + setlocale(LC_ALL, "C"); // NON-COMPLIANT + tmpnam(""); // NON-COMPLIANT + rand(); // NON-COMPLIANT + srand(0); // NON-COMPLIANT + getenv("PATH"); // NON-COMPLIANT + getenv_s(NULL, NULL, 0, NULL); // NON-COMPLIANT + strtok("a", "b"); // NON-COMPLIANT + strerror(0); // NON-COMPLIANT + asctime(NULL); // NON-COMPLIANT + ctime(NULL); // NON-COMPLIANT + gmtime(NULL); // NON-COMPLIANT + localtime(NULL); // NON-COMPLIANT + mbrtoc16(NULL, NULL, 0, NULL); // NON-COMPLIANT + mbrtoc32(NULL, NULL, 0, NULL); // NON-COMPLIANT + c16rtomb(NULL, 0, NULL); // NON-COMPLIANT + c32rtomb(NULL, 0, NULL); // NON-COMPLIANT + mbrlen(NULL, 0, NULL); // NON-COMPLIANT + mbrtowc(NULL, NULL, 0, NULL); // NON-COMPLIANT + wcrtomb(NULL, 0, NULL); // NON-COMPLIANT + mbsrtowcs(NULL, NULL, 0, NULL); // NON-COMPLIANT + wcsrtombs(NULL, NULL, 0, NULL); // NON-COMPLIANT +} + +void main() { + thrd_t single_thread1; + thrd_t many_thread2; + thrd_t single_thread3; + thrd_t single_thread4; + thrd_t many_thread5; + thrd_t single_thread6; + thrd_t single_thread7; + thrd_t many_thread8; + thrd_t single_thread9; + thrd_t single_thread10; + thrd_t single_thread11; + thrd_t single_thread12; + thrd_t many_thread13; + + thrd_create(&single_thread1, single_thread1_reads_g1, NULL); + thrd_create(&single_thread3, single_thread3_reads_g2, NULL); + thrd_create(&single_thread4, single_thread4_writes_g2, NULL); + thrd_create(&single_thread6, single_thread6_reads_g4_locked, NULL); + thrd_create(&single_thread7, single_thread7_writes_g4_locked, NULL); + thrd_create(&single_thread9, single_thread9_writes_g6_m1, NULL); + thrd_create(&single_thread10, single_thread10_writes_g6_m2, NULL); + thrd_create(&single_thread11, single_thread11_writes_g7_m1, NULL); + thrd_create(&single_thread12, single_thread12_writes_g7_m1, NULL); + for (;;) { + thrd_create(&many_thread2, many_thread2_reads_g1, NULL); + thrd_create(&many_thread5, many_thread5_writes_g3, NULL); + thrd_create(&many_thread8, many_thread8_writes_g5_locked, NULL); + thrd_create(&many_thread13, many_thread13_calls_nonreentrant_funcs, NULL); + } +} \ 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 index d8f1770ad8..c664a08dc3 100644 --- a/c/misra/test/rules/RULE-22-14/test.c +++ b/c/misra/test/rules/RULE-22-14/test.c @@ -143,4 +143,16 @@ void invalid_mtx_init_types() { mtx_init(&m, mtx_plain & mtx_recursive); // NON-COMPLIANT mtx_init(&m, mtx_plain * mtx_recursive); // NON-COMPLIANT mtx_init(&m, -1); // NON-COMPLIANT +} + +void function_pointer_uses_global_mutexes() { + // If the function has been used as a function pointer, we don't attempt to + // analyze this. + mtx_lock(&g1); // COMPLIANT + mtx_lock(&g2.m1); // COMPLIANT + mtx_lock(g3); // COMPLIANT +} + +void take_function_pointer() { + void (*f)(void) = function_pointer_uses_global_mutexes; } \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.expected b/c/misra/test/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.expected new file mode 100644 index 0000000000..49f1b74c15 --- /dev/null +++ b/c/misra/test/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.expected @@ -0,0 +1,12 @@ +| test.c:16:3:16:13 | call to mtx_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:3:7:3:8 | g1 | g1 | test.c:64:3:64:13 | call to thrd_create | t2_use_all | +| test.c:16:3:16:13 | call to mtx_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:3:7:3:8 | g1 | g1 | test.c:72:3:72:13 | call to thrd_create | t2_use_all | +| test.c:16:3:16:13 | call to mtx_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:3:7:3:8 | g1 | g1 | test.c:91:3:91:10 | call to mtx_lock | main thread | +| test.c:17:3:17:12 | call to tss_delete | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:4:7:4:8 | g2 | g2 | test.c:64:3:64:13 | call to thrd_create | t2_use_all | +| test.c:17:3:17:12 | call to tss_delete | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:4:7:4:8 | g2 | g2 | test.c:72:3:72:13 | call to thrd_create | t2_use_all | +| test.c:17:3:17:12 | call to tss_delete | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:4:7:4:8 | g2 | g2 | test.c:92:3:92:9 | call to tss_get | main thread | +| test.c:18:3:18:13 | call to cnd_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:5:7:5:8 | g3 | g3 | test.c:64:3:64:13 | call to thrd_create | t2_use_all | +| test.c:18:3:18:13 | call to cnd_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:5:7:5:8 | g3 | g3 | test.c:72:3:72:13 | call to thrd_create | t2_use_all | +| test.c:18:3:18:13 | call to cnd_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:5:7:5:8 | g3 | g3 | test.c:93:3:93:10 | call to cnd_wait | main thread | +| test.c:42:3:42:13 | call to mtx_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:3:7:3:8 | g1 | g1 | test.c:41:3:41:13 | call to thrd_create | t2_use_all | +| test.c:43:3:43:12 | call to tss_delete | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:4:7:4:8 | g2 | g2 | test.c:41:3:41:13 | call to thrd_create | t2_use_all | +| test.c:44:3:44:13 | call to cnd_destroy | Thread resource $@ disposed before joining thread $@ which uses it. | test.c:5:7:5:8 | g3 | g3 | test.c:41:3:41:13 | call to thrd_create | t2_use_all | diff --git a/c/misra/test/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.qlref b/c/misra/test/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.qlref new file mode 100644 index 0000000000..809eae6faf --- /dev/null +++ b/c/misra/test/rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.qlref @@ -0,0 +1 @@ +rules/RULE-22-15/ThreadResourceDisposedBeforeThreadsJoined.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-15/test.c b/c/misra/test/rules/RULE-22-15/test.c new file mode 100644 index 0000000000..7679730fc9 --- /dev/null +++ b/c/misra/test/rules/RULE-22-15/test.c @@ -0,0 +1,113 @@ +#include "threads.h" + +mtx_t g1; +tss_t g2; +cnd_t g3; + +int t1_use_none(void *p) { return 0; } + +int t2_use_all(void *p) { + mtx_lock(&g1); + tss_get(&g2); + cnd_wait(&g3, &g1); +} + +int t3_dispose_all(void *p) { + mtx_destroy(&g1); + tss_delete(&g2); + cnd_destroy(&g3); +} + +int t4_use_then_dispose(void *p) { + mtx_lock(&g1); + tss_get(&g2); + cnd_wait(&g3, &g1); + + mtx_destroy(&g1); + tss_delete(&g2); + cnd_destroy(&g3); +} + +void f1() { + thrd_t t; + thrd_create(&t, t1_use_none, NULL); + mtx_destroy(&g1); + tss_delete(&g2); + cnd_destroy(&g3); +} + +void f2() { + thrd_t t; + thrd_create(&t, t2_use_all, NULL); + mtx_destroy(&g1); // NON-COMPLIANT + tss_delete(&g2); // NON-COMPLIANT + cnd_destroy(&g3); // NON-COMPLIANT +} + +void f3() { + thrd_t t; + thrd_create(&t, t2_use_all, NULL); // COMPLIANT +} + +void f4() { + thrd_t t; + thrd_create(&t, t2_use_all, NULL); // COMPLIANT + thrd_join(&t, NULL); + mtx_destroy(&g1); // COMPLIANT + tss_delete(&g2); // COMPLIANT + cnd_destroy(&g3); // COMPLIANT +} + +void f5() { + thrd_t t1; + thrd_t t2; + thrd_create(&t1, t2_use_all, NULL); // COMPLIANT + thrd_create(&t2, t3_dispose_all, NULL); // NON-COMPLIANT +} + +void f6() { + thrd_t t1; + thrd_t t2; + thrd_create(&t1, t3_dispose_all, NULL); // NON-COMPLIANT + thrd_create(&t2, t2_use_all, NULL); // COMPLIANT +} + +void f7() { + thrd_t t1; + thrd_t t2; + thrd_create(&t1, t2_use_all, NULL); // COMPLIANT + thrd_join(&t1, NULL); + thrd_create(&t2, t3_dispose_all, NULL); // COMPLIANT +} + +void f8() { + thrd_t t; + thrd_create(&t, t4_use_then_dispose, NULL); // COMPLIANT +} + +void f9() { + thrd_t t; + thrd_create(&t, t3_dispose_all, NULL); // NON-COMPLIANT + mtx_lock(&g1); + tss_get(&g2); + cnd_wait(&g3, &g1); +} + +void f10() { + thrd_t t; + mtx_lock(&g1); + tss_get(&g2); + cnd_wait(&g3, &g1); + thrd_create(&t, t3_dispose_all, NULL); // COMPLIANT +} + +void f11() { + thrd_t t; + thrd_create(&t, t1_use_none, NULL); + mtx_lock(&g1); + tss_get(&g2); + cnd_wait(&g3, &g1); + mtx_destroy(&g1); // COMPLIANT + tss_delete(&g2); // COMPLIANT + cnd_destroy(&g3); // COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.expected b/c/misra/test/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.expected new file mode 100644 index 0000000000..254d55adc2 --- /dev/null +++ b/c/misra/test/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.expected @@ -0,0 +1,16 @@ +| test.c:19:3:19:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:11:9:11:10 | l1 | l1 | +| test.c:20:3:20:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:11:9:11:10 | l1 | l1 | +| test.c:25:3:25:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:14:5:14:6 | l2 | l2.m1 | +| test.c:26:3:26:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:14:5:14:6 | l2 | l2.m1 | +| test.c:31:3:31:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:3:7:3:8 | g1 | g1 | +| test.c:32:3:32:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:3:7:3:8 | g1 | g1 | +| test.c:37:3:37:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:6:3:6:4 | g2 | g2.m1 | +| test.c:38:3:38:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:6:3:6:4 | g2 | g2.m1 | +| test.c:47:3:47:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:11:9:11:10 | l1 | l1 | +| test.c:48:3:48:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:14:5:14:6 | l2 | l2.m1 | +| test.c:49:3:49:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:3:7:3:8 | g1 | g1 | +| test.c:50:3:50:10 | call to cnd_wait | Invalid operation on mutex '$@' not locked by the current thread. | test.c:6:3:6:4 | g2 | g2.m1 | +| test.c:51:3:51:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:11:9:11:10 | l1 | l1 | +| test.c:52:3:52:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:14:5:14:6 | l2 | l2.m1 | +| test.c:53:3:53:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:3:7:3:8 | g1 | g1 | +| test.c:54:3:54:12 | call to mtx_unlock | Invalid operation on mutex '$@' not locked by the current thread. | test.c:6:3:6:4 | g2 | g2.m1 | diff --git a/c/misra/test/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.qlref b/c/misra/test/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.qlref new file mode 100644 index 0000000000..4ac06f10ed --- /dev/null +++ b/c/misra/test/rules/RULE-22-17/InvalidOperationOnUnlockedMutex.qlref @@ -0,0 +1 @@ +rules/RULE-22-17/InvalidOperationOnUnlockedMutex.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-17/test.c b/c/misra/test/rules/RULE-22-17/test.c new file mode 100644 index 0000000000..fc841bb3e1 --- /dev/null +++ b/c/misra/test/rules/RULE-22-17/test.c @@ -0,0 +1,70 @@ +#include "threads.h" + +mtx_t g1; +struct { + mtx_t m1; +} g2; + +cnd_t cnd; + +void f1(int p) { + mtx_t l1; + struct { + mtx_t m1; + } l2; + + mtx_lock(&l1); + cnd_wait(&cnd, &l1); // COMPLIANT + mtx_unlock(&l1); // COMPLIANT + cnd_wait(&cnd, &l1); // NON-COMPLIANT + mtx_unlock(&l1); // NON-COMPLIANT + + mtx_lock(&l2.m1); + cnd_wait(&cnd, &l2.m1); // COMPLIANT + mtx_unlock(&l2.m1); // COMPLIANT + cnd_wait(&cnd, &l2.m1); // NON-COMPLIANT + mtx_unlock(&l2.m1); // NON-COMPLIANT + + mtx_lock(&g1); + cnd_wait(&cnd, &g1); // COMPLIANT + mtx_unlock(&g1); // COMPLIANT + cnd_wait(&cnd, &g1); // NON-COMPLIANT + mtx_unlock(&g1); // NON-COMPLIANT + + mtx_lock(&g2.m1); + cnd_wait(&cnd, &g2.m1); // COMPLIANT + mtx_unlock(&g2.m1); // COMPLIANT + cnd_wait(&cnd, &g2.m1); // NON-COMPLIANT + mtx_unlock(&g2.m1); // NON-COMPLIANT + + // We should report when a mutex is unlocked in the wrong block: + if (p) { + mtx_lock(&l1); + mtx_lock(&l2.m1); + mtx_lock(&g1); + mtx_lock(&g2.m1); + } + cnd_wait(&cnd, &l1); // NON-COMPLIANT + cnd_wait(&cnd, &l2.m1); // NON-COMPLIANT + cnd_wait(&cnd, &g1); // NON-COMPLIANT + cnd_wait(&cnd, &g2.m1); // NON-COMPLIANT + mtx_unlock(&l1); // NON-COMPLIANT + mtx_unlock(&l2.m1); // NON-COMPLIANT + mtx_unlock(&g1); // NON-COMPLIANT + mtx_unlock(&g2.m1); // NON-COMPLIANT + + // The above requires dominance analysis. Check dominator sets don't cause + // false positives: + if (p) { + mtx_lock(&l1); + } else { + mtx_lock(&l1); + } + mtx_unlock(&l1); // COMPLIANT + + // Invalid but satisfies the rule: + mtx_lock(&l1); + if (p) { + mtx_unlock(&l1); // COMPLIANT + } +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.expected b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.expected new file mode 100644 index 0000000000..fd947dee51 --- /dev/null +++ b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.expected @@ -0,0 +1,2 @@ +| test.c:19:3:19:10 | call to mtx_lock | Non-recursive mutex nonrec locked after it is $@. | test.c:18:3:18:10 | call to mtx_lock | already locked | +| test.c:22:3:22:10 | call to mtx_lock | Non-recursive mutex s.m locked after it is $@. | test.c:21:3:21:10 | call to mtx_lock | already locked | diff --git a/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.qlref b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.qlref new file mode 100644 index 0000000000..131e0476bf --- /dev/null +++ b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.qlref @@ -0,0 +1 @@ +rules/RULE-22-18/NonRecursiveMutexRecursivelyLocked.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.expected b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.expected new file mode 100644 index 0000000000..e268f5367e --- /dev/null +++ b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.expected @@ -0,0 +1,6 @@ +| test.c:44:3:44:10 | call to mtx_lock | Mutex locked after it was already $@. | test.c:43:3:43:10 | call to mtx_lock | previously locked | +| test.c:49:3:49:10 | call to mtx_lock | Mutex locked after it was already $@. | test.c:48:3:48:10 | call to mtx_lock | previously locked | +| test.c:54:3:54:10 | call to mtx_lock | Mutex locked after it was already $@. | test.c:53:3:53:10 | call to mtx_lock | previously locked | +| test.c:59:3:59:10 | call to mtx_lock | Mutex locked after it was already $@. | test.c:58:3:58:10 | call to mtx_lock | previously locked | +| test.c:76:3:76:10 | call to mtx_lock | Mutex locked after it was already $@. | test.c:75:3:75:10 | call to mtx_lock | previously locked | +| test.c:81:3:81:10 | call to mtx_lock | Mutex locked after it was already $@. | test.c:80:3:80:10 | call to mtx_lock | previously locked | diff --git a/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.qlref b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.qlref new file mode 100644 index 0000000000..77a81deb69 --- /dev/null +++ b/c/misra/test/rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.qlref @@ -0,0 +1 @@ +rules/RULE-22-18/NonRecursiveMutexRecursivelyLockedAudit.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-18/test.c b/c/misra/test/rules/RULE-22-18/test.c new file mode 100644 index 0000000000..f71066b1bc --- /dev/null +++ b/c/misra/test/rules/RULE-22-18/test.c @@ -0,0 +1,122 @@ +#include "threads.h" + +mtx_t rec; +mtx_t nonrec; +mtx_t both; +mtx_t unknown; + +struct { + mtx_t m; +} s; + +mtx_t arr[2]; + +int t1(void *arg) { + mtx_lock(&rec); // COMPLIANT + mtx_lock(&rec); // COMPLIANT + + mtx_lock(&nonrec); // COMPLIANT + mtx_lock(&nonrec); // NON-COMPLIANT + + mtx_lock(&s.m); // COMPLIANT + mtx_lock(&s.m); // NON-COMPLIANT +} + +void f1() { + mtx_init(&rec, mtx_plain | mtx_recursive); + mtx_init(&nonrec, mtx_plain); + mtx_init(&both, mtx_plain); + mtx_init(&both, mtx_plain | mtx_recursive); + // Do not initialize `unknown`. + mtx_init(&s.m, mtx_plain); + mtx_init(&arr[0], mtx_plain); + mtx_init(&arr[1], mtx_plain); + + thrd_t t; + thrd_create(t, t1, NULL); +} + +mtx_t *p; + +// Results for the audit query: +void t2(void *arg) { + mtx_lock(&arr[0]); + mtx_lock(&arr[(int)arg]); // NON-COMPLIANT +} + +void t3(void *arg) { + mtx_lock(arg); + mtx_lock(p); // NON-COMPLIANT +} + +void t4() { + mtx_lock(&both); + mtx_lock(&both); // NON-COMPLIANT +} + +void t5() { + mtx_lock(&unknown); + mtx_lock(&unknown); // NON-COMPLIANT +} + +void t6() { + // Cannot be locks of the same mutex: + mtx_lock(&nonrec); + mtx_lock(&unknown); // COMPLIANT +} + +void t7() { + mtx_lock(p); + // Definitely a recursive mutex: + mtx_lock(&rec); // COMPLIANT +} + +void t8() { + mtx_lock(p); + mtx_lock(&nonrec); // NON-COMPLIANT +} + +void t9() { + mtx_lock(&nonrec); + mtx_lock(p); // NON-COMPLIANT +} + +void f2() { + thrd_t t; + thrd_create(t, t2, NULL); +} + +void f3() { + thrd_t t; + thrd_create(t, t3, &rec); +} + +void f4() { + thrd_t t; + thrd_create(t, t4, NULL); +} + +void f5() { + thrd_t t; + thrd_create(t, t5, NULL); +} + +void f6() { + thrd_t t; + thrd_create(t, t6, NULL); +} + +void f7() { + thrd_t t; + thrd_create(t, t7, NULL); +} + +void f8() { + thrd_t t; + thrd_create(t, t8, NULL); +} + +void f9() { + thrd_t t; + thrd_create(t, t9, NULL); +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.expected b/c/misra/test/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.expected new file mode 100644 index 0000000000..c9785067c6 --- /dev/null +++ b/c/misra/test/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.expected @@ -0,0 +1,2 @@ +| test.c:19:3:19:10 | call to cnd_wait | Condition variable $@ associated with multiple mutexes, operation uses mutex $@ while $@ uses other mutex $@. | test.c:16:9:16:12 | cnd1 | cnd1 | test.c:17:9:17:12 | mtx1 | mtx1 | test.c:19:3:19:10 | call to cnd_wait | another operation | test.c:18:9:18:12 | mtx2 | mtx2 | +| test.c:41:3:41:10 | call to cnd_wait | Condition variable $@ associated with multiple mutexes, operation uses mutex $@ while $@ uses other mutex $@. | test.c:37:7:37:11 | gcnd1 | gcnd1 | test.c:38:7:38:11 | gmtx1 | gmtx1 | test.c:41:3:41:10 | call to cnd_wait | another operation | test.c:39:7:39:11 | gmtx2 | gmtx2 | diff --git a/c/misra/test/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.qlref b/c/misra/test/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.qlref new file mode 100644 index 0000000000..d43a824ec8 --- /dev/null +++ b/c/misra/test/rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.qlref @@ -0,0 +1 @@ +rules/RULE-22-19/ConditionVariableUsedWithMultipleMutexes.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-19/test.c b/c/misra/test/rules/RULE-22-19/test.c new file mode 100644 index 0000000000..f4b46d4077 --- /dev/null +++ b/c/misra/test/rules/RULE-22-19/test.c @@ -0,0 +1,46 @@ +#include "threads.h" + +void f1(void) { + cnd_t cnd1; + mtx_t mtx1; + cnd_wait(&cnd1, &mtx1); // COMPLIANT + cnd_wait(&cnd1, &mtx1); // COMPLIANT + + cnd_t cnd2; + mtx_t mtx2; + cnd_wait(&cnd2, &mtx2); // COMPLIANT + cnd_wait(&cnd2, &mtx2); // COMPLIANT +} + +void f2(void) { + cnd_t cnd1; + mtx_t mtx1; + mtx_t mtx2; + cnd_wait(&cnd1, &mtx1); // NON-COMPLIANT + cnd_wait(&cnd1, &mtx2); // NON-COMPLIANT +} + +void f3(void) { + cnd_t cnd1; + cnd_t cnd2; + mtx_t mtx1; + cnd_wait(&cnd1, &mtx1); // COMPLIANT + cnd_wait(&cnd2, &mtx1); // COMPLIANT +} + +void f4(cnd_t *cnd1, mtx_t *mtx1, mtx_t *mtx2) { + cnd_wait(cnd1, mtx1); // COMPLIANT + // Compliant, mtx1 and mtx2 may point to the same object + cnd_wait(cnd1, mtx2); // COMPLIANT +} + +cnd_t gcnd1; +mtx_t gmtx1; +mtx_t gmtx2; +void f5(void) { + cnd_wait(&gcnd1, &gmtx1); // NON-COMPLIANT +} + +void f6(void) { + cnd_wait(&gcnd1, &gmtx2); // NON-COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.expected b/c/misra/test/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.expected new file mode 100644 index 0000000000..301debd7e8 --- /dev/null +++ b/c/misra/test/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.expected @@ -0,0 +1,5 @@ +| test.c:6:11:6:13 | & ... | Thread specific storage pointer '$@' 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:11:11:11:12 | l4 | Thread specific storage pointer '$@' used before initialization from entry point function '$@'. | test.c:10:15:10:20 | call to malloc | call to malloc | test.c:16:6:16:31 | root1_calls_use_local_mtxs | root1_calls_use_local_mtxs | +| test.c:25:11:25:13 | & ... | Thread specific storage pointer '$@' used before initialization from entry point function '$@'. | test.c:22:7:22:8 | g1 | g1 | test.c:24:6:24:28 | root2_uses_global_tss_t | root2_uses_global_tss_t | +| test.c:38:11:38:13 | & ... | Thread specific storage pointer '$@' used before initialization from entry point function '$@'. | test.c:22:7:22:8 | g1 | g1 | test.c:41:6:41:45 | root4_call_thread_without_initialization | root4_call_thread_without_initialization | +| test.c:58:11:58:13 | & ... | Thread specific storage pointer '$@' used before initialization from entry point function '$@'. | test.c:56:7:56:8 | g5 | g5 | test.c:67:6:67:50 | root6_spawn_thread_uninitialized_thread_local | root6_spawn_thread_uninitialized_thread_local | diff --git a/c/misra/test/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.qlref b/c/misra/test/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.qlref new file mode 100644 index 0000000000..10d9aadf1b --- /dev/null +++ b/c/misra/test/rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.qlref @@ -0,0 +1 @@ +rules/RULE-22-20/ThreadStorageNotInitializedBeforeUse.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.expected b/c/misra/test/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.expected new file mode 100644 index 0000000000..75e9825074 --- /dev/null +++ b/c/misra/test/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.expected @@ -0,0 +1 @@ +| test.c:61:3:61:12 | call to tss_create | Thread specific storage object initialization reachable from threaded function '$@'. | test.c:57:6:57:41 | from_root6_init_and_use_thread_local | from_root6_init_and_use_thread_local | diff --git a/c/misra/test/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.qlref b/c/misra/test/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.qlref new file mode 100644 index 0000000000..d299808814 --- /dev/null +++ b/c/misra/test/rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.qlref @@ -0,0 +1 @@ +rules/RULE-22-20/ThreadStoragePointerInitializedInsideThread.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-20/test.c b/c/misra/test/rules/RULE-22-20/test.c new file mode 100644 index 0000000000..0fe58abdcd --- /dev/null +++ b/c/misra/test/rules/RULE-22-20/test.c @@ -0,0 +1,70 @@ +#include "stdlib.h" +#include "threads.h" + +void use_local_mtxs(int x, int y) { + tss_t l1; + tss_get(&l1); // NON-COMPLIANT + tss_create(&l1, NULL); + tss_get(&l1); // COMPLIANT + + tss_t *l4 = malloc(sizeof(tss_t)); + tss_get(l4); // NON-COMPLIANT + tss_create(l4, NULL); + tss_get(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); +} + +tss_t g1; + +void root2_uses_global_tss_t() { + tss_get(&g1); // NON-COMPLIANT +} + +void from_root3_use_global_tss_t() { + tss_get(&g1); // COMPLIANT +} + +void root3_initializes_and_uses_global_tss_t() { + tss_create(&g1, NULL); + from_root3_use_global_tss_t(); +} + +void from_root4_use_global_tss_t(void *arg) { + tss_get(&g1); // NON-COMPLIANT +} + +void root4_call_thread_without_initialization() { + thrd_t t; + thrd_create(&t, &from_root4_use_global_tss_t, NULL); +} + +void from_root5_use_global_tss_t(void *arg) { + tss_get(&g1); // COMPLIANT +} + +void root5_thread_with_initialization() { + tss_create(&g1, NULL); + thrd_t t; + thrd_create(&t, &from_root5_use_global_tss_t, NULL); +} + +mtx_t g5; +void from_root6_init_and_use_thread_local() { + tss_get(&g5); // NON-COMPLIANT + + // Violates recommendation, tss_t initialized within a thread. + tss_create(&g5, NULL); // NON-COMPLIANT + + // Valid if we except the above initialization. + tss_get(&g5); // COMPLIANT +} + +void root6_spawn_thread_uninitialized_thread_local() { + thrd_t t; + thrd_create(&t, &from_root6_init_and_use_thread_local, NULL); +} \ No newline at end of file diff --git a/change_notes/2024-12-10-refactor-concurrency-library.md b/change_notes/2024-12-10-refactor-concurrency-library.md new file mode 100644 index 0000000000..ccefe85f19 --- /dev/null +++ b/change_notes/2024-12-10-refactor-concurrency-library.md @@ -0,0 +1,2 @@ + - `Concurrency.qll` - for all queries using this library + - This has been refactored into a set of smaller utility files. No impact on query results or performance expected. \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll index cfb0f03f72..0e2afb8ece 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -1,1015 +1,15 @@ import cpp import semmle.code.cpp.dataflow.TaintTracking - -/** - * Models CFG nodes which should be added to a thread context. - */ -abstract class ThreadedCFGPathExtension extends ControlFlowNode { - /** - * Returns the next `ControlFlowNode` in this thread context. - */ - abstract ControlFlowNode getNext(); -} - -/** - * Models a `FunctionCall` invoked from a threaded context. - */ -class ThreadContextFunctionCall extends FunctionCall, ThreadedCFGPathExtension { - override ControlFlowNode getNext() { getTarget().getEntryPoint() = result } -} - -/** - * Models a specialized `FunctionCall` that may create a thread. - */ -abstract class ThreadCreationFunction extends FunctionCall, ThreadedCFGPathExtension { - /** - * Returns the function that will be invoked. - */ - abstract Function getFunction(); -} - -/** - * Models a call to a thread constructor via `std::thread`. - */ -class ThreadConstructorCall extends ConstructorCall, ThreadCreationFunction { - Function f; - - ThreadConstructorCall() { - getTarget().getDeclaringType().hasQualifiedName("std", "thread") and - f = getArgument(0).(FunctionAccess).getTarget() - } - - /** - * Returns the function that will be invoked by this `std::thread`. - */ - override Function getFunction() { result = f } - - override ControlFlowNode getNext() { result = getFunction().getEntryPoint() } -} - -/** - * Models a call to a thread creation via `thrd_create` or `pthread_create`. - */ -class CThreadCreateCall extends FunctionCall { - Function f; - int fArgIdx; - - CThreadCreateCall() { - ( - getTarget().getName() = "thrd_create" and - fArgIdx = 1 - or - getTarget().getName() = "pthread_create" and - fArgIdx = 2 - ) and - ( - f = getArgument(fArgIdx).(FunctionAccess).getTarget() or - f = getArgument(fArgIdx).(AddressOfExpr).getOperand().(FunctionAccess).getTarget() - ) - } - - /** - * Returns the function that will be invoked by this thread. - */ - Function getFunction() { result = f } -} - -/** - * Models a call to a thread constructor via `thrd_create`. - */ -class C11ThreadCreateCall extends ThreadCreationFunction, CThreadCreateCall { - C11ThreadCreateCall() { getTarget().getName() = "thrd_create" } - - /** - * Returns the function that will be invoked by this thread. - */ - override Function getFunction() { result = f } - - 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. - */ -abstract class MutexFunctionCall extends LockingOperation { - abstract predicate isRecursive(); - - abstract predicate isSpeculativeLock(); - - abstract predicate unlocks(MutexFunctionCall fc); -} - -/** - * Models calls to various mutex types found in CPP. - */ -class CPPMutexFunctionCall extends MutexFunctionCall { - VariableAccess var; - - CPPMutexFunctionCall() { - ( - // the non recursive kinds - getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "mutex") or - getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "timed_mutex") or - getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "shared_timed_mutex") or - // the recursive ones - getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "recursive_mutex") or - getTarget() - .(MemberFunction) - .getDeclaringType() - .hasQualifiedName("std", "recursive_timed_mutex") - ) and - var = getQualifier() - } - - /** - * Holds if this mutex is a recursive mutex. - */ - override predicate isRecursive() { - getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "recursive_mutex") or - getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "recursive_timed_mutex") - } - - /** - * Holds if this `CPPMutexFunctionCall` is a lock. - */ - override predicate isLock() { - not isLockingOperationWithinLockingOperation(this) and - getTarget().getName() = "lock" - } - - /** - * Holds if this `CPPMutexFunctionCall` is a speculative lock, defined as calling - * one of the speculative locking functions such as `try_lock`. - */ - override predicate isSpeculativeLock() { - getTarget().getName() in [ - "try_lock", "try_lock_for", "try_lock_until", "try_lock_shared_for", "try_lock_shared_until" - ] - } - - /** - * Returns the lock to which this `CPPMutexFunctionCall` refers to. - */ - override Variable getLock() { result = getQualifier().(VariableAccess).getTarget() } - - /** - * Returns the qualifier for this `CPPMutexFunctionCall`. - */ - override Expr getLockExpr() { result = var } - - /** - * Holds if this is a `unlock` and *may* unlock the previously locked `MutexFunctionCall`. - * This predicate does not check that the mutex is currently locked. - */ - override predicate unlocks(MutexFunctionCall fc) { - isUnlock() and - fc.getQualifier().(VariableAccess).getTarget() = getQualifier().(VariableAccess).getTarget() - } - - /** - * Holds if this is an unlock call. - */ - override predicate isUnlock() { getTarget().getName() = "unlock" } -} - -/** - * Models calls to various mutex types specialized to C code. - */ -class CMutexFunctionCall extends MutexFunctionCall { - Expr arg; - - CMutexFunctionCall() { - // the non recursive kinds - getTarget().getName() = ["mtx_lock", "mtx_unlock", "mtx_timedlock", "mtx_trylock"] and - arg = getArgument(0) - } - - /** - * Holds if this mutex is a recursive mutex. - */ - override predicate isRecursive() { none() } - - /** - * Holds if this `CMutexFunctionCall` is a lock. - */ - override predicate isLock() { - not isLockingOperationWithinLockingOperation(this) and - getTarget().getName() = ["mtx_lock", "mtx_timedlock", "mtx_trylock"] - } - - /** - * Holds if this `CMutexFunctionCall` is a speculative lock, defined as calling - * one of the speculative locking functions such as `try_lock`. - */ - override predicate isSpeculativeLock() { - getTarget().getName() in ["mtx_timedlock", "mtx_trylock"] - } - - /** - * Returns the `Variable` to which this `CMutexFunctionCall` refers to. For this - * style of lock it can reference a number of different variables. - */ - override Variable getLock() { - exists(VariableAccess va | - TaintTracking::localTaint(DataFlow::exprNode(va), DataFlow::exprNode(getLockExpr())) and - result = va.getTarget() - ) - } - - /** - * Returns the expression for this `CMutexFunctionCall`. - */ - override Expr getLockExpr() { result = arg } - - /** - * Holds if this is a `unlock` and *may* unlock the previously locked `CMutexFunctionCall`. - * This predicate does not check that the mutex is currently locked. - */ - override predicate unlocks(MutexFunctionCall fc) { - isUnlock() and - fc.getLock() = getLock() - } - - /** - * Holds if this is an unlock call. - */ - override predicate isUnlock() { getTarget().getName() = "mtx_unlock" } -} - -/** - * The thread-aware predecessor function is defined in terms of the thread aware - * successor function. This is because it is simpler to construct the forward - * paths of a thread's execution than the backwards paths. For this reason we - * require a `start` and `end` node. - * - * The logic of this function is that a thread aware predecessor is one that - * follows a `start` node, is not equal to the ending node, and does not follow - * the `end` node. Such nodes can only be predecessors of `end`. - * - * For this reason this function requires a `start` node from which to start - * considering something a predecessor of `end`. - */ -pragma[inline] -ControlFlowNode getAThreadContextAwarePredecessor(ControlFlowNode start, ControlFlowNode end) { - result = getAThreadContextAwareSuccessor(start) and - not result = getAThreadContextAwareSuccessor(end) and - not result = end -} - -/** - * A predicate for finding successors of `ControlFlowNode`s that are aware of - * the objects that my flow into a thread's context. This is achieved by adding - * additional edges to thread entry points and function calls. - */ -ControlFlowNode getAThreadContextAwareSuccessorR(ControlFlowNode cfn) { - result = cfn.getASuccessor() - or - result = cfn.(ThreadedCFGPathExtension).getNext() -} - -ControlFlowNode getAThreadContextAwareSuccessor(ControlFlowNode m) { - result = getAThreadContextAwareSuccessorR*(m) and - // for performance reasons we handle back edges by enforcing a lexical - // ordering restriction on these nodes if they are both in - // the same loop. One way of doing this is as follows: - // - // ````and ( - // exists(Loop loop | - // loop.getAChild*() = m and - // loop.getAChild*() = result - // ) - // implies - // not result.getLocation().isBefore(m.getLocation()) - // )``` - // In this implementation we opt for the more generic form below - // which seems to have reasonable performance. - ( - m.getEnclosingStmt().getParentStmt*() = result.getEnclosingStmt().getParentStmt*() - implies - not exists(Location l1, Location l2 | - l1 = result.getLocation() and - l2 = m.getLocation() - | - l1.getEndLine() < l2.getStartLine() - or - l1.getStartLine() = l2.getEndLine() and - l1.getEndColumn() < l2.getStartColumn() - ) - ) -} - -abstract class LockingOperation extends FunctionCall { - /** - * Returns the target of the lock underlying this RAII-style lock. - */ - abstract Variable getLock(); - - /** - * Returns the lock underlying this RAII-style lock. - */ - abstract Expr getLockExpr(); - - /** - * Holds if this is a lock operation - */ - abstract predicate isLock(); - - /** - * Holds if this is an unlock operation - */ - abstract predicate isUnlock(); - - /** - * Holds if this locking operation is really a locking operation within a - * designated locking operation. This library assumes the underlying locking - * operations are implemented correctly in that calling a `LockingOperation` - * results in the creation of a singular lock. - */ - predicate isLockingOperationWithinLockingOperation(LockingOperation inner) { - exists(LockingOperation outer | outer.getTarget() = inner.getEnclosingFunction()) - } -} - -/** - * Models a RAII-Style lock. - */ -class RAIIStyleLock extends LockingOperation { - VariableAccess lock; - - 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*() - or - this instanceof DestructorCall and - exists(RAIIStyleLock constructor | - constructor = getQualifier().(VariableAccess).getTarget().getInitializer().getExpr() and - lock = constructor.getArgument(0).getAChild*() - ) - ) - } - - /** - * Holds if this is a lock operation - */ - 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 - exp.(VariableAccess) - .getTarget() - .getUnderlyingType() - .(Class) - .hasQualifiedName("std", "defer_lock_t") - ) - } - - /** - * Holds if this is an unlock operation - */ - override predicate isUnlock() { this instanceof DestructorCall } - - /** - * Returns the target of the lock underlying this RAII-style lock. - */ - override Variable getLock() { result = lock.getTarget() } - - /** - * Returns the lock underlying this RAII-style lock. - */ - override Expr getLockExpr() { result = lock } -} - -/** - * Models a function that may be executed by some thread. - */ -abstract class ThreadedFunction extends Function { } - -/** - * Models a function that may be executed by some thread via - * C++ standard classes. - */ -class CPPThreadedFunction extends ThreadedFunction { - CPPThreadedFunction() { exists(ThreadConstructorCall tcc | tcc.getFunction() = this) } -} - -/** - * Models a function that may be executed by some thread via - * C11 standard functions. - */ -class C11ThreadedFunction extends ThreadedFunction { - C11ThreadedFunction() { exists(C11ThreadCreateCall cc | cc.getFunction() = this) } -} - -/** - * Models a control flow node within a function that may be executed by some - * thread. - */ -class ThreadedCFN extends ControlFlowNode { - ThreadedCFN() { - exists(ThreadedFunction tf | this = getAThreadContextAwareSuccessor(tf.getEntryPoint())) - } -} - -/** - * Models a `ControlFlowNode` that is protected by some sort of lock. - */ -class LockProtectedControlFlowNode extends ThreadedCFN { - FunctionCall lockingFunction; - - LockProtectedControlFlowNode() { - exists(LockingOperation lock | - // there is a node that is a lock - lockingFunction = lock and - lock.isLock() and - // this node should be a successor of this lock - this = getAThreadContextAwareSuccessor(lock) and - // and there should not exist a predecessor of this - // node that is an unlock. Since we are doing thread context - // aware tracking it is easier to go forwards than backwards - // in constructing the call graph. Thus we can define predecessor - // in terms of a node that is a successor of the lock but NOT a - // successor of the current node. - not exists(ControlFlowNode unlock | - // it's an unlock - unlock = getAThreadContextAwarePredecessor(lock, this) and - unlock.(MutexFunctionCall).isUnlock() and - // note that we don't check that it's the same lock -- this is left - // to the caller to enforce this condition. - // Because of the way that `getAThreadContextAwarePredecessor` works, it is possible - // for operations PAST it to be technically part of the predecessors. - // Thus, we need to make sure that this node is a - // successor of the unlock in the CFG - getAThreadContextAwareSuccessor(unlock) = this - ) and - (lock instanceof MutexFunctionCall implies not this.(MutexFunctionCall).isUnlock()) - ) - } - - /** - * The `MutexFunctionCall` holding the lock that locks this node. - */ - FunctionCall coveredByLock() { result = lockingFunction } - - /** - * The lock underlying this `LockProtectedControlFlowNode`. - */ - Variable getAProtectingLock() { result = lockingFunction.(LockingOperation).getLock() } -} - -/** - * Models a function that conditionally waits. - */ -abstract class ConditionalWait extends FunctionCall { } - -/** - * Models a function in CPP that will conditionally wait. - */ -class CPPConditionalWait extends ConditionalWait { - CPPConditionalWait() { - exists(MemberFunction mf | - mf = getTarget() and - mf.getDeclaringType().hasQualifiedName("std", "condition_variable") and - mf.getName() in ["wait", "wait_for", "wait_until"] - ) - } -} - -/** - * Models a function in C that will conditionally wait. - */ -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. - */ -class MutexDependentThreadConstructor extends ThreadConstructorCall { - Expr mutexExpr; - - MutexDependentThreadConstructor() { - mutexExpr = getAnArgument() and - mutexExpr.getUnderlyingType().stripType() instanceof MutexType - } - - Expr dependentMutex() { result = mutexExpr } -} - -/** - * Models thread waiting functions. - */ -abstract class ThreadWait extends FunctionCall { } - -/** - * Models a call to a `std::thread` join. - */ -class CPPThreadWait extends ThreadWait { - VariableAccess var; - - CPPThreadWait() { - getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "thread") and - getTarget().getName() = "join" - } -} - -/** - * Models a call to `thrd_join` in C11. - */ -class C11ThreadWait extends ThreadWait { - VariableAccess var; - - C11ThreadWait() { getTarget().getName() = "thrd_join" } -} - -/** - * Models thread detach functions. - */ -abstract class ThreadDetach extends FunctionCall { } - -/** - * Models a call to `thrd_detach` in C11. - */ -class C11ThreadDetach extends ThreadWait { - VariableAccess var; - - C11ThreadDetach() { getTarget().getName() = "thrd_detach" } -} - -abstract class MutexSource extends FunctionCall { } - -/** - * Models a C++ style mutex. - */ -class CPPMutexSource extends MutexSource, ConstructorCall { - CPPMutexSource() { getTarget().getDeclaringType().hasQualifiedName("std", "mutex") } -} - -/** - * Models a C11 style mutex. - */ -class C11MutexSource extends MutexSource, FunctionCall { - C11MutexSource() { getTarget().hasName("mtx_init") } - - Expr getMutexExpr() { result = getArgument(0) } - - Expr getMutexTypeExpr() { result = getArgument(1) } -} - -/** - * Models a thread dependent mutex. A thread dependent mutex is a mutex - * that is used by a thread. This dependency is established either by directly - * passing in a mutex or by referencing a mutex that is in the local scope. The utility - * of this class is it captures the `DataFlow::Node` source at which the mutex - * came from. For example, if it is passed in from a local function to a thread. - * This functionality is critical, since it allows one to inspect how the thread - * behaves with respect to the owner of a resource. - * - * To model the myriad ways this can happen, the subclasses of this class are - * responsible for implementing the various usage patterns. - */ -abstract class ThreadDependentMutex extends DataFlow::Node { - DataFlow::Node sink; - - DataFlow::Node getASource() { - // the source is either the thing that declared - // the mutex - result = this - or - // or the thread we are using it in - result = getAThreadSource() - } - - /** - * Gets the dataflow nodes corresponding to thread local usages of the - * dependent mutex. - */ - DataFlow::Node getAThreadSource() { - // here we line up the actual parameter at the thread creation - // site with the formal parameter in the target thread. - // Note that there are differences between the C and C++ versions - // of the argument ordering in the thread creation function. However, - // since the C version only takes one parameter (as opposed to multiple) - // we can simplify this search by considering only the first argument. - exists(FunctionCall fc, Function f, int n | - // Get the argument to which the mutex flowed. - fc.getArgument(n) = sink.asExpr() and - // Get the thread function we are calling. - f = fc.getArgument(0).(FunctionAccess).getTarget() and - // in C++, there is an extra argument to the `std::thread` call - // so we must subtract 1 since this is not passed to the thread. - ( - result = DataFlow::exprNode(f.getParameter(n - 1).getAnAccess()) - or - // In C, only one argument is allowed. Thus IF the flow predicate holds, - // it will be to the first argument - result = DataFlow::exprNode(f.getParameter(0).getAnAccess()) - ) - ) - } - - /** - * Produces the set of dataflow nodes to thread creation for threads - * that are dependent on this mutex. - */ - DataFlow::Node getADependentThreadCreationExpr() { - exists(FunctionCall fc | - fc.getAnArgument() = sink.asExpr() and - result = DataFlow::exprNode(fc) - ) - } - - /** - * Gets a set of usages of this mutex in both the local and thread scope. - * In the case of scoped usage, this also captures typical accesses of variables. - */ - DataFlow::Node getAUsage() { TaintTracking::localTaint(getASource(), result) } -} - -/** - * This class models the type of thread/mutex dependency that is established - * through the typical parameter passing mechanisms found in C++. - */ -class FlowBasedThreadDependentMutex extends ThreadDependentMutex { - FlowBasedThreadDependentMutex() { - // some sort of dataflow, likely through parameter passing. - ThreadDependentMutexFlow::flow(this, sink) - } -} - -/** - * This class models the type of thread/mutex dependency that is established by - * either scope based accesses (e.g., global variables) or block scope differences. - */ -class AccessBasedThreadDependentMutex extends ThreadDependentMutex { - Variable variableSource; - - AccessBasedThreadDependentMutex() { - // encapsulates usages from outside scopes not directly expressed - // in dataflow. - exists(MutexSource mutexSrc, ThreadedFunction f | - DataFlow::exprNode(mutexSrc) = this and - // find a variable that was assigned the mutex - TaintTracking::localTaint(DataFlow::exprNode(mutexSrc), - DataFlow::exprNode(variableSource.getAnAssignedValue())) and - // find all subsequent accesses of that variable that are within a - // function and set those to the sink - exists(VariableAccess va | - va = variableSource.getAnAccess() and - va.getEnclosingFunction() = f and - sink = DataFlow::exprNode(va) - ) - ) - } - - override DataFlow::Node getAUsage() { DataFlow::exprNode(variableSource.getAnAccess()) = result } -} - -/** - * In the typical C thread model, a mutex is a created by a function that is not responsible - * for creating the variable. Thus this class encodes a slightly different semantics - * wherein the usage pattern is that of variables that have been both initialized - * and then subsequently passed into a thread directly. - */ -class DeclarationInitBasedThreadDependentMutex extends ThreadDependentMutex { - Variable variableSource; - - DeclarationInitBasedThreadDependentMutex() { - exists(MutexSource ms, ThreadCreationFunction tcf | - this = DataFlow::exprNode(ms) and - // accessed as a mutex source - TaintTracking::localTaint(DataFlow::exprNode(variableSource.getAnAccess()), - DataFlow::exprNode(ms.getAnArgument())) and - // subsequently passed to a thread creation function (order not strictly - // enforced for performance reasons) - sink = DataFlow::exprNode(tcf.getAnArgument()) and - TaintTracking::localTaint(DataFlow::exprNode(variableSource.getAnAccess()), sink) - ) - } - - override DataFlow::Node getAUsage() { - TaintTracking::localTaint(getASource(), result) or - DataFlow::exprNode(variableSource.getAnAccess()) = result - } - - override DataFlow::Node getASource() { - // the source is either the thing that declared - // the mutex - result = this - or - // or the thread we are using it in - result = getAThreadSource() - } - - DataFlow::Node getSink() { result = sink } - - /** - * Gets the dataflow nodes corresponding to thread local usages of the - * dependent mutex. - */ - override DataFlow::Node getAThreadSource() { - // here we line up the actual parameter at the thread creation - // site with the formal parameter in the target thread. - // Note that there are differences between the C and C++ versions - // of the argument ordering in the thread creation function. However, - // since the C version only takes one parameter (as opposed to multiple) - // we can simplify this search by considering only the first argument. - exists( - FunctionCall fc, Function f, int n // CPP Version - | - fc.getArgument(n) = sink.asExpr() and - f = fc.getArgument(0).(FunctionAccess).getTarget() and - // in C++, there is an extra argument to the `std::thread` call - // so we must subtract 1 since this is not passed to the thread. - result = DataFlow::exprNode(f.getParameter(n - 1).getAnAccess()) - ) - or - exists( - FunctionCall fc, Function f // C Version - | - fc.getAnArgument() = sink.asExpr() and - // in C, the second argument is the function - f = fc.getArgument(1).(FunctionAccess).getTarget() and - // in C, the passed argument is always the zeroth argument - result = DataFlow::exprNode(f.getParameter(0).getAnAccess()) - ) - } -} - -/** - * In the typical C model, another way to use mutexes is to work with global variables - * that can be initialized at various points -- one of which must be inside a thread. - * This class encapsulates this pattern. - */ -class DeclarationInitAccessBasedThreadDependentMutex extends ThreadDependentMutex { - Variable variableSource; - - DeclarationInitAccessBasedThreadDependentMutex() { - exists(MutexSource ms, ThreadedFunction tf, VariableAccess va | - this = DataFlow::exprNode(ms) and - // accessed as a mutex source - TaintTracking::localTaint(DataFlow::exprNode(variableSource.getAnAccess()), - DataFlow::exprNode(ms.getAnArgument())) and - // is accessed somewhere else - va = variableSource.getAnAccess() and - sink = DataFlow::exprNode(va) and - // one of which must be a thread - va.getEnclosingFunction() = tf - ) - } - - override DataFlow::Node getAUsage() { result = DataFlow::exprNode(variableSource.getAnAccess()) } -} - -module ThreadDependentMutexConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node node) { node.asExpr() instanceof MutexSource } - - predicate isSink(DataFlow::Node node) { - exists(ThreadCreationFunction f | f.getAnArgument() = node.asExpr()) - } -} - -module ThreadDependentMutexFlow = TaintTracking::Global<ThreadDependentMutexConfig>; - -/** - * Models expressions that destroy mutexes. - */ -abstract class MutexDestroyer extends StmtParent { - /** - * Gets the expression that references the mutex being destroyed. - */ - abstract Expr getMutexExpr(); -} - -/** - * Models C style mutex destruction via `mtx_destroy`. - */ -class C11MutexDestroyer extends MutexDestroyer, FunctionCall { - C11MutexDestroyer() { getTarget().getName() = "mtx_destroy" } - - /** - * Returns the `Expr` being destroyed. - */ - override Expr getMutexExpr() { result = getArgument(0) } -} - -/** - * Models a delete expression -- note it is necessary to add this in - * addition to destructors to handle certain implementations of the - * standard library which obscure the destructors of mutexes. - */ -class DeleteMutexDestroyer extends MutexDestroyer { - DeleteMutexDestroyer() { this instanceof DeleteExpr } - - override Expr getMutexExpr() { this.(DeleteExpr).getExpr() = result } -} - -/** - * Models a possible mutex variable that if it goes - * out of scope would destroy an underlying mutex. - */ -class LocalMutexDestroyer extends MutexDestroyer { - Expr assignedValue; - - LocalMutexDestroyer() { - exists(LocalVariable lv | - // static types aren't destroyers - not lv.isStatic() and - // neither are pointers - not lv.getType() instanceof PointerType and - lv.getAnAssignedValue() = assignedValue and - // map the location to the return statements of the - // enclosing function - exists(ReturnStmt rs | - rs.getEnclosingFunction() = assignedValue.getEnclosingFunction() and - rs = this - ) - ) - } - - override Expr getMutexExpr() { result = assignedValue } -} - -/** - * Models implicit or explicit calls to the destructor of a mutex, either via - * a `delete` statement or a variable going out of scope. - */ -class DestructorMutexDestroyer extends MutexDestroyer, DestructorCall { - DestructorMutexDestroyer() { getTarget().getDeclaringType().hasQualifiedName("std", "mutex") } - - /** - * Returns the `Expr` being deleted. - */ - override Expr getMutexExpr() { getQualifier() = result } -} - -/** - * Models a conditional variable denoted by `std::condition_variable`. - */ -class ConditionalVariable extends Variable { - ConditionalVariable() { - getUnderlyingType().(Class).hasQualifiedName("std", "condition_variable") - } -} - -/** - * Models a conditional function, which is a function that depends on the value - * of a conditional variable. - */ -class ConditionalFunction extends Function { - ConditionalFunction() { - exists(ConditionalVariable cv | cv.getAnAccess().getEnclosingFunction() = this) - } -} - -/** - * Models calls to thread specific storage function calls. - */ -abstract class ThreadSpecificStorageFunctionCall extends FunctionCall { - /** - * Gets the key to which this call references. - */ - Expr getKey() { getArgument(0) = result } -} - -/** - * Models calls to `tss_get`. - */ -class TSSGetFunctionCall extends ThreadSpecificStorageFunctionCall { - TSSGetFunctionCall() { getTarget().getName() = "tss_get" } -} - -/** - * Models calls to `tss_set`. - */ -class TSSSetFunctionCall extends ThreadSpecificStorageFunctionCall { - TSSSetFunctionCall() { getTarget().getName() = "tss_set" } -} - -/** - * Models calls to `tss_create` - */ -class TSSCreateFunctionCall extends ThreadSpecificStorageFunctionCall { - TSSCreateFunctionCall() { getTarget().getName() = "tss_create" } - - predicate hasDeallocator() { - not exists(MacroInvocation mi, NullMacro nm | - getArgument(1) = mi.getExpr() and - mi = nm.getAnInvocation() - ) - } -} - -/** - * Models calls to `tss_delete` - */ -class TSSDeleteFunctionCall extends ThreadSpecificStorageFunctionCall { - TSSDeleteFunctionCall() { getTarget().getName() = "tss_delete" } -} - -/** - * Gets a call to `DeallocationExpr` that deallocates memory owned by thread specific - * storage. - */ -predicate getAThreadSpecificStorageDeallocationCall(C11ThreadCreateCall tcc, DeallocationExpr dexp) { - exists(TSSGetFunctionCall tsg | - tcc.getFunction().getEntryPoint().getASuccessor*() = tsg and - DataFlow::localFlow(DataFlow::exprNode(tsg), DataFlow::exprNode(dexp.getFreedExpr())) - ) -} - -/** - * Models calls to routines `atomic_compare_exchange_weak` and - * `atomic_compare_exchange_weak_explicit` in the `stdatomic` library. - * Note that these are typically implemented as macros within Clang and - * GCC's standard libraries. - */ -class AtomicCompareExchange extends MacroInvocation { - AtomicCompareExchange() { - getMacroName() = "atomic_compare_exchange_weak" - or - // some compilers model `atomic_compare_exchange_weak` as a macro that - // expands to `atomic_compare_exchange_weak_explicit` so this defeats that - // and other similar modeling. - getMacroName() = "atomic_compare_exchange_weak_explicit" and - not exists(MacroInvocation m | - m.getMacroName() = "atomic_compare_exchange_weak" and - m.getAnExpandedElement() = getAnExpandedElement() - ) - } -} - -/** - * Models calls to routines `atomic_store` and - * `atomic_store_explicit` in the `stdatomic` library. - * Note that these are typically implemented as macros within Clang and - * GCC's standard libraries. - */ -class AtomicStore extends MacroInvocation { - AtomicStore() { - getMacroName() = "atomic_store" - or - // some compilers model `atomic_compare_exchange_weak` as a macro that - // expands to `atomic_compare_exchange_weak_explicit` so this defeats that - // and other similar modeling. - getMacroName() = "atomic_store_explicit" and - not exists(MacroInvocation m | - m.getMacroName() = "atomic_store" and - m.getAnExpandedElement() = getAnExpandedElement() - ) - } -} +import codingstandards.cpp.concurrency.Atomic +import codingstandards.cpp.concurrency.CConditionOperation +import codingstandards.cpp.concurrency.ControlFlow +import codingstandards.cpp.concurrency.ConditionalWait +import codingstandards.cpp.concurrency.LockingOperation +import codingstandards.cpp.concurrency.LockProtectedControlFlow +import codingstandards.cpp.concurrency.MutexDestroyer +import codingstandards.cpp.concurrency.ThreadCreation +import codingstandards.cpp.concurrency.ThreadedFunction +import codingstandards.cpp.concurrency.ThreadDependentMutex +import codingstandards.cpp.concurrency.ThreadSpecificStorage +import codingstandards.cpp.concurrency.ThreadWaitDetach +import codingstandards.cpp.concurrency.Types diff --git a/cpp/common/src/codingstandards/cpp/concurrency/Atomic.qll b/cpp/common/src/codingstandards/cpp/concurrency/Atomic.qll new file mode 100644 index 0000000000..44101f08bb --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/Atomic.qll @@ -0,0 +1,43 @@ +import cpp + +/** + * Models calls to routines `atomic_compare_exchange_weak` and + * `atomic_compare_exchange_weak_explicit` in the `stdatomic` library. + * Note that these are typically implemented as macros within Clang and + * GCC's standard libraries. + */ +class AtomicCompareExchange extends MacroInvocation { + AtomicCompareExchange() { + getMacroName() = "atomic_compare_exchange_weak" + or + // some compilers model `atomic_compare_exchange_weak` as a macro that + // expands to `atomic_compare_exchange_weak_explicit` so this defeats that + // and other similar modeling. + getMacroName() = "atomic_compare_exchange_weak_explicit" and + not exists(MacroInvocation m | + m.getMacroName() = "atomic_compare_exchange_weak" and + m.getAnExpandedElement() = getAnExpandedElement() + ) + } +} + +/** + * Models calls to routines `atomic_store` and + * `atomic_store_explicit` in the `stdatomic` library. + * Note that these are typically implemented as macros within Clang and + * GCC's standard libraries. + */ +class AtomicStore extends MacroInvocation { + AtomicStore() { + getMacroName() = "atomic_store" + or + // some compilers model `atomic_compare_exchange_weak` as a macro that + // expands to `atomic_compare_exchange_weak_explicit` so this defeats that + // and other similar modeling. + getMacroName() = "atomic_store_explicit" and + not exists(MacroInvocation m | + m.getMacroName() = "atomic_store" and + m.getAnExpandedElement() = getAnExpandedElement() + ) + } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/CConditionOperation.qll b/cpp/common/src/codingstandards/cpp/concurrency/CConditionOperation.qll new file mode 100644 index 0000000000..adf230f08d --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/CConditionOperation.qll @@ -0,0 +1,31 @@ +import cpp + +/** + * 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 C style condition destruction via `cnd_destroy`. + */ +class C11ConditionDestroyer extends FunctionCall { + C11ConditionDestroyer() { getTarget().getName() = "cnd_destroy" } + + /** + * Returns the `Expr` being destroyed. + */ + Expr getConditionExpr() { result = getArgument(0) } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/ConditionalWait.qll b/cpp/common/src/codingstandards/cpp/concurrency/ConditionalWait.qll new file mode 100644 index 0000000000..e69ea2fee5 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/ConditionalWait.qll @@ -0,0 +1,26 @@ +import cpp + +/** + * Models a function that conditionally waits. + */ +abstract class ConditionalWait extends FunctionCall { } + +/** + * Models a function in CPP that will conditionally wait. + */ +class CPPConditionalWait extends ConditionalWait { + CPPConditionalWait() { + exists(MemberFunction mf | + mf = getTarget() and + mf.getDeclaringType().hasQualifiedName("std", "condition_variable") and + mf.getName() in ["wait", "wait_for", "wait_until"] + ) + } +} + +/** + * Models a function in C that will conditionally wait. + */ +class CConditionalWait extends ConditionalWait { + CConditionalWait() { getTarget().getName() in ["cnd_wait"] } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/ControlFlow.qll b/cpp/common/src/codingstandards/cpp/concurrency/ControlFlow.qll new file mode 100644 index 0000000000..15f8ab5a61 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/ControlFlow.qll @@ -0,0 +1,101 @@ +import cpp +private import codingstandards.cpp.concurrency.ThreadedFunction + +/** + * Models a control flow node within a function that may be executed by some + * thread. + */ +class ThreadedCFN extends ControlFlowNode { + ThreadedCFN() { + exists(ThreadedFunction tf | this = getAThreadContextAwareSuccessor(tf.getEntryPoint())) + } +} + +/** + * Models CFG nodes which should be added to a thread context. + */ +abstract class ThreadedCFGPathExtension extends ControlFlowNode { + /** + * Returns the next `ControlFlowNode` in this thread context. + */ + abstract ControlFlowNode getNext(); +} + +/** + * Models a `FunctionCall` invoked from a threaded context. + */ +class ThreadContextFunctionCall extends FunctionCall, ThreadedCFGPathExtension { + override ControlFlowNode getNext() { getTarget().getEntryPoint() = result } +} + +/** + * Models a specialized `FunctionCall` that may create a thread. + */ +abstract class ThreadCreationFunction extends FunctionCall, ThreadedCFGPathExtension { + /** + * Returns the function that will be invoked. + */ + abstract Function getFunction(); +} + +/** + * The thread-aware predecessor function is defined in terms of the thread aware + * successor function. This is because it is simpler to construct the forward + * paths of a thread's execution than the backwards paths. For this reason we + * require a `start` and `end` node. + * + * The logic of this function is that a thread aware predecessor is one that + * follows a `start` node, is not equal to the ending node, and does not follow + * the `end` node. Such nodes can only be predecessors of `end`. + * + * For this reason this function requires a `start` node from which to start + * considering something a predecessor of `end`. + */ +pragma[inline] +ControlFlowNode getAThreadContextAwarePredecessor(ControlFlowNode start, ControlFlowNode end) { + result = getAThreadContextAwareSuccessor(start) and + not result = getAThreadContextAwareSuccessor(end) and + not result = end +} + +/** + * A predicate for finding successors of `ControlFlowNode`s that are aware of + * the objects that my flow into a thread's context. This is achieved by adding + * additional edges to thread entry points and function calls. + */ +ControlFlowNode getAThreadContextAwareSuccessorR(ControlFlowNode cfn) { + result = cfn.getASuccessor() + or + result = cfn.(ThreadedCFGPathExtension).getNext() +} + +ControlFlowNode getAThreadContextAwareSuccessor(ControlFlowNode m) { + result = getAThreadContextAwareSuccessorR*(m) and + // for performance reasons we handle back edges by enforcing a lexical + // ordering restriction on these nodes if they are both in + // the same loop. One way of doing this is as follows: + // + // ````and ( + // exists(Loop loop | + // loop.getAChild*() = m and + // loop.getAChild*() = result + // ) + // implies + // not result.getLocation().isBefore(m.getLocation()) + // )``` + // In this implementation we opt for the more generic form below + // which seems to have reasonable performance. + ( + m.getEnclosingStmt().getParentStmt*() = result.getEnclosingStmt().getParentStmt*() + implies + not exists(Location l1, Location l2 | + l1 = result.getLocation() and + l2 = m.getLocation() + | + l1.getEndLine() < l2.getStartLine() + or + l1.getStartLine() = l2.getEndLine() and + l1.getEndColumn() < l2.getStartColumn() + ) + ) +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/LockProtectedControlFlow.qll b/cpp/common/src/codingstandards/cpp/concurrency/LockProtectedControlFlow.qll new file mode 100644 index 0000000000..a828ec8768 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/LockProtectedControlFlow.qll @@ -0,0 +1,49 @@ +import cpp +private import codingstandards.cpp.concurrency.ControlFlow +private import codingstandards.cpp.concurrency.LockingOperation + +/** + * Models a `ControlFlowNode` that is protected by some sort of lock. + */ +class LockProtectedControlFlowNode extends ThreadedCFN { + FunctionCall lockingFunction; + + LockProtectedControlFlowNode() { + exists(LockingOperation lock | + // there is a node that is a lock + lockingFunction = lock and + lock.isLock() and + // this node should be a successor of this lock + this = getAThreadContextAwareSuccessor(lock) and + // and there should not exist a predecessor of this + // node that is an unlock. Since we are doing thread context + // aware tracking it is easier to go forwards than backwards + // in constructing the call graph. Thus we can define predecessor + // in terms of a node that is a successor of the lock but NOT a + // successor of the current node. + not exists(ControlFlowNode unlock | + // it's an unlock + unlock = getAThreadContextAwarePredecessor(lock, this) and + unlock.(MutexFunctionCall).isUnlock() and + // note that we don't check that it's the same lock -- this is left + // to the caller to enforce this condition. + // Because of the way that `getAThreadContextAwarePredecessor` works, it is possible + // for operations PAST it to be technically part of the predecessors. + // Thus, we need to make sure that this node is a + // successor of the unlock in the CFG + getAThreadContextAwareSuccessor(unlock) = this + ) and + (lock instanceof MutexFunctionCall implies not this.(MutexFunctionCall).isUnlock()) + ) + } + + /** + * The `MutexFunctionCall` holding the lock that locks this node. + */ + FunctionCall coveredByLock() { result = lockingFunction } + + /** + * The lock underlying this `LockProtectedControlFlowNode`. + */ + Variable getAProtectingLock() { result = lockingFunction.(LockingOperation).getLock() } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/LockingOperation.qll b/cpp/common/src/codingstandards/cpp/concurrency/LockingOperation.qll new file mode 100644 index 0000000000..95404b114a --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/LockingOperation.qll @@ -0,0 +1,235 @@ +import cpp +import semmle.code.cpp.dataflow.TaintTracking + +abstract class LockingOperation extends FunctionCall { + /** + * Returns the target of the lock underlying this RAII-style lock. + */ + abstract Variable getLock(); + + /** + * Returns the lock underlying this RAII-style lock. + */ + abstract Expr getLockExpr(); + + /** + * Holds if this is a lock operation + */ + abstract predicate isLock(); + + /** + * Holds if this is an unlock operation + */ + abstract predicate isUnlock(); + + /** + * Holds if this locking operation is really a locking operation within a + * designated locking operation. This library assumes the underlying locking + * operations are implemented correctly in that calling a `LockingOperation` + * results in the creation of a singular lock. + */ + predicate isLockingOperationWithinLockingOperation(LockingOperation inner) { + exists(LockingOperation outer | outer.getTarget() = inner.getEnclosingFunction()) + } +} + +/** + * Common base class providing an interface into function call + * based mutex locks. + */ +abstract class MutexFunctionCall extends LockingOperation { + abstract predicate isRecursive(); + + abstract predicate isSpeculativeLock(); + + abstract predicate unlocks(MutexFunctionCall fc); +} + +/** + * Models calls to various mutex types found in CPP. + */ +class CPPMutexFunctionCall extends MutexFunctionCall { + VariableAccess var; + + CPPMutexFunctionCall() { + getTarget() + .(MemberFunction) + .getDeclaringType() + .hasQualifiedName("std", + ["mutex", "timed_mutex", "shared_timed_mutex", "recursive_mutex", "recursive_timed_mutex"]) and + var = getQualifier() + } + + /** + * Holds if this mutex is a recursive mutex. + */ + override predicate isRecursive() { + getTarget() + .(MemberFunction) + .getDeclaringType() + .hasQualifiedName("std", ["recursive_mutex", "recursive_timed_mutex"]) + } + + /** + * Holds if this `CPPMutexFunctionCall` is a lock. + */ + override predicate isLock() { + not isLockingOperationWithinLockingOperation(this) and + getTarget().getName() = "lock" + } + + /** + * Holds if this `CPPMutexFunctionCall` is a speculative lock, defined as calling + * one of the speculative locking functions such as `try_lock`. + */ + override predicate isSpeculativeLock() { + getTarget().getName() in [ + "try_lock", "try_lock_for", "try_lock_until", "try_lock_shared_for", "try_lock_shared_until" + ] + } + + /** + * Returns the lock to which this `CPPMutexFunctionCall` refers to. + */ + override Variable getLock() { result = getQualifier().(VariableAccess).getTarget() } + + /** + * Returns the qualifier for this `CPPMutexFunctionCall`. + */ + override Expr getLockExpr() { result = var } + + /** + * Holds if this is a `unlock` and *may* unlock the previously locked `MutexFunctionCall`. + * This predicate does not check that the mutex is currently locked. + */ + override predicate unlocks(MutexFunctionCall fc) { + isUnlock() and + fc.getQualifier().(VariableAccess).getTarget() = getQualifier().(VariableAccess).getTarget() + } + + /** + * Holds if this is an unlock call. + */ + override predicate isUnlock() { getTarget().getName() = "unlock" } +} + +/** + * Models calls to various mutex types specialized to C code. + */ +class CMutexFunctionCall extends MutexFunctionCall { + Expr arg; + + CMutexFunctionCall() { + // the non recursive kinds + getTarget().getName() = ["mtx_lock", "mtx_unlock", "mtx_timedlock", "mtx_trylock"] and + arg = getArgument(0) + } + + /** + * Holds if this mutex is a recursive mutex. + */ + override predicate isRecursive() { none() } + + /** + * Holds if this `CMutexFunctionCall` is a lock. + */ + override predicate isLock() { + not isLockingOperationWithinLockingOperation(this) and + getTarget().getName() = ["mtx_lock", "mtx_timedlock", "mtx_trylock"] + } + + /** + * Holds if this `CMutexFunctionCall` is a speculative lock, defined as calling + * one of the speculative locking functions such as `try_lock`. + */ + override predicate isSpeculativeLock() { + getTarget().getName() in ["mtx_timedlock", "mtx_trylock"] + } + + /** + * Returns the `Variable` to which this `CMutexFunctionCall` refers to. For this + * style of lock it can reference a number of different variables. + */ + override Variable getLock() { + exists(VariableAccess va | + TaintTracking::localTaint(DataFlow::exprNode(va), DataFlow::exprNode(getLockExpr())) and + result = va.getTarget() + ) + } + + /** + * Returns the expression for this `CMutexFunctionCall`. + */ + override Expr getLockExpr() { result = arg } + + /** + * Holds if this is a `unlock` and *may* unlock the previously locked `CMutexFunctionCall`. + * This predicate does not check that the mutex is currently locked. + */ + override predicate unlocks(MutexFunctionCall fc) { + isUnlock() and + fc.getLock() = getLock() + } + + /** + * Holds if this is an unlock call. + */ + override predicate isUnlock() { getTarget().getName() = "mtx_unlock" } +} + +/** + * Models a RAII-Style lock. + */ +class RAIIStyleLock extends LockingOperation { + VariableAccess lock; + + 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*() + or + this instanceof DestructorCall and + exists(RAIIStyleLock constructor | + constructor = getQualifier().(VariableAccess).getTarget().getInitializer().getExpr() and + lock = constructor.getArgument(0).getAChild*() + ) + ) + } + + /** + * Holds if this is a lock operation + */ + 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 + exp.(VariableAccess) + .getTarget() + .getUnderlyingType() + .(Class) + .hasQualifiedName("std", "defer_lock_t") + ) + } + + /** + * Holds if this is an unlock operation + */ + override predicate isUnlock() { this instanceof DestructorCall } + + /** + * Returns the target of the lock underlying this RAII-style lock. + */ + override Variable getLock() { result = lock.getTarget() } + + /** + * Returns the lock underlying this RAII-style lock. + */ + override Expr getLockExpr() { result = lock } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/MutexDestroyer.qll b/cpp/common/src/codingstandards/cpp/concurrency/MutexDestroyer.qll new file mode 100644 index 0000000000..915efc6077 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/MutexDestroyer.qll @@ -0,0 +1,73 @@ +import cpp + +/** + * Models expressions that destroy mutexes. + */ +abstract class MutexDestroyer extends StmtParent { + /** + * Gets the expression that references the mutex being destroyed. + */ + abstract Expr getMutexExpr(); +} + +/** + * Models C style mutex destruction via `mtx_destroy`. + */ +class C11MutexDestroyer extends MutexDestroyer, FunctionCall { + C11MutexDestroyer() { getTarget().getName() = "mtx_destroy" } + + /** + * Returns the `Expr` being destroyed. + */ + override Expr getMutexExpr() { result = getArgument(0) } +} + +/** + * Models a delete expression -- note it is necessary to add this in + * addition to destructors to handle certain implementations of the + * standard library which obscure the destructors of mutexes. + */ +class DeleteMutexDestroyer extends MutexDestroyer { + DeleteMutexDestroyer() { this instanceof DeleteExpr } + + override Expr getMutexExpr() { this.(DeleteExpr).getExpr() = result } +} + +/** + * Models a possible mutex variable that if it goes + * out of scope would destroy an underlying mutex. + */ +class LocalMutexDestroyer extends MutexDestroyer { + Expr assignedValue; + + LocalMutexDestroyer() { + exists(LocalVariable lv | + // static types aren't destroyers + not lv.isStatic() and + // neither are pointers + not lv.getType() instanceof PointerType and + lv.getAnAssignedValue() = assignedValue and + // map the location to the return statements of the + // enclosing function + exists(ReturnStmt rs | + rs.getEnclosingFunction() = assignedValue.getEnclosingFunction() and + rs = this + ) + ) + } + + override Expr getMutexExpr() { result = assignedValue } +} + +/** + * Models implicit or explicit calls to the destructor of a mutex, either via + * a `delete` statement or a variable going out of scope. + */ +class DestructorMutexDestroyer extends MutexDestroyer, DestructorCall { + DestructorMutexDestroyer() { getTarget().getDeclaringType().hasQualifiedName("std", "mutex") } + + /** + * Returns the `Expr` being deleted. + */ + override Expr getMutexExpr() { getQualifier() = result } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/ThreadCreation.qll b/cpp/common/src/codingstandards/cpp/concurrency/ThreadCreation.qll new file mode 100644 index 0000000000..4499b993ad --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/ThreadCreation.qll @@ -0,0 +1,62 @@ +import cpp +private import codingstandards.cpp.concurrency.ControlFlow + +/** + * Models a call to a thread constructor via `std::thread`. + */ +class ThreadConstructorCall extends ConstructorCall, ThreadCreationFunction { + Function f; + + ThreadConstructorCall() { + getTarget().getDeclaringType().hasQualifiedName("std", "thread") and + f = getArgument(0).(FunctionAccess).getTarget() + } + + /** + * Returns the function that will be invoked by this `std::thread`. + */ + override Function getFunction() { result = f } + + override ControlFlowNode getNext() { result = getFunction().getEntryPoint() } +} + +/** + * Models a call to a thread creation via `thrd_create` or `pthread_create`. + */ +class CThreadCreateCall extends FunctionCall { + Function f; + int fArgIdx; + + CThreadCreateCall() { + ( + getTarget().getName() = "thrd_create" and + fArgIdx = 1 + or + getTarget().getName() = "pthread_create" and + fArgIdx = 2 + ) and + ( + f = getArgument(fArgIdx).(FunctionAccess).getTarget() or + f = getArgument(fArgIdx).(AddressOfExpr).getOperand().(FunctionAccess).getTarget() + ) + } + + /** + * Returns the function that will be invoked by this thread. + */ + Function getFunction() { result = f } +} + +/** + * Models a call to a thread constructor via `thrd_create`. + */ +class C11ThreadCreateCall extends ThreadCreationFunction, CThreadCreateCall { + C11ThreadCreateCall() { getTarget().getName() = "thrd_create" } + + /** + * Returns the function that will be invoked by this thread. + */ + override Function getFunction() { result = f } + + override ControlFlowNode getNext() { result = getFunction().getEntryPoint() } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/ThreadDependentMutex.qll b/cpp/common/src/codingstandards/cpp/concurrency/ThreadDependentMutex.qll new file mode 100644 index 0000000000..f86e94566f --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/ThreadDependentMutex.qll @@ -0,0 +1,246 @@ +import cpp +import semmle.code.cpp.dataflow.TaintTracking +private import codingstandards.cpp.concurrency.ControlFlow +private import codingstandards.cpp.concurrency.ThreadedFunction + +abstract class MutexSource extends FunctionCall { } + +/** + * Models a C++ style mutex. + */ +class CPPMutexSource extends MutexSource, ConstructorCall { + CPPMutexSource() { getTarget().getDeclaringType().hasQualifiedName("std", "mutex") } +} + +/** + * Models a C11 style mutex. + */ +class C11MutexSource extends MutexSource, FunctionCall { + C11MutexSource() { getTarget().hasName("mtx_init") } + + Expr getMutexExpr() { result = getArgument(0) } + + Expr getMutexTypeExpr() { result = getArgument(1) } + + predicate isRecursive() { + exists(EnumConstantAccess recursive | + recursive = getMutexTypeExpr().getAChild*() and + recursive.getTarget().hasName("mtx_recursive") + ) + } +} + +/** + * Models a thread dependent mutex. A thread dependent mutex is a mutex + * that is used by a thread. This dependency is established either by directly + * passing in a mutex or by referencing a mutex that is in the local scope. The utility + * of this class is it captures the `DataFlow::Node` source at which the mutex + * came from. For example, if it is passed in from a local function to a thread. + * This functionality is critical, since it allows one to inspect how the thread + * behaves with respect to the owner of a resource. + * + * To model the myriad ways this can happen, the subclasses of this class are + * responsible for implementing the various usage patterns. + */ +abstract class ThreadDependentMutex extends DataFlow::Node { + DataFlow::Node sink; + + DataFlow::Node getASource() { + // the source is either the thing that declared + // the mutex + result = this + or + // or the thread we are using it in + result = getAThreadSource() + } + + /** + * Gets the dataflow nodes corresponding to thread local usages of the + * dependent mutex. + */ + DataFlow::Node getAThreadSource() { + // here we line up the actual parameter at the thread creation + // site with the formal parameter in the target thread. + // Note that there are differences between the C and C++ versions + // of the argument ordering in the thread creation function. However, + // since the C version only takes one parameter (as opposed to multiple) + // we can simplify this search by considering only the first argument. + exists(FunctionCall fc, Function f, int n | + // Get the argument to which the mutex flowed. + fc.getArgument(n) = sink.asExpr() and + // Get the thread function we are calling. + f = fc.getArgument(0).(FunctionAccess).getTarget() and + // in C++, there is an extra argument to the `std::thread` call + // so we must subtract 1 since this is not passed to the thread. + ( + result = DataFlow::exprNode(f.getParameter(n - 1).getAnAccess()) + or + // In C, only one argument is allowed. Thus IF the flow predicate holds, + // it will be to the first argument + result = DataFlow::exprNode(f.getParameter(0).getAnAccess()) + ) + ) + } + + /** + * Produces the set of dataflow nodes to thread creation for threads + * that are dependent on this mutex. + */ + DataFlow::Node getADependentThreadCreationExpr() { + exists(FunctionCall fc | + fc.getAnArgument() = sink.asExpr() and + result = DataFlow::exprNode(fc) + ) + } + + /** + * Gets a set of usages of this mutex in both the local and thread scope. + * In the case of scoped usage, this also captures typical accesses of variables. + */ + DataFlow::Node getAUsage() { TaintTracking::localTaint(getASource(), result) } +} + +/** + * This class models the type of thread/mutex dependency that is established + * through the typical parameter passing mechanisms found in C++. + */ +class FlowBasedThreadDependentMutex extends ThreadDependentMutex { + FlowBasedThreadDependentMutex() { + // some sort of dataflow, likely through parameter passing. + ThreadDependentMutexFlow::flow(this, sink) + } +} + +/** + * This class models the type of thread/mutex dependency that is established by + * either scope based accesses (e.g., global variables) or block scope differences. + */ +class AccessBasedThreadDependentMutex extends ThreadDependentMutex { + Variable variableSource; + + AccessBasedThreadDependentMutex() { + // encapsulates usages from outside scopes not directly expressed + // in dataflow. + exists(MutexSource mutexSrc, ThreadedFunction f | + DataFlow::exprNode(mutexSrc) = this and + // find a variable that was assigned the mutex + TaintTracking::localTaint(DataFlow::exprNode(mutexSrc), + DataFlow::exprNode(variableSource.getAnAssignedValue())) and + // find all subsequent accesses of that variable that are within a + // function and set those to the sink + exists(VariableAccess va | + va = variableSource.getAnAccess() and + va.getEnclosingFunction() = f and + sink = DataFlow::exprNode(va) + ) + ) + } + + override DataFlow::Node getAUsage() { DataFlow::exprNode(variableSource.getAnAccess()) = result } +} + +/** + * In the typical C thread model, a mutex is a created by a function that is not responsible + * for creating the variable. Thus this class encodes a slightly different semantics + * wherein the usage pattern is that of variables that have been both initialized + * and then subsequently passed into a thread directly. + */ +class DeclarationInitBasedThreadDependentMutex extends ThreadDependentMutex { + Variable variableSource; + + DeclarationInitBasedThreadDependentMutex() { + exists(MutexSource ms, ThreadCreationFunction tcf | + this = DataFlow::exprNode(ms) and + // accessed as a mutex source + TaintTracking::localTaint(DataFlow::exprNode(variableSource.getAnAccess()), + DataFlow::exprNode(ms.getAnArgument())) and + // subsequently passed to a thread creation function (order not strictly + // enforced for performance reasons) + sink = DataFlow::exprNode(tcf.getAnArgument()) and + TaintTracking::localTaint(DataFlow::exprNode(variableSource.getAnAccess()), sink) + ) + } + + override DataFlow::Node getAUsage() { + TaintTracking::localTaint(getASource(), result) or + DataFlow::exprNode(variableSource.getAnAccess()) = result + } + + override DataFlow::Node getASource() { + // the source is either the thing that declared + // the mutex + result = this + or + // or the thread we are using it in + result = getAThreadSource() + } + + DataFlow::Node getSink() { result = sink } + + /** + * Gets the dataflow nodes corresponding to thread local usages of the + * dependent mutex. + */ + override DataFlow::Node getAThreadSource() { + // here we line up the actual parameter at the thread creation + // site with the formal parameter in the target thread. + // Note that there are differences between the C and C++ versions + // of the argument ordering in the thread creation function. However, + // since the C version only takes one parameter (as opposed to multiple) + // we can simplify this search by considering only the first argument. + exists( + FunctionCall fc, Function f, int n // CPP Version + | + fc.getArgument(n) = sink.asExpr() and + f = fc.getArgument(0).(FunctionAccess).getTarget() and + // in C++, there is an extra argument to the `std::thread` call + // so we must subtract 1 since this is not passed to the thread. + result = DataFlow::exprNode(f.getParameter(n - 1).getAnAccess()) + ) + or + exists( + FunctionCall fc, Function f // C Version + | + fc.getAnArgument() = sink.asExpr() and + // in C, the second argument is the function + f = fc.getArgument(1).(FunctionAccess).getTarget() and + // in C, the passed argument is always the zeroth argument + result = DataFlow::exprNode(f.getParameter(0).getAnAccess()) + ) + } +} + +/** + * In the typical C model, another way to use mutexes is to work with global variables + * that can be initialized at various points -- one of which must be inside a thread. + * This class encapsulates this pattern. + */ +class DeclarationInitAccessBasedThreadDependentMutex extends ThreadDependentMutex { + Variable variableSource; + + DeclarationInitAccessBasedThreadDependentMutex() { + exists(MutexSource ms, ThreadedFunction tf, VariableAccess va | + this = DataFlow::exprNode(ms) and + // accessed as a mutex source + TaintTracking::localTaint(DataFlow::exprNode(variableSource.getAnAccess()), + DataFlow::exprNode(ms.getAnArgument())) and + // is accessed somewhere else + va = variableSource.getAnAccess() and + sink = DataFlow::exprNode(va) and + // one of which must be a thread + va.getEnclosingFunction() = tf + ) + } + + override DataFlow::Node getAUsage() { result = DataFlow::exprNode(variableSource.getAnAccess()) } +} + +module ThreadDependentMutexConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node.asExpr() instanceof MutexSource } + + predicate isSink(DataFlow::Node node) { + exists(ThreadCreationFunction f | f.getAnArgument() = node.asExpr()) + } +} + +module ThreadDependentMutexFlow = TaintTracking::Global<ThreadDependentMutexConfig>; diff --git a/cpp/common/src/codingstandards/cpp/concurrency/ThreadSpecificStorage.qll b/cpp/common/src/codingstandards/cpp/concurrency/ThreadSpecificStorage.qll new file mode 100644 index 0000000000..aa7daf972c --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/ThreadSpecificStorage.qll @@ -0,0 +1,59 @@ +import cpp +private import semmle.code.cpp.dataflow.DataFlow +private import codingstandards.cpp.concurrency.ThreadCreation + +/** + * Models calls to thread specific storage function calls. + */ +abstract class ThreadSpecificStorageFunctionCall extends FunctionCall { + /** + * Gets the key to which this call references. + */ + Expr getKey() { getArgument(0) = result } +} + +/** + * Models calls to `tss_get`. + */ +class TSSGetFunctionCall extends ThreadSpecificStorageFunctionCall { + TSSGetFunctionCall() { getTarget().getName() = "tss_get" } +} + +/** + * Models calls to `tss_set`. + */ +class TSSSetFunctionCall extends ThreadSpecificStorageFunctionCall { + TSSSetFunctionCall() { getTarget().getName() = "tss_set" } +} + +/** + * Models calls to `tss_create` + */ +class TSSCreateFunctionCall extends ThreadSpecificStorageFunctionCall { + TSSCreateFunctionCall() { getTarget().getName() = "tss_create" } + + predicate hasDeallocator() { + not exists(MacroInvocation mi, NullMacro nm | + getArgument(1) = mi.getExpr() and + mi = nm.getAnInvocation() + ) + } +} + +/** + * Models calls to `tss_delete` + */ +class TSSDeleteFunctionCall extends ThreadSpecificStorageFunctionCall { + TSSDeleteFunctionCall() { getTarget().getName() = "tss_delete" } +} + +/** + * Gets a call to `DeallocationExpr` that deallocates memory owned by thread specific + * storage. + */ +predicate getAThreadSpecificStorageDeallocationCall(C11ThreadCreateCall tcc, DeallocationExpr dexp) { + exists(TSSGetFunctionCall tsg | + tcc.getFunction().getEntryPoint().getASuccessor*() = tsg and + DataFlow::localFlow(DataFlow::exprNode(tsg), DataFlow::exprNode(dexp.getFreedExpr())) + ) +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/ThreadWaitDetach.qll b/cpp/common/src/codingstandards/cpp/concurrency/ThreadWaitDetach.qll new file mode 100644 index 0000000000..6898dc54df --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/ThreadWaitDetach.qll @@ -0,0 +1,41 @@ +import cpp + +/** + * Models thread waiting functions. + */ +abstract class ThreadWait extends FunctionCall { } + +/** + * Models a call to a `std::thread` join. + */ +class CPPThreadWait extends ThreadWait { + VariableAccess var; + + CPPThreadWait() { + getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", "thread") and + getTarget().getName() = "join" + } +} + +/** + * Models a call to `thrd_join` in C11. + */ +class C11ThreadWait extends ThreadWait { + VariableAccess var; + + C11ThreadWait() { getTarget().getName() = "thrd_join" } +} + +/** + * Models thread detach functions. + */ +abstract class ThreadDetach extends FunctionCall { } + +/** + * Models a call to `thrd_detach` in C11. + */ +class C11ThreadDetach extends ThreadWait { + VariableAccess var; + + C11ThreadDetach() { getTarget().getName() = "thrd_detach" } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/ThreadedFunction.qll b/cpp/common/src/codingstandards/cpp/concurrency/ThreadedFunction.qll new file mode 100644 index 0000000000..a8d2c609c5 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/ThreadedFunction.qll @@ -0,0 +1,37 @@ +import cpp +private import codingstandards.cpp.concurrency.ThreadCreation + +/** + * Models a function that may be executed by some thread. + */ +abstract class ThreadedFunctionBase extends Function { + abstract Expr getSpawnExpr(); + + predicate isMultiplySpawned() { getSpawnExpr().getBasicBlock().inLoop() } +} + +final class ThreadedFunction = ThreadedFunctionBase; + +/** + * Models a function that may be executed by some thread via + * C++ standard classes. + */ +class CPPThreadedFunction extends ThreadedFunctionBase { + ThreadConstructorCall tcc; + + CPPThreadedFunction() { tcc.getFunction() = this } + + override Expr getSpawnExpr() { result = tcc } +} + +/** + * Models a function that may be executed by some thread via + * C11 standard functions. + */ +class C11ThreadedFunction extends ThreadedFunctionBase { + C11ThreadCreateCall cc; + + C11ThreadedFunction() { cc.getFunction() = this } + + override Expr getSpawnExpr() { result = cc } +} diff --git a/cpp/common/src/codingstandards/cpp/concurrency/Types.qll b/cpp/common/src/codingstandards/cpp/concurrency/Types.qll new file mode 100644 index 0000000000..3b865d5171 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/concurrency/Types.qll @@ -0,0 +1,29 @@ +import cpp + +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 + } +} diff --git a/cpp/common/src/codingstandards/cpp/dominance/BehavioralSet.qll b/cpp/common/src/codingstandards/cpp/dominance/BehavioralSet.qll new file mode 100644 index 0000000000..8609e3213b --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/dominance/BehavioralSet.qll @@ -0,0 +1,40 @@ +import cpp +import semmle.code.cpp.controlflow.ControlFlowGraph + +signature class TargetNode extends ControlFlowNode; + +signature module DominatingSetConfigSig<TargetNode Target> { + predicate isTargetBehavior(ControlFlowNode behavior, Target target); + + default predicate isBlockingBehavior(ControlFlowNode behavior, Target target) { none() } +} + +/** + * A module to find whether there exists a dominator set for a node which performs a relevant + * behavior. + * + * For instance, we may wish to see that all paths leading to an `abort()` statement include a + * logging call. In this case, the `abort()` statement is the `Target` node, and the config module + * predicate `isTargetBehavior` logging statements. + * + * Additionally, the config may specify `isBlockingBehavior` to prevent searching too far for the + * relevant behavior. For instance, if analyzing that all paths to an `fflush()` call are preceded + * by a write, we should ignore paths from write operations that have already been flushed through + * an intermediary `fflush()` call. + */ +module DominatingBehavioralSet<TargetNode Target, DominatingSetConfigSig<Target> Config> { + /** + * Holds if this search step can reach the entry or a blocking node, without passing through a + * target behavior, indicating that the target is has no relevant dominator set. + */ + private predicate searchStep(ControlFlowNode node, Target target) { + Config::isBlockingBehavior(node, target) + or + not Config::isTargetBehavior(node, target) and + exists(ControlFlowNode prev | prev = node.getAPredecessor() | searchStep(prev, target)) + } + + predicate isDominatedByBehavior(Target target) { + forex(ControlFlowNode prev | prev = target.getAPredecessor() | not searchStep(prev, target)) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency9.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency9.qll new file mode 100644 index 0000000000..b013bbdabb --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency9.qll @@ -0,0 +1,146 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Concurrency9Query = + TPossibleDataRaceBetweenThreadsQuery() or + TThreadResourceDisposedBeforeThreadsJoinedQuery() or + TInvalidOperationOnUnlockedMutexQuery() or + TNonRecursiveMutexRecursivelyLockedQuery() or + TNonRecursiveMutexRecursivelyLockedAuditQuery() or + TConditionVariableUsedWithMultipleMutexesQuery() or + TThreadStorageNotInitializedBeforeUseQuery() or + TThreadStoragePointerInitializedInsideThreadQuery() + +predicate isConcurrency9QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `possibleDataRaceBetweenThreads` query + Concurrency9Package::possibleDataRaceBetweenThreadsQuery() and + queryId = + // `@id` for the `possibleDataRaceBetweenThreads` query + "c/misra/possible-data-race-between-threads" and + ruleId = "DIR-5-1" and + category = "required" + or + query = + // `Query` instance for the `threadResourceDisposedBeforeThreadsJoined` query + Concurrency9Package::threadResourceDisposedBeforeThreadsJoinedQuery() and + queryId = + // `@id` for the `threadResourceDisposedBeforeThreadsJoined` query + "c/misra/thread-resource-disposed-before-threads-joined" and + ruleId = "RULE-22-15" and + category = "required" + or + query = + // `Query` instance for the `invalidOperationOnUnlockedMutex` query + Concurrency9Package::invalidOperationOnUnlockedMutexQuery() and + queryId = + // `@id` for the `invalidOperationOnUnlockedMutex` query + "c/misra/invalid-operation-on-unlocked-mutex" and + ruleId = "RULE-22-17" and + category = "required" + or + query = + // `Query` instance for the `nonRecursiveMutexRecursivelyLocked` query + Concurrency9Package::nonRecursiveMutexRecursivelyLockedQuery() and + queryId = + // `@id` for the `nonRecursiveMutexRecursivelyLocked` query + "c/misra/non-recursive-mutex-recursively-locked" and + ruleId = "RULE-22-18" and + category = "required" + or + query = + // `Query` instance for the `nonRecursiveMutexRecursivelyLockedAudit` query + Concurrency9Package::nonRecursiveMutexRecursivelyLockedAuditQuery() and + queryId = + // `@id` for the `nonRecursiveMutexRecursivelyLockedAudit` query + "c/misra/non-recursive-mutex-recursively-locked-audit" and + ruleId = "RULE-22-18" and + category = "required" + or + query = + // `Query` instance for the `conditionVariableUsedWithMultipleMutexes` query + Concurrency9Package::conditionVariableUsedWithMultipleMutexesQuery() and + queryId = + // `@id` for the `conditionVariableUsedWithMultipleMutexes` query + "c/misra/condition-variable-used-with-multiple-mutexes" and + ruleId = "RULE-22-19" and + category = "required" + or + query = + // `Query` instance for the `threadStorageNotInitializedBeforeUse` query + Concurrency9Package::threadStorageNotInitializedBeforeUseQuery() and + queryId = + // `@id` for the `threadStorageNotInitializedBeforeUse` query + "c/misra/thread-storage-not-initialized-before-use" and + ruleId = "RULE-22-20" and + category = "mandatory" + or + query = + // `Query` instance for the `threadStoragePointerInitializedInsideThread` query + Concurrency9Package::threadStoragePointerInitializedInsideThreadQuery() and + queryId = + // `@id` for the `threadStoragePointerInitializedInsideThread` query + "c/misra/thread-storage-pointer-initialized-inside-thread" and + ruleId = "RULE-22-20" and + category = "mandatory" +} + +module Concurrency9Package { + Query possibleDataRaceBetweenThreadsQuery() { + //autogenerate `Query` type + result = + // `Query` type for `possibleDataRaceBetweenThreads` query + TQueryC(TConcurrency9PackageQuery(TPossibleDataRaceBetweenThreadsQuery())) + } + + Query threadResourceDisposedBeforeThreadsJoinedQuery() { + //autogenerate `Query` type + result = + // `Query` type for `threadResourceDisposedBeforeThreadsJoined` query + TQueryC(TConcurrency9PackageQuery(TThreadResourceDisposedBeforeThreadsJoinedQuery())) + } + + Query invalidOperationOnUnlockedMutexQuery() { + //autogenerate `Query` type + result = + // `Query` type for `invalidOperationOnUnlockedMutex` query + TQueryC(TConcurrency9PackageQuery(TInvalidOperationOnUnlockedMutexQuery())) + } + + Query nonRecursiveMutexRecursivelyLockedQuery() { + //autogenerate `Query` type + result = + // `Query` type for `nonRecursiveMutexRecursivelyLocked` query + TQueryC(TConcurrency9PackageQuery(TNonRecursiveMutexRecursivelyLockedQuery())) + } + + Query nonRecursiveMutexRecursivelyLockedAuditQuery() { + //autogenerate `Query` type + result = + // `Query` type for `nonRecursiveMutexRecursivelyLockedAudit` query + TQueryC(TConcurrency9PackageQuery(TNonRecursiveMutexRecursivelyLockedAuditQuery())) + } + + Query conditionVariableUsedWithMultipleMutexesQuery() { + //autogenerate `Query` type + result = + // `Query` type for `conditionVariableUsedWithMultipleMutexes` query + TQueryC(TConcurrency9PackageQuery(TConditionVariableUsedWithMultipleMutexesQuery())) + } + + Query threadStorageNotInitializedBeforeUseQuery() { + //autogenerate `Query` type + result = + // `Query` type for `threadStorageNotInitializedBeforeUse` query + TQueryC(TConcurrency9PackageQuery(TThreadStorageNotInitializedBeforeUseQuery())) + } + + Query threadStoragePointerInitializedInsideThreadQuery() { + //autogenerate `Query` type + result = + // `Query` type for `threadStoragePointerInitializedInsideThread` query + TQueryC(TConcurrency9PackageQuery(TThreadStoragePointerInitializedInsideThreadQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index 63bdcfa4df..2444bbfa0e 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -15,6 +15,7 @@ import Concurrency5 import Concurrency6 import Concurrency7 import Concurrency8 +import Concurrency9 import Contracts import Contracts1 import Contracts2 @@ -102,6 +103,7 @@ newtype TCQuery = TConcurrency6PackageQuery(Concurrency6Query q) or TConcurrency7PackageQuery(Concurrency7Query q) or TConcurrency8PackageQuery(Concurrency8Query q) or + TConcurrency9PackageQuery(Concurrency9Query q) or TContractsPackageQuery(ContractsQuery q) or TContracts1PackageQuery(Contracts1Query q) or TContracts2PackageQuery(Contracts2Query q) or @@ -189,6 +191,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isConcurrency6QueryMetadata(query, queryId, ruleId, category) or isConcurrency7QueryMetadata(query, queryId, ruleId, category) or isConcurrency8QueryMetadata(query, queryId, ruleId, category) or + isConcurrency9QueryMetadata(query, queryId, ruleId, category) or isContractsQueryMetadata(query, queryId, ruleId, category) or isContracts1QueryMetadata(query, queryId, ruleId, category) or isContracts2QueryMetadata(query, queryId, ruleId, category) or diff --git a/rule_packages/c/Concurrency9.json b/rule_packages/c/Concurrency9.json new file mode 100644 index 0000000000..6ae1df8173 --- /dev/null +++ b/rule_packages/c/Concurrency9.json @@ -0,0 +1,158 @@ +{ + "MISRA-C-2012": { + "DIR-5-1": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Threads shall not access the same memory location concurrently without utilization of thread synchronization objects.", + "kind": "problem", + "name": "There shall be no data races between threads", + "precision": "medium", + "severity": "error", + "short_name": "PossibleDataRaceBetweenThreads", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "There shall be no data races between threads" + }, + "RULE-22-15": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Thread synchronization objects and thread-specific storage pointers shall not be destroyed until after all threads accessing them have terminated.", + "kind": "problem", + "name": "Thread synchronization objects and thread-specific storage pointers shall not be disposed unsafely", + "precision": "medium", + "severity": "error", + "short_name": "ThreadResourceDisposedBeforeThreadsJoined", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "Thread synchronization objects and thread-specific storage pointers shall not be destroyed until after all threads accessing them have terminated" + }, + "RULE-22-17": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked before.", + "kind": "problem", + "name": "No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked", + "precision": "high", + "severity": "error", + "short_name": "InvalidOperationOnUnlockedMutex", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked before" + }, + "RULE-22-18": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Mutexes initialized with mtx_init() without mtx_recursive shall not be locked by a thread that has previously locked it.", + "kind": "problem", + "name": "Non-recursive mutexes shall not be recursively locked", + "precision": "very-high", + "severity": "error", + "short_name": "NonRecursiveMutexRecursivelyLocked", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + }, + { + "description": "Mutexes that may be initialized without mtx_recursive shall not be locked by a thread that may have previously locked it.", + "kind": "problem", + "name": "(Audit) Non-recursive mutexes shall not be recursively locked", + "precision": "high", + "severity": "error", + "short_name": "NonRecursiveMutexRecursivelyLockedAudit", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4", + "external/misra/audit" + ] + } + ], + "title": "Non-recursive mutexes shall not be recursively locked" + }, + "RULE-22-19": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Standard library functions cnd_wait() and cnd_timedwait() shall specify the same mutex object for each condition object in all calls.", + "kind": "problem", + "name": "A condition variable shall be associated with at most one mutex object", + "precision": "very-high", + "severity": "error", + "short_name": "ConditionVariableUsedWithMultipleMutexes", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "A condition variable shall be associated with at most one mutex object" + }, + "RULE-22-20": { + "properties": { + "obligation": "mandatory" + }, + "queries": [ + { + "description": "Thread specific storage pointers shall be initialized with the standard library functions before using them.", + "kind": "problem", + "name": "Thread-specific storage pointers shall be created before being accessed", + "precision": "high", + "severity": "error", + "short_name": "ThreadStorageNotInitializedBeforeUse", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + }, + { + "description": "Thread specific storage pointers initialized inside of threads may result in indeterministic state.", + "kind": "problem", + "name": "Thread specific storage pointers shall be initialized deterministically", + "precision": "very-high", + "severity": "recommendation", + "short_name": "ThreadStoragePointerInitializedInsideThread", + "tags": [ + "readability", + "maintainability", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "Thread-specific storage pointers shall be created before being accessed" + } + } +} \ No newline at end of file diff --git a/schemas/rule-package.schema.json b/schemas/rule-package.schema.json index 64fac6f396..b4f729afe2 100644 --- a/schemas/rule-package.schema.json +++ b/schemas/rule-package.schema.json @@ -342,6 +342,7 @@ "external/autosar/strict", "scope/single-translation-unit", "scope/system", + "external/misra/audit", "external/misra/c/2012/third-edition-first-revision", "external/misra/c/2012/amendment2", "external/misra/c/2012/amendment3",